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.

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

Upstream merge 2024 06 24 #1661

Merged
merged 4 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 6 additions & 11 deletions crypto/x509/v3_purp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
99 changes: 90 additions & 9 deletions crypto/x509/x509_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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-----
)";

Expand Down Expand Up @@ -1740,6 +1741,86 @@ TEST(X509Test, VerifyThreads) {
thread.join();
}
}

// Using the same CRL on two threads should be safe.
TEST(X509Test, CRLThreads) {
bssl::UniquePtr<X509> root(CertFromPEM(kCRLTestRoot));
bssl::UniquePtr<X509> leaf(CertFromPEM(kCRLTestLeaf));
bssl::UniquePtr<X509_CRL> basic_crl(CRLFromPEM(kBasicCRL));
bssl::UniquePtr<X509_CRL> 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<std::thread> 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<X509> root(CertFromPEM(kRootCAPEM));
bssl::UniquePtr<X509> intermediate(CertFromPEM(kIntermediatePEM));
bssl::UniquePtr<X509> leaf(CertFromPEM(kLeafPEM));
ASSERT_TRUE(root);
ASSERT_TRUE(intermediate);
ASSERT_TRUE(leaf);

bssl::UniquePtr<STACK_OF(X509)> intermediates =
CertsToStack({intermediate.get()});
ASSERT_TRUE(intermediates);

// Some unrelated certificates.
bssl::UniquePtr<X509> other1(CertFromPEM(kCRLTestRoot));
bssl::UniquePtr<X509> other2(CertFromPEM(kCRLTestLeaf));
ASSERT_TRUE(other1);
ASSERT_TRUE(other2);

bssl::UniquePtr<X509_STORE> store(X509_STORE_new());
ASSERT_TRUE(store);
ASSERT_TRUE(X509_STORE_add_cert(store.get(), root.get()));

const size_t kNumThreads = 10;
std::vector<std::thread> threads;
for (size_t i = 0; i < kNumThreads; i++) {
threads.emplace_back([&] {
bssl::UniquePtr<X509_STORE_CTX> 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";
Expand Down
12 changes: 0 additions & 12 deletions crypto/x509/x509_trs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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}};

Expand Down Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions include/openssl/target.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
2 changes: 0 additions & 2 deletions include/openssl/x509.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading