Ignore duplicates in |X509_STORE_add_*|
This change imports upstream's
https://github.com/openssl/openssl/commit/c0452248ea1a59a41023a4765ef7d9825e80a62b
Change-Id: Ib50ff9eb8c48d9580aa2ffcae92d3990cc987e30
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50905
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/err/x509.errordata b/crypto/err/x509.errordata
index ffa4267..65181bf 100644
--- a/crypto/err/x509.errordata
+++ b/crypto/err/x509.errordata
@@ -25,8 +25,11 @@
X509,119,NEWER_CRL_NOT_NEWER
X509,120,NOT_PKCS7_SIGNED_DATA
X509,121,NO_CERTIFICATES_INCLUDED
+X509,141,NO_CERTIFICATE_FOUND
+X509,142,NO_CERTIFICATE_OR_CRL_FOUND
X509,122,NO_CERT_SET_FOR_US_TO_VERIFY
X509,123,NO_CRLS_INCLUDED
+X509,143,NO_CRL_FOUND
X509,124,NO_CRL_NUMBER
X509,125,PUBLIC_KEY_DECODE_ERROR
X509,126,PUBLIC_KEY_ENCODE_ERROR
diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c
index a630cdf..2f16944 100644
--- a/crypto/x509/by_dir.c
+++ b/crypto/x509/by_dir.c
@@ -438,6 +438,13 @@
ok = 1;
ret->type = tmp->type;
OPENSSL_memcpy(&ret->data, &tmp->data, sizeof(ret->data));
+
+ /*
+ * Clear any errors that might have been raised processing empty
+ * or malformed files.
+ */
+ ERR_clear_error();
+
/*
* If we were going to up the reference count, we would need
* to do it on a perl 'type' basis
diff --git a/crypto/x509/by_file.c b/crypto/x509/by_file.c
index 1614c8c..0714b4f 100644
--- a/crypto/x509/by_file.c
+++ b/crypto/x509/by_file.c
@@ -126,8 +126,6 @@
int i, count = 0;
X509 *x = NULL;
- if (file == NULL)
- return (1);
in = BIO_new(BIO_s_file());
if ((in == NULL) || (BIO_read_filename(in, file) <= 0)) {
@@ -171,6 +169,11 @@
OPENSSL_PUT_ERROR(X509, X509_R_BAD_X509_FILETYPE);
goto err;
}
+
+ if (ret == 0) {
+ OPENSSL_PUT_ERROR(X509, X509_R_NO_CERTIFICATE_FOUND);
+ }
+
err:
if (x != NULL)
X509_free(x);
@@ -186,8 +189,6 @@
int i, count = 0;
X509_CRL *x = NULL;
- if (file == NULL)
- return (1);
in = BIO_new(BIO_s_file());
if ((in == NULL) || (BIO_read_filename(in, file) <= 0)) {
@@ -231,6 +232,11 @@
OPENSSL_PUT_ERROR(X509, X509_R_BAD_X509_FILETYPE);
goto err;
}
+
+ if (ret == 0) {
+ OPENSSL_PUT_ERROR(X509, X509_R_NO_CRL_FOUND);
+ }
+
err:
if (x != NULL)
X509_CRL_free(x);
@@ -246,6 +252,7 @@
BIO *in;
size_t i;
int count = 0;
+
if (type != X509_FILETYPE_PEM)
return X509_load_cert_file(ctx, file, type);
in = BIO_new_file(file, "r");
@@ -262,14 +269,24 @@
for (i = 0; i < sk_X509_INFO_num(inf); i++) {
itmp = sk_X509_INFO_value(inf, i);
if (itmp->x509) {
- X509_STORE_add_cert(ctx->store_ctx, itmp->x509);
+ if (!X509_STORE_add_cert(ctx->store_ctx, itmp->x509)) {
+ goto err;
+ }
count++;
}
if (itmp->crl) {
- X509_STORE_add_crl(ctx->store_ctx, itmp->crl);
+ if (!X509_STORE_add_crl(ctx->store_ctx, itmp->crl)) {
+ goto err;
+ }
count++;
}
}
+
+ if (count == 0) {
+ OPENSSL_PUT_ERROR(X509, X509_R_NO_CERTIFICATE_OR_CRL_FOUND);
+ }
+
+err:
sk_X509_INFO_pop_free(inf, X509_INFO_free);
return count;
}
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 041a5fd..4680e61 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -332,76 +332,54 @@
return 1;
}
-int X509_STORE_add_cert(X509_STORE *ctx, X509 *x)
+static int x509_store_add(X509_STORE *ctx, void *x, int is_crl)
{
- X509_OBJECT *obj;
- int ret = 1;
-
- if (x == NULL)
+ if (x == NULL) {
return 0;
- obj = (X509_OBJECT *)OPENSSL_malloc(sizeof(X509_OBJECT));
+ }
+
+ X509_OBJECT *const obj = (X509_OBJECT *)OPENSSL_malloc(sizeof(X509_OBJECT));
if (obj == NULL) {
OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
return 0;
}
- obj->type = X509_LU_X509;
- obj->data.x509 = x;
+
+ if (is_crl) {
+ obj->type = X509_LU_CRL;
+ obj->data.crl = (X509_CRL *)x;
+ } else {
+ obj->type = X509_LU_X509;
+ obj->data.x509 = (X509 *)x;
+ }
+ X509_OBJECT_up_ref_count(obj);
CRYPTO_MUTEX_lock_write(&ctx->objs_lock);
- X509_OBJECT_up_ref_count(obj);
-
- if (X509_OBJECT_retrieve_match(ctx->objs, obj)) {
- X509_OBJECT_free_contents(obj);
- OPENSSL_free(obj);
- OPENSSL_PUT_ERROR(X509, X509_R_CERT_ALREADY_IN_HASH_TABLE);
- ret = 0;
- } else if (!sk_X509_OBJECT_push(ctx->objs, obj)) {
- X509_OBJECT_free_contents(obj);
- OPENSSL_free(obj);
- OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
- ret = 0;
+ int ret = 1;
+ int added = 0;
+ /* Duplicates are silently ignored */
+ if (!X509_OBJECT_retrieve_match(ctx->objs, obj)) {
+ ret = added = (sk_X509_OBJECT_push(ctx->objs, obj) != 0);
}
CRYPTO_MUTEX_unlock_write(&ctx->objs_lock);
+ if (!added) {
+ X509_OBJECT_free_contents(obj);
+ OPENSSL_free(obj);
+ }
+
return ret;
}
+int X509_STORE_add_cert(X509_STORE *ctx, X509 *x)
+{
+ return x509_store_add(ctx, x, /*is_crl=*/0);
+}
+
int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x)
{
- X509_OBJECT *obj;
- int ret = 1;
-
- if (x == NULL)
- return 0;
- obj = (X509_OBJECT *)OPENSSL_malloc(sizeof(X509_OBJECT));
- if (obj == NULL) {
- OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
- return 0;
- }
- obj->type = X509_LU_CRL;
- obj->data.crl = x;
-
- CRYPTO_MUTEX_lock_write(&ctx->objs_lock);
-
- X509_OBJECT_up_ref_count(obj);
-
- if (X509_OBJECT_retrieve_match(ctx->objs, obj)) {
- X509_OBJECT_free_contents(obj);
- OPENSSL_free(obj);
- OPENSSL_PUT_ERROR(X509, X509_R_CERT_ALREADY_IN_HASH_TABLE);
- ret = 0;
- } else if (!sk_X509_OBJECT_push(ctx->objs, obj)) {
- X509_OBJECT_free_contents(obj);
- OPENSSL_free(obj);
- OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
- ret = 0;
- }
-
- CRYPTO_MUTEX_unlock_write(&ctx->objs_lock);
-
- return ret;
+ return x509_store_add(ctx, x, /*is_crl=*/1);
}
int X509_OBJECT_up_ref_count(X509_OBJECT *a)
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 691b452..38414e9 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -3855,3 +3855,22 @@
}
}
}
+
+TEST(X509Test, AddDuplicates) {
+ bssl::UniquePtr<X509_STORE> store(X509_STORE_new());
+ bssl::UniquePtr<X509> a(CertFromPEM(kCrossSigningRootPEM));
+ bssl::UniquePtr<X509> b(CertFromPEM(kRootCAPEM));
+
+ ASSERT_TRUE(store);
+ ASSERT_TRUE(a);
+ ASSERT_TRUE(b);
+
+ 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()));
+
+ EXPECT_EQ(sk_X509_OBJECT_num(X509_STORE_get0_objects(store.get())), 2u);
+}
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 3c47789..6696988 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -2404,5 +2404,8 @@
#define X509_R_DELTA_CRL_WITHOUT_CRL_NUMBER 138
#define X509_R_INVALID_FIELD_FOR_VERSION 139
#define X509_R_INVALID_VERSION 140
+#define X509_R_NO_CERTIFICATE_FOUND 141
+#define X509_R_NO_CERTIFICATE_OR_CRL_FOUND 142
+#define X509_R_NO_CRL_FOUND 143
#endif
