Add X509_parse_with_algorithms
This controls the algorithms that the SPKI will be parsed with. Note
there is a decision point here with our API: is an X509 a holder of an
EVP_PKEY, and thus parsed with algorithms in mind, or is it just an
abstractly parsed certificate, and algorithms are instead passed to an
X509_get_publickey_with_algorithms.
This CL takes the first route, largely because upstream already went
down this path in many ways:
- X509_get0_pubkey expose the fact that X509's retain a cached EVP_PKEY.
- Upstream OpenSSL added X509_new_ex which passes an OSSL_LIB_CTX into
the X509. It's a little unfortunate that this pattern relies on object
reuse in d2i_X509, but so it goes.
- A caller using the same X509 to verify mutiple signatures (e.g. a root
CA) might wish to hold on to the EVP_PKEY to avoid importing the key a
bunch. (This probably doesn't matter too much, but if we ever add an
API to precompute a table to speed up ECDSA verify...)
This changes follows in those footsteps. Note this means that the same
certificate bytes might produce semantically different X509 objects
(with or without the SPKI in EVP_PKEY form) depending on how it was
parsed. This is a little unfortunate but probably the most
straightforward model given where we are.
Bug: 42290364, 384818542
Change-Id: I8c0c95cb1d3221fc0b79a7749833c4f0d87646eb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81787
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 4e3a72b..b0085ed 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -2144,6 +2144,23 @@
return bssl::UniquePtr<X509>(d2i_X509(nullptr, &inp, len));
}
+static bssl::UniquePtr<X509> ReencodeCertificateWithAlgorithms(
+ X509 *cert, bssl::Span<const EVP_PKEY_ALG *const> algs) {
+ uint8_t *der = nullptr;
+ int len = i2d_X509(cert, &der);
+ bssl::UniquePtr<uint8_t> free_der(der);
+ if (len <= 0) {
+ return nullptr;
+ }
+
+ bssl::UniquePtr<CRYPTO_BUFFER> buf(CRYPTO_BUFFER_new(der, len, nullptr));
+ if (buf == nullptr) {
+ return nullptr;
+ }
+ return bssl::UniquePtr<X509>(
+ X509_parse_with_algorithms(buf.get(), algs.data(), algs.size()));
+}
+
static bssl::UniquePtr<X509_CRL> ReencodeCRL(X509_CRL *crl) {
uint8_t *der = nullptr;
int len = i2d_X509_CRL(crl, &der);
@@ -8935,8 +8952,40 @@
EXPECT_FALSE(X509_get0_pubkey(reparsed.get()));
EXPECT_EQ(X509_check_private_key(reparsed.get(), pkey.get()), 0);
- // TODO(crbug.com/42290364): Add an API to parse certificates with a custom
- // algorithm list. That should be able to extract the key.
+ // Reparsing with RSA-PSS enabled does enable it.
+ bssl::UniquePtr<X509> cert_with_key =
+ ReencodeCertificateWithAlgorithms(cert.get(), bssl::Span(&alg, 1));
+ ASSERT_TRUE(cert_with_key);
+ // The public key can be extracted from |cert|.
+ const EVP_PKEY *cert_pkey = X509_get0_pubkey(cert_with_key.get());
+ ASSERT_TRUE(cert_pkey);
+ EXPECT_EQ(EVP_PKEY_cmp(pkey.get(), cert_pkey), 1);
+ // |X509_check_private_key| should work.
+ EXPECT_EQ(X509_check_private_key(cert_with_key.get(), pkey.get()), 1);
+
+ // Verifying a certificate chain using |EVP_PKEY_RSA_PSS| should work as long
+ // as all CA certificates have the key available. The end-entity key is not
+ // checked.
+ bssl::UniquePtr<X509> root =
+ MakeTestCert("Test Issuer", "Test Issuer", pkey.get(), /*is_ca=*/true);
+ ASSERT_TRUE(root);
+ ASSERT_TRUE(X509_sign(root.get(), pkey.get(), EVP_sha256()));
+ root = ReencodeCertificate(root.get());
+ ASSERT_TRUE(root);
+ bssl::UniquePtr<X509> root_with_key =
+ ReencodeCertificateWithAlgorithms(root.get(), bssl::Span(&alg, 1));
+ ASSERT_TRUE(root_with_key);
+ EXPECT_EQ(X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY,
+ Verify(cert.get(), /*roots=*/{root.get()},
+ /*intermediates=*/{}, /*crls=*/{}));
+ EXPECT_EQ(X509_V_OK, Verify(cert.get(), /*roots=*/{root_with_key.get()},
+ /*intermediates=*/{}, /*crls=*/{}));
+ EXPECT_EQ(X509_V_ERR_UNABLE_TO_DECODE_ISSUER_PUBLIC_KEY,
+ Verify(cert_with_key.get(), /*roots=*/{root.get()},
+ /*intermediates=*/{}, /*crls=*/{}));
+ EXPECT_EQ(X509_V_OK,
+ Verify(cert_with_key.get(), /*roots=*/{root_with_key.get()},
+ /*intermediates=*/{}, /*crls=*/{}));
}
} // namespace
diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index 520ae63..442c827 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc
@@ -124,8 +124,9 @@
return 1;
}
-
-X509 *X509_parse_from_buffer(CRYPTO_BUFFER *buf) {
+X509 *X509_parse_with_algorithms(CRYPTO_BUFFER *buf,
+ const EVP_PKEY_ALG *const *algs,
+ size_t num_algs) {
bssl::UniquePtr<X509> ret(x509_new_null());
if (ret == nullptr) {
return nullptr;
@@ -188,9 +189,7 @@
/*allow_utc_timezone_offset=*/1) ||
CBS_len(&validity) != 0 || //
!parse_name(&tbs, &ret->subject) ||
- // TODO(crbug.com/42290364): Expose an API to use different algorithms.
- !x509_parse_public_key(&tbs, &ret->key,
- bssl::GetDefaultEVPAlgorithms())) {
+ !x509_parse_public_key(&tbs, &ret->key, bssl::Span(algs, num_algs))) {
OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
return nullptr;
}
@@ -238,6 +237,11 @@
return ret.release();
}
+X509 *X509_parse_from_buffer(CRYPTO_BUFFER *buf) {
+ auto algs = bssl::GetDefaultEVPAlgorithms();
+ return X509_parse_with_algorithms(buf, algs.data(), algs.size());
+}
+
static bssl::UniquePtr<X509> x509_parse(CBS *cbs) {
CBS cert;
if (!CBS_get_asn1_element(cbs, &cert, CBS_ASN1_SEQUENCE)) {
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 2fe11e3..667eede5 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -100,10 +100,20 @@
// Certificate (RFC 5280), as described in |d2i_SAMPLE|.
OPENSSL_EXPORT X509 *d2i_X509(X509 **out, const uint8_t **inp, long len);
-// X509_parse_from_buffer parses an X.509 structure from |buf| and returns a
+// X509_parse_with_algorithms parses an X.509 structure from |buf| and returns a
// fresh X509 or NULL on error. There must not be any trailing data in |buf|.
-// The returned structure (if any) holds a reference to |buf| rather than
-// copying parts of it as a normal |d2i_X509| call would do.
+// The returned structure (if any) increment's |buf|'s reference count and
+// retains a reference to it.
+//
+// Only the |num_algs| algorithms from |algs| will be considered when parsing
+// the certificate's public key. If the certificate uses a different algorithm,
+// it will still be parsed, but |X509_get0_pubkey| will return NULL.
+OPENSSL_EXPORT X509 *X509_parse_with_algorithms(CRYPTO_BUFFER *buf,
+ const EVP_PKEY_ALG *const *algs,
+ size_t num_algs);
+
+// X509_parse_from_buffer behaves like |X509_parse_with_algorithms| but uses a
+// default algorithm list.
OPENSSL_EXPORT X509 *X509_parse_from_buffer(CRYPTO_BUFFER *buf);
// i2d_X509 marshals |x509| as a DER-encoded X.509 Certificate (RFC 5280), as
