Revert "Check AlgorithmIdentifier parameters for RSA and ECDSA signatures."
This reverts commit 9dd9d4fc242f31584f5a42e41e63d132c790421f.
BUG=b/167375496,342
Original change's description:
> Check AlgorithmIdentifier parameters for RSA and ECDSA signatures.
>
> This aligns with the Chromium certificate verifier, which allows NULL or
> empty for RSA and requires empty for ECDSA.
>
> Bug: 342
> Change-Id: I34acf68f63b4d133dd47b73144b2f27224c499ee
> Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41804
> Reviewed-by: Adam Langley <agl@google.com>
> Commit-Queue: David Benjamin <davidben@google.com>
TBR=agl@google.com,davidben@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: If8f136a09fea68e64c9f4f9ffae88b6209ede124
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42804
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/x509/algorithm.c b/crypto/x509/algorithm.c
index b9f3314..8f53fff 100644
--- a/crypto/x509/algorithm.c
+++ b/crypto/x509/algorithm.c
@@ -142,14 +142,6 @@
return 0;
}
- /* RSA signature algorithms include an explicit NULL parameter but we also
- * accept omitted values for compatibility. Other algorithms must omit it. */
- if (sigalg->parameter != NULL && (pkey_nid != EVP_PKEY_RSA ||
- sigalg->parameter->type != V_ASN1_NULL)) {
- OPENSSL_PUT_ERROR(X509, X509_R_INVALID_PARAMETER);
- return 0;
- }
-
/* Otherwise, initialize with the digest from the OID. */
const EVP_MD *digest = EVP_get_digestbynid(digest_nid);
if (digest == NULL) {
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 38c9bc5..d71dee9 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -233,13 +233,6 @@
"moZWgjHvB2W9Ckn7sDqsPB+U2tyX0joDdQEyuiMECDY8oQ==\n"
"-----END RSA PRIVATE KEY-----\n";
-static const char kP256Key[] =
- "-----BEGIN PRIVATE KEY-----\n"
- "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgBw8IcnrUoEqc3VnJ\n"
- "TYlodwi1b8ldMHcO6NHJzgqLtGqhRANCAATmK2niv2Wfl74vHg2UikzVl2u3qR4N\n"
- "Rvvdqakendy6WgHn1peoChj5w8SjHlbifINI2xYaHPUdfvGULUvPciLB\n"
- "-----END PRIVATE KEY-----\n";
-
// kCRLTestRoot is a test root certificate. It has private key:
//
// -----BEGIN RSA PRIVATE KEY-----
@@ -2452,112 +2445,3 @@
EXPECT_FALSE(CertFromPEM(kV1WithIssuerUniqueIDPEM));
EXPECT_FALSE(CertFromPEM(kV1WithSubjectUniqueIDPEM));
}
-
-// The following strings are test certificates signed by kP256Key and kRSAKey,
-// with missing, NULL, or invalid algorithm parameters.
-static const char kP256NoParam[] =
- "-----BEGIN CERTIFICATE-----\n"
- "MIIBIDCBxqADAgECAgIE0jAKBggqhkjOPQQDAjAPMQ0wCwYDVQQDEwRUZXN0MCAX\n"
- "DTAwMDEwMTAwMDAwMFoYDzIxMDAwMTAxMDAwMDAwWjAPMQ0wCwYDVQQDEwRUZXN0\n"
- "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5itp4r9ln5e+Lx4NlIpM1Zdrt6ke\n"
- "DUb73ampHp3culoB59aXqAoY+cPEox5W4nyDSNsWGhz1HX7xlC1Lz3IiwaMQMA4w\n"
- "DAYDVR0TBAUwAwEB/zAKBggqhkjOPQQDAgNJADBGAiEAqdIiF+bN9Cl44oUeICpy\n"
- "aXd7HqhpVUaglYKw9ChmNUACIQCpMdL0fNkFNDbRww9dSl/y7kBdk/tp16HiqeSy\n"
- "gGzFYg==\n"
- "-----END CERTIFICATE-----\n";
-static const char kP256NullParam[] =
- "-----BEGIN CERTIFICATE-----\n"
- "MIIBJDCByKADAgECAgIE0jAMBggqhkjOPQQDAgUAMA8xDTALBgNVBAMTBFRlc3Qw\n"
- "IBcNMDAwMTAxMDAwMDAwWhgPMjEwMDAxMDEwMDAwMDBaMA8xDTALBgNVBAMTBFRl\n"
- "c3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATmK2niv2Wfl74vHg2UikzVl2u3\n"
- "qR4NRvvdqakendy6WgHn1peoChj5w8SjHlbifINI2xYaHPUdfvGULUvPciLBoxAw\n"
- "DjAMBgNVHRMEBTADAQH/MAwGCCqGSM49BAMCBQADSQAwRgIhAKILHmyo+F3Cn/VX\n"
- "UUeSXOQQKX5aLzsQitwwmNF3ZgH3AiEAsYHcrVj/ftmoQIORARkQ/+PrqntXev8r\n"
- "t6uPxHrmpUY=\n"
- "-----END CERTIFICATE-----\n";
-static const char kP256InvalidParam[] =
- "-----BEGIN CERTIFICATE-----\n"
- "MIIBMTCBz6ADAgECAgIE0jATBggqhkjOPQQDAgQHZ2FyYmFnZTAPMQ0wCwYDVQQD\n"
- "EwRUZXN0MCAXDTAwMDEwMTAwMDAwMFoYDzIxMDAwMTAxMDAwMDAwWjAPMQ0wCwYD\n"
- "VQQDEwRUZXN0MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5itp4r9ln5e+Lx4N\n"
- "lIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY+cPEox5W4nyDSNsWGhz1HX7xlC1L\n"
- "z3IiwaMQMA4wDAYDVR0TBAUwAwEB/zATBggqhkjOPQQDAgQHZ2FyYmFnZQNIADBF\n"
- "AiAglpDf/YhN89LeJ2WAs/F0SJIrsuhS4uoInIz6WXUiuQIhAIu5Pwhp5E3Pbo8y\n"
- "fLULTZnynuQUULQkRcF7S7T2WpIL\n"
- "-----END CERTIFICATE-----\n";
-static const char kRSANoParam[] =
- "-----BEGIN CERTIFICATE-----\n"
- "MIIBWzCBx6ADAgECAgIE0jALBgkqhkiG9w0BAQswDzENMAsGA1UEAxMEVGVzdDAg\n"
- "Fw0wMDAxMDEwMDAwMDBaGA8yMTAwMDEwMTAwMDAwMFowDzENMAsGA1UEAxMEVGVz\n"
- "dDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABOYraeK/ZZ+Xvi8eDZSKTNWXa7ep\n"
- "Hg1G+92pqR6d3LpaAefWl6gKGPnDxKMeVuJ8g0jbFhoc9R1+8ZQtS89yIsGjEDAO\n"
- "MAwGA1UdEwQFMAMBAf8wCwYJKoZIhvcNAQELA4GBAC1f8W3W0Ao7CPfIBQYDSbPh\n"
- "brZpbxdBU5x27JOS7iSa+Lc9pEH5VCX9vIypHVHXLPEfZ38yIt11eiyrmZB6w62N\n"
- "l9kIeZ6FVPmC30d3sXx70Jjs+ZX9yt7kD1gLyNAQQfeYfa4rORAZT1n2YitD74NY\n"
- "TWUH2ieFP3l+ecj1SeQR\n"
- "-----END CERTIFICATE-----\n";
-static const char kRSANullParam[] =
- "-----BEGIN CERTIFICATE-----\n"
- "MIIBXzCByaADAgECAgIE0jANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDEwRUZXN0\n"
- "MCAXDTAwMDEwMTAwMDAwMFoYDzIxMDAwMTAxMDAwMDAwWjAPMQ0wCwYDVQQDEwRU\n"
- "ZXN0MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5itp4r9ln5e+Lx4NlIpM1Zdr\n"
- "t6keDUb73ampHp3culoB59aXqAoY+cPEox5W4nyDSNsWGhz1HX7xlC1Lz3IiwaMQ\n"
- "MA4wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOBgQAzVcfIv+Rq1KrMXqIL\n"
- "fPq/cWZjgqFZA1RGaGElNaqp+rkJfamq5tDGzckWpebrK+jjRN7yIlcWDtPpy3Gy\n"
- "seZfvtBDR0TwJm0S/pQl8prKB4wgALcwe3bmi56Rq85nzY5ZLNcP16LQxL+jAAua\n"
- "SwmQUz4bRpckRBj+sIyp1We+pg==\n"
- "-----END CERTIFICATE-----\n";
-static const char kRSAInvalidParam[] =
- "-----BEGIN CERTIFICATE-----\n"
- "MIIBbTCB0KADAgECAgIE0jAUBgkqhkiG9w0BAQsEB2dhcmJhZ2UwDzENMAsGA1UE\n"
- "AxMEVGVzdDAgFw0wMDAxMDEwMDAwMDBaGA8yMTAwMDEwMTAwMDAwMFowDzENMAsG\n"
- "A1UEAxMEVGVzdDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABOYraeK/ZZ+Xvi8e\n"
- "DZSKTNWXa7epHg1G+92pqR6d3LpaAefWl6gKGPnDxKMeVuJ8g0jbFhoc9R1+8ZQt\n"
- "S89yIsGjEDAOMAwGA1UdEwQFMAMBAf8wFAYJKoZIhvcNAQELBAdnYXJiYWdlA4GB\n"
- "AHTJ6cWWjCNrZhqiWWVI3jdK+h5xpRG8jGMXxR4JnjtoYRRusJLOXhmapwCB6fA0\n"
- "4vc+66O27v36yDmQX+tIc/hDrTpKNJptU8q3n2VagREvoHhkOTYkcCeS8vmnMtn8\n"
- "5OMNZ/ajVwOssw61GcAlScRqEHkZFBoGp7e+QpgB2tf9\n"
- "-----END CERTIFICATE-----\n";
-
-TEST(X509Test, AlgorithmParameters) {
- // P-256 requires the parameter be omitted.
- bssl::UniquePtr<EVP_PKEY> key = PrivateKeyFromPEM(kP256Key);
- ASSERT_TRUE(key);
-
- bssl::UniquePtr<X509> cert = CertFromPEM(kP256NoParam);
- ASSERT_TRUE(cert);
- EXPECT_TRUE(X509_verify(cert.get(), key.get()));
-
- cert = CertFromPEM(kP256NullParam);
- ASSERT_TRUE(cert);
- EXPECT_FALSE(X509_verify(cert.get(), key.get()));
- uint32_t err = ERR_get_error();
- EXPECT_EQ(ERR_LIB_X509, ERR_GET_LIB(err));
- EXPECT_EQ(X509_R_INVALID_PARAMETER, ERR_GET_REASON(err));
-
- cert = CertFromPEM(kP256InvalidParam);
- ASSERT_TRUE(cert);
- EXPECT_FALSE(X509_verify(cert.get(), key.get()));
- err = ERR_get_error();
- EXPECT_EQ(ERR_LIB_X509, ERR_GET_LIB(err));
- EXPECT_EQ(X509_R_INVALID_PARAMETER, ERR_GET_REASON(err));
-
- // RSA parameters should be NULL, but we accept omitted ones.
- key = PrivateKeyFromPEM(kRSAKey);
- ASSERT_TRUE(key);
-
- cert = CertFromPEM(kRSANoParam);
- ASSERT_TRUE(cert);
- EXPECT_TRUE(X509_verify(cert.get(), key.get()));
-
- cert = CertFromPEM(kRSANullParam);
- ASSERT_TRUE(cert);
- EXPECT_TRUE(X509_verify(cert.get(), key.get()));
-
- cert = CertFromPEM(kRSAInvalidParam);
- ASSERT_TRUE(cert);
- EXPECT_FALSE(X509_verify(cert.get(), key.get()));
- err = ERR_get_error();
- EXPECT_EQ(ERR_LIB_X509, ERR_GET_LIB(err));
- EXPECT_EQ(X509_R_INVALID_PARAMETER, ERR_GET_REASON(err));
-}
