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

Support long C_x #320

Merged
merged 8 commits into from
Nov 7, 2024
Merged

Conversation

chrysn
Copy link
Collaborator

@chrysn chrysn commented Nov 4, 2024

Closes: #258

This is a straightforward change and relatively complete; noteworthy points are:

  • On some parts that I don't usually touch, things were not changed -- they should keep working, but they may still only support the short identifiers.
    Grep for from_int_raw to see all possible places; in particular, I known of the C API, and of the automatic generation of identifiers.
  • There are unit tests in the docs, and I enabled the previously disabled tests that planned ahead for this and things just do check alright. There are no new integration tests, but I don't know whether we need any. I tested things from aiocoap, and things worked fine; I can still try an interop test with Marco later this week.
  • The backing data for identifiers is an interesting thing because it's an owned small array that has no length information. CBOR is self-delimited, and I'm making use of that, thus fitting the 7 byte sensible string length (typical OSCORE KIDs are up to 7 byte) in 8 byte of RAM. The type describes its invariant, and explicitly panics in a place. I'd guess that that's just one of the panics where hax could show that no safe operation on that type will ever get it in a state where that invariant is violated, but I know too little about it to tell whether it already does that, or whether it'll need some handholding, or whether it doesn't matter anyway.

The previously used vector was valid but merely unsupported.
This used the wrong encoding function, which made no difference before
when only short C_x were supported.
@chrysn
Copy link
Collaborator Author

chrysn commented Nov 4, 2024

I had to add one more fix where we used .as_slice() instead of .as_cbor() (which made no difference so far), and now interop'd with Marco with aiocoap/lakers as the initiator (producing both short and long C_I) and Marco as responder (producing both b'' and numeric C_R).

Please review.

@chrysn chrysn requested a review from geonnave November 4, 2024 13:48
@geonnave
Copy link
Collaborator

geonnave commented Nov 7, 2024

Thanks for this PR! I went through and actually have no extra comments. Merging.

@geonnave geonnave merged commit 91f7d77 into openwsn-berkeley:main Nov 7, 2024
37 checks passed
@chrysn chrysn deleted the long-c_x branch November 25, 2024 14:15
@geonnave geonnave mentioned this pull request Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C_x only support u8
2 participants