Avoid errors when loading a cert multiple times.

Manual backport of #2830 to 1.1.0

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6861)

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index b80cc8e3f2a08d4a3571006ca9ec2b81decd2f34..e5bea5b2764e92d421b7fc4f72ae96952e6e1fc9 100644 (file)
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -310,26 +310,30 @@ int X509_STORE_CTX_get_by_subject(X509_STORE_CTX *vs, X509_LOOKUP_TYPE type,
     return 1;
 }
 
-int X509_STORE_add_cert(X509_STORE *ctx, X509 *x)
+static int x509_store_add(X509_STORE *ctx, void *x, int crl)
 {
     X509_OBJECT *obj;
-    int ret = 1, added = 1;
+    int ret = 0, added = 0;
 
     if (x == NULL)
         return 0;
     obj = X509_OBJECT_new();
     if (obj == NULL)
         return 0;
-    obj->type = X509_LU_X509;
-    obj->data.x509 = x;
+
+    if (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_THREAD_write_lock(ctx->lock);
 
     if (X509_OBJECT_retrieve_match(ctx->objs, obj)) {
-        X509err(X509_F_X509_STORE_ADD_CERT,
-                X509_R_CERT_ALREADY_IN_HASH_TABLE);
-        ret = 0;
+        ret = 1;
     } else {
         added = sk_X509_OBJECT_push(ctx->objs, obj);
         ret = added != 0;
@@ -337,46 +341,28 @@ int X509_STORE_add_cert(X509_STORE *ctx, X509 *x)
 
     CRYPTO_THREAD_unlock(ctx->lock);
 
-    if (!ret)                   /* obj not pushed */
+    if (added == 0)             /* obj not pushed */
         X509_OBJECT_free(obj);
-    if (!added)                 /* on push failure */
-        X509err(X509_F_X509_STORE_ADD_CERT, ERR_R_MALLOC_FAILURE);
 
     return ret;
 }
 
-int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x)
+int X509_STORE_add_cert(X509_STORE *ctx, X509 *x)
 {
-    X509_OBJECT *obj;
-    int ret = 1, added = 1;
-
-    if (x == NULL)
-        return 0;
-    obj = X509_OBJECT_new();
-    if (obj == NULL)
+   if (!x509_store_add(ctx, x, 0)) {
+        X509err(X509_F_X509_STORE_ADD_CERT, ERR_R_MALLOC_FAILURE);
         return 0;
-    obj->type = X509_LU_CRL;
-    obj->data.crl = x;
-    X509_OBJECT_up_ref_count(obj);
-
-    CRYPTO_THREAD_write_lock(ctx->lock);
-
-    if (X509_OBJECT_retrieve_match(ctx->objs, obj)) {
-        X509err(X509_F_X509_STORE_ADD_CRL, X509_R_CERT_ALREADY_IN_HASH_TABLE);
-        ret = 0;
-    } else {
-        added = sk_X509_OBJECT_push(ctx->objs, obj);
-        ret = added != 0;
     }
+    return 1;
+}
 
-    CRYPTO_THREAD_unlock(ctx->lock);
-
-    if (!ret)                   /* obj not pushed */
-        X509_OBJECT_free(obj);
-    if (!added)                 /* on push failure */
+int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x)
+{
+    if (!x509_store_add(ctx, x, 1)) {
         X509err(X509_F_X509_STORE_ADD_CRL, ERR_R_MALLOC_FAILURE);
-
-    return ret;
+        return 0;
+    }
+    return 1;
 }
 
 int X509_OBJECT_up_ref_count(X509_OBJECT *a)

diff --git a/test/build.info b/test/build.info
index 87961bc834525eec35be8c2cec88874e2afde4b2..d850b5229cee63e750468b39a3236599722dbae6 100644 (file)
--- a/test/build.info
+++ b/test/build.info
@@ -18,7 +18,7 @@ IF[{- !$disabled{tests} -}]
           dtlsv1listentest ct_test threadstest afalgtest d2i_test \
           ssl_test_ctx_test ssl_test x509aux cipherlist_test asynciotest \
           bioprinttest sslapitest dtlstest sslcorrupttest bio_enc_test \
-          ocspapitest fatalerrtest x509_time_test errtest
+          ocspapitest fatalerrtest x509_time_test x509_dup_cert_test errtest
 
   SOURCE[versions]=versions.c
   INCLUDE[versions]=../include
@@ -301,6 +301,10 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[x509_time_test]=.. ../include
   DEPEND[x509_time_test]=../libcrypto
 
+  SOURCE[x509_dup_cert_test]=x509_dup_cert_test.c
+  INCLUDE[x509_dup_cert_test]=../include
+  DEPEND[x509_dup_cert_test]=../libcrypto
+
   IF[{- !$disabled{shared} -}]
     PROGRAMS_NO_INST=shlibloadtest
     SOURCE[shlibloadtest]=shlibloadtest.c

diff --git a/test/recipes/60-test_x509_dup_cert.t b/test/recipes/60-test_x509_dup_cert.t
new file mode 100644 (file)
index 0000000..8e1c313
--- /dev/null
+++ b/test/recipes/60-test_x509_dup_cert.t
@@ -0,0 +1,19 @@
+#! /usr/bin/env perl
+# Copyright 2017-2018 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the OpenSSL license (the "License").  You may not use
+# this file except in compliance with the License.  You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+#
+# ======================================================================
+# Copyright (c) 2017 Oracle and/or its affiliates.  All rights reserved.
+
+
+use OpenSSL::Test qw/:DEFAULT srctop_file/;
+
+setup("test_x509_dup_cert");
+
+plan tests => 1;
+
+ok(run(test(["x509_dup_cert_test", srctop_file("test", "certs", "leaf.pem")])));

diff --git a/test/x509_dup_cert_test.c b/test/x509_dup_cert_test.c
new file mode 100644 (file)
index 0000000..7f7adeb
--- /dev/null
+++ b/test/x509_dup_cert_test.c
@@ -0,0 +1,70 @@
+/*
+ * Copyright 2017-2018 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL license (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+/* ====================================================================
+ * Copyright (c) 2017, 2018 Oracle and/or its affiliates.  All rights reserved.
+ */
+
+#include <stdio.h>
+#include <openssl/err.h>
+#include <openssl/x509_vfy.h>
+
+static int test_509_dup_cert(const char *cert_f)
+{
+    int ret = 0;
+    X509_STORE_CTX *sctx = NULL;
+    X509_STORE *store = NULL;
+    X509_LOOKUP *lookup = NULL;
+
+    store = X509_STORE_new();
+    if (store == NULL)
+        goto err;
+
+    lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file());
+    if (lookup == NULL)
+        goto err;
+
+    if (!X509_load_cert_file(lookup, cert_f, X509_FILETYPE_PEM))
+        goto err;
+    if (!X509_load_cert_file(lookup, cert_f, X509_FILETYPE_PEM))
+        goto err;
+
+    ret = 1;
+
+ err:
+    X509_STORE_CTX_free(sctx);
+    X509_STORE_free(store);
+    if (ret != 1)
+        ERR_print_errors_fp(stderr);
+    return ret;
+}
+
+int main(int argc, char **argv)
+{
+    CRYPTO_set_mem_debug(1);
+    CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
+
+    if (argc != 2) {
+        fprintf(stderr, "usage: x509_dup_cert_test cert.pem\n");
+        return 1;
+    }
+
+    if (!test_509_dup_cert(argv[1])) {
+        fprintf(stderr, "Test X509 duplicate cert failed\n");
+        return 1;
+    }
+
+#ifndef OPENSSL_NO_CRYPTO_MDEBUG
+    if (CRYPTO_mem_leaks_fp(stderr) <= 0)
+        return 1;
+#endif
+
+    printf("PASS\n");
+    return 0;
+}