Test (and, for CSRs, fix) TBS cache invalidation on signing.

We didn't actually have a test that would have caught
https://github.com/openssl/openssl/issues/19388. This fixes this by
further generalizing the signing tests to run through all combinations
of {new object, reused object} x {X509_sign, X509_set_signature_value}.

In doing so, align X509_REQ_sign and X509_REQ_sign_ctx, which were
missing the TBS invalidation.

Change-Id: I5028aa2a00e71da0ebc7a03b23823b1337a56fca
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54726
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
fips-20230428
David Benjamin 2 years ago committed by Boringssl LUCI CQ
parent d66bba9128
commit da96eeb958
  1. 418
      crypto/x509/x509_test.cc
  2. 2
      crypto/x509/x_all.c

@ -1943,8 +1943,9 @@ TEST(X509Test, RSASign) {
EXPECT_FALSE(X509_sign_ctx(cert.get(), md_ctx.get()));
}
// Test the APIs for manually signing a certificate.
TEST(X509Test, RSASignManual) {
// Test the APIs for signing a certificate, particularly whether they correctly
// handle the TBSCertificate cache.
TEST(X509Test, SignCertificate) {
const int kSignatureNID = NID_sha384WithRSAEncryption;
const EVP_MD *kSignatureHash = EVP_sha384();
@ -1955,74 +1956,86 @@ TEST(X509Test, RSASignManual) {
ASSERT_TRUE(X509_ALGOR_set0(algor.get(), OBJ_nid2obj(kSignatureNID),
V_ASN1_NULL, nullptr));
// Test certificates made both from other certificates and |X509_new|, in case
// there are bugs in filling in fields from different states. (Parsed
// certificates contain a TBSCertificate cache, and |X509_new| initializes
// fields based on complex ASN.1 template logic.)
for (bool new_cert : {true, false}) {
SCOPED_TRACE(new_cert);
bssl::UniquePtr<X509> cert;
if (new_cert) {
cert.reset(X509_new());
ASSERT_TRUE(cert);
// Fill in some fields for the certificate arbitrarily.
EXPECT_TRUE(X509_set_version(cert.get(), X509_VERSION_3));
EXPECT_TRUE(ASN1_INTEGER_set_int64(X509_get_serialNumber(cert.get()), 1));
EXPECT_TRUE(X509_gmtime_adj(X509_getm_notBefore(cert.get()), 0));
EXPECT_TRUE(
X509_gmtime_adj(X509_getm_notAfter(cert.get()), 60 * 60 * 24));
X509_NAME *subject = X509_get_subject_name(cert.get());
X509_NAME_add_entry_by_txt(subject, "CN", MBSTRING_ASC,
reinterpret_cast<const uint8_t *>("Test"), -1,
-1, 0);
EXPECT_TRUE(X509_set_issuer_name(cert.get(), subject));
EXPECT_TRUE(X509_set_pubkey(cert.get(), pkey.get()));
} else {
// Extract fields from a parsed certificate.
cert = CertFromPEM(kLeafPEM);
ASSERT_TRUE(cert);
// Test both signing with |X509_sign| and constructing a signature manually.
for (bool sign_manual : {true, false}) {
SCOPED_TRACE(sign_manual);
// Test certificates made both from other certificates and |X509_new|, in
// case there are bugs in filling in fields from different states. (Parsed
// certificates contain a TBSCertificate cache, and |X509_new| initializes
// fields based on complex ASN.1 template logic.)
for (bool new_cert : {true, false}) {
SCOPED_TRACE(new_cert);
bssl::UniquePtr<X509> cert;
if (new_cert) {
cert.reset(X509_new());
ASSERT_TRUE(cert);
// Fill in some fields for the certificate arbitrarily.
EXPECT_TRUE(X509_set_version(cert.get(), X509_VERSION_3));
EXPECT_TRUE(
ASN1_INTEGER_set_int64(X509_get_serialNumber(cert.get()), 1));
EXPECT_TRUE(X509_gmtime_adj(X509_getm_notBefore(cert.get()), 0));
EXPECT_TRUE(
X509_gmtime_adj(X509_getm_notAfter(cert.get()), 60 * 60 * 24));
X509_NAME *subject = X509_get_subject_name(cert.get());
X509_NAME_add_entry_by_txt(subject, "CN", MBSTRING_ASC,
reinterpret_cast<const uint8_t *>("Test"),
-1, -1, 0);
EXPECT_TRUE(X509_set_issuer_name(cert.get(), subject));
EXPECT_TRUE(X509_set_pubkey(cert.get(), pkey.get()));
} else {
// Extract fields from a parsed certificate.
cert = CertFromPEM(kLeafPEM);
ASSERT_TRUE(cert);
// We should test with a different algorithm from what is already in the
// certificate.
EXPECT_NE(kSignatureNID, X509_get_signature_nid(cert.get()));
}
// We should test with a different algorithm from what is already in the
// certificate.
EXPECT_NE(kSignatureNID, X509_get_signature_nid(cert.get()));
}
// Fill in the signature algorithm.
ASSERT_TRUE(X509_set1_signature_algo(cert.get(), algor.get()));
// Extract the TBSCertificiate.
uint8_t *tbs_cert = nullptr;
int tbs_cert_len = i2d_re_X509_tbs(cert.get(), &tbs_cert);
bssl::UniquePtr<uint8_t> free_tbs_cert(tbs_cert);
ASSERT_GT(tbs_cert_len, 0);
// Generate a signature externally and fill it in.
bssl::ScopedEVP_MD_CTX md_ctx;
ASSERT_TRUE(EVP_DigestSignInit(md_ctx.get(), nullptr, kSignatureHash,
nullptr, pkey.get()));
size_t sig_len;
ASSERT_TRUE(EVP_DigestSign(md_ctx.get(), nullptr, &sig_len, tbs_cert,
tbs_cert_len));
std::vector<uint8_t> sig(sig_len);
ASSERT_TRUE(EVP_DigestSign(md_ctx.get(), sig.data(), &sig_len, tbs_cert,
tbs_cert_len));
sig.resize(sig_len);
ASSERT_TRUE(X509_set1_signature_value(cert.get(), sig.data(), sig.size()));
// Check the signature.
EXPECT_TRUE(X509_verify(cert.get(), pkey.get()));
if (sign_manual) {
// Fill in the signature algorithm.
ASSERT_TRUE(X509_set1_signature_algo(cert.get(), algor.get()));
// Extract the TBSCertificiate.
uint8_t *tbs_cert = nullptr;
int tbs_cert_len = i2d_re_X509_tbs(cert.get(), &tbs_cert);
bssl::UniquePtr<uint8_t> free_tbs_cert(tbs_cert);
ASSERT_GT(tbs_cert_len, 0);
// Generate a signature externally and fill it in.
bssl::ScopedEVP_MD_CTX md_ctx;
ASSERT_TRUE(EVP_DigestSignInit(md_ctx.get(), nullptr, kSignatureHash,
nullptr, pkey.get()));
size_t sig_len;
ASSERT_TRUE(EVP_DigestSign(md_ctx.get(), nullptr, &sig_len, tbs_cert,
tbs_cert_len));
std::vector<uint8_t> sig(sig_len);
ASSERT_TRUE(EVP_DigestSign(md_ctx.get(), sig.data(), &sig_len, tbs_cert,
tbs_cert_len));
sig.resize(sig_len);
ASSERT_TRUE(
X509_set1_signature_value(cert.get(), sig.data(), sig.size()));
} else {
ASSERT_TRUE(X509_sign(cert.get(), pkey.get(), EVP_sha384()));
}
// Re-encode the certificate. X509 objects contain a cached TBSCertificate
// encoding and |i2d_re_X509_tbs| should have refreshed that cache.
bssl::UniquePtr<X509> copy = ReencodeCertificate(cert.get());
ASSERT_TRUE(copy);
EXPECT_TRUE(X509_verify(copy.get(), pkey.get()));
// Check the signature.
EXPECT_TRUE(X509_verify(cert.get(), pkey.get()));
// Re-encode the certificate. X509 objects contain a cached TBSCertificate
// encoding and |i2d_re_X509_tbs| should have refreshed that cache.
bssl::UniquePtr<X509> copy = ReencodeCertificate(cert.get());
ASSERT_TRUE(copy);
EXPECT_TRUE(X509_verify(copy.get(), pkey.get()));
}
}
}
// Test the APIs for manually signing a CSR.
TEST(X509Test, RSASignCRLManual) {
// Test the APIs for signing a CRL, particularly whether they correctly handle
// the TBSCertList cache.
TEST(X509Test, SignCRL) {
const int kSignatureNID = NID_sha384WithRSAEncryption;
const EVP_MD *kSignatureHash = EVP_sha384();
@ -2033,69 +2046,80 @@ TEST(X509Test, RSASignCRLManual) {
ASSERT_TRUE(X509_ALGOR_set0(algor.get(), OBJ_nid2obj(kSignatureNID),
V_ASN1_NULL, nullptr));
// Test CRLs made both from other CRLs and |X509_CRL_new|, in case there are
// bugs in filling in fields from different states. (Parsed CRLs contain a
// TBSCertList cache, and |X509_CRL_new| initializes fields based on complex
// ASN.1 template logic.)
for (bool new_crl : {true, false}) {
SCOPED_TRACE(new_crl);
bssl::UniquePtr<X509_CRL> crl;
if (new_crl) {
crl.reset(X509_CRL_new());
ASSERT_TRUE(crl);
// Fill in some fields for the certificate arbitrarily.
ASSERT_TRUE(X509_CRL_set_version(crl.get(), X509_CRL_VERSION_2));
bssl::UniquePtr<ASN1_TIME> last_update(ASN1_TIME_new());
ASSERT_TRUE(last_update);
ASSERT_TRUE(ASN1_TIME_set(last_update.get(), kReferenceTime));
ASSERT_TRUE(X509_CRL_set1_lastUpdate(crl.get(), last_update.get()));
bssl::UniquePtr<X509_NAME> issuer(X509_NAME_new());
ASSERT_TRUE(issuer);
ASSERT_TRUE(X509_NAME_add_entry_by_txt(
issuer.get(), "CN", MBSTRING_ASC,
reinterpret_cast<const uint8_t *>("Test"), -1, -1, 0));
EXPECT_TRUE(X509_CRL_set_issuer_name(crl.get(), issuer.get()));
} else {
// Extract fields from a parsed CRL.
crl = CRLFromPEM(kBasicCRL);
ASSERT_TRUE(crl);
// Test both signing with |X509_CRL_sign| and constructing a signature
// manually.
for (bool sign_manual : {true, false}) {
SCOPED_TRACE(sign_manual);
// Test CRLs made both from other CRLs and |X509_CRL_new|, in case there are
// bugs in filling in fields from different states. (Parsed CRLs contain a
// TBSCertList cache, and |X509_CRL_new| initializes fields based on complex
// ASN.1 template logic.)
for (bool new_crl : {true, false}) {
SCOPED_TRACE(new_crl);
bssl::UniquePtr<X509_CRL> crl;
if (new_crl) {
crl.reset(X509_CRL_new());
ASSERT_TRUE(crl);
// Fill in some fields for the certificate arbitrarily.
ASSERT_TRUE(X509_CRL_set_version(crl.get(), X509_CRL_VERSION_2));
bssl::UniquePtr<ASN1_TIME> last_update(ASN1_TIME_new());
ASSERT_TRUE(last_update);
ASSERT_TRUE(ASN1_TIME_set(last_update.get(), kReferenceTime));
ASSERT_TRUE(X509_CRL_set1_lastUpdate(crl.get(), last_update.get()));
bssl::UniquePtr<X509_NAME> issuer(X509_NAME_new());
ASSERT_TRUE(issuer);
ASSERT_TRUE(X509_NAME_add_entry_by_txt(
issuer.get(), "CN", MBSTRING_ASC,
reinterpret_cast<const uint8_t *>("Test"), -1, -1, 0));
EXPECT_TRUE(X509_CRL_set_issuer_name(crl.get(), issuer.get()));
} else {
// Extract fields from a parsed CRL.
crl = CRLFromPEM(kBasicCRL);
ASSERT_TRUE(crl);
// We should test with a different algorithm from what is already in the
// CRL.
EXPECT_NE(kSignatureNID, X509_CRL_get_signature_nid(crl.get()));
}
// We should test with a different algorithm from what is already in the
// CRL.
EXPECT_NE(kSignatureNID, X509_CRL_get_signature_nid(crl.get()));
}
if (sign_manual) {
// Fill in the signature algorithm.
ASSERT_TRUE(X509_CRL_set1_signature_algo(crl.get(), algor.get()));
// Extract the TBSCertList.
uint8_t *tbs = nullptr;
int tbs_len = i2d_re_X509_CRL_tbs(crl.get(), &tbs);
bssl::UniquePtr<uint8_t> free_tbs(tbs);
ASSERT_GT(tbs_len, 0);
// Generate a signature externally and fill it in.
bssl::ScopedEVP_MD_CTX md_ctx;
ASSERT_TRUE(EVP_DigestSignInit(md_ctx.get(), nullptr, kSignatureHash,
nullptr, pkey.get()));
size_t sig_len;
ASSERT_TRUE(
EVP_DigestSign(md_ctx.get(), nullptr, &sig_len, tbs, tbs_len));
std::vector<uint8_t> sig(sig_len);
ASSERT_TRUE(
EVP_DigestSign(md_ctx.get(), sig.data(), &sig_len, tbs, tbs_len));
sig.resize(sig_len);
ASSERT_TRUE(
X509_CRL_set1_signature_value(crl.get(), sig.data(), sig.size()));
} else {
ASSERT_TRUE(X509_CRL_sign(crl.get(), pkey.get(), EVP_sha384()));
}
// Check the signature.
EXPECT_TRUE(X509_CRL_verify(crl.get(), pkey.get()));
// Fill in the signature algorithm.
ASSERT_TRUE(X509_CRL_set1_signature_algo(crl.get(), algor.get()));
// Extract the TBSCertList.
uint8_t *tbs = nullptr;
int tbs_len = i2d_re_X509_CRL_tbs(crl.get(), &tbs);
bssl::UniquePtr<uint8_t> free_tbs(tbs);
ASSERT_GT(tbs_len, 0);
// Generate a signature externally and fill it in.
bssl::ScopedEVP_MD_CTX md_ctx;
ASSERT_TRUE(EVP_DigestSignInit(md_ctx.get(), nullptr, kSignatureHash,
nullptr, pkey.get()));
size_t sig_len;
ASSERT_TRUE(EVP_DigestSign(md_ctx.get(), nullptr, &sig_len, tbs, tbs_len));
std::vector<uint8_t> sig(sig_len);
ASSERT_TRUE(
EVP_DigestSign(md_ctx.get(), sig.data(), &sig_len, tbs, tbs_len));
sig.resize(sig_len);
ASSERT_TRUE(
X509_CRL_set1_signature_value(crl.get(), sig.data(), sig.size()));
// Check the signature.
EXPECT_TRUE(X509_CRL_verify(crl.get(), pkey.get()));
// Re-encode the CRL. X509_CRL objects contain a cached TBSCertList encoding
// and |i2d_re_X509_tbs| should have refreshed that cache.
bssl::UniquePtr<X509_CRL> copy = ReencodeCRL(crl.get());
ASSERT_TRUE(copy);
EXPECT_TRUE(X509_CRL_verify(copy.get(), pkey.get()));
// Re-encode the CRL. X509_CRL objects contain a cached TBSCertList
// encoding and |i2d_re_X509_tbs| should have refreshed that cache.
bssl::UniquePtr<X509_CRL> copy = ReencodeCRL(crl.get());
ASSERT_TRUE(copy);
EXPECT_TRUE(X509_CRL_verify(copy.get(), pkey.get()));
}
}
}
@ -2117,8 +2141,9 @@ rZGEJG3+X9OuhczVKGJyg+3gU7oDbecc
-----END CERTIFICATE REQUEST-----
)";
// Test the APIs for manually signing a certificate.
TEST(X509Test, RSASignCSRManual) {
// Test the APIs for signing a CSR, particularly whether they correctly handle
// the CertificationRequestInfo cache.
TEST(X509Test, SignCSR) {
const int kSignatureNID = NID_sha384WithRSAEncryption;
const EVP_MD *kSignatureHash = EVP_sha384();
@ -2129,71 +2154,82 @@ TEST(X509Test, RSASignCSRManual) {
ASSERT_TRUE(X509_ALGOR_set0(algor.get(), OBJ_nid2obj(kSignatureNID),
V_ASN1_NULL, nullptr));
// Test CSRs made both from other CSRs and |X509_REQ_new|, in case there are
// bugs in filling in fields from different states. (Parsed CSRs contain a
// CertificationRequestInfo cache, and |X509_REQ_new| initializes fields based
// on complex ASN.1 template logic.)
for (bool new_csr : {true, false}) {
SCOPED_TRACE(new_csr);
bssl::UniquePtr<X509_REQ> csr;
if (new_csr) {
csr.reset(X509_REQ_new());
ASSERT_TRUE(csr);
bssl::UniquePtr<X509_NAME> subject(X509_NAME_new());
ASSERT_TRUE(subject);
ASSERT_TRUE(X509_NAME_add_entry_by_txt(
subject.get(), "CN", MBSTRING_ASC,
reinterpret_cast<const uint8_t *>("New CSR"), -1, -1, 0));
EXPECT_TRUE(X509_REQ_set_subject_name(csr.get(), subject.get()));
} else {
// Extract fields from a parsed CSR.
csr = CSRFromPEM(kTestCSR);
ASSERT_TRUE(csr);
}
// Test both signing with |X509_REQ_sign| and constructing a signature
// manually.
for (bool sign_manual : {true, false}) {
SCOPED_TRACE(sign_manual);
// Test CSRs made both from other CSRs and |X509_REQ_new|, in case there are
// bugs in filling in fields from different states. (Parsed CSRs contain a
// CertificationRequestInfo cache, and |X509_REQ_new| initializes fields
// based on complex ASN.1 template logic.)
for (bool new_csr : {true, false}) {
SCOPED_TRACE(new_csr);
bssl::UniquePtr<X509_REQ> csr;
if (new_csr) {
csr.reset(X509_REQ_new());
ASSERT_TRUE(csr);
bssl::UniquePtr<X509_NAME> subject(X509_NAME_new());
ASSERT_TRUE(subject);
ASSERT_TRUE(X509_NAME_add_entry_by_txt(
subject.get(), "CN", MBSTRING_ASC,
reinterpret_cast<const uint8_t *>("New CSR"), -1, -1, 0));
EXPECT_TRUE(X509_REQ_set_subject_name(csr.get(), subject.get()));
} else {
// Extract fields from a parsed CSR.
csr = CSRFromPEM(kTestCSR);
ASSERT_TRUE(csr);
}
// Override the public key from the CSR unconditionally. Unlike
// certificates and CRLs, CSRs do not contain a signed copy of the
// signature algorithm, so we use a different field to confirm
// |i2d_re_X509_REQ_tbs| clears the cache as expected.
EXPECT_TRUE(X509_REQ_set_pubkey(csr.get(), pkey.get()));
if (sign_manual) {
// Fill in the signature algorithm.
ASSERT_TRUE(X509_REQ_set1_signature_algo(csr.get(), algor.get()));
// Extract the CertificationRequestInfo.
uint8_t *tbs = nullptr;
int tbs_len = i2d_re_X509_REQ_tbs(csr.get(), &tbs);
bssl::UniquePtr<uint8_t> free_tbs(tbs);
ASSERT_GT(tbs_len, 0);
// Generate a signature externally and fill it in.
bssl::ScopedEVP_MD_CTX md_ctx;
ASSERT_TRUE(EVP_DigestSignInit(md_ctx.get(), nullptr, kSignatureHash,
nullptr, pkey.get()));
size_t sig_len;
ASSERT_TRUE(
EVP_DigestSign(md_ctx.get(), nullptr, &sig_len, tbs, tbs_len));
std::vector<uint8_t> sig(sig_len);
ASSERT_TRUE(
EVP_DigestSign(md_ctx.get(), sig.data(), &sig_len, tbs, tbs_len));
sig.resize(sig_len);
ASSERT_TRUE(
X509_REQ_set1_signature_value(csr.get(), sig.data(), sig.size()));
} else {
ASSERT_TRUE(X509_REQ_sign(csr.get(), pkey.get(), EVP_sha384()));
}
// Check the signature.
EXPECT_TRUE(X509_REQ_verify(csr.get(), pkey.get()));
// Override the public key from the CSR unconditionally. Unlike certificates
// and CRLs, CSRs do not contain a signed copy of the signature algorithm,
// so we use a different field to confirm |i2d_re_X509_REQ_tbs| clears the
// cache as expected.
EXPECT_TRUE(X509_REQ_set_pubkey(csr.get(), pkey.get()));
// Fill in the signature algorithm.
ASSERT_TRUE(X509_REQ_set1_signature_algo(csr.get(), algor.get()));
// Extract the CertificationRequestInfo.
uint8_t *tbs = nullptr;
int tbs_len = i2d_re_X509_REQ_tbs(csr.get(), &tbs);
bssl::UniquePtr<uint8_t> free_tbs(tbs);
ASSERT_GT(tbs_len, 0);
// Generate a signature externally and fill it in.
bssl::ScopedEVP_MD_CTX md_ctx;
ASSERT_TRUE(EVP_DigestSignInit(md_ctx.get(), nullptr, kSignatureHash,
nullptr, pkey.get()));
size_t sig_len;
ASSERT_TRUE(EVP_DigestSign(md_ctx.get(), nullptr, &sig_len, tbs, tbs_len));
std::vector<uint8_t> sig(sig_len);
ASSERT_TRUE(
EVP_DigestSign(md_ctx.get(), sig.data(), &sig_len, tbs, tbs_len));
sig.resize(sig_len);
ASSERT_TRUE(
X509_REQ_set1_signature_value(csr.get(), sig.data(), sig.size()));
// Check the signature.
EXPECT_TRUE(X509_REQ_verify(csr.get(), pkey.get()));
// Re-encode the CSR. X509_REQ objects contain a cached
// CertificationRequestInfo encoding and |i2d_re_X509_REQ_tbs| should have
// refreshed that cache.
bssl::UniquePtr<X509_REQ> copy = ReencodeCSR(csr.get());
ASSERT_TRUE(copy);
EXPECT_TRUE(X509_REQ_verify(copy.get(), pkey.get()));
// Check the signature was over the new public key.
bssl::UniquePtr<EVP_PKEY> copy_pubkey(X509_REQ_get_pubkey(copy.get()));
ASSERT_TRUE(copy_pubkey);
EXPECT_EQ(1, EVP_PKEY_cmp(pkey.get(), copy_pubkey.get()));
// Re-encode the CSR. X509_REQ objects contain a cached
// CertificationRequestInfo encoding and |i2d_re_X509_REQ_tbs| should have
// refreshed that cache.
bssl::UniquePtr<X509_REQ> copy = ReencodeCSR(csr.get());
ASSERT_TRUE(copy);
EXPECT_TRUE(X509_REQ_verify(copy.get(), pkey.get()));
// Check the signature was over the new public key.
bssl::UniquePtr<EVP_PKEY> copy_pubkey(X509_REQ_get_pubkey(copy.get()));
ASSERT_TRUE(copy_pubkey);
EXPECT_EQ(1, EVP_PKEY_cmp(pkey.get(), copy_pubkey.get()));
}
}
}

@ -96,11 +96,13 @@ int X509_sign_ctx(X509 *x, EVP_MD_CTX *ctx) {
}
int X509_REQ_sign(X509_REQ *x, EVP_PKEY *pkey, const EVP_MD *md) {
x->req_info->enc.modified = 1;
return (ASN1_item_sign(ASN1_ITEM_rptr(X509_REQ_INFO), x->sig_alg, NULL,
x->signature, x->req_info, pkey, md));
}
int X509_REQ_sign_ctx(X509_REQ *x, EVP_MD_CTX *ctx) {
x->req_info->enc.modified = 1;
return ASN1_item_sign_ctx(ASN1_ITEM_rptr(X509_REQ_INFO), x->sig_alg, NULL,
x->signature, x->req_info, ctx);
}

Loading…
Cancel
Save