Skip to content

Commit 1f367c9

Browse files
authored
Apply additional X509 validation checks on certificates sourced from trust store (#2230)
### Description of changes: Improves validation of certificates by also applying additional checks for certificates retrieved from the trust store. This aligns AWS-LC with OpenSSL 1.1.1 and later behavior. ### Testing: Add a root certificate that does not have the expected basicConstraints extension with the cA bit set to true. This certificate would be rejected by OpenSSL, but is currently allowed to be trusted by AWS-LC due to it being in the trust store. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
1 parent 8d77e4c commit 1f367c9

File tree

2 files changed

+63
-2
lines changed

2 files changed

+63
-2
lines changed

crypto/x509/x509_test.cc

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,6 +1469,55 @@ TXHOSQQD8Dl4BK0wOet+TP6LBEjHlRFjAqK4bu9xpxV2
14691469
-----END CERTIFICATE-----
14701470
)";
14711471

1472+
// This is a "root certificate" where the basicConstraints extension
1473+
// has marked this with cA:false. The root certificate though has the
1474+
// keyCertSign bit set for the keyUsage extension.
1475+
//
1476+
// Private Key:
1477+
// -----BEGIN PRIVATE KEY-----
1478+
// MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgfVMH4tqIaJ6OzyxY
1479+
// mqWXNwmK7gpXYDFhX80mXKgzrGGhRANCAATCqXrfbdTjFimzdBHxj71Ejcc/stea
1480+
// 5xAU/xxK+s77yXzB5lfy/zEbcYxuOrnwHrWsX9sugWgCy74ZRNWJPTDW
1481+
// -----END PRIVATE KEY-----
1482+
static const char kRootBadBasicConstraints[] = R"(
1483+
-----BEGIN CERTIFICATE-----
1484+
MIIBmDCCAT+gAwIBAgIUdUDg7B26eXlz1kIvnpH+xuRxxWQwCgYIKoZIzj0EAwIw
1485+
KzEXMBUGA1UECgwOQVdTLUxDIFRlc3RpbmcxEDAOBgNVBAMMB1Jvb3QgQ0EwIBcN
1486+
MjUwMjI4MjIyODM5WhgPMjA5OTEyMjQyMjI4MzlaMCsxFzAVBgNVBAoMDkFXUy1M
1487+
QyBUZXN0aW5nMRAwDgYDVQQDDAdSb290IENBMFkwEwYHKoZIzj0CAQYIKoZIzj0D
1488+
AQcDQgAEwql6323U4xYps3QR8Y+9RI3HP7LXmucQFP8cSvrO+8l8weZX8v8xG3GM
1489+
bjq58B61rF/bLoFoAsu+GUTViT0w1qM/MD0wDgYDVR0PAQH/BAQDAgIEMAwGA1Ud
1490+
EwEB/wQCMAAwHQYDVR0OBBYEFBkZ4YwJ4l1cFgThnHRmGf24UlvfMAoGCCqGSM49
1491+
BAMCA0cAMEQCIBzU4Tsvcro7Ngh7OAvPoxeRJjV5Cxas3wpEeDWkVow0AiB+DCN+
1492+
fx6O+iOJWecFfO2w756B+z9LvBhfV8HA7Dflyw==
1493+
-----END CERTIFICATE-----
1494+
)";
1495+
1496+
// This is an end-entity certificate signed by |kRootBadBasicConstraints|.
1497+
// This should not be considered as the kRootBadBasicConstraints is a v3 certificate
1498+
// which has a basicConstraints extension indicating that it is not a CA.
1499+
//
1500+
// Private Key:
1501+
// -----BEGIN PRIVATE KEY-----
1502+
// MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgRpZ8cAJjAb0htcq+
1503+
// UkqSo8Ud1wsOrNE28cmjwFjBHsuhRANCAASyt7018uvahtXcQETHIxT50KVAFzCF
1504+
// tsYROMLbLMW8DBkR2Ghh1qOSa4oYUizchqetKa2RrH7fhyQ787RxK05Y
1505+
// -----END PRIVATE KEY-----
1506+
static const char kEndEntitySignedByBadRoot[] = R"(
1507+
-----BEGIN CERTIFICATE-----
1508+
MIIB1DCCAXqgAwIBAgIUeRFjDJSyJybmzn8Uf1u2NfICP7QwCgYIKoZIzj0EAwIw
1509+
KzEXMBUGA1UECgwOQVdTLUxDIFRlc3RpbmcxEDAOBgNVBAMMB1Jvb3QgQ0EwIBcN
1510+
MjUwMjI4MjIyODQ1WhgPMjA5OTEyMjQyMjI4NDVaMCYxFzAVBgNVBAoMDkFXUy1M
1511+
QyBUZXN0aW5nMQswCQYDVQQDDAJFRTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IA
1512+
BLK3vTXy69qG1dxARMcjFPnQpUAXMIW2xhE4wtssxbwMGRHYaGHWo5JrihhSLNyG
1513+
p60prZGsft+HJDvztHErTlijfzB9MA4GA1UdDwEB/wQEAwIHgDAMBgNVHRMBAf8E
1514+
AjAAMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAdBgNVHQ4EFgQUyHhk
1515+
6fecD1biHc7u7STgnx1Lo78wHwYDVR0jBBgwFoAUGRnhjAniXVwWBOGcdGYZ/bhS
1516+
W98wCgYIKoZIzj0EAwIDSAAwRQIhAPknNhPpJBWKSAVtY7yrwkmx2yCPU4u22kW0
1517+
C2Z6lTDpAiBUkWczZ5By5KTXVE/TX/dpNHhOr8VO/rTCY0YNjBiZ1w==
1518+
-----END CERTIFICATE-----
1519+
)";
1520+
14721521
// CRLFromPEM parses the given, NUL-terminated PEM block and returns an
14731522
// |X509_CRL*|.
14741523
static bssl::UniquePtr<X509_CRL> CRLFromPEM(const char *pem) {
@@ -8204,3 +8253,14 @@ TEST(X509Test, Trust) {
82048253
{intermediate.normal.get()}, {},
82058254
/*flags=*/0, set_server_trust));
82068255
}
8256+
8257+
TEST(X509Test, CertificatesFromTrustStoreValidated) {
8258+
bssl::UniquePtr<X509> root = CertFromPEM(kRootBadBasicConstraints);
8259+
ASSERT_TRUE(root);
8260+
bssl::UniquePtr<X509> leaf = CertFromPEM(kEndEntitySignedByBadRoot);
8261+
ASSERT_TRUE(leaf);
8262+
8263+
EXPECT_EQ(X509_V_ERR_INVALID_CA,
8264+
Verify(leaf.get(), /*roots=*/{root.get()}, /*intermediates=*/{},
8265+
/*crls=*/{}, /*flags=*/0));
8266+
}

crypto/x509/x509_vfy.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,8 +568,9 @@ static int check_chain_extensions(X509_STORE_CTX *ctx) {
568568
int ok = 0, plen = 0;
569569
int purpose = ctx->param->purpose;
570570

571-
// Check all untrusted certificates
572-
for (int i = 0; i < ctx->last_untrusted; i++) {
571+
int num = (int)sk_X509_num(ctx->chain);
572+
573+
for (int i = 0; i < num; i++) {
573574
X509 *x = sk_X509_value(ctx->chain, i);
574575
if (!(ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL) &&
575576
(x->ex_flags & EXFLAG_CRITICAL)) {

0 commit comments

Comments
 (0)