Normalize all EVP_PKEY_CTX_ctrl return values. No code within BoringSSL or Google (grep for EVP_PKEY_CTX_(ctrl|get|set)) is sensitive to the various failure cases. Normalize it all to 0/1 for simplicity. This does carry a slight risk: any new ctrl hooks we import from upstream that, like EVP_PKEY_CTX_get_rsa_oaep_md, return something other than success/failure cannot be called directly via EVP_PKEY_CTX_ctrl. They instead need to internally be routed through a struct like CBS and only called through the wrappers. To that end, unexport EVP_PKEY_CTX_ctrl and require that callers use the wrappers. No code in Google uses it directly and, if need be, switching to the wrapper would be an incredibly upstreamable patch. Change-Id: I3fd4e5a1a0f3d4d1c4122c52d4c74a5105b99cd5 Reviewed-on: https://boringssl-review.googlesource.com/3874 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/evp/evp_ctx.c b/crypto/evp/evp_ctx.c index daf1a28..e94ca1c 100644 --- a/crypto/evp/evp_ctx.c +++ b/crypto/evp/evp_ctx.c
@@ -214,20 +214,20 @@ int p1, void *p2) { if (!ctx || !ctx->pmeth || !ctx->pmeth->ctrl) { OPENSSL_PUT_ERROR(EVP, EVP_PKEY_CTX_ctrl, EVP_R_COMMAND_NOT_SUPPORTED); - return -2; + return 0; } if (keytype != -1 && ctx->pmeth->pkey_id != keytype) { - return -1; + return 0; } if (ctx->operation == EVP_PKEY_OP_UNDEFINED) { OPENSSL_PUT_ERROR(EVP, EVP_PKEY_CTX_ctrl, EVP_R_NO_OPERATION_SET); - return -1; + return 0; } if (optype != -1 && !(ctx->operation & optype)) { OPENSSL_PUT_ERROR(EVP, EVP_PKEY_CTX_ctrl, EVP_R_INVALID_OPERATION); - return -1; + return 0; } return ctx->pmeth->ctrl(ctx, cmd, p1, p2);
diff --git a/crypto/evp/internal.h b/crypto/evp/internal.h index 2b0f608..08a7bfb 100644 --- a/crypto/evp/internal.h +++ b/crypto/evp/internal.h
@@ -170,8 +170,49 @@ #define EVP_PKEY_OP_TYPE_GEN (EVP_PKEY_OP_PARAMGEN | EVP_PKEY_OP_KEYGEN) +/* EVP_PKEY_CTX_ctrl performs |cmd| on |ctx|. The |keytype| and |optype| + * arguments can be -1 to specify that any type and operation are acceptable, + * otherwise |keytype| must match the type of |ctx| and the bits of |optype| + * must intersect the operation flags set on |ctx|. + * + * The |p1| and |p2| arguments depend on the value of |cmd|. + * + * It returns one on success and zero on error. */ +OPENSSL_EXPORT int EVP_PKEY_CTX_ctrl(EVP_PKEY_CTX *ctx, int keytype, int optype, + int cmd, int p1, void *p2); + +/* EVP_PKEY_CTRL_DIGESTINIT is an internal value. It's called by + * EVP_DigestInit_ex to signal the |EVP_PKEY| that a digest operation is + * starting. + * + * TODO(davidben): This is only needed to support the deprecated HMAC |EVP_PKEY| + * types. */ +#define EVP_PKEY_CTRL_DIGESTINIT 3 + +/* EVP_PKEY_CTRL_PEER_KEY is called with different values of |p1|: + * 0: Is called from |EVP_PKEY_derive_set_peer| and |p2| contains a peer key. + * If the return value is <= 0, the key is rejected. + * 1: Is called at the end of |EVP_PKEY_derive_set_peer| and |p2| contains a + * peer key. If the return value is <= 0, the key is rejected. + * 2: Is called with |p2| == NULL to test whether the peer's key was used. + * (EC)DH always return one in this case. + * 3: Is called with |p2| == NULL to set whether the peer's key was used. + * (EC)DH always return one in this case. This was only used for GOST. */ +#define EVP_PKEY_CTRL_PEER_KEY 4 + +/* EVP_PKEY_CTRL_SET_MAC_KEY sets a MAC key. For example, this can be done an + * |EVP_PKEY_CTX| prior to calling |EVP_PKEY_keygen| in order to generate an + * HMAC |EVP_PKEY| with the given key. It returns one on success and zero on + * error. */ +#define EVP_PKEY_CTRL_SET_MAC_KEY 5 + +/* EVP_PKEY_ALG_CTRL is the base value from which key-type specific ctrl + * commands are numbered. */ +#define EVP_PKEY_ALG_CTRL 0x1000 + #define EVP_PKEY_CTRL_MD 1 #define EVP_PKEY_CTRL_GET_MD 2 + #define EVP_PKEY_CTRL_RSA_PADDING (EVP_PKEY_ALG_CTRL + 1) #define EVP_PKEY_CTRL_GET_RSA_PADDING (EVP_PKEY_ALG_CTRL + 2) #define EVP_PKEY_CTRL_RSA_PSS_SALTLEN (EVP_PKEY_ALG_CTRL + 3) @@ -185,6 +226,8 @@ #define EVP_PKEY_CTRL_RSA_OAEP_LABEL (EVP_PKEY_ALG_CTRL + 11) #define EVP_PKEY_CTRL_GET_RSA_OAEP_LABEL (EVP_PKEY_ALG_CTRL + 12) +#define EVP_PKEY_CTRL_EC_PARAMGEN_CURVE_NID (EVP_PKEY_ALG_CTRL + 1) + struct evp_pkey_ctx_st { /* Method associated with this operation */ const EVP_PKEY_METHOD *pmeth;
diff --git a/crypto/evp/p_ec.c b/crypto/evp/p_ec.c index 0fe805d..bbf470b 100644 --- a/crypto/evp/p_ec.c +++ b/crypto/evp/p_ec.c
@@ -242,7 +242,7 @@ default: OPENSSL_PUT_ERROR(EVP, pkey_ec_ctrl, EVP_R_COMMAND_NOT_SUPPORTED); - return -2; + return 0; } }
diff --git a/crypto/evp/p_hmac.c b/crypto/evp/p_hmac.c index 89859b4..21703ed 100644 --- a/crypto/evp/p_hmac.c +++ b/crypto/evp/p_hmac.c
@@ -205,7 +205,7 @@ default: OPENSSL_PUT_ERROR(EVP, pkey_hmac_ctrl, EVP_R_COMMAND_NOT_SUPPORTED); - return -2; + return 0; } return 1; }
diff --git a/crypto/evp/p_rsa.c b/crypto/evp/p_rsa.c index be2229b..89f5739 100644 --- a/crypto/evp/p_rsa.c +++ b/crypto/evp/p_rsa.c
@@ -371,7 +371,7 @@ 0 == (ctx->operation & EVP_PKEY_OP_TYPE_CRYPT))) { OPENSSL_PUT_ERROR(EVP, pkey_rsa_ctrl, EVP_R_ILLEGAL_OR_UNSUPPORTED_PADDING_MODE); - return -2; + return 0; } if ((p1 == RSA_PKCS1_PSS_PADDING || p1 == RSA_PKCS1_OAEP_PADDING) && rctx->md == NULL) { @@ -388,13 +388,13 @@ case EVP_PKEY_CTRL_GET_RSA_PSS_SALTLEN: if (rctx->pad_mode != RSA_PKCS1_PSS_PADDING) { OPENSSL_PUT_ERROR(EVP, pkey_rsa_ctrl, EVP_R_INVALID_PSS_SALTLEN); - return -2; + return 0; } if (type == EVP_PKEY_CTRL_GET_RSA_PSS_SALTLEN) { *(int *)p2 = rctx->saltlen; } else { if (p1 < -2) { - return -2; + return 0; } rctx->saltlen = p1; } @@ -403,14 +403,14 @@ case EVP_PKEY_CTRL_RSA_KEYGEN_BITS: if (p1 < 256) { OPENSSL_PUT_ERROR(EVP, pkey_rsa_ctrl, EVP_R_INVALID_KEYBITS); - return -2; + return 0; } rctx->nbits = p1; return 1; case EVP_PKEY_CTRL_RSA_KEYGEN_PUBEXP: if (!p2) { - return -2; + return 0; } BN_free(rctx->pub_exp); rctx->pub_exp = p2; @@ -420,7 +420,7 @@ case EVP_PKEY_CTRL_GET_RSA_OAEP_MD: if (rctx->pad_mode != RSA_PKCS1_OAEP_PADDING) { OPENSSL_PUT_ERROR(EVP, pkey_rsa_ctrl, EVP_R_INVALID_PADDING_MODE); - return -2; + return 0; } if (type == EVP_PKEY_CTRL_GET_RSA_OAEP_MD) { *(const EVP_MD **)p2 = rctx->md; @@ -445,7 +445,7 @@ if (rctx->pad_mode != RSA_PKCS1_PSS_PADDING && rctx->pad_mode != RSA_PKCS1_OAEP_PADDING) { OPENSSL_PUT_ERROR(EVP, pkey_rsa_ctrl, EVP_R_INVALID_MGF1_MD); - return -2; + return 0; } if (type == EVP_PKEY_CTRL_GET_RSA_MGF1_MD) { if (rctx->mgf1md) { @@ -461,7 +461,7 @@ case EVP_PKEY_CTRL_RSA_OAEP_LABEL: if (rctx->pad_mode != RSA_PKCS1_OAEP_PADDING) { OPENSSL_PUT_ERROR(EVP, pkey_rsa_ctrl, EVP_R_INVALID_PADDING_MODE); - return -2; + return 0; } if (rctx->oaep_label) { OPENSSL_free(rctx->oaep_label); @@ -480,7 +480,7 @@ case EVP_PKEY_CTRL_GET_RSA_OAEP_LABEL: if (rctx->pad_mode != RSA_PKCS1_OAEP_PADDING) { OPENSSL_PUT_ERROR(EVP, pkey_rsa_ctrl, EVP_R_INVALID_PADDING_MODE); - return -2; + return 0; } CBS_init((CBS *)p2, rctx->oaep_label, rctx->oaep_labellen); return 1; @@ -490,7 +490,7 @@ default: OPENSSL_PUT_ERROR(EVP, pkey_rsa_ctrl, EVP_R_COMMAND_NOT_SUPPORTED); - return -2; + return 0; } } @@ -587,7 +587,7 @@ size_t label_len) { int label_len_int = label_len; if (((size_t) label_len_int) != label_len) { - return -2; + return 0; } return EVP_PKEY_CTX_ctrl(ctx, EVP_PKEY_RSA, EVP_PKEY_OP_TYPE_CRYPT,
diff --git a/include/openssl/evp.h b/include/openssl/evp.h index 1e17a46..b1497f1 100644 --- a/include/openssl/evp.h +++ b/include/openssl/evp.h
@@ -444,18 +444,6 @@ * set. */ OPENSSL_EXPORT void *EVP_PKEY_CTX_get_app_data(EVP_PKEY_CTX *ctx); -/* EVP_PKEY_CTX_ctrl performs |cmd| on |ctx|. The |keytype| and |optype| - * arguments can be -1 to specify that any type and operation are acceptable, - * otherwise |keytype| must match the type of |ctx| and the bits of |optype| - * must intersect the operation flags set on |ctx|. - * - * The |p1| and |p2| arguments depend on the value of |cmd|. - * - * It returns -2 if |cmd| is not recognised, -1 on error or a |cmd| specific - * value otherwise. */ -OPENSSL_EXPORT int EVP_PKEY_CTX_ctrl(EVP_PKEY_CTX *ctx, int keytype, int optype, - int cmd, int p1, void *p2); - /* EVP_PKEY_sign_init initialises an |EVP_PKEY_CTX| for a signing operation. It * should be called before |EVP_PKEY_sign|. * @@ -569,67 +557,28 @@ OPENSSL_EXPORT int EVP_PKEY_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY **ppkey); -/* EVP_PKEY_CTX_ctrl operations. - * - * These values are passed as the |cmd| argument to - * EVP_PKEY_CTX_ctrl */ - -/* Generic. */ +/* Generic control functions. */ /* EVP_PKEY_CTX_set_signature_md sets |md| as the digest to be used in a - * signature operation. It returns one on success or otherwise on error. See - * the return values of |EVP_PKEY_CTX_ctrl| for details. */ + * signature operation. It returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_set_signature_md(EVP_PKEY_CTX *ctx, const EVP_MD *md); /* EVP_PKEY_CTX_get_signature_md sets |*out_md| to the digest to be used in a - * signature operation. It returns one on success or otherwise on error. See - * the return values of |EVP_PKEY_CTX_ctrl| for details. */ + * signature operation. It returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_get_signature_md(EVP_PKEY_CTX *ctx, const EVP_MD **out_md); -/* EVP_PKEY_CTRL_DIGESTINIT is an internal value. It's called by - * EVP_DigestInit_ex to signal the |EVP_PKEY| that a digest operation is - * starting. - * - * TODO(davidben): This is only needed to support the deprecated HMAC |EVP_PKEY| - * types. */ -#define EVP_PKEY_CTRL_DIGESTINIT 3 - -/* EVP_PKEY_CTRL_PEER_KEY is called with different values of |p1|: - * 0: Is called from |EVP_PKEY_derive_set_peer| and |p2| contains a peer key. - * If the return value is <= 0, the key is rejected. - * 1: Is called at the end of |EVP_PKEY_derive_set_peer| and |p2| contains a - * peer key. If the return value is <= 0, the key is rejected. - * 2: Is called with |p2| == NULL to test whether the peer's key was used. - * (EC)DH always return one in this case. - * 3: Is called with |p2| == NULL to set whether the peer's key was used. - * (EC)DH always return one in this case. This was only used for GOST. */ -#define EVP_PKEY_CTRL_PEER_KEY 4 - -/* EVP_PKEY_CTRL_SET_MAC_KEY sets a MAC key. For example, this can be done an - * |EVP_PKEY_CTX| prior to calling |EVP_PKEY_keygen| in order to generate an - * HMAC |EVP_PKEY| with the given key. It returns one on success and zero on - * error. */ -#define EVP_PKEY_CTRL_SET_MAC_KEY 5 - -/* EVP_PKEY_ALG_CTRL is the base value from which key-type specific ctrl - * commands are numbered. */ -#define EVP_PKEY_ALG_CTRL 0x1000 - /* RSA specific control functions. */ /* EVP_PKEY_CTX_set_rsa_padding sets the padding type to use. It should be one - * of the |RSA_*_PADDING| values. Returns one on success or another value on - * error. See |EVP_PKEY_CTX_ctrl| for the other return values, which are - * non-standard. */ + * of the |RSA_*_PADDING| values. Returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_set_rsa_padding(EVP_PKEY_CTX *ctx, int padding); /* EVP_PKEY_CTX_get_rsa_padding sets |*out_padding| to the current padding * value, which is one of the |RSA_*_PADDING| values. Returns one on success or - * another value on error. See |EVP_PKEY_CTX_ctrl| for the other return values, - * which are non-standard. */ + * zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_get_rsa_padding(EVP_PKEY_CTX *ctx, int *out_padding); @@ -638,8 +587,7 @@ * in the signature. A value of -2 causes the salt to be the maximum length * that will fit. Otherwise the value gives the size of the salt in bytes. * - * Returns one on success or another value on error. See |EVP_PKEY_CTX_ctrl| - * for the other return values, which are non-standard. */ + * Returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_set_rsa_pss_saltlen(EVP_PKEY_CTX *ctx, int salt_len); @@ -648,70 +596,59 @@ * |EVP_PKEY_CTX_set_rsa_pss_saltlen| for details of the special values that it * can take. * - * Returns one on success or another value on error. See |EVP_PKEY_CTX_ctrl| - * for the other return values, which are non-standard. */ + * Returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_get_rsa_pss_saltlen(EVP_PKEY_CTX *ctx, int *out_salt_len); /* EVP_PKEY_CTX_set_rsa_keygen_bits sets the size of the desired RSA modulus, - * in bits, for key generation. Returns one on success or another value on - * error. See |EVP_PKEY_CTX_ctrl| for the other return values, which are - * non-standard. */ + * in bits, for key generation. Returns one on success or zero on + * error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_set_rsa_keygen_bits(EVP_PKEY_CTX *ctx, int bits); /* EVP_PKEY_CTX_set_rsa_keygen_pubexp sets |e| as the public exponent for key - * generation. Returns one on success or another value on error. See - * |EVP_PKEY_CTX_ctrl| for the other return values, which are non-standard. */ + * generation. Returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *e); /* EVP_PKEY_CTX_set_rsa_oaep_md sets |md| as the digest used in OAEP padding. - * Returns one on success or another value on error. See |EVP_PKEY_CTX_ctrl| - * for the other return values, which are non-standard. */ + * Returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_set_rsa_oaep_md(EVP_PKEY_CTX *ctx, const EVP_MD *md); /* EVP_PKEY_CTX_get_rsa_oaep_md sets |*out_md| to the digest function used in - * OAEP padding. Returns one on success or another value on error. See - * |EVP_PKEY_CTX_ctrl| for the other return values, which are non-standard. */ + * OAEP padding. Returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_get_rsa_oaep_md(EVP_PKEY_CTX *ctx, const EVP_MD **out_md); /* EVP_PKEY_CTX_set_rsa_mgf1_md sets |md| as the digest used in MGF1. Returns - * one on success or another value on error. See |EVP_PKEY_CTX_ctrl| for the - * other return values, which are non-standard. */ + * one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_set_rsa_mgf1_md(EVP_PKEY_CTX *ctx, const EVP_MD *md); /* EVP_PKEY_CTX_get_rsa_mgf1_md sets |*out_md| to the digest function used in - * MGF1. Returns one on success or another value on error. See - * |EVP_PKEY_CTX_ctrl| for the other return values, which are non-standard. */ + * MGF1. Returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_get_rsa_mgf1_md(EVP_PKEY_CTX *ctx, const EVP_MD **out_md); /* EVP_PKEY_CTX_set0_rsa_oaep_label sets |label_len| bytes from |label| as the - * label used in OAEP. DANGER: this call takes ownership of |label| and will - * call |free| on it when |ctx| is destroyed. + * label used in OAEP. DANGER: On success, this call takes ownership of |label| + * and will call |OPENSSL_free| on it when |ctx| is destroyed. * - * Returns one on success or another value on error. See |EVP_PKEY_CTX_ctrl| - * for the other return values, which are non-standard. */ + * Returns one on success or zero on error. */ OPENSSL_EXPORT int EVP_PKEY_CTX_set0_rsa_oaep_label(EVP_PKEY_CTX *ctx, const uint8_t *label, size_t label_len); /* EVP_PKEY_CTX_get0_rsa_oaep_label sets |*out_label| to point to the internal * buffer containing the OAEP label (which may be NULL) and returns the length - * of the label or a negative value on error. */ + * of the label or a negative value on error. + * + * WARNING: the return value differs from the usual return value convention. */ OPENSSL_EXPORT int EVP_PKEY_CTX_get0_rsa_oaep_label(EVP_PKEY_CTX *ctx, const uint8_t **out_label); -/* EC specific */ - -#define EVP_PKEY_CTRL_EC_PARAMGEN_CURVE_NID (EVP_PKEY_ALG_CTRL + 1) - - /* Private functions */ /* OpenSSL_add_all_algorithms does nothing. */