Deprecate the EVP_PKEY controls for CMS and PKCS#7

Improve the ossl_rsa_check_key() to prevent non-signature
operations with PSS keys.

Do not invoke the EVP_PKEY controls for CMS and PKCS#7 anymore
as they are not needed anymore and deprecate them.

Fixes #14276

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/14760)
This commit is contained in:
Tomas Mraz 2021-04-01 17:14:43 +02:00
parent 5ad3e6c56e
commit 0cfbc828e0
11 changed files with 79 additions and 175 deletions

View File

@ -31,6 +31,15 @@ OpenSSL 3.0
*Shane Lontis*
* The EVP_PKEY_CTRL_PKCS7_ENCRYPT, EVP_PKEY_CTRL_PKCS7_DECRYPT,
EVP_PKEY_CTRL_PKCS7_SIGN, EVP_PKEY_CTRL_CMS_ENCRYPT,
EVP_PKEY_CTRL_CMS_DECRYPT, and EVP_PKEY_CTRL_CMS_SIGN control operations
are deprecated. They are not invoked by the OpenSSL library anymore and
are replaced by direct checks of the key operation against the key type
when the operation is initialized.
*Tomáš Mráz*
* The EVP_PKEY_public_check() and EVP_PKEY_param_check() functions now work for
more key types including RSA, DSA, ED25519, X25519, ED448 and X448.
Previously (in 1.1.1) they would return -2. For key types that do not have

View File

@ -485,12 +485,6 @@ static int cms_RecipientInfo_ktri_encrypt(const CMS_ContentInfo *cms,
goto err;
}
if (EVP_PKEY_CTX_ctrl(pctx, -1, EVP_PKEY_OP_ENCRYPT,
EVP_PKEY_CTRL_CMS_ENCRYPT, 0, ri) <= 0) {
ERR_raise(ERR_LIB_CMS, CMS_R_CTRL_ERROR);
goto err;
}
if (EVP_PKEY_encrypt(pctx, NULL, &eklen, ec->key, ec->keylen) <= 0)
goto err;
@ -574,12 +568,6 @@ static int cms_RecipientInfo_ktri_decrypt(CMS_ContentInfo *cms,
if (!ossl_cms_env_asn1_ctrl(ri, 1))
goto err;
if (EVP_PKEY_CTX_ctrl(ktri->pctx, -1, EVP_PKEY_OP_DECRYPT,
EVP_PKEY_CTRL_CMS_DECRYPT, 0, ri) <= 0) {
ERR_raise(ERR_LIB_CMS, CMS_R_CTRL_ERROR);
goto err;
}
if (EVP_PKEY_decrypt(ktri->pctx, NULL, &eklen,
ktri->encryptedKey->data,
ktri->encryptedKey->length) <= 0)

View File

@ -749,24 +749,6 @@ int CMS_SignerInfo_sign(CMS_SignerInfo *si)
si->pctx = pctx;
}
/*
* TODO(3.0): This causes problems when providers are in use, so disabled
* for now. Can we get rid of this completely? AFAICT this ctrl has been
* present since CMS was first put in - but has never been used to do
* anything. All internal implementations just return 1 and ignore this ctrl
* and have always done so by the looks of things. To fix this we could
* convert this ctrl into a param, which would require us to send all the
* signer info data as a set of params...but that is non-trivial and since
* this isn't used by anything it may be better just to remove it.
*/
#if 0
if (EVP_PKEY_CTX_ctrl(pctx, -1, EVP_PKEY_OP_SIGN,
EVP_PKEY_CTRL_CMS_SIGN, 0, si) <= 0) {
ERR_raise(ERR_LIB_CMS, CMS_R_CTRL_ERROR);
goto err;
}
#endif
alen = ASN1_item_i2d((ASN1_VALUE *)si->signedAttrs, &abuf,
ASN1_ITEM_rptr(CMS_Attributes_Sign));
if (!abuf)
@ -782,24 +764,6 @@ int CMS_SignerInfo_sign(CMS_SignerInfo *si)
if (EVP_DigestSignFinal(mctx, abuf, &siglen) <= 0)
goto err;
/*
* TODO(3.0): This causes problems when providers are in use, so disabled
* for now. Can we get rid of this completely? AFAICT this ctrl has been
* present since CMS was first put in - but has never been used to do
* anything. All internal implementations just return 1 and ignore this ctrl
* and have always done so by the looks of things. To fix this we could
* convert this ctrl into a param, which would require us to send all the
* signer info data as a set of params...but that is non-trivial and since
* this isn't used by anything it may be better just to remove it.
*/
#if 0
if (EVP_PKEY_CTX_ctrl(pctx, -1, EVP_PKEY_OP_SIGN,
EVP_PKEY_CTRL_CMS_SIGN, 1, si) <= 0) {
ERR_raise(ERR_LIB_CMS, CMS_R_CTRL_ERROR);
goto err;
}
#endif
EVP_MD_CTX_reset(mctx);
ASN1_STRING_set0(si->signature, abuf, siglen);

View File

@ -1432,34 +1432,6 @@ static int fix_hkdf_mode(enum state state,
return 1;
}
static int hack_pkcs7_cms(enum state state,
const struct translation_st *translation,
struct translation_ctx_st *ctx)
{
int ret = 1;
/* Make sure that this has no further effect */
ctx->action_type = 0;
switch (state) {
case PRE_CTRL_TO_PARAMS:
/* TODO (3.0) Temporary hack, this should probe */
if (EVP_PKEY_is_a(EVP_PKEY_CTX_get0_pkey(ctx->pctx), "RSASSA-PSS")) {
ERR_raise(ERR_LIB_EVP,
EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
ret = -2;
}
break;
case POST_CTRL_TO_PARAMS:
break;
default:
ERR_raise(ERR_LIB_EVP, EVP_R_COMMAND_NOT_SUPPORTED);
ret = -2;
break;
}
return ret;
}
/*-
* Payload getters
* ===============
@ -2121,16 +2093,6 @@ static const struct translation_st evp_pkey_ctx_translations[] = {
EVP_PKEY_CTRL_RSA_KEYGEN_PRIMES, "rsa_keygen_primes", NULL,
OSSL_PKEY_PARAM_RSA_PRIMES, OSSL_PARAM_UNSIGNED_INTEGER, NULL },
/* PKCS#7 and CMS hacks */
{ SET, -1, -1, EVP_PKEY_OP_ENCRYPT,
EVP_PKEY_CTRL_PKCS7_ENCRYPT, NULL, NULL, NULL, 0, hack_pkcs7_cms },
{ SET, -1, -1, EVP_PKEY_OP_DECRYPT,
EVP_PKEY_CTRL_PKCS7_DECRYPT, NULL, NULL, NULL, 0, hack_pkcs7_cms },
{ SET, -1, -1, EVP_PKEY_OP_ENCRYPT,
EVP_PKEY_CTRL_CMS_ENCRYPT, NULL, NULL, NULL, 0, hack_pkcs7_cms },
{ SET, -1, -1, EVP_PKEY_OP_DECRYPT,
EVP_PKEY_CTRL_CMS_DECRYPT, NULL, NULL, NULL, 0, hack_pkcs7_cms },
/*-
* TLS1-PRF
* ========

View File

@ -122,12 +122,6 @@ static int pkcs7_encode_rinfo(PKCS7_RECIP_INFO *ri,
if (EVP_PKEY_encrypt_init(pctx) <= 0)
goto err;
if (EVP_PKEY_CTX_ctrl(pctx, -1, EVP_PKEY_OP_ENCRYPT,
EVP_PKEY_CTRL_PKCS7_ENCRYPT, 0, ri) <= 0) {
ERR_raise(ERR_LIB_PKCS7, PKCS7_R_CTRL_ERROR);
goto err;
}
if (EVP_PKEY_encrypt(pctx, NULL, &eklen, key, keylen) <= 0)
goto err;
@ -171,12 +165,6 @@ static int pkcs7_decrypt_rinfo(unsigned char **pek, int *peklen,
if (EVP_PKEY_decrypt_init(pctx) <= 0)
goto err;
if (EVP_PKEY_CTX_ctrl(pctx, -1, EVP_PKEY_OP_DECRYPT,
EVP_PKEY_CTRL_PKCS7_DECRYPT, 0, ri) <= 0) {
ERR_raise(ERR_LIB_PKCS7, PKCS7_R_CTRL_ERROR);
goto err;
}
if (EVP_PKEY_decrypt(pctx, NULL, &eklen,
ri->enc_key->data, ri->enc_key->length) <= 0)
goto err;
@ -932,30 +920,6 @@ int PKCS7_SIGNER_INFO_sign(PKCS7_SIGNER_INFO *si)
NULL) <= 0)
goto err;
/*
* TODO(3.0): This causes problems when providers are in use, so disabled
* for now. Can we get rid of this completely? AFAICT this ctrl has never
* been used since it was first put in. All internal implementations just
* return 1 and ignore this ctrl and have always done so by the looks of
* things. To fix this we could convert this ctrl into a param, which would
* require us to send all the signer info data as a set of params...but that
* is non-trivial and since this isn't used by anything it may be better
* just to remove it. The original commit that added it had this
* justification in CHANGES:
*
* "During PKCS7 signing pass the PKCS7 SignerInfo structure to the
* EVP_PKEY_METHOD before and after signing via the
* EVP_PKEY_CTRL_PKCS7_SIGN ctrl. It can then customise the structure
* before and/or after signing if necessary."
*/
#if 0
if (EVP_PKEY_CTX_ctrl(pctx, -1, EVP_PKEY_OP_SIGN,
EVP_PKEY_CTRL_PKCS7_SIGN, 0, si) <= 0) {
ERR_raise(ERR_LIB_PKCS7, PKCS7_R_CTRL_ERROR);
goto err;
}
#endif
alen = ASN1_item_i2d((ASN1_VALUE *)si->auth_attr, &abuf,
ASN1_ITEM_rptr(PKCS7_ATTR_SIGN));
if (!abuf)
@ -972,30 +936,6 @@ int PKCS7_SIGNER_INFO_sign(PKCS7_SIGNER_INFO *si)
if (EVP_DigestSignFinal(mctx, abuf, &siglen) <= 0)
goto err;
/*
* TODO(3.0): This causes problems when providers are in use, so disabled
* for now. Can we get rid of this completely? AFAICT this ctrl has never
* been used since it was first put in. All internal implementations just
* return 1 and ignore this ctrl and have always done so by the looks of
* things. To fix this we could convert this ctrl into a param, which would
* require us to send all the signer info data as a set of params...but that
* is non-trivial and since this isn't used by anything it may be better
* just to remove it. The original commit that added it had this
* justification in CHANGES:
*
* "During PKCS7 signing pass the PKCS7 SignerInfo structure to the
* EVP_PKEY_METHOD before and after signing via the
* EVP_PKEY_CTRL_PKCS7_SIGN ctrl. It can then customise the structure
* before and/or after signing if necessary."
*/
#if 0
if (EVP_PKEY_CTX_ctrl(pctx, -1, EVP_PKEY_OP_SIGN,
EVP_PKEY_CTRL_PKCS7_SIGN, 1, si) <= 0) {
ERR_raise(ERR_LIB_PKCS7, PKCS7_R_CTRL_ERROR);
goto err;
}
#endif
EVP_MD_CTX_free(mctx);
ASN1_STRING_set0(si->enc_digest, abuf, siglen);

View File

@ -1632,16 +1632,18 @@ int EVP_PKEY_CTX_set_mac_key(EVP_PKEY_CTX *ctx, const unsigned char *key,
# define EVP_PKEY_CTRL_MD 1
# define EVP_PKEY_CTRL_PEER_KEY 2
# define EVP_PKEY_CTRL_PKCS7_ENCRYPT 3
# define EVP_PKEY_CTRL_PKCS7_DECRYPT 4
# define EVP_PKEY_CTRL_PKCS7_SIGN 5
# define EVP_PKEY_CTRL_SET_MAC_KEY 6
# define EVP_PKEY_CTRL_DIGESTINIT 7
/* Used by GOST key encryption in TLS */
# define EVP_PKEY_CTRL_SET_IV 8
# define EVP_PKEY_CTRL_CMS_ENCRYPT 9
# define EVP_PKEY_CTRL_CMS_DECRYPT 10
# define EVP_PKEY_CTRL_CMS_SIGN 11
# ifndef OPENSSL_NO_DEPRECATED_3_0
# define EVP_PKEY_CTRL_PKCS7_ENCRYPT 3
# define EVP_PKEY_CTRL_PKCS7_DECRYPT 4
# define EVP_PKEY_CTRL_PKCS7_SIGN 5
# define EVP_PKEY_CTRL_CMS_ENCRYPT 9
# define EVP_PKEY_CTRL_CMS_DECRYPT 10
# define EVP_PKEY_CTRL_CMS_SIGN 11
# endif
# define EVP_PKEY_CTRL_CIPHER 12
# define EVP_PKEY_CTRL_GET_MD 13
# define EVP_PKEY_CTRL_SET_DIGEST_SIZE 14

View File

@ -10,7 +10,7 @@
#include "crypto/types.h"
/* Functions that are common */
int ossl_rsa_check_key(const RSA *rsa, int protect);
int ossl_rsa_check_key(const RSA *rsa, int operation);
int ossl_ec_check_key(const EC_KEY *ec, int protect);
int ossl_dsa_check_key(const DSA *dsa, int sign);
int ossl_dh_check_key(const DH *dh);

View File

@ -13,6 +13,7 @@
#include <openssl/dsa.h>
#include <openssl/dh.h>
#include <openssl/ec.h>
#include <openssl/evp.h>
#include <openssl/err.h>
#include <openssl/proverr.h>
#include <openssl/core_names.h>
@ -25,14 +26,50 @@
* Set protect = 1 for encryption or signing operations, or 0 otherwise. See
* https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf.
*/
int ossl_rsa_check_key(const RSA *rsa, int protect)
int ossl_rsa_check_key(const RSA *rsa, int operation)
{
int protect = 0;
switch (operation) {
case EVP_PKEY_OP_SIGN:
protect = 1;
/* fallthrough */
case EVP_PKEY_OP_VERIFY:
break;
case EVP_PKEY_OP_ENCAPSULATE:
case EVP_PKEY_OP_ENCRYPT:
protect = 1;
/* fallthrough */
case EVP_PKEY_OP_VERIFYRECOVER:
case EVP_PKEY_OP_DECAPSULATE:
case EVP_PKEY_OP_DECRYPT:
if (RSA_test_flags(rsa,
RSA_FLAG_TYPE_MASK) == RSA_FLAG_TYPE_RSASSAPSS) {
ERR_raise_data(ERR_LIB_PROV,
PROV_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE,
"operation: %d", operation);
return 0;
}
break;
default:
ERR_raise_data(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR,
"invalid operation: %d", operation);
return 0;
}
#if !defined(OPENSSL_NO_FIPS_SECURITYCHECKS)
if (ossl_securitycheck_enabled()) {
int sz = RSA_bits(rsa);
return protect ? (sz >= 2048) : (sz >= 1024);
if (protect ? (sz < 2048) : (sz < 1024)) {
ERR_raise_data(ERR_LIB_PROV, PROV_R_INVALID_KEY_LENGTH,
"operation: %d", operation);
return 0;
}
}
#else
/* make protect used */
(void)protect;
#endif /* OPENSSL_NO_FIPS_SECURITYCHECKS */
return 1;
}

View File

@ -96,10 +96,13 @@ static int rsa_init(void *vprsactx, void *vrsa, const OSSL_PARAM params[],
{
PROV_RSA_CTX *prsactx = (PROV_RSA_CTX *)vprsactx;
if (!ossl_prov_is_running()
|| prsactx == NULL
|| vrsa == NULL
|| !RSA_up_ref(vrsa))
if (!ossl_prov_is_running() || prsactx == NULL || vrsa == NULL)
return 0;
if (!ossl_rsa_check_key(vrsa, operation))
return 0;
if (!RSA_up_ref(vrsa))
return 0;
RSA_free(prsactx->rsa);
prsactx->rsa = vrsa;
@ -110,11 +113,8 @@ static int rsa_init(void *vprsactx, void *vrsa, const OSSL_PARAM params[],
prsactx->pad_mode = RSA_PKCS1_PADDING;
break;
default:
ERR_raise(ERR_LIB_PROV, PROV_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
return 0;
}
if (!ossl_rsa_check_key(vrsa, operation == EVP_PKEY_OP_ENCRYPT)) {
ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_KEY_LENGTH);
/* This should not happen due to the check above */
ERR_raise(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR);
return 0;
}
return rsa_set_ctx_params(prsactx, params);

View File

@ -122,15 +122,17 @@ static int rsakem_init(void *vprsactx, void *vrsa,
{
PROV_RSA_CTX *prsactx = (PROV_RSA_CTX *)vprsactx;
if (prsactx == NULL || vrsa == NULL || !RSA_up_ref(vrsa))
if (prsactx == NULL || vrsa == NULL)
return 0;
if (!ossl_rsa_check_key(vrsa, operation))
return 0;
if (!RSA_up_ref(vrsa))
return 0;
RSA_free(prsactx->rsa);
prsactx->rsa = vrsa;
if (!ossl_rsa_check_key(vrsa, operation == EVP_PKEY_OP_ENCAPSULATE)) {
ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_KEY_LENGTH);
return 0;
}
return rsakem_set_ctx_params(prsactx, params);
}

View File

@ -365,9 +365,14 @@ static int rsa_signverify_init(void *vprsactx, void *vrsa,
if (!ossl_prov_is_running())
return 0;
if (prsactx == NULL || vrsa == NULL || !RSA_up_ref(vrsa))
if (prsactx == NULL || vrsa == NULL)
return 0;
if (!ossl_rsa_check_key(vrsa, operation))
return 0;
if (!RSA_up_ref(vrsa))
return 0;
RSA_free(prsactx->rsa);
prsactx->rsa = vrsa;
prsactx->operation = operation;
@ -375,11 +380,6 @@ static int rsa_signverify_init(void *vprsactx, void *vrsa,
if (!rsa_set_ctx_params(prsactx, params))
return 0;
if (!ossl_rsa_check_key(vrsa, operation == EVP_PKEY_OP_SIGN)) {
ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_KEY_LENGTH);
return 0;
}
/* Maximum for sign, auto for verify */
prsactx->saltlen = RSA_PSS_SALTLEN_AUTO;
prsactx->min_saltlen = -1;