openwsn-berkeley / lakers

EDHOC implemented in Rust, optimized for microcontrollers, with bindings for C and Python.
https://crates.io/crates/lakers
BSD 3-Clause "New" or "Revised" License
12 stars 10 forks source link

Split message processing #174

Closed geonnave closed 6 months ago

geonnave commented 6 months ago

On top of PR #170. Aims to provide the split message processing discussed in issue #99.

Main tasks:

In addition, as a result of the split there is now IdCred and IdCredOwned. Thus,

Finally, with respect to the EAD:

geonnave commented 6 months ago

Edit: the mapping of tasks in this comment have been moved to the first comment.

geonnave commented 6 months ago

This is ready for an initial review round. Note that the split begins at 5dd6c72 (previous commits are from PR #170).

chrysn commented 6 months ago

This looks to me like it's really independent of #170 (apart from commits such as 49cff1a2e13f501fb8e38ff262a9770cb0c75cee that applies small fixes).

I'm not sure how you do / want reviews done here, but to me this would be way easier to review if the splitting changes could happen in a branch of their own. Depends a bit on the level of review you want here -- if it's more a rough look you're after, that works here as well. If you prefer commit-by-commit review, that might be more productive on a dedicated branch.

geonnave commented 6 months ago

Thanks for the review so far. The type of review I am looking for is about the current state of the API and an overall rough look.

chrysn commented 6 months ago

I think the rough API is good from a high-level PoV. There are lots of details we can enhance, but we can do that progressively once this is in.

One thing probably makes sense to do now because it's a pain to do later: look through the newly introduce type and function names. (Now this can be one commit with grep -i s/.../.../ **/*.rs, later that's bound to cause merge conflicts). Particular candidates are the split functions that are now a and b variants. The process_message_n / prepare_message_n+1 split already works well, so maybe process_message_3a / process_message_3b can become parse_message_3 / verify_message_3, and prepare_message_3a / preparere_mesage_3b can become prepare_message_3 / finalize_message_3 (same for prepare_message_1a/b)

geonnave commented 6 months ago

Did changes suggested by @malishav (un-split prepare_message_1 and 3) and the renamings proposed by @chrysn. Another critical item pointed by @malishav (private key accessed by application) is tracked in #182 and will be fixed ASAP.