diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index a7a4e888e2..fdd920f779 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -118,26 +118,21 @@ static const X509_PURPOSE xstandard[] = { (char *)"timestampsign", NULL}, }; -// As much as I'd like to make X509_check_purpose use a "const" X509* I -// really can't because it does recalculate hashes and do other non-const -// things. If |id| is -1 it just calls |x509v3_cache_extensions| for its -// side-effect. -// Returns 1 on success, 0 if x does not allow purpose, -1 on (internal) error. int X509_check_purpose(X509 *x, int id, int ca) { - int idx; - const X509_PURPOSE *pt; + // This differs from OpenSSL, which uses -1 to indicate a fatal error and 0 to + // indicate an invalid certificate. BoringSSL uses 0 for both. if (!x509v3_cache_extensions(x)) { - return -1; + return 0; } if (id == -1) { return 1; } - idx = X509_PURPOSE_get_by_id(id); + int idx = X509_PURPOSE_get_by_id(id); if (idx == -1) { - return -1; + return 0; } - pt = X509_PURPOSE_get0(idx); + const X509_PURPOSE *pt = X509_PURPOSE_get0(idx); return pt->check_purpose(pt, x, ca); } diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index 9cb794de16..6d78309855 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -400,16 +400,17 @@ diyu0fZ/bPAM3VAGawatf/SyWfBMyKpoPXEG39oAzmjjOj8en82psn7m474IGaho static const char kRevokedCRL[] = R"( -----BEGIN X509 CRL----- -MIIBvjCBpwIBATANBgkqhkiG9w0BAQsFADBOMQswCQYDVQQGEwJVUzETMBEGA1UE +MIIB6DCB0QIBATANBgkqhkiG9w0BAQsFADBOMQswCQYDVQQGEwJVUzETMBEGA1UE CAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNTW91bnRhaW4gVmlldzESMBAGA1UECgwJ -Qm9yaW5nU1NMFw0xNjA5MjYxNTEyNDRaFw0xNjEwMjYxNTEyNDRaMBUwEwICEAAX -DTE2MDkyNjE1MTIyNlqgDjAMMAoGA1UdFAQDAgECMA0GCSqGSIb3DQEBCwUAA4IB -AQCUGaM4DcWzlQKrcZvI8TMeR8BpsvQeo5BoI/XZu2a8h//PyRyMwYeaOM+3zl0d -sjgCT8b3C1FPgT+P2Lkowv7rJ+FHJRNQkogr+RuqCSPTq65ha4WKlRGWkMFybzVH -NloxC+aU3lgp/NlX9yUtfqYmJek1CDrOOGPrAEAwj1l/BUeYKNGqfBWYJQtPJu+5 -OaSvIYGpETCZJscUWODmLEb/O3DM438vLvxonwGqXqS0KX37+CHpUlyhnSovxXxp -Pz4aF+L7OtczxL0GYtD2fR9B7TDMqsNmHXgQrixvvOY7MUdLGbd4RfJL3yA53hyO -xzfKY2TzxLiOmctG0hXFkH5J +Qm9yaW5nU1NMFw0xNjA5MjYxNTEyNDRaFw0xNjEwMjYxNTEyNDRaMD8wEwICEAAX +DTE2MDkyNjE1MTIyNlowEwICD/8XDTE2MDkyNjE1MTIyNlowEwICEAEXDTE2MDky +NjE1MTIyNlqgDjAMMAoGA1UdFAQDAgECMA0GCSqGSIb3DQEBCwUAA4IBAQAuepA9 +byX4PkpJcYKb+Ofn6f/mgB4Sh1BBRp0HMFm7Q3jryXB6yPZYp7UtwAufZUzbV42U +kOifOtDyQbu+fcpnpb4D9oWoFlr4DGQ+n23wujHizoTEYhhcZMj4U5yooDfmK4lI +GU6zzQZKG+1PaS6Dm4f6+kA+ekVUP+ZVjPqHP/K7Uv6akSKV7gAWs49N5QjrJKMQ +3Igrbg4Et2ipaYgThGj8t1MUZdVY4UPtQ8oltSHkFEvH4PxOW/xKpHT4QQMl/WTT +QsQqOlRJBG2ci7pu1fzOylY35YFEAApkND/MkQjQfylNNgCzDWWAPQx0pDvVKQ0v +WXdfcGJRL1D3xRXk -----END X509 CRL----- )"; @@ -1740,6 +1741,86 @@ TEST(X509Test, VerifyThreads) { thread.join(); } } + +// Using the same CRL on two threads should be safe. +TEST(X509Test, CRLThreads) { + bssl::UniquePtr root(CertFromPEM(kCRLTestRoot)); + bssl::UniquePtr leaf(CertFromPEM(kCRLTestLeaf)); + bssl::UniquePtr basic_crl(CRLFromPEM(kBasicCRL)); + bssl::UniquePtr revoked_crl(CRLFromPEM(kRevokedCRL)); + ASSERT_TRUE(root); + ASSERT_TRUE(leaf); + ASSERT_TRUE(basic_crl); + ASSERT_TRUE(revoked_crl); + + const size_t kNumThreads = 10; + std::vector threads; + for (size_t i = 0; i < kNumThreads; i++) { + threads.emplace_back([&] { + EXPECT_EQ(X509_V_OK, Verify(leaf.get(), {root.get()}, {root.get()}, + {basic_crl.get()}, X509_V_FLAG_CRL_CHECK)); + }); + threads.emplace_back([&] { + EXPECT_EQ(X509_V_ERR_CERT_REVOKED, + Verify(leaf.get(), {root.get()}, {root.get()}, + {revoked_crl.get()}, X509_V_FLAG_CRL_CHECK)); + }); + } + + for (auto &thread : threads) { + thread.join(); + } + + // TODO(crbug.com/boringssl/600): Add a thread that iterates + // |X509_CRL_get_REVOKED| and a thread that calls |X509_CRL_print|. Those + // currently do not work correctly. +} + +TEST(X509Test, StoreThreads) { + bssl::UniquePtr root(CertFromPEM(kRootCAPEM)); + bssl::UniquePtr intermediate(CertFromPEM(kIntermediatePEM)); + bssl::UniquePtr leaf(CertFromPEM(kLeafPEM)); + ASSERT_TRUE(root); + ASSERT_TRUE(intermediate); + ASSERT_TRUE(leaf); + + bssl::UniquePtr intermediates = + CertsToStack({intermediate.get()}); + ASSERT_TRUE(intermediates); + + // Some unrelated certificates. + bssl::UniquePtr other1(CertFromPEM(kCRLTestRoot)); + bssl::UniquePtr other2(CertFromPEM(kCRLTestLeaf)); + ASSERT_TRUE(other1); + ASSERT_TRUE(other2); + + bssl::UniquePtr store(X509_STORE_new()); + ASSERT_TRUE(store); + ASSERT_TRUE(X509_STORE_add_cert(store.get(), root.get())); + + const size_t kNumThreads = 10; + std::vector threads; + for (size_t i = 0; i < kNumThreads; i++) { + threads.emplace_back([&] { + bssl::UniquePtr ctx(X509_STORE_CTX_new()); + ASSERT_TRUE(ctx); + ASSERT_TRUE(X509_STORE_CTX_init(ctx.get(), store.get(), leaf.get(), + intermediates.get())); + X509_STORE_CTX_set_time_posix(ctx.get(), /*flags=*/0, kReferenceTime); + ASSERT_TRUE(X509_verify_cert(ctx.get())); + ASSERT_EQ(X509_STORE_CTX_get_error(ctx.get()), X509_V_OK); + }); + threads.emplace_back([&] { + ASSERT_TRUE(X509_STORE_add_cert(store.get(), other1.get())); + }); + threads.emplace_back([&] { + ASSERT_TRUE(X509_STORE_add_cert(store.get(), other2.get())); + }); + } + for (auto &thread : threads) { + thread.join(); + } +} #endif // OPENSSL_THREADS static const char kHostname[] = "example.com"; diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c index 680f6d651e..95daf46657 100644 --- a/crypto/x509/x509_trs.c +++ b/crypto/x509/x509_trs.c @@ -67,7 +67,6 @@ static int trust_1oidany(const X509_TRUST *trust, X509 *x, int flags); -static int trust_1oid(const X509_TRUST *trust, X509 *x, int flags); static int trust_compat(const X509_TRUST *trust, X509 *x, int flags); static int obj_trust(int id, X509 *x, int flags); @@ -82,10 +81,6 @@ static const X509_TRUST trstandard[] = { NID_email_protect, NULL}, {X509_TRUST_OBJECT_SIGN, 0, trust_1oidany, (char *)"Object Signer", NID_code_sign, NULL}, - {X509_TRUST_OCSP_SIGN, 0, trust_1oid, (char *)"OCSP responder", - NID_OCSP_sign, NULL}, - {X509_TRUST_OCSP_REQUEST, 0, trust_1oid, (char *)"OCSP request", - NID_ad_OCSP, NULL}, {X509_TRUST_TSA, 0, trust_1oidany, (char *)"TSA server", NID_time_stamp, NULL}}; @@ -155,13 +150,6 @@ static int trust_1oidany(const X509_TRUST *trust, X509 *x, int flags) { return trust_compat(trust, x, flags); } -static int trust_1oid(const X509_TRUST *trust, X509 *x, int flags) { - if (x->aux) { - return obj_trust(trust->arg1, x, flags); - } - return X509_TRUST_UNTRUSTED; -} - static int trust_compat(const X509_TRUST *trust, X509 *x, int flags) { if (!x509v3_cache_extensions(x)) { return X509_TRUST_UNTRUSTED; diff --git a/include/openssl/target.h b/include/openssl/target.h index feb0bdfd4c..35f1add1aa 100644 --- a/include/openssl/target.h +++ b/include/openssl/target.h @@ -87,7 +87,8 @@ #endif // Trusty and Android baremetal aren't Linux but currently define __linux__. -// As a workaround, we exclude them here. We also exclude nanolibc. nanolibc +// As a workaround, we exclude them here. +// We also exclude nanolibc/CrOS EC/Zephyr. nanolibc/CrOS EC/Zephyr // sometimes build for a non-Linux target (which should not define __linux__), // but also sometimes build for Linux. Although technically running in Linux // userspace, this lacks all the libc APIs we'd normally expect on Linux, so we @@ -97,7 +98,8 @@ // TODO(b/291101350): Remove this workaround once Android baremetal no longer // defines it. #if defined(__linux__) && !defined(__TRUSTY__) && \ - !defined(ANDROID_BAREMETAL) && !defined(OPENSSL_NANOLIBC) + !defined(ANDROID_BAREMETAL) && !defined(OPENSSL_NANOLIBC) && \ + !defined(CROS_EC) && !defined(CROS_ZEPHYR) #define OPENSSL_LINUX #endif diff --git a/include/openssl/x509.h b/include/openssl/x509.h index e157f15c35..a0cf4ce9f2 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -3652,8 +3652,6 @@ DEFINE_STACK_OF(X509_TRUST) #define X509_TRUST_SSL_SERVER 3 #define X509_TRUST_EMAIL 4 #define X509_TRUST_OBJECT_SIGN 5 -#define X509_TRUST_OCSP_SIGN 6 -#define X509_TRUST_OCSP_REQUEST 7 #define X509_TRUST_TSA 8 // check_trust return codes