search

google / coset

A set of Rust types for supporting COSE
Apache License 2.0
28 stars 16 forks source link

Add structure for ACE OSCORE profile #58

Open chrysn opened 1 year ago

chrysn commented 1 year ago

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[bot] commented 1 year ago

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 commented 1 year ago

Force pushed to clear CLA check.

chrysn commented 1 year ago

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 commented 1 year ago

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 https://github.com/namib-project/dcaf-rs/pull/10 is done.