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

IdCred enhancements #325

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

chrysn
Copy link
Collaborator

@chrysn chrysn commented Nov 25, 2024

Some small fixes I found when updating Ariel OS to use the latest lakers:

  • Some documentation on how IdCred is used would have helped, so I added an example that doubles as a doctest.
  • Implementing that I found a typo (KCSS instead of KCCS); the fix is API compatible by deprecating the old version.
  • The from_encoded_value constructor accepted some values that look like KIDs but are really longer. It may be a good idea to accept non-1-byte- KIDs, but this PR is designed for "all no-brainers so let's just merge it". I'm happy to provide longer KID support in there, but I don't know whether there's a limit on KID lengths in other places (won't hurt for this function to support more, though).
    [edit] Note that the way it is used internally (eg. decode_plaintext_3), the length is pre-checked to be correct due to decoder.any_as_encoded(), but that also means we're doing double work – still, a public function should do proper error handling.

@geonnave
Copy link
Collaborator

Thanks, just went through (including latest commit for larger KIDs) and is looking good. Happy to merge once CI is green.

@chrysn
Copy link
Collaborator Author

chrysn commented Nov 25, 2024

That latest commit was pushed by accident and is now in #326.

CI gets upset because apparently the doc comments get translated to C (cbindgen?), and there the compiler is not super happy about comments-in-comments (no way!); I'll look briefly into whether cbindgen can be made to play ball, but chances are I'll just remove those comments.

@chrysn
Copy link
Collaborator Author

chrysn commented Nov 25, 2024

The bindgen issue seems to be previously unnoticed; I'm adding a workaround.

@chrysn
Copy link
Collaborator Author

chrysn commented Nov 25, 2024

I think this is ready now; fixing things on cbindgen would have been excessive, so the comments in the doctest are in a less-than-ideal position as a workaround.

@geonnave
Copy link
Collaborator

Looks good!

@geonnave geonnave merged commit eb32a07 into openwsn-berkeley:main Nov 25, 2024
37 checks passed
@chrysn chrysn deleted the id-cred-docs branch November 25, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants