Const-correct a few more crypto/x509 APIs Previously, I kept anything that serializes or compares X509_NAME non-const due to crbug.com/42290269. That has been fixed now, so we can const-correct a few more APIs. I've not switched the CRL parameter in X509_CRL_get0_* to const yet. It also should be const, but we still need to deal with crbug.com/42290473 I've also not switched the EVP_PKEY parameter to const because functions that use an EVP_PKEY are currently non-const due to internal refcount-bumping. This honestly should probably be fixed, but let's ponder that at the EVP side first. Bug: 42290269 Change-Id: Ib1bd381817c67c167922ad9a9002a5806f116fbc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/89347 Reviewed-by: Xiangfei Ding <xfding@google.com> Commit-Queue: Xiangfei Ding <xfding@google.com> Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/x509cset.cc b/crypto/x509/x509cset.cc index 54ae135..3f4e75e 100644 --- a/crypto/x509/x509cset.cc +++ b/crypto/x509/x509cset.cc
@@ -207,7 +207,7 @@ return i2d_X509_CRL_INFO(crl->crl, outp); } -int i2d_X509_CRL_tbs(X509_CRL *crl, unsigned char **outp) { +int i2d_X509_CRL_tbs(const X509_CRL *crl, unsigned char **outp) { return i2d_X509_CRL_INFO(crl->crl, outp); }
diff --git a/crypto/x509/x_all.cc b/crypto/x509/x_all.cc index 3d9474d..e9e59c4 100644 --- a/crypto/x509/x_all.cc +++ b/crypto/x509/x_all.cc
@@ -35,7 +35,7 @@ using namespace bssl; -int X509_verify(X509 *x509, EVP_PKEY *pkey) { +int X509_verify(const X509 *x509, EVP_PKEY *pkey) { auto *impl = FromOpaque(x509); if (X509_ALGOR_cmp(&impl->sig_alg, &impl->tbs_sig_alg)) { OPENSSL_PUT_ERROR(X509, X509_R_SIGNATURE_ALGORITHM_MISMATCH); @@ -50,7 +50,7 @@ CBBAsSpan(cbb.get()), pkey); } -int X509_REQ_verify(X509_REQ *req, EVP_PKEY *pkey) { +int X509_REQ_verify(const X509_REQ *req, EVP_PKEY *pkey) { return ASN1_item_verify(ASN1_ITEM_rptr(X509_REQ_INFO), req->sig_alg, req->signature, req->req_info, pkey); }
diff --git a/crypto/x509/x_crl.cc b/crypto/x509/x_crl.cc index faca41f..83b1bce 100644 --- a/crypto/x509/x_crl.cc +++ b/crypto/x509/x_crl.cc
@@ -46,7 +46,7 @@ BSSL_NAMESPACE_END static int crl_lookup(X509_CRL *crl, X509_REVOKED **ret, - const ASN1_INTEGER *serial, X509_NAME *issuer); + const ASN1_INTEGER *serial, const X509_NAME *issuer); // The X509_CRL_INFO structure needs a bit of customisation. Since we cache // the original encoding the signature won't be affected by reordering of the @@ -295,8 +295,7 @@ } int X509_CRL_add0_revoked(X509_CRL *crl, X509_REVOKED *rev) { - X509_CRL_INFO *inf; - inf = crl->crl; + X509_CRL_INFO *inf = crl->crl; if (!inf->revoked) { inf->revoked = sk_X509_REVOKED_new(X509_REVOKED_cmp); } @@ -307,7 +306,7 @@ return 1; } -int X509_CRL_verify(X509_CRL *crl, EVP_PKEY *pkey) { +int X509_CRL_verify(const X509_CRL *crl, EVP_PKEY *pkey) { if (X509_ALGOR_cmp(crl->sig_alg, crl->crl->sig_alg) != 0) { OPENSSL_PUT_ERROR(X509, X509_R_SIGNATURE_ALGORITHM_MISMATCH); return 0; @@ -322,24 +321,24 @@ return crl_lookup(crl, ret, serial, nullptr); } -int X509_CRL_get0_by_cert(X509_CRL *crl, X509_REVOKED **ret, X509 *x) { - return crl_lookup(crl, ret, X509_get_serialNumber(x), +int X509_CRL_get0_by_cert(X509_CRL *crl, X509_REVOKED **ret, const X509 *x) { + return crl_lookup(crl, ret, X509_get0_serialNumber(x), X509_get_issuer_name(x)); } -static int crl_revoked_issuer_match(X509_CRL *crl, X509_NAME *nm, - X509_REVOKED *rev) { +static int crl_revoked_issuer_match(const X509_CRL *crl, const X509_NAME *nm, + const X509_REVOKED *rev) { return nm == nullptr || X509_NAME_cmp(nm, X509_CRL_get_issuer(crl)) == 0; } static CRYPTO_MUTEX g_crl_sort_lock = CRYPTO_MUTEX_INIT; static int crl_lookup(X509_CRL *crl, X509_REVOKED **ret, - const ASN1_INTEGER *serial, X509_NAME *issuer) { + const ASN1_INTEGER *serial, const X509_NAME *issuer) { // Use an assert, rather than a runtime error, because returning nothing for a // CRL is arguably failing open, rather than closed. assert(serial->type == V_ASN1_INTEGER || serial->type == V_ASN1_NEG_INTEGER); - X509_REVOKED rtmp, *rev; + X509_REVOKED rtmp; size_t idx; rtmp.serialNumber = (ASN1_INTEGER *)serial; // Sort revoked into serial number order if not already sorted. Do this @@ -362,7 +361,7 @@ } // Need to look for matching name for (; idx < sk_X509_REVOKED_num(crl->crl->revoked); idx++) { - rev = sk_X509_REVOKED_value(crl->crl->revoked, idx); + X509_REVOKED *rev = sk_X509_REVOKED_value(crl->crl->revoked, idx); if (ASN1_INTEGER_cmp(rev->serialNumber, serial)) { return 0; }
diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 926f365..765de55 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h
@@ -393,7 +393,7 @@ // one if the signature is valid and zero otherwise. Note this function only // checks the signature itself and does not perform a full certificate // validation. -OPENSSL_EXPORT int X509_verify(X509 *x509, EVP_PKEY *pkey); +OPENSSL_EXPORT int X509_verify(const X509 *x509, EVP_PKEY *pkey); // X509_get1_email returns a newly-allocated list of NUL-terminated strings // containing all email addresses in |x509|'s subject and all rfc822name names @@ -730,7 +730,7 @@ // On success, |*out| continues to be owned by |crl|. It is an error to free or // otherwise modify |*out|. // -// TODO(crbug.com/boringssl/600): Ideally |crl| would be const. It is broadly +// TODO(crbug.com/42290473): Ideally |crl| would be const. It is broadly // thread-safe, but changes the order of entries in |crl|. It cannot be called // concurrently with |i2d_X509_CRL|. OPENSSL_EXPORT int X509_CRL_get0_by_serial(X509_CRL *crl, X509_REVOKED **out, @@ -738,8 +738,12 @@ // X509_CRL_get0_by_cert behaves like |X509_CRL_get0_by_serial|, except it looks // for the entry that matches |x509|. +// +// TODO(crbug.com/42290473): Ideally |crl| would be const. It is broadly +// thread-safe, but changes the order of entries in |crl|. It cannot be called +// concurrently with |i2d_X509_CRL|. OPENSSL_EXPORT int X509_CRL_get0_by_cert(X509_CRL *crl, X509_REVOKED **out, - X509 *x509); + const X509 *x509); // X509_CRL_get_REVOKED returns the list of revoked certificates in |crl|, or // NULL if |crl| omits it. @@ -810,11 +814,11 @@ // reflect modifications made to |crl|. It may be used to manually verify the // signature of an existing CRL. To generate CRLs, use |i2d_re_X509_CRL_tbs| // instead. -OPENSSL_EXPORT int i2d_X509_CRL_tbs(X509_CRL *crl, unsigned char **outp); +OPENSSL_EXPORT int i2d_X509_CRL_tbs(const X509_CRL *crl, unsigned char **outp); // X509_CRL_verify checks that |crl| has a valid signature by |pkey|. It returns // one if the signature is valid and zero otherwise. -OPENSSL_EXPORT int X509_CRL_verify(X509_CRL *crl, EVP_PKEY *pkey); +OPENSSL_EXPORT int X509_CRL_verify(const X509_CRL *crl, EVP_PKEY *pkey); // Issuing certificate revocation lists. @@ -1163,7 +1167,7 @@ // X509_REQ_verify checks that |req| has a valid signature by |pkey|. It returns // one if the signature is valid and zero otherwise. -OPENSSL_EXPORT int X509_REQ_verify(X509_REQ *req, EVP_PKEY *pkey); +OPENSSL_EXPORT int X509_REQ_verify(const X509_REQ *req, EVP_PKEY *pkey); // X509_REQ_get1_email returns a newly-allocated list of NUL-terminated strings // containing all email addresses in |req|'s subject and all rfc822name names