Use the certificate issuer in CMS/PKCS7 signer IDs We were using the subject and not noticing because all our tests test with self-signed certificates. Change-Id: I16774442c463a50030547fc57f078bc387cf643e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/95287 Auto-Submit: David Benjamin <davidben@google.com> Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Lily Chen <chlily@google.com> Commit-Queue: Lily Chen <chlily@google.com>
diff --git a/crypto/cms/cms_test.cc b/crypto/cms/cms_test.cc index 9a96f38..205cee1 100644 --- a/crypto/cms/cms_test.cc +++ b/crypto/cms/cms_test.cc
@@ -41,11 +41,16 @@ // Sign a message with the same call that the Linux kernel's sign-file.c // makes. std::string cert_pem = GetTestData("crypto/pkcs7/test/sign_cert.pem"); + std::string cert2_pem = GetTestData("crypto/pkcs7/test/sign_cert2.pem"); std::string key_pem = GetTestData("crypto/pkcs7/test/sign_key.pem"); bssl::UniquePtr<BIO> cert_bio( BIO_new_mem_buf(cert_pem.data(), cert_pem.size())); bssl::UniquePtr<X509> cert( PEM_read_bio_X509(cert_bio.get(), nullptr, nullptr, nullptr)); + bssl::UniquePtr<BIO> cert2_bio( + BIO_new_mem_buf(cert2_pem.data(), cert2_pem.size())); + bssl::UniquePtr<X509> cert2( + PEM_read_bio_X509(cert2_bio.get(), nullptr, nullptr, nullptr)); bssl::UniquePtr<BIO> key_bio(BIO_new_mem_buf(key_pem.data(), key_pem.size())); bssl::UniquePtr<EVP_PKEY> key( @@ -181,4 +186,15 @@ EXPECT_FALSE(cms); EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_CMS, CMS_R_CERTIFICATE_HAS_NO_KEYID)); + + // Sign with a certificate that isn't self-signed. The issuer and serial tuple + // should use the correct name. + ASSERT_TRUE(BIO_reset(data_bio.get())); + cms.reset(CMS_sign(cert2.get(), key.get(), nullptr, data_bio.get(), + CMS_DETACHED | CMS_NOCERTS | CMS_NOATTR | CMS_BINARY)); + ASSERT_TRUE(cms); + ASSERT_TRUE(BIO_reset(out.get())); + ASSERT_TRUE(i2d_CMS_bio(out.get(), cms.get())); + expected = GetTestData("crypto/pkcs7/test/sign_sha256_cert2.p7s"); + EXPECT_EQ(Bytes(BIOMemContents(out.get())), Bytes(expected)); }
diff --git a/crypto/pkcs7/pkcs7_x509.cc b/crypto/pkcs7/pkcs7_x509.cc index dad05af..07975b2 100644 --- a/crypto/pkcs7/pkcs7_x509.cc +++ b/crypto/pkcs7/pkcs7_x509.cc
@@ -431,7 +431,7 @@ } } else { if (!CBB_add_asn1(&seq, &child, CBS_ASN1_SEQUENCE) || - !x509_marshal_name(&child, X509_get_subject_name(si_data->sign_cert)) || + !x509_marshal_name(&child, X509_get_issuer_name(si_data->sign_cert)) || !asn1_marshal_integer(&child, X509_get0_serialNumber(si_data->sign_cert), /*tag=*/0)) {
diff --git a/crypto/pkcs7/test/sign_cert2.pem b/crypto/pkcs7/test/sign_cert2.pem new file mode 100644 index 0000000..95f7350 --- /dev/null +++ b/crypto/pkcs7/test/sign_cert2.pem
@@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIIE2DCCAsACCQCeL08RMjYgJjANBgkqhkiG9w0BAQsFADBFMQswCQYDVQQGEwJB +VTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0 +cyBQdHkgTHRkMB4XDTI2MDUxNDAzMzIxMFoXDTI3MDUxNDAzMzIxMFowFzEVMBMG +A1UEAwwMT3RoZXIgU2lnbmVyMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKC +AgEAtfjDp/gaKXBFUeEDL413QMRpFhZZV8kBQ06C/qjoj0VNcjewvCgrZGzgBbbS +SDuDv4tImeyqmG55gemFAPzjQHGpKHJ63aBEoESXFTHgMQ5P9dTrTBeciH3jjjv0 +1HlTZgFGk+t4BWysv2naaocybTr/QQZeVXGDRhkhPtMLW5o6ZjbMbpxEEdSiw3Pe +3xmB3kXyqEsqwiBBPCGgBKShkdkpLt7/ScIe0aY7qYIlAJB+qSZO1TGZub5dsBY2 +L38rII3jeWbbMRQzGj2GUracTMaTXeQvQ7Dti46eTTKRnzRphuGaomkIPM9nCDOh +EpjdcwyXVIeSF3ndXzsQWQ/6aFBR9MA0RVp785Wj5oyRBa2CzCvLL5xOZxEKfb7v +dyEhQce3KH7Pp4jGlFzxKmkUqq7onzdjNYrd5Dn//GGZdb7q7PlbRw+pziaOm3dd +ft+2gYnSAfwWlp8y6w5kk3DxWtbZEx8KC6nPfo04U6vY2esN5QDD6bKHp8g9fVJk +AefPfuJDadkiDv07c1UAhoED1xNowwpehvWiDzJH76T2SuEx1Ypd1kEQhWJ2HQzu +B0F6NmbguEYqoGOcoH3qC/n2vZN/N5YMcQB0C0u8bG2lQeNEZn3W3Xr4D2CMGTTT +u/HXUZjHUr+iJJ0lvYMnRs2P/27WPrdUlnAWVs59RMKvINkCAwEAATANBgkqhkiG +9w0BAQsFAAOCAgEAbI1udg8Lhg4+mFz0nDmLMM1MPIKxDkD6hLgpI0BmALEXeVIm +XkCngsl6tKjgZ9RslnJjTfaQB9PdjE3TJ1rktnWZgadKofelznYbn4y3gPISUJ2U +mQPYa9LTsCLDVcCusgs8iW6xqZBSWYZjylEjnhqWEt3u5FFBtNFFxWuUfRRbR+of +Ttu8yJyli4TtsKR+IHDgznx+ohOVAr9AO6fmE3VcvDal82pJv8M016EPxzQ4kmAJ +YQAD4LM0VgNfL4rDNsve8kKONAzn7XW8TS460SqHcDeDx90gx/WyoMpHm21sBPw8 +4PQqJzi/CMHga5phav4c2vt73UlKKt9bEoi/HJy5QJTtW3uYizS8mKQUhU36Aj7K +ye3Dnr6674joSSo5aPbzTsSSrChzbNFfqk0zP2ev6QYxoyEHqDYM2zqctP++T8EZ +kSY1/9GuduGysi5hRej1NnSYqeoUhOW99Ye+V4Z3Y5fbnsuLAuCDzb9HF5IeNKvc +nckMUa7FGoP4H7wOMo4auXxiM5fGKwAj5B5+8jw5YnSSiIY1w+ldp3vJemWz3WVy +jRO4kSUj+SoQaM129JU7O9yQ8PlEjyF3x8kvJKPJ7buHtRIq5m1htNhOhbmrRFH6 +bhE8Rp6MJtc4nWAjD5RXmYW1hp/xqyJaa36W6j+vTFjRziTR7Yfm27XxjcA= +-----END CERTIFICATE-----
diff --git a/crypto/pkcs7/test/sign_sha256_cert2.p7s b/crypto/pkcs7/test/sign_sha256_cert2.p7s new file mode 100644 index 0000000..1c68eda --- /dev/null +++ b/crypto/pkcs7/test/sign_sha256_cert2.p7s Binary files differ
diff --git a/gen/sources.bzl b/gen/sources.bzl index c30b959..913c7aa 100644 --- a/gen/sources.bzl +++ b/gen/sources.bzl
@@ -921,10 +921,12 @@ "crypto/pkcs7/test/nss.p7c", "crypto/pkcs7/test/openssl_crl.p7c", "crypto/pkcs7/test/sign_cert.pem", + "crypto/pkcs7/test/sign_cert2.pem", "crypto/pkcs7/test/sign_key.pem", "crypto/pkcs7/test/sign_sha1.p7s", "crypto/pkcs7/test/sign_sha1_key_id.p7s", "crypto/pkcs7/test/sign_sha256.p7s", + "crypto/pkcs7/test/sign_sha256_cert2.p7s", "crypto/pkcs7/test/sign_sha256_key_id.p7s", "crypto/pkcs7/test/windows.p7c", "crypto/pkcs8/test/bad1.p12",
diff --git a/gen/sources.cmake b/gen/sources.cmake index 27f50ff..44b1a09 100644 --- a/gen/sources.cmake +++ b/gen/sources.cmake
@@ -951,10 +951,12 @@ crypto/pkcs7/test/nss.p7c crypto/pkcs7/test/openssl_crl.p7c crypto/pkcs7/test/sign_cert.pem + crypto/pkcs7/test/sign_cert2.pem crypto/pkcs7/test/sign_key.pem crypto/pkcs7/test/sign_sha1.p7s crypto/pkcs7/test/sign_sha1_key_id.p7s crypto/pkcs7/test/sign_sha256.p7s + crypto/pkcs7/test/sign_sha256_cert2.p7s crypto/pkcs7/test/sign_sha256_key_id.p7s crypto/pkcs7/test/windows.p7c crypto/pkcs8/test/bad1.p12
diff --git a/gen/sources.gni b/gen/sources.gni index acaab66..147cc0f 100644 --- a/gen/sources.gni +++ b/gen/sources.gni
@@ -921,10 +921,12 @@ "crypto/pkcs7/test/nss.p7c", "crypto/pkcs7/test/openssl_crl.p7c", "crypto/pkcs7/test/sign_cert.pem", + "crypto/pkcs7/test/sign_cert2.pem", "crypto/pkcs7/test/sign_key.pem", "crypto/pkcs7/test/sign_sha1.p7s", "crypto/pkcs7/test/sign_sha1_key_id.p7s", "crypto/pkcs7/test/sign_sha256.p7s", + "crypto/pkcs7/test/sign_sha256_cert2.p7s", "crypto/pkcs7/test/sign_sha256_key_id.p7s", "crypto/pkcs7/test/windows.p7c", "crypto/pkcs8/test/bad1.p12",
diff --git a/gen/sources.json b/gen/sources.json index d866d05..ad7e940 100644 --- a/gen/sources.json +++ b/gen/sources.json
@@ -901,10 +901,12 @@ "crypto/pkcs7/test/nss.p7c", "crypto/pkcs7/test/openssl_crl.p7c", "crypto/pkcs7/test/sign_cert.pem", + "crypto/pkcs7/test/sign_cert2.pem", "crypto/pkcs7/test/sign_key.pem", "crypto/pkcs7/test/sign_sha1.p7s", "crypto/pkcs7/test/sign_sha1_key_id.p7s", "crypto/pkcs7/test/sign_sha256.p7s", + "crypto/pkcs7/test/sign_sha256_cert2.p7s", "crypto/pkcs7/test/sign_sha256_key_id.p7s", "crypto/pkcs7/test/windows.p7c", "crypto/pkcs8/test/bad1.p12",
diff --git a/gen/sources.mk b/gen/sources.mk index 7eaf370..478d1d7 100644 --- a/gen/sources.mk +++ b/gen/sources.mk
@@ -907,10 +907,12 @@ crypto/pkcs7/test/nss.p7c \ crypto/pkcs7/test/openssl_crl.p7c \ crypto/pkcs7/test/sign_cert.pem \ + crypto/pkcs7/test/sign_cert2.pem \ crypto/pkcs7/test/sign_key.pem \ crypto/pkcs7/test/sign_sha1.p7s \ crypto/pkcs7/test/sign_sha1_key_id.p7s \ crypto/pkcs7/test/sign_sha256.p7s \ + crypto/pkcs7/test/sign_sha256_cert2.p7s \ crypto/pkcs7/test/sign_sha256_key_id.p7s \ crypto/pkcs7/test/windows.p7c \ crypto/pkcs8/test/bad1.p12 \