cose-wg / HPKE

3 stars 3 forks source link

Replace COSE_KDF_Context with new Recipient structure #58

Closed laurencelundblade closed 4 months ago

laurencelundblade commented 7 months ago

This is my attempt at the outcome of the discussion between Ori, Illari and myself on the list.

I went back to next_alg so it can be used with multiple layers of COSE_Recipient, but say that it is for the next COSE layer down. It is expected that when a COSE_Recipient employs multiple algorithms like HPKE and -29 do that they lock down all the algorithms they use internally. It was unclear what the alg ID in COSE_KDF_Context was for.

I haven't implemented this, but I'm pretty confident about it. I also haven't updated the examples.

dajiaji commented 7 months ago

I support this PR, but let me ask just one question.

Why not use the existing names 'protected' and 'external_aad', instead of giving them new names?

I understand this is a bikeshed discussion.

laurencelundblade commented 7 months ago

Feel free to merge. I don't have the privilege to do so.

OR13 commented 7 months ago

@hannestschofenig can you review? I will merge if there is no objection.

laurencelundblade commented 7 months ago

@dajiaji

I support this PR, but let me ask just one question.

Why not use the existing names 'protected' ~and 'external_aad'~, instead of giving them new names?

I understand this is a bikeshed discussion.

I think the new names are more clear. There are many places in 9052 and 9053 where the wording is unclear. I've had to go read Jim's code to make sure I understood. If things still aren't clearly, let's add more text to be sure they are.

I want to be sure it's clear that the external_aad at level 0 is not the same data as recipient_aad at level 1. There is only one external_aad for the whole message no matter how many recipients. If there are multiple recipients, there might be multiple recipient_aad.

Similarly, I want to be clear that the headers that are protected are the level 1 headers, the COSE_Recipient headers, not the level 0 COSE_Encrypt headers.

dajiaji commented 7 months ago

I think the new names are more clear. There are many places in 9052 and 9053 where the wording is unclear. I've had to go read Jim's code to make sure I understood. If things still aren't clearly, let's add more text to be sure they are.

I want to be sure it's clear that the external_aad at level 0 is not the same data as recipient_aad at level 1. There is only one external_aad for the whole message no matter how many recipients. If there are multiple recipients, there might be multiple recipient_aad.

Similarly, I want to be clear that the headers that are protected are the level 1 headers, the COSE_Recipient headers, not the level 0 COSE_Encrypt headers.

Understood. It might be a good idea to refactor the wording with the introduction of COSE-HPKE.

@hannestschofenig @OR13 I'm glad if you can merge this PR.

OR13 commented 7 months ago

We're still looking for code review and approvals from @hannestschofenig and @dajiaji , ideally we merge after 1 week with no objection.

OR13 commented 5 months ago

I'd like to merge this pull request.