Skip to content

Commit

Permalink
Add misc. x509 un/lock and set1 functions
Browse files Browse the repository at this point in the history
The set1 functions are cribbed from [OpenSSL's implementation][1], but
notably differ in that we don't free the `X509_OBJECT`. This is pursuant
to OpenSSL's [own convention][2] around "set1" functions:

> In the 1 version the ownership of the object is not passed to or
> retained by the parent object. Instead a copy or "up ref" of the
> object is performed.

OpenSSL [frees][3] the input `X509_OBJECT`; we do not.

[1]: https://github.com/openssl/openssl/blob/4a6f70c03182b421d326831532edca32bcdb3fb1/crypto/x509/x509_lu.c#L506
[2]: https://www.openssl.org/docs/man3.2/man7/ossl-guide-libraries-introduction.html
[3]: https://github.com/openssl/openssl/blob/4a6f70c03182b421d326831532edca32bcdb3fb1/crypto/x509/x509_lu.c#L511
  • Loading branch information
WillChilds-Klein committed Feb 23, 2024
1 parent f618701 commit f8f98e4
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 4 deletions.
30 changes: 30 additions & 0 deletions crypto/x509/x509_lu.c
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,16 @@ X509_STORE *X509_STORE_new(void) {
return NULL;
}

int X509_STORE_lock(X509_STORE *v) {
CRYPTO_MUTEX_lock_write(&v->objs_lock);
return 1;
}

int X509_STORE_unlock(X509_STORE *v) {
CRYPTO_MUTEX_unlock_write(&v->objs_lock);
return 1;
}

int X509_STORE_up_ref(X509_STORE *store) {
CRYPTO_refcount_inc(&store->references);
return 1;
Expand Down Expand Up @@ -410,6 +420,26 @@ X509_CRL *X509_OBJECT_get0_X509_CRL(const X509_OBJECT *a) {
return a->data.crl;
}

int X509_OBJECT_set1_X509(X509_OBJECT *a, X509 *obj) {
if (a == NULL || !X509_up_ref(obj)) {
return 0;
}

a->type = X509_LU_X509;
a->data.x509 = obj;
return 1;
}

int X509_OBJECT_set1_X509_CRL(X509_OBJECT *a, X509_CRL *obj) {
if (a == NULL || !X509_CRL_up_ref(obj)) {
return 0;
}

a->type = X509_LU_CRL;
a->data.crl = obj;
return 1;
}

static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, int type,
X509_NAME *name, int *pnmatch) {
X509_OBJECT stmp;
Expand Down
55 changes: 51 additions & 4 deletions crypto/x509/x509_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <openssl/nid.h>
#include <openssl/pem.h>
#include <openssl/pool.h>
#include <openssl/rand.h>
#include <openssl/x509.h>
#include <openssl/x509v3.h>

Expand Down Expand Up @@ -1945,6 +1946,24 @@ TEST(X509Test, TestCRL) {
ASSERT_EQ(nullptr, X509_OBJECT_get0_X509_CRL(&invalidCRL));
}

TEST(X509Test, TestX509GettersSetters) {
bssl::UniquePtr<X509_OBJECT> obj(X509_OBJECT_new());
bssl::UniquePtr<X509> x509(CertFromPEM(kCRLTestRoot));
bssl::UniquePtr<X509_CRL> crl(CRLFromPEM(kBasicCRL));

ASSERT_TRUE(obj);
ASSERT_TRUE(x509);
ASSERT_TRUE(crl);

EXPECT_EQ(0, X509_OBJECT_get0_X509(obj.get()));
EXPECT_EQ(0, X509_OBJECT_get0_X509_CRL(obj.get()));

EXPECT_EQ(1, X509_OBJECT_set1_X509(obj.get(), x509.get()));
EXPECT_EQ(x509.get(), X509_OBJECT_get0_X509(obj.get()));
EXPECT_EQ(1, X509_OBJECT_set1_X509_CRL(obj.get(), crl.get()));
EXPECT_EQ(crl.get(), X509_OBJECT_get0_X509_CRL(obj.get()));
}

TEST(X509Test, ManyNamesAndConstraints) {
bssl::UniquePtr<X509> many_constraints(CertFromPEM(
GetTestData("crypto/x509/test/many_constraints.pem").c_str()));
Expand Down Expand Up @@ -5022,12 +5041,40 @@ TEST(X509Test, AddDuplicates) {
ASSERT_TRUE(a);
ASSERT_TRUE(b);

// To begin, add the certs to the store. Subsequent adds will be duplicative.
EXPECT_TRUE(X509_STORE_add_cert(store.get(), a.get()));
EXPECT_TRUE(X509_STORE_add_cert(store.get(), b.get()));
EXPECT_TRUE(X509_STORE_add_cert(store.get(), a.get()));
EXPECT_TRUE(X509_STORE_add_cert(store.get(), b.get()));
EXPECT_TRUE(X509_STORE_add_cert(store.get(), a.get()));
EXPECT_TRUE(X509_STORE_add_cert(store.get(), b.get()));

const size_t kNumThreads = 50;
std::vector<std::thread> threads;
for (size_t i = 0; i < kNumThreads; i++) {
threads.emplace_back([&] {
// Firstly, save off |i| in the thread's context.
const size_t idx = i;
// Sleep with some jitter to offset thread execution
uint8_t sleep_buf[1];
ASSERT_TRUE(RAND_bytes(sleep_buf, sizeof(sleep_buf)));
std::this_thread::sleep_for(std::chrono::milliseconds(sleep_buf[0] % 100));
// Half the threads add duplicate certs, the other half take a lock and
// look them up to exercise un/locking functions.
if (idx % 2 == 0) {
EXPECT_TRUE(X509_STORE_add_cert(store.get(), a.get()));
EXPECT_TRUE(X509_STORE_add_cert(store.get(), b.get()));
} else {
ASSERT_TRUE(X509_STORE_lock(store.get()));
EXPECT_TRUE(X509_OBJECT_retrieve_by_subject(
store->objs, X509_LU_X509, X509_get_subject_name(a.get())
));
EXPECT_TRUE(X509_OBJECT_retrieve_by_subject(
store->objs, X509_LU_X509, X509_get_subject_name(b.get())
));
ASSERT_TRUE(X509_STORE_unlock(store.get()));
}
});
}
for (auto &thread : threads) {
thread.join();
}

EXPECT_EQ(sk_X509_OBJECT_num(X509_STORE_get0_objects(store.get())), 2u);
}
Expand Down
5 changes: 5 additions & 0 deletions include/openssl/x509.h
Original file line number Diff line number Diff line change
Expand Up @@ -2800,7 +2800,11 @@ OPENSSL_EXPORT int X509_OBJECT_get_type(const X509_OBJECT *a);
OPENSSL_EXPORT X509 *X509_OBJECT_get0_X509(const X509_OBJECT *a);
// X509_OBJECT_get0_X509_CRL returns the |X509_CRL| associated with |a|
OPENSSL_EXPORT X509_CRL *X509_OBJECT_get0_X509_CRL(const X509_OBJECT *a);
int X509_OBJECT_set1_X509(X509_OBJECT *a, X509 *obj);
int X509_OBJECT_set1_X509_CRL(X509_OBJECT *a, X509_CRL *obj);
OPENSSL_EXPORT X509_STORE *X509_STORE_new(void);
OPENSSL_EXPORT int X509_STORE_lock(X509_STORE *v);
OPENSSL_EXPORT int X509_STORE_unlock(X509_STORE *v);
OPENSSL_EXPORT int X509_STORE_up_ref(X509_STORE *store);
OPENSSL_EXPORT void X509_STORE_free(X509_STORE *v);

Expand Down Expand Up @@ -3122,6 +3126,7 @@ BORINGSSL_MAKE_UP_REF(X509_CRL, X509_CRL_up_ref)
BORINGSSL_MAKE_DELETER(X509_EXTENSION, X509_EXTENSION_free)
BORINGSSL_MAKE_DELETER(X509_INFO, X509_INFO_free)
BORINGSSL_MAKE_DELETER(X509_LOOKUP, X509_LOOKUP_free)
BORINGSSL_MAKE_DELETER(X509_OBJECT, X509_OBJECT_free)
BORINGSSL_MAKE_DELETER(X509_NAME, X509_NAME_free)
BORINGSSL_MAKE_DELETER(X509_NAME_ENTRY, X509_NAME_ENTRY_free)
BORINGSSL_MAKE_DELETER(X509_PKEY, X509_PKEY_free)
Expand Down

0 comments on commit f8f98e4

Please sign in to comment.