Use X509_get0_pubkey to simplify things slightly Also X509_REQ_check_private_key didn't handle unknown key type case. Clean those up and align with X509_check_private_key. Change-Id: I7b16f85662943e4b226221a01e1092cf62afc643 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65051 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/t_req.c b/crypto/x509/t_req.c index 3cfab19..bc41b45 100644 --- a/crypto/x509/t_req.c +++ b/crypto/x509/t_req.c
@@ -80,7 +80,6 @@ int X509_REQ_print_ex(BIO *bio, X509_REQ *x, unsigned long nmflags, unsigned long cflag) { long l; - EVP_PKEY *pkey; STACK_OF(X509_ATTRIBUTE) *sk; char mlch = ' '; @@ -127,13 +126,12 @@ goto err; } - pkey = X509_REQ_get_pubkey(x); + const EVP_PKEY *pkey = X509_REQ_get0_pubkey(x); if (pkey == NULL) { BIO_printf(bio, "%12sUnable to load Public Key\n", ""); ERR_print_errors(bio); } else { EVP_PKEY_print_public(bio, pkey, 16, NULL); - EVP_PKEY_free(pkey); } }
diff --git a/crypto/x509/t_x509.c b/crypto/x509/t_x509.c index 9596b65..9046928 100644 --- a/crypto/x509/t_x509.c +++ b/crypto/x509/t_x509.c
@@ -212,13 +212,12 @@ return 0; } - EVP_PKEY *pkey = X509_get_pubkey(x); + const EVP_PKEY *pkey = X509_get0_pubkey(x); if (pkey == NULL) { BIO_printf(bp, "%12sUnable to load Public Key\n", ""); ERR_print_errors(bp); } else { EVP_PKEY_print_public(bp, pkey, 16, NULL); - EVP_PKEY_free(pkey); } }
diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c index 9574c9b..87518ff 100644 --- a/crypto/x509/x509_cmp.c +++ b/crypto/x509/x509_cmp.c
@@ -218,6 +218,13 @@ return NULL; } +EVP_PKEY *X509_get0_pubkey(const X509 *x) { + if (x == NULL) { + return NULL; + } + return X509_PUBKEY_get0(x->cert_info->key); +} + EVP_PKEY *X509_get_pubkey(const X509 *x) { if (x == NULL) { return NULL; @@ -232,36 +239,29 @@ return x->cert_info->key->public_key; } -int X509_check_private_key(X509 *x, const EVP_PKEY *k) { - EVP_PKEY *xk; - int ret; - - xk = X509_get_pubkey(x); - - if (xk) { - ret = EVP_PKEY_cmp(xk, k); - } else { - ret = -2; +int X509_check_private_key(const X509 *x, const EVP_PKEY *k) { + const EVP_PKEY *xk = X509_get0_pubkey(x); + if (xk == NULL) { + return 0; } - switch (ret) { - case 1: - break; - case 0: - OPENSSL_PUT_ERROR(X509, X509_R_KEY_VALUES_MISMATCH); - break; - case -1: - OPENSSL_PUT_ERROR(X509, X509_R_KEY_TYPE_MISMATCH); - break; - case -2: - OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_KEY_TYPE); - } - if (xk) { - EVP_PKEY_free(xk); - } + int ret = EVP_PKEY_cmp(xk, k); if (ret > 0) { return 1; } + + switch (ret) { + case 0: + OPENSSL_PUT_ERROR(X509, X509_R_KEY_VALUES_MISMATCH); + return 0; + case -1: + OPENSSL_PUT_ERROR(X509, X509_R_KEY_TYPE_MISMATCH); + return 0; + case -2: + OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_KEY_TYPE); + return 0; + } + return 0; }
diff --git a/crypto/x509/x509_req.c b/crypto/x509/x509_req.c index 3c192ff..c7ea009 100644 --- a/crypto/x509/x509_req.c +++ b/crypto/x509/x509_req.c
@@ -77,37 +77,47 @@ } EVP_PKEY *X509_REQ_get_pubkey(const X509_REQ *req) { - if ((req == NULL) || (req->req_info == NULL)) { + if (req == NULL) { return NULL; } - return (X509_PUBKEY_get(req->req_info->pubkey)); + return X509_PUBKEY_get(req->req_info->pubkey); } -int X509_REQ_check_private_key(X509_REQ *x, const EVP_PKEY *k) { - EVP_PKEY *xk = NULL; - int ok = 0; +EVP_PKEY *X509_REQ_get0_pubkey(const X509_REQ *req) { + if (req == NULL) { + return NULL; + } + return X509_PUBKEY_get0(req->req_info->pubkey); +} - xk = X509_REQ_get_pubkey(x); - switch (EVP_PKEY_cmp(xk, k)) { - case 1: - ok = 1; - break; +int X509_REQ_check_private_key(const X509_REQ *x, const EVP_PKEY *k) { + const EVP_PKEY *xk = X509_REQ_get0_pubkey(x); + if (xk == NULL) { + return 0; + } + + int ret = EVP_PKEY_cmp(xk, k); + if (ret > 0) { + return 1; + } + + switch (ret) { case 0: OPENSSL_PUT_ERROR(X509, X509_R_KEY_VALUES_MISMATCH); - break; + return 0; case -1: OPENSSL_PUT_ERROR(X509, X509_R_KEY_TYPE_MISMATCH); - break; + return 0; case -2: if (EVP_PKEY_id(k) == EVP_PKEY_EC) { OPENSSL_PUT_ERROR(X509, ERR_R_EC_LIB); - break; + } else { + OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_KEY_TYPE); } - OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_KEY_TYPE); + return 0; } - EVP_PKEY_free(xk); - return ok; + return 0; } int X509_REQ_extension_nid(int req_nid) {
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 6bf366b..15121d2 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c
@@ -1171,8 +1171,6 @@ // Check CRL validity static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl) { X509 *issuer = NULL; - EVP_PKEY *ikey = NULL; - int ok = 0; int cnum = ctx->error_depth; int chnum = (int)sk_X509_num(ctx->chain) - 1; // if we have an alternative CRL issuer cert use that @@ -1189,9 +1187,8 @@ // If not self signed, can't check signature if (!x509_check_issued_with_callback(ctx, issuer, issuer)) { ctx->error = X509_V_ERR_UNABLE_TO_GET_CRL_ISSUER; - ok = ctx->verify_cb(0, ctx); - if (!ok) { - goto err; + if (!ctx->verify_cb(0, ctx)) { + return 0; } } } @@ -1201,61 +1198,50 @@ if ((issuer->ex_flags & EXFLAG_KUSAGE) && !(issuer->ex_kusage & X509v3_KU_CRL_SIGN)) { ctx->error = X509_V_ERR_KEYUSAGE_NO_CRL_SIGN; - ok = ctx->verify_cb(0, ctx); - if (!ok) { - goto err; + if (!ctx->verify_cb(0, ctx)) { + return 0; } } if (!(ctx->current_crl_score & CRL_SCORE_SCOPE)) { ctx->error = X509_V_ERR_DIFFERENT_CRL_SCOPE; - ok = ctx->verify_cb(0, ctx); - if (!ok) { - goto err; + if (!ctx->verify_cb(0, ctx)) { + return 0; } } if (crl->idp_flags & IDP_INVALID) { ctx->error = X509_V_ERR_INVALID_EXTENSION; - ok = ctx->verify_cb(0, ctx); - if (!ok) { - goto err; + if (!ctx->verify_cb(0, ctx)) { + return 0; } } if (!(ctx->current_crl_score & CRL_SCORE_TIME)) { - ok = check_crl_time(ctx, crl, 1); - if (!ok) { - goto err; + if (!check_crl_time(ctx, crl, 1)) { + return 0; } } // Attempt to get issuer certificate public key - ikey = X509_get_pubkey(issuer); - + EVP_PKEY *ikey = X509_get0_pubkey(issuer); if (!ikey) { ctx->error = X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY; - ok = ctx->verify_cb(0, ctx); - if (!ok) { - goto err; + if (!ctx->verify_cb(0, ctx)) { + return 0; } } else { // Verify CRL signature if (X509_CRL_verify(crl, ikey) <= 0) { ctx->error = X509_V_ERR_CRL_SIGNATURE_FAILURE; - ok = ctx->verify_cb(0, ctx); - if (!ok) { - goto err; + if (!ctx->verify_cb(0, ctx)) { + return 0; } } } } - ok = 1; - -err: - EVP_PKEY_free(ikey); - return ok; + return 1; } // Check certificate against CRL @@ -1365,7 +1351,6 @@ static int internal_verify(X509_STORE_CTX *ctx) { int ok = 0; X509 *xs, *xi; - EVP_PKEY *pkey = NULL; int n = (int)sk_X509_num(ctx->chain); ctx->error_depth = n - 1; @@ -1399,7 +1384,8 @@ // explicitly asked for. It doesn't add any security and just wastes // time. if (xs != xi || (ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE)) { - if ((pkey = X509_get_pubkey(xi)) == NULL) { + EVP_PKEY *pkey = X509_get0_pubkey(xi); + if (pkey == NULL) { ctx->error = X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY; ctx->current_cert = xi; ok = ctx->verify_cb(0, ctx); @@ -1411,12 +1397,9 @@ ctx->current_cert = xs; ok = ctx->verify_cb(0, ctx); if (!ok) { - EVP_PKEY_free(pkey); goto end; } } - EVP_PKEY_free(pkey); - pkey = NULL; } check_cert:
diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 697f193..72bca77 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h
@@ -219,7 +219,8 @@ // X509_check_private_key returns one if |x509|'s public key matches |pkey| and // zero otherwise. -OPENSSL_EXPORT int X509_check_private_key(X509 *x509, const EVP_PKEY *pkey); +OPENSSL_EXPORT int X509_check_private_key(const X509 *x509, + const EVP_PKEY *pkey); // X509_get0_uids sets |*out_issuer_uid| to a non-owning pointer to the // issuerUID field of |x509|, or NULL if |x509| has no issuerUID. It similarly @@ -1128,7 +1129,7 @@ // X509_REQ_check_private_key returns one if |req|'s public key matches |pkey| // and zero otherwise. -OPENSSL_EXPORT int X509_REQ_check_private_key(X509_REQ *req, +OPENSSL_EXPORT int X509_REQ_check_private_key(const X509_REQ *req, const EVP_PKEY *pkey); // X509_REQ_get_attr_count returns the number of attributes in |req|.