openpgp-pqc / draft-openpgp-pqc

Repository of the WIP draft-ietf-openpgp-pqc
Other
8 stars 2 forks source link

change KDF from KMAC to SHA3 #48

Closed falko-strenzke closed 1 year ago

falko-strenzke commented 1 year ago

Some concerns regarding the availability and ease of implementation of KMAC have arisen: https://github.com/openpgp-pqc/draft-openpgp-pqc/issues/46

Accordingly, the use of readily available SHA-3 functions seems to be the superior choice.

ahuelsing commented 1 year ago

I do not think this is a good idea. I agree with @wussler that KMAC is the better solution. The problem that motivated the change, i.e., that the Keccak core function is not available will also manifest when implementing SHAKE which is required for Dilithium and Kyber. Hence, I do not consider it a solution to switch to SHA3 or SHA2-HMAC here.

falko-strenzke commented 1 year ago

I do not think this is a good idea. I agree with @wussler that KMAC is the better solution. The problem that motivated the change, i.e., that the Keccak core function is not available will also manifest when implementing SHAKE which is required for Dilithium and Kyber. Hence, I do not consider it a solution to switch to SHA3 or SHA2-HMAC here.

This is not entirely true since KMAC requires cSHAKE and not SHAKE. If a library, like Botan, implements SHA-3 and SHAKE this is still not enough to implement KMAC. In such cases the Keccak[c] function has to be made available, which forces the developer to deal with its internals to some degree since the final bit padding for cSHAKE differs from that of SHAKE:

SHAKE128(M, d) = KECCAK[256] (M || 1111, d)

cSHAKE128(...) = KECCAK[256](... || X || 00, L)

This is what I experienced with Botan.

falko-strenzke commented 1 year ago

KMAC is defined as follows:

KMAC256(K, X, L, S):
Validity Conditions: len(K) < 2²⁰⁴⁰ and 0 ≤ L < 2²⁰⁴⁰ and len(S) < 2²⁰⁴⁰

1. newX = bytepad(encode_string(K), 136) || X || right_encode(L).
2. T = bytepad(encode_string(“KMAC”) || encode_string(S), 136).
3. return KECCAK[512](T || newX || 00, L)  // "00" is the 2-bit padding

Besides some encodings it doesn't to anything different than SHA-3 hash. The necessary length encodings can easily be modelled into the KDF construction.

Note also the virtually unlimitted key length. This is another implementation problem we experience in Botan since allowing an extremely large key length may be unexpected by the caller who might simply try to allocate max_key_length. Limiting it arbirtrarily might on the other hand limit the usability of KMAC.

I think it makes sense to hold back this PR for now and wait how the acceptance of KMAC by the community will turn out. This will probably depend on the availability of cSHAKE or KMAC in crypto libraries at that point.

wussler commented 1 year ago

I would close the PR and keep the issue open. The reason is that this PR IMO does not cover the requirements to simply perform the swap, for instance adding the output length to the fixed info, even though now we just output a single length to keywrap AES-256. A PR in this field would require some additional discussion.

The reasons why I would keep the issue open is because implementation complexity is still very important to me. We should not forget that if we find hard to implement this in the first two implementations, it might be very hard for others too. As much as I prefer KMAC, I'm willing to switch to SHA-3 to make it easier to implement, as long as we keep the security guarantees at the cost of a more complex hash construction (to avoid collisions with the ECDH KDF)

falko-strenzke commented 1 year ago

OK, let's close it as we have no immediate plans to accept it. Issue #46 anyway still points to it and we can reopen and refine it whenever needed.