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 missing parameter to ECC-KEM #66

Merged
merged 2 commits into from Feb 13, 2024
Merged

Add missing parameter to ECC-KEM #66

merged 2 commits into from Feb 13, 2024

Conversation

wussler
Copy link
Collaborator

@wussler wussler commented Nov 2, 2023

The eccPublicKey parameter is also used into the KDF, therefore required for the decaps

Copy link
Collaborator

@fluppe2 fluppe2 left a comment

Choose a reason for hiding this comment

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

@wussler : Nice catch!

To recall, the history of the appearance of eccPublicKey in the final step of our ECC-KEMs, i.e. x25519kem, x448kem and ecdhkem, is that

  • we introduced eccKeyShare = Hash(X || eccCipherText) (previously it was just eccKeyShare = X) to have IND-CCA2 secure ECC-KEMs following Cramer-Shoup. The crucial aspect of Cramer-Shoup is the inclusion of eccCipherText.
  • we raised this security aspect during the inclusion of X25519 and X448 in the crypto-refresh and pointed the OpenPGP WG to the security considerations of RFC7748 which states "Alice and Bob can then use a key-derivation function that includes K, K_A, and K_B to derive a symmetric key.". The K_A, K_B map to our eccPublicKey and eccCipherText. It has been applied to the crypto-refresh such that they included both in their KDF.
  • we adopted the additional inclusion of eccPublicKey in our ECC-KEMs to align to the approach of the crypto-refresh.

ML-KEM doas not need the explicit mentioning of the public key since it does extract the public key from the decapsulation key as @falko-strenzke and @wussler state it, i.e. already incorporates such an IND-CCA2 conversion and accomodates the needed data in the decapsulation key.

Since the mlkemSecretKey contains the actual secret key AND the public key I guess we have two options here:

  1. Do it the way @wussler proposed it here and just have a slightly different interface for the ECC-KEMs.
  2. Mimic the way ML-KEM does it and define the ECC public key as part of the ML-KEM+ECC secret key packet in §5.3.2 from which the eccPublicKey could then be extracted in the decapsulation function ECC-KEM.Decaps() without having it to specify it in the interface.

We are defining the key material packets for new algorithms and hence from scratch anyway so I guess we have the liberty to do this.

I guess both options are fine. I would (personally) maybe tend to option 2 as I feel it is more in the mindset of modern KEMs to have the public key always be part of the secret key.

Option 2 will trigger some changes in implementations as it would change the specification of the secret key material packet. In my opinion, we are in such an early stage that this should not hinder us in considering this approach.

What do you think/prefer?

@wussler
Copy link
Collaborator Author

wussler commented Nov 5, 2023

@fluppe2

Option 2 will trigger some changes in implementations as it would change the specification of the secret key material packet.

This would probably make it less OpenPGP-like but definitely makes it easy to verify the keys against KOpenpgp attacks

@TJ-91
Copy link
Collaborator

TJ-91 commented Nov 6, 2023

Personally, I prefer option 1) since it seems like the "obvious approach". For 2) I feel like we need some justification. IMO KOpenpgp attacks should be mitigated on a different level: use authenticated encryption (which is introduced in the Crypto Refresh).

Copy link
Collaborator

@fluppe2 fluppe2 left a comment

Choose a reason for hiding this comment

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

Yes, lets go for option 1, I think that is a good way forward.

@wussler wussler merged commit 28c1e0f into main Feb 13, 2024
2 checks passed
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.

None yet

4 participants