Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply additional X509 validation checks on certificates sourced from trust store #2230

Open
wants to merge 2 commits into
base: x509
Choose a base branch
from
+63 −2
Open

Apply additional X509 validation checks on certificates sourced from trust store #2230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af1660d
Iterate full chain in check_chain_extensions to verify trusted certif…
skmcgrail Oct 4, 2024
97d12a1
Add test case with certificate
skmcgrail Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions crypto/x509/x509_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,55 @@ TXHOSQQD8Dl4BK0wOet+TP6LBEjHlRFjAqK4bu9xpxV2
-----END CERTIFICATE-----
)";

// This is a "root certificate" where the basicConstraints extension
// has marked this with cA:false. The root certificate though has the
// keyCertSign bit set for the keyUsage extension.
//
// Private Key:
// -----BEGIN PRIVATE KEY-----
// MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgfVMH4tqIaJ6OzyxY
// mqWXNwmK7gpXYDFhX80mXKgzrGGhRANCAATCqXrfbdTjFimzdBHxj71Ejcc/stea
// 5xAU/xxK+s77yXzB5lfy/zEbcYxuOrnwHrWsX9sugWgCy74ZRNWJPTDW
// -----END PRIVATE KEY-----
static const char kRootBadBasicConstraints[] = R"(
-----BEGIN CERTIFICATE-----
MIIBmDCCAT+gAwIBAgIUdUDg7B26eXlz1kIvnpH+xuRxxWQwCgYIKoZIzj0EAwIw
KzEXMBUGA1UECgwOQVdTLUxDIFRlc3RpbmcxEDAOBgNVBAMMB1Jvb3QgQ0EwIBcN
MjUwMjI4MjIyODM5WhgPMjA5OTEyMjQyMjI4MzlaMCsxFzAVBgNVBAoMDkFXUy1M
QyBUZXN0aW5nMRAwDgYDVQQDDAdSb290IENBMFkwEwYHKoZIzj0CAQYIKoZIzj0D
AQcDQgAEwql6323U4xYps3QR8Y+9RI3HP7LXmucQFP8cSvrO+8l8weZX8v8xG3GM
bjq58B61rF/bLoFoAsu+GUTViT0w1qM/MD0wDgYDVR0PAQH/BAQDAgIEMAwGA1Ud
EwEB/wQCMAAwHQYDVR0OBBYEFBkZ4YwJ4l1cFgThnHRmGf24UlvfMAoGCCqGSM49
BAMCA0cAMEQCIBzU4Tsvcro7Ngh7OAvPoxeRJjV5Cxas3wpEeDWkVow0AiB+DCN+
fx6O+iOJWecFfO2w756B+z9LvBhfV8HA7Dflyw==
-----END CERTIFICATE-----
)";

// This is an end-entity certificate signed by |kRootBadBasicConstraints|.
// This should not be considered as the kRootBadBasicConstraints is a v3 certificate
// which has a basicConstraints extension indicating that it is not a CA.
//
// Private Key:
// -----BEGIN PRIVATE KEY-----
// MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgRpZ8cAJjAb0htcq+
// UkqSo8Ud1wsOrNE28cmjwFjBHsuhRANCAASyt7018uvahtXcQETHIxT50KVAFzCF
// tsYROMLbLMW8DBkR2Ghh1qOSa4oYUizchqetKa2RrH7fhyQ787RxK05Y
// -----END PRIVATE KEY-----
static const char kEndEntitySignedByBadRoot[] = R"(
-----BEGIN CERTIFICATE-----
MIIB1DCCAXqgAwIBAgIUeRFjDJSyJybmzn8Uf1u2NfICP7QwCgYIKoZIzj0EAwIw
KzEXMBUGA1UECgwOQVdTLUxDIFRlc3RpbmcxEDAOBgNVBAMMB1Jvb3QgQ0EwIBcN
MjUwMjI4MjIyODQ1WhgPMjA5OTEyMjQyMjI4NDVaMCYxFzAVBgNVBAoMDkFXUy1M
QyBUZXN0aW5nMQswCQYDVQQDDAJFRTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IA
BLK3vTXy69qG1dxARMcjFPnQpUAXMIW2xhE4wtssxbwMGRHYaGHWo5JrihhSLNyG
p60prZGsft+HJDvztHErTlijfzB9MA4GA1UdDwEB/wQEAwIHgDAMBgNVHRMBAf8E
AjAAMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAdBgNVHQ4EFgQUyHhk
6fecD1biHc7u7STgnx1Lo78wHwYDVR0jBBgwFoAUGRnhjAniXVwWBOGcdGYZ/bhS
W98wCgYIKoZIzj0EAwIDSAAwRQIhAPknNhPpJBWKSAVtY7yrwkmx2yCPU4u22kW0
C2Z6lTDpAiBUkWczZ5By5KTXVE/TX/dpNHhOr8VO/rTCY0YNjBiZ1w==
-----END CERTIFICATE-----
)";

// CRLFromPEM parses the given, NUL-terminated PEM block and returns an
// |X509_CRL*|.
static bssl::UniquePtr<X509_CRL> CRLFromPEM(const char *pem) {
Expand Down Expand Up @@ -8204,3 +8253,14 @@ TEST(X509Test, Trust) {
{intermediate.normal.get()}, {},
/*flags=*/0, set_server_trust));
}

TEST(X509Test, CertificatesFromTrustStoreValidated) {
bssl::UniquePtr<X509> root = CertFromPEM(kRootBadBasicConstraints);
ASSERT_TRUE(root);
bssl::UniquePtr<X509> leaf = CertFromPEM(kEndEntitySignedByBadRoot);
ASSERT_TRUE(leaf);

EXPECT_EQ(X509_V_ERR_INVALID_CA,
Verify(leaf.get(), /*roots=*/{root.get()}, /*intermediates=*/{},
/*crls=*/{}, /*flags=*/0));
}
5 changes: 3 additions & 2 deletions crypto/x509/x509_vfy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,9 @@ static int check_chain_extensions(X509_STORE_CTX *ctx) {
int ok = 0, plen = 0;
int purpose = ctx->param->purpose;

// Check all untrusted certificates
for (int i = 0; i < ctx->last_untrusted; i++) {
int num = (int)sk_X509_num(ctx->chain);

for (int i = 0; i < num; i++) {
X509 *x = sk_X509_value(ctx->chain, i);
if (!(ctx->param->flags & X509_V_FLAG_IGNORE_CRITICAL) &&
(x->ex_flags & EXFLAG_CRITICAL)) {
Expand Down
Loading