Remove a TODO(3.0) from EVP_PKEY_derive_set_peer()

The TODO described a case where a legacy derive operation is called, but
the peer key is provider based. In practice this will almost never be a
problem. We should never end up in our own legacy EVP_PKEY_METHOD
implementations if no ENGINE has been configured. If an ENGINE has been
configured then we we will be using a third party EVP_PKEY_METHOD
implementation and public APIs will be used to obtain the key data from the
peer key so there will be no "reaching inside" the pkey.

There is a theoretical case where a third party ENGINE wraps our own
internal EVP_PKEY_METHODs using EVP_PKEY_meth_find() or
EVP_PKEY_meth_get0(). For these cases we just ensure all our
EVP_PKEY_METHODs never reach "inside" the implementation of a peer key. We
can never assume that it is a legacy key.

Fixes #14399

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14555)
This commit is contained in:
Matt Caswell 2021-03-15 16:21:45 +00:00
parent d11f644ba5
commit 19ad1e9d37
4 changed files with 23 additions and 13 deletions

View File

@ -421,23 +421,30 @@ static int pkey_dh_derive(EVP_PKEY_CTX *ctx, unsigned char *key,
{ {
int ret; int ret;
DH *dh; DH *dh;
const DH *dhpub;
DH_PKEY_CTX *dctx = ctx->data; DH_PKEY_CTX *dctx = ctx->data;
BIGNUM *dhpub; BIGNUM *dhpubbn;
if (!ctx->pkey || !ctx->peerkey) {
if (ctx->pkey == NULL || ctx->peerkey == NULL) {
ERR_raise(ERR_LIB_DH, DH_R_KEYS_NOT_SET); ERR_raise(ERR_LIB_DH, DH_R_KEYS_NOT_SET);
return 0; return 0;
} }
dh = ctx->pkey->pkey.dh; dh = ctx->pkey->pkey.dh;
dhpub = ctx->peerkey->pkey.dh->pub_key; dhpub = EVP_PKEY_get0_DH(ctx->peerkey);
if (dhpub == NULL) {
ERR_raise(ERR_LIB_DH, DH_R_KEYS_NOT_SET);
return 0;
}
dhpubbn = dhpub->pub_key;
if (dctx->kdf_type == EVP_PKEY_DH_KDF_NONE) { if (dctx->kdf_type == EVP_PKEY_DH_KDF_NONE) {
if (key == NULL) { if (key == NULL) {
*keylen = DH_size(dh); *keylen = DH_size(dh);
return 1; return 1;
} }
if (dctx->pad) if (dctx->pad)
ret = DH_compute_key_padded(key, dhpub, dh); ret = DH_compute_key_padded(key, dhpubbn, dh);
else else
ret = DH_compute_key(key, dhpub, dh); ret = DH_compute_key(key, dhpubbn, dh);
if (ret < 0) if (ret < 0)
return ret; return ret;
*keylen = ret; *keylen = ret;
@ -461,7 +468,7 @@ static int pkey_dh_derive(EVP_PKEY_CTX *ctx, unsigned char *key,
if (Z == NULL) { if (Z == NULL) {
goto err; goto err;
} }
if (DH_compute_key_padded(Z, dhpub, dh) <= 0) if (DH_compute_key_padded(Z, dhpubbn, dh) <= 0)
goto err; goto err;
if (!DH_KDF_X9_42(key, *keylen, Z, Zlen, dctx->kdf_oid, if (!DH_KDF_X9_42(key, *keylen, Z, Zlen, dctx->kdf_oid,
dctx->kdf_ukm, dctx->kdf_ukmlen, dctx->kdf_md)) dctx->kdf_ukm, dctx->kdf_ukmlen, dctx->kdf_md))

View File

@ -161,8 +161,15 @@ static int pkey_ec_derive(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen)
size_t outlen; size_t outlen;
const EC_POINT *pubkey = NULL; const EC_POINT *pubkey = NULL;
EC_KEY *eckey; EC_KEY *eckey;
const EC_KEY *eckeypub;
EC_PKEY_CTX *dctx = ctx->data; EC_PKEY_CTX *dctx = ctx->data;
if (!ctx->pkey || !ctx->peerkey) {
if (ctx->pkey == NULL || ctx->peerkey == NULL) {
ERR_raise(ERR_LIB_EC, EC_R_KEYS_NOT_SET);
return 0;
}
eckeypub = EVP_PKEY_get0_EC_KEY(ctx->peerkey);
if (eckeypub == NULL) {
ERR_raise(ERR_LIB_EC, EC_R_KEYS_NOT_SET); ERR_raise(ERR_LIB_EC, EC_R_KEYS_NOT_SET);
return 0; return 0;
} }
@ -178,7 +185,7 @@ static int pkey_ec_derive(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen)
*keylen = (EC_GROUP_get_degree(group) + 7) / 8; *keylen = (EC_GROUP_get_degree(group) + 7) / 8;
return 1; return 1;
} }
pubkey = EC_KEY_get0_public_key(ctx->peerkey->pkey.ec); pubkey = EC_KEY_get0_public_key(eckeypub);
/* /*
* NB: unlike PKCS#3 DH, if *outlen is less than maximum size this is not * NB: unlike PKCS#3 DH, if *outlen is less than maximum size this is not

View File

@ -766,7 +766,7 @@ static int validate_ecx_derive(EVP_PKEY_CTX *ctx, unsigned char *key,
return 0; return 0;
} }
ecxkey = ctx->pkey->pkey.ecx; ecxkey = ctx->pkey->pkey.ecx;
peerkey = ctx->peerkey->pkey.ecx; peerkey = EVP_PKEY_get0(ctx->peerkey);
if (ecxkey == NULL || ecxkey->privkey == NULL) { if (ecxkey == NULL || ecxkey->privkey == NULL) {
ERR_raise(ERR_LIB_EC, EC_R_INVALID_PRIVATE_KEY); ERR_raise(ERR_LIB_EC, EC_R_INVALID_PRIVATE_KEY);
return 0; return 0;

View File

@ -349,10 +349,6 @@ int EVP_PKEY_derive_set_peer(EVP_PKEY_CTX *ctx, EVP_PKEY *peer)
#ifdef FIPS_MODULE #ifdef FIPS_MODULE
return ret; return ret;
#else #else
/*
* TODO(3.0) investigate the case where the operation is deemed legacy,
* but the given peer key is provider only.
*/
if (ctx->pmeth == NULL if (ctx->pmeth == NULL
|| !(ctx->pmeth->derive != NULL || !(ctx->pmeth->derive != NULL
|| ctx->pmeth->encrypt != NULL || ctx->pmeth->encrypt != NULL