From f96d3c1fc65fae4193bdda464819bb1180feba5a Mon Sep 17 00:00:00 2001 From: Pauli Date: Mon, 6 Aug 2018 07:31:49 +1000 Subject: [PATCH] Avoid errors when loading a cert multiple times. Manual backport of #2830 to 1.1.0 Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/6861) --- crypto/x509/x509_lu.c | 62 ++++++++++-------------- test/build.info | 6 ++- test/recipes/60-test_x509_dup_cert.t | 19 ++++++++ test/x509_dup_cert_test.c | 70 ++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 39 deletions(-) create mode 100644 test/recipes/60-test_x509_dup_cert.t create mode 100644 test/x509_dup_cert_test.c diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index b80cc8e3f2..e5bea5b276 100644 --- 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 87961bc834..d850b5229c 100644 --- 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 index 0000000000..8e1c313814 --- /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 index 0000000000..7f7adebbb0 --- /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 +#include +#include + +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; +} -- 2.25.1