Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor process_message_3 to follow implementation guidelines #130

Conversation

malishav
Copy link
Contributor

@malishav malishav commented Nov 5, 2023

This PR updates r_process_message_3 routine to allow generic handling of EAD items and of credentials, as per draft-tiloca-lake-implem-cons.

It also updated the decode_plaintext_3 routine to enable the credential to be passed in the message by value, instead only by reference.

Closes #129

@malishav malishav changed the title 129 refactor process message 3 to follow implementation guidelines Refactor process_message_3 to follow implementation guidelines Nov 5, 2023
@malishav malishav marked this pull request as ready for review November 5, 2023 13:59
@malishav malishav requested a review from geonnave November 5, 2023 13:59
@malishav malishav force-pushed the 129-refactor-process_message_3-to-follow-implementation-guidelines branch from bf9075b to 0513f02 Compare November 5, 2023 18:04
Copy link
Collaborator

@geonnave geonnave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, looks good to me! Left two minor comments below.

let ead_success = if let Some(ead_3) = ead_3 {
r_process_ead_3(ead_3).is_ok()
cred_i = if let Some(cred_i_expected) = cred_i_expected {
// 1. Does ID_CRED_X point to a stored authentication credential? YES
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could leave a pointer to Marco's diagram, just to make clear where these comments come from.

Suggestion: Comments tagged with a number refer to Marco Tiloca's implementation guidelines diagram (yet to be published).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -287,19 +289,86 @@ pub fn r_process_message_3(
let decoded_p3_res = decode_plaintext_3(&plaintext_3);

if decoded_p3_res.is_ok() {
let (kid, mac_3, ead_3) = decoded_p3_res.unwrap();
let (id_cred_i, mac_3, ead_3) = decoded_p3_res.unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to leave this as a note, for now this will only work for the following cases:

  1. R receives KID and has the associated CRED_I locally stored
  2. R receives CRED_I and uses it

According to lake-authz, R (V) could also support receiving KID and calling W to resolve CRED_I. We could leave a TO-DO for that in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@malishav malishav force-pushed the 129-refactor-process_message_3-to-follow-implementation-guidelines branch from 0513f02 to 0906fa0 Compare November 6, 2023 10:35
@malishav
Copy link
Contributor Author

malishav commented Nov 6, 2023

@geonnave fstar generation seems to be triggered for this PR now and it fails. could you check if the command is OK?

@malishav malishav force-pushed the 129-refactor-process_message_3-to-follow-implementation-guidelines branch from 0906fa0 to 2fbf6f7 Compare November 6, 2023 13:53
@geonnave geonnave merged commit 21ec5cd into openwsn-berkeley:main Nov 6, 2023
23 of 24 checks passed
@malishav malishav deleted the 129-refactor-process_message_3-to-follow-implementation-guidelines branch November 7, 2023 10:43
@geonnave geonnave added new feature enhancement New feature or request and removed new feature labels Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor process_message_3 to follow implementation guidelines
2 participants