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|.
