lib: ecdh1 derive: add support for CKD_SHA1_KDF key derivation function#918
lib: ecdh1 derive: add support for CKD_SHA1_KDF key derivation function#918hnez wants to merge 6 commits into
Conversation
079b290 to
581b3d9
Compare
| key[0].shr = malloc(shr_len); | ||
| key[1].shr = malloc(shr_len); | ||
| memset(key[0].shr, 0, shr_len); | ||
| memset(key[0].shr, 1, shr_len); |
There was a problem hiding this comment.
Yes, yes indeed. I have replaced the copied steps with a loop.
| }; | ||
|
|
||
| key[0].shr = malloc(shr_len); | ||
| key[1].shr = malloc(shr_len); |
There was a problem hiding this comment.
Check for malloc failures. Use the cmocka macros for null check. I just want an OOM failure to be obvious.
There was a problem hiding this comment.
Makes sense. I have added checks.
| shared_secret_attr.pValue = calloc(shared_secret_attr.ulValueLen, | ||
| sizeof(CK_BYTE)); | ||
|
|
||
| EVP_KDF *kdf = EVP_KDF_fetch(NULL, OSSL_KDF_NAME_X963KDF, NULL); |
There was a problem hiding this comment.
This can return NULL, so we need a check
There was a problem hiding this comment.
I was not to sure about which error messages to return in that case. I hope the ones I chose make some sense.
| sizeof(CK_BYTE)); | ||
|
|
||
| EVP_KDF *kdf = EVP_KDF_fetch(NULL, OSSL_KDF_NAME_X963KDF, NULL); | ||
| EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf); |
There was a problem hiding this comment.
This could fail as well, so we need a check
| EVP_KDF *kdf = EVP_KDF_fetch(NULL, OSSL_KDF_NAME_X963KDF, NULL); | ||
| EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf); | ||
|
|
||
| OSSL_PARAM params[4], *p = params; |
There was a problem hiding this comment.
Why do I need p here? Can't I just do something like parms[0] =, parms[1] =. This hides the underlying storage IMO. All those code paths succeed, so p always gets incremented, the constants I think will make that more clear as well.
There was a problem hiding this comment.
This seems to be a common pattern in code that sets up OSSL_PARAM arrays, which I just copied.
I guess it makes sense when setting up an array in code with a lot of conditional expressions, because it becomes difficult to keep track of the current array index in that case.
Since the control flow is rather easy here I agree that we do not have to do it this way.
I have changed the code accordingly.
This more closely matches the TPM2 ESAPI function it wraps in both name and functionality. A complete derivation may involve running a key derivation function on the output tpm_ec_ecdh_zgen(). Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
Move the check for which kdf function to run down to after the Z point generation. This requires some more cleanup if a non-supported KDF is used, but makes it easier to add more KDFs (besides CKD_NULL). Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
This allows generating shared secrets of arbitrary length by using SHA1 as a key derivation function. There are other possible KDFs, but this one is the default used by `openssl cms -encrypt` and thus one of the most interesting ones to implement. Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
This is in preparation to adding other KDFs to the test suite. Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
When a key derivation function is used the shared secret can have an arbitrary length, independent from the elliptic curve key size. Prepare for adding KDF support by making the shared secret size dynamic. Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
Signed-off-by: Leonard Göhrs <l.goehrs@pengutronix.de>
2 participants
This PR adds support for the
CKD_SHA1_KDFkey derivation function to the Elliptic Curve Diffie Helman functionality in tpm2-pkcs11.This Fixes #914 (at least partially).
Some more work is required to get it fully working:
lib: ecdh1 derive: register CKM_ECDH1_DERIVE as available mechanism #913 should be merged as well.
opensslwith thepkcs11-provideruses aCKS_RO_USER_FUNCTIONSsession state when calling derive. This makesobject_create()insrc/lib/object.cfail, because it assumes that all objects are token objects, when in fact a session object should be created here.See tpm2-pkcs11 does not support session objects / only token objects #921 for more information.