Christian's implementation comments #110
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…roupcomm into impl-comments-christian
marco-tiloca-sics
requested review from
rikard-sics,
gselander,
fpalombini,
emanjon and
chrysn
rikard-sics
reviewed
Sep 16, 2024
draft-ietf-core-oscore-groupcomm.md
Outdated
| A. and then XOR with the X least significant bits of the Common IV, where X is the length in bits of the AEAD nonce. | ||
| For example, if X = 7 and the Common IV is 0x00112233445566778899aabbcc (13 bytes), then the bytes to XOR are 0x00112233445566 (7 bytes). | ||
|
|
||
| The constructed nonce is in fact used as AEAD nonce by the AEAD Algorithm (see {{ssec-common-context-aead-alg}}) and by the Group Encryption Algorithm when this is an AEAD algorithm (see {{ssec-common-context-cs-enc-alg}}). |
There was a problem hiding this comment.
I would remove "in fact" and just put this as:
The constructed nonce is used as AEAD nonce by the AEAD Algorithm (see {{ssec-common-context-aead-alg}}) and by the Group Encryption Algorithm when this is an AEAD algorithm (see {{ssec-common-context-cs-enc-alg}}).
There was a problem hiding this comment.
Fixed in 486ca64
rikard-sics
approved these changes
Sep 16, 2024
fpalombini
requested changes
Sep 16, 2024
draft-ietf-core-oscore-groupcomm.md
Outdated
| @@ -324,6 +333,10 @@ The new parameter Group Manager Authentication Credential specifies the authenti | |||
|
|
|||
| The new parameter Group Encryption Algorithm identifies the algorithm to use for encryption and decryption, when messages are protected in group mode (see {{mess-processing}}). This algorithm MAY provide integrity protection. If this parameter is not set, the group mode is not used in the group. | |||
|
|
|||
| The following non-authenticated algorithms can be used as Group Encryption Algorithm: A128CBC, A192CBC, and A256CBC {{RFC9459}}. The non-authenticated algorithm ChaCha20 {{ChaCha}} is also suitable to consider, although using it will first require its registration in the "COSE Algorithms" Registry. | |||
There was a problem hiding this comment.
So here we either register ChaCha20 or we skip mentioning I think.
There was a problem hiding this comment.
Addressed at d249213 by skipping ChaCha20.
draft-ietf-core-oscore-groupcomm.md
Outdated
| @@ -324,6 +333,10 @@ The new parameter Group Manager Authentication Credential specifies the authenti | |||
|
|
|||
| The new parameter Group Encryption Algorithm identifies the algorithm to use for encryption and decryption, when messages are protected in group mode (see {{mess-processing}}). This algorithm MAY provide integrity protection. If this parameter is not set, the group mode is not used in the group. | |||
|
|
|||
| The following non-authenticated algorithms can be used as Group Encryption Algorithm: A128CBC, A192CBC, and A256CBC {{RFC9459}}. The non-authenticated algorithm ChaCha20 {{ChaCha}} is also suitable to consider, although using it will first require its registration in the "COSE Algorithms" Registry. | |||
|
|
|||
| The following non-authenticated algorithms MUST NOT be used as Group Encryption Algorithm: A128CTR, A192CTR, and A256CTR {{RFC9459}}. | |||
There was a problem hiding this comment.
the problem with a list like this is that it's never going to be exhaustive... can we skip it? Christian only asked to add which parameter are particularly suitable...
There was a problem hiding this comment.
Addressed at d249213
draft-ietf-core-oscore-groupcomm.md
Outdated
|
|
||
| The AEAD nonce is constructed like in OSCORE, with the difference that step 4 in {{Section 5.2 of RFC8613}} is replaced with: | ||
| A. and then XOR with the X bytes from the Common IV's start, where X is the length in bytes of the nonce. |
There was a problem hiding this comment.
s/with the X bytes/with X bytes
There was a problem hiding this comment.
Fixed in 486ca64
…roupcomm into impl-comments-christian
rikard-sics
approved these changes
Sep 18, 2024
|
Looks good to me. |
fpalombini
approved these changes
Sep 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses the comments from Christian Amsüss archived at https://mailarchive.ietf.org/arch/msg/core/oQT5vcbEsui9fvf7P9QH4PEsX-c/
In addition to that:
Where appropriate, it replaces "AEAD nonce" with "nonce" and "AEAD key" with "key", consistent with the fact that the Group Encryption Algorithm might not be an AEAD algorithm.
For endpoints that use non-authenticated encryption, it makes A128CBC mandatory to implement as non-authenticated Group Encryption Algorithm.
It clarifies that the HKDF Algorithm must be an HMAC-based HKDF.