tireddy2 / jose-hybrid-encrypt

Other
1 stars 0 forks source link

Comments Set - 3 #4

Closed auriee closed 9 months ago

auriee commented 9 months ago

The KDF looks like it is intended to be NIST SP 800-56C KDF, but it misuses the salt parameter (the key to KMAC). (Indeed it is, modified the text)

The salt can not be fixed. NIST SP 800-56C requires that the application can give explicit salt to use (which goes raw into KMAC key) or if the application does not provode salt, then the default salt is used. Additionally, the default default salt has off-by-one problem: It is one byte too big. (Addressed: Reduced the salt by one byte and https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-56Cr2.pdf discusses in Table 3 that the Maximum Byte Length of salt can be 132 bytes but in our draft K is variable-length. The size of "K" will change based on the PQ/T hybrid algorithm. For instance, "x25519-ES_kyber512" and "secp256r1-ES_kyber512" will result in two different sizes of K after the concat operation. We have two choices either go with default salt or pad the resulting string with X to get a 132 byte output.)

The key format should be . There is no need for lengths: Those are either impiled by or if those are variable, it is not sufficient to just include lengths, there must be inner hash. (We address the variable lengths using rlen and we keep the format according to draft-ounsworth-cfrg-kem-combiners-04)

And the algorithm choices seem pretty bad:

1) KMAC128 should not be used with any variant of Kyber, the capacity does not match. Use KMAC256 instead. (Addressed, changed to KMAC256)

2) Don't use HKDF-256 with Kyber. Especially not in COSE! (We are not using HKDF-256 with Kyber but rather with DHKEM-based shared secret)

3) Don't use AES128KW, This is post-quantum. Use AES256KW instead. (The authors intend to keep AES128 into consideration as long as standardisation bodies considers AES128 to quantum safe)

tireddy2 commented 9 months ago

https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-56Cr2.pdf discusses in Table 3 that the Maximum Byte Length of salt can be 132 bytes but in our draft K is variable-length. The size of "K" will change based on the PQ/T hybrid algorithm. For instance, "x25519-ES_kyber512" and "secp256r1-ES_kyber512" will result in two different sizes of K after the concat operation. We have two choices either go with default salt or pad the resulting string with X to get a 132 byte output.

tireddy2 commented 9 months ago

Rest of the changes look good.