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

Add structure for ACE OSCORE profile #58

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chrysn
Copy link

@chrysn chrysn commented Nov 29, 2022

This adds support for the main data structure of RFC 9203.

Status

This is currently incomplete, as evidenced by the todo!() conversion direction, and the lack of tests.

I'm putting it here to make the next step (integration in dcaf) traceable; in the course of that, I may yet find the need to adjust things here.

@google-cla
Copy link

google-cla bot commented Nov 29, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@chrysn
Copy link
Author

chrysn commented Nov 30, 2022

Force pushed to clear CLA check.

@chrysn
Copy link
Author

chrysn commented Nov 30, 2022

These parameters typically occur twice in an ACE OSCORE profile token response: Once in the outer response, once encrypted in the token.

The outer occurrence is part of the access token response (which AIU is not modelled in coset), where crates building on this one (such as dcaf) can easily build an OscoreInputMaterial from what they find. This is fine.

The inner occurrence gets reported by this crate as part of the .rest of a ClaimsSet that can be decrypted (in dacf) and built (in coset, indirectly from dcaf after decryption) from a token. There, it is stored unstructured, and as the patches are now, it's up to the user to dissect that rest into an OscoreInputMaterial and other details. Not great, not terrible.

I'd like to make that a bit smoother, but given that OscoreInputMaterial is just one possible manifestation of PoP confirmation ("cnf") that this cnf can be. The dcaf crate has an implementation of that already, all of whose nontrivial variants are already coset types.

Suggestion (for which I'm pinging the dcaf maintainer @falko17): Move the type dcaf::common::cbor_values::ProofOfPossessionKey into coset. (This may involve renaming, also of its method, to be aligned with the crate's conventions). Then, the cnf claim previously kept as a Value inside ClaimSet.rest could be parsed semantically, and moved into the struct itself.

(All of that can be executed after this PR, but given that it justifies and/or influences how the type introduced here, I think it's as good place as any to discuss this here).

@falko17
Copy link
Contributor

falko17 commented Nov 30, 2022

Suggestion (for which I'm pinging the dcaf maintainer @falko17): Move the type dcaf::common::cbor_values::ProofOfPossessionKey into coset. (This may involve renaming, also of its method, to be aligned with the crate's conventions). Then, the cnf claim previously kept as a Value inside ClaimSet.rest could be parsed semantically, and moved into the struct itself.

I agree, that seems like a better solution, thanks for the ping.

This is slightly off-topic for this PR, but I've also thought about moving the cipher trait definitions from dcaf into coset, because they are much more closely related to COSE rather than ACE-OAuth (as they are basically just abstractions over ciphers for CoseEncrypt, CoseSign, and CoseMac). Some methods in that module could also be partially refactored and moved into coset, specifically the parts responsible for decoding COSE structures for multiple recipients (e.g., decrypt_access_token_multiple handling retrieval of the Content Encryption Key via the Key Encryption Key).
This becomes even more relevant once we implement cipher implementations, something that really seems much more suitable for a COSE crate instead of an ACE-OAuth crate like dcaf.

As long as this isn't deemed out of scope for coset and fine with the maintainers, I'd open corresponding issues and/or PRs to further plan this out once work on the cipher trait rewrite in namib-project/dcaf-rs#10 is done.

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