mbedtls
mbedtls copied to clipboard
Ad-hoc KDF for EC J-PAKE in TLS 1.2
Fixes https://github.com/Mbed-TLS/mbedtls/issues/5978.
This PR adds an algorithm to prepare SHA256(K.X) for the use of TLS 1.2.
Open questions:
- Should the input type be restricted to only accept
PSA_KEY_DERIVATION_INPUT_SECRET? - Should the operation capacity restriction be added to
psa_key_derivation_setup_kdf?
Requires backport
NO - new feature
Requires ChangeLog entry
[mpg] Unsure - on one hand, that's a new feature, on the other hand, it's really only meant to be used in the library: the only use case is implementing TLS 1.2's ECJPAKE key exchange.
Note: the ABI-API check outputs information that's expected and harmless (new field added in a union), so CI is as good as green.
@mpg this solution requires a bit of changes in psa_key_derivation_setup_kdf to account for the lack of passed hash algorithm. Let me know if this approach is fine, and if yes - I'll proceed with minor things to add around - like a check in check_crypto_config.h, or a changelog entry.
Added all requested changes apart from two: removal of one newline and a proposal of restricting the reading to one chunk of 32 bytes.
Btw we also need to make a decision regarding the ChangeLog entry. I previously wrote:
Unsure - on one hand, that's a new feature, on the other hand, it's really only meant to be used in the library: the only use case is implementing TLS 1.2's ECJPAKE key exchange.
But now I'm thinking, if we consider this important enough to be included in the official PSA API, then surely this is also important enough for our ChangeLog. So, please add a ChangeLog entry unless someone disagrees.
we should allow piecewise output
It doesn't have to gate this PR though. Only supporting a single output could be an acceptable temporary limitation, and then we could build a generic implementation of piecewise output from fixed-size-output KDF without the pressure of delivering ECJPAKE.
It doesn't have to gate this PR though. Only supporting a single output could be an acceptable temporary limitation, and then we could build a generic implementation of piecewise output from fixed-size-output KDF without the pressure of delivering ECJPAKE.
Ok, good point, let's do this (unless @AndrzejKurek has already implemented it in the meantime). I believe you already created an issue to track that yesterday, so that's covered. I'll change my review to "approved" as soon as the ChangeLog entry is there.
It doesn't have to gate this PR though. Only supporting a single output could be an acceptable temporary limitation, and then we could build a generic implementation of piecewise output from fixed-size-output KDF without the pressure of delivering ECJPAKE.
Ok, good point, let's do this (unless @AndrzejKurek has already implemented it in the meantime). I believe you already created an issue to track that yesterday, so that's covered. I'll change my review to "approved" as soon as the ChangeLog entry is there.
I was in the progress of doing it, but I'll just add a changelog entry instead. Thanks!
The only failure here is the API/ABI check.
I should probably have thought about it before hitting merge, but this needs side-porting to the PSA Crypto spec. @gilles-peskine-arm @athoelke can you remind me what the procedure is these days?
@mpg The procedure is to file an issue either in https://github.com/Mbed-TLS/mbedtls or https://github.com/ARM-software/psa-crypto-api, or to directly make a PR to psa-crypto-api. In this case Andrew has already filed an issue: https://github.com/ARM-software/psa-crypto-api/issues/564
Ok, great, thanks!