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

AAD Usage Clarifications #51

Merged
merged 5 commits into from
Feb 14, 2024
Merged

AAD Usage Clarifications #51

merged 5 commits into from
Feb 14, 2024

Conversation

hannestschofenig
Copy link
Collaborator

Based on interop testing with Daisuke

Based on interop testing with Daisuke
Copy link
Contributor

@laurencelundblade laurencelundblade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Agree with use of the "aad" parameter.
  • A few comments and typos
  • Lament about the info param / structure

discouraged by RFC 9052, this profile allows the use of the 'kid' parameter
(or other parameters) to identify the static recipient public key used by
the sender. If the COSE_Encrypt0 contains the 'kid' then the recipient may
discouraged by RFC 9052, this profile RECOMMENDS the use of the 'kid' parameter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COSE-HKPE is not really a "profile". It is a new standard encryption mechanism for COSE (not a big deal, but I thought I'd mention it).

(or other parameters) to identify the static recipient public key used by
the sender. If the COSE_Encrypt0 contains the 'kid' then the recipient may
discouraged by RFC 9052, this profile RECOMMENDS the use of the 'kid' parameter
(or other parameters) to explicitly identify the static recipient public key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the use or non-use of kid depends on the use case. If you have a use case where the key is known already then it is not needed (not a big deal, but I thought I'd mention it).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. You don't have to use it but we recommend it in the generic case.

@@ -208,7 +197,7 @@ structure, i.e. one COSE_recipient structure per recipient.
In this approach the following layers are involved:

- Layer 0 (corresponding to the COSE_Encrypt structure) contains the content (plaintext)
encrypted with the CEK. This ciphertext MAY be detached. If not detached, then
encrypted with the CEK. This ciphertext may be detached, and if not detached, then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think detached / non-detached needs to be mention in this document. (not a big deal, but I thought I'd mention it).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I downgraded the sentence to avoid the use of the MAY to may

Section 5.3 of {{RFC9052}} need to be followed, which includes the
calculation of the authenticated data strcture.

At layer 1 where HPKE is used to encrypted the CEK, the "aad" parameter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"encrypt", not "encrypted"

@@ -251,6 +240,34 @@ header_map = {

The COSE_Encrypt MAY be tagged or untagged.

When encrypting the content at layer 0 then the instructions in
Section 5.3 of {{RFC9052}} need to be followed, which includes the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"need" -> "MUST"

@@ -251,6 +240,34 @@ header_map = {

The COSE_Encrypt MAY be tagged or untagged.

When encrypting the content at layer 0 then the instructions in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems correct to me.

Section 5.3 of {{RFC9052}} need to be followed, which includes the
calculation of the authenticated data strcture.

At layer 1 where HPKE is used to encrypted the CEK, the "aad" parameter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is progress on #42. It is saying what the "aad" parameter input to the Seal() API is.

from the COSE_recipient structure at layer 1, encoded in a bstr type.

The external_aad field in the Enc_structure contains the Externally Supplied
Data described in Section 4.3 and Section 5.3 in RFC 9052. If this field is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is restating some of 9052. I kind of thought it's better not to do restatements and if you do, say the original is normative. (not a big deal, but I thought I'd mention it).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Laurence that there is no need to restate what is written in another spec. However, I also think that this PR should be merged once and editorial corrections can be done later as a PR that resolves the issue (#41) Laurence filed.

not supplied, it defaults to a zero-length byte string.

The HPKE APIs also use an "info" parameter as input and the details are
provided in {{info}}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wish we could improve this a lot. Kind of seems like the user of COSE-HPKE has to do a deep threat analysis of their use case to know what to do here. Probably they have to do a lot less than in regular COSE, because COSE-HPKE internally takes care of a lot of this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, and I think it would be better to solve this with another PR.

The protected field in the Enc_structure contains the protected attributes
from the COSE_recipient structure at layer 1, encoded in a bstr type.

The external_aad field in the Enc_structure contains the Externally Supplied
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The external_aad field in the Enc_structure contains the Externally Supplied

from the COSE_recipient structure at layer 1, encoded in a bstr type.

The external_aad field in the Enc_structure contains the Externally Supplied
Data described in Section 4.3 and Section 5.3 in RFC 9052. If this field is
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Data described in Section 4.3 and Section 5.3 in RFC 9052. If this field is


The external_aad field in the Enc_structure contains the Externally Supplied
Data described in Section 4.3 and Section 5.3 in RFC 9052. If this field is
not supplied, it defaults to a zero-length byte string.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
not supplied, it defaults to a zero-length byte string.

@hannestschofenig hannestschofenig merged commit 3afc9cf into main Feb 14, 2024
@hannestschofenig hannestschofenig deleted the hannestschofenig-patch-4 branch February 14, 2024 10:23
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.

4 participants