Commit c0452248 authored by Rich Salz's avatar Rich Salz
Browse files

Ignore dups in X509_STORE_add_* 



X509_STORE_add_cert and X509_STORE_add_crl are changed to return
success if the object to be added was already found in the store, rather
than returning an error.

Raise errors if empty or malformed files are read when loading certificates
and CRLs.

Remove NULL checks and allow a segv to occur.
Add error handing for all calls to X509_STORE_add_c{ert|tl}

Refactor these two routines into one.

Bring the unit test for duplicate certificates up to date using the test
framework.

Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2830)
parent 0444c52a

CHANGES

+6 −0
Original line number Diff line number Diff line
@@ -22,6 +22,12 @@ 
     platform rather than 'mingw'.

     [Richard Levitte]













  *) The functions X509_STORE_add_cert and X509_STORE_add_crl return













     success if they are asked to add an object which already exists













     in the store. This change cascades to other functions which load













     certificates and CRLs.













     [Paul Dale]



























  *) x86_64 assembly pack: annotate code with DWARF CFI directives to















     facilitate stack unwinding even from assembly subroutines.















     [Andy Polyakov]

































crypto/x509/by_dir.c



+8
−1






























Original line number
Diff line number
Diff line















/*













 * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.













 * Copyright 1995-2017 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










@@ -373,6 +373,13 @@ static int get_cert_by_subject(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,













            ok = 1;















            ret->type = tmp->type;















            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

































crypto/x509/by_file.c



+14
−7






























Original line number
Diff line number
Diff line















/*













 * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.













 * Copyright 1995-2017 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










@@ -79,8 +79,6 @@ int X509_load_cert_file(X509_LOOKUP *ctx, const char *file, int type)













    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)) {










@@ -123,6 +121,8 @@ int X509_load_cert_file(X509_LOOKUP *ctx, const char *file, int type)













        X509err(X509_F_X509_LOAD_CERT_FILE, X509_R_BAD_X509_FILETYPE);















        goto err;















    }













    if (ret == 0)













        X509err(X509_F_X509_LOAD_CERT_FILE, X509_R_NO_CERTIFICATE_FOUND);















 err:















    X509_free(x);















    BIO_free(in);









@@ -136,8 +136,6 @@ int X509_load_crl_file(X509_LOOKUP *ctx, const char *file, int type)













    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)) {










@@ -180,6 +178,8 @@ int X509_load_crl_file(X509_LOOKUP *ctx, const char *file, int type)













        X509err(X509_F_X509_LOAD_CRL_FILE, X509_R_BAD_X509_FILETYPE);















        goto err;















    }













    if (ret == 0)













        X509err(X509_F_X509_LOAD_CRL_FILE, X509_R_NO_CRL_FOUND);















 err:















    X509_CRL_free(x);















    BIO_free(in);









@@ -192,6 +192,7 @@ int X509_load_cert_crl_file(X509_LOOKUP *ctx, const char *file, int type)













    X509_INFO *itmp;















    BIO *in;















    int i, count = 0;





























    if (type != X509_FILETYPE_PEM)















        return X509_load_cert_file(ctx, file, type);















    in = BIO_new_file(file, "r");









@@ -208,14 +209,20 @@ int X509_load_cert_crl_file(X509_LOOKUP *ctx, const char *file, int type)













    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)













        X509err(X509_F_X509_LOAD_CERT_CRL_FILE,













                X509_R_NO_CERTIFICATE_OR_CRL_FOUND);













 err:















    sk_X509_INFO_pop_free(inf, X509_INFO_free);















    return count;















}
























crypto/x509/x509_err.c



+5
−1






























Original line number
Diff line number
Diff line















/*















 * Generated by util/mkerr.pl DO NOT EDIT













 * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.













 * Copyright 1995-2017 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










@@ -107,8 +107,12 @@ static ERR_STRING_DATA X509_str_reasons[] = {













    {ERR_REASON(X509_R_METHOD_NOT_SUPPORTED), "method not supported"},















    {ERR_REASON(X509_R_NAME_TOO_LONG), "name too long"},















    {ERR_REASON(X509_R_NEWER_CRL_NOT_NEWER), "newer crl not newer"},













    {ERR_REASON(X509_R_NO_CERTIFICATE_FOUND), "no certificate found"},













    {ERR_REASON(X509_R_NO_CERTIFICATE_OR_CRL_FOUND),













     "no certificate or crl found"},















    {ERR_REASON(X509_R_NO_CERT_SET_FOR_US_TO_VERIFY),















     "no cert set for us to verify"},













    {ERR_REASON(X509_R_NO_CRL_FOUND), "no crl found"},















    {ERR_REASON(X509_R_NO_CRL_NUMBER), "no crl number"},















    {ERR_REASON(X509_R_PUBLIC_KEY_DECODE_ERROR), "public key decode error"},















    {ERR_REASON(X509_R_PUBLIC_KEY_ENCODE_ERROR), "public key encode error"},

































crypto/x509/x509_lu.c



+24
−39






























Original line number
Diff line number
Diff line















/*













 * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.













 * Copyright 1995-2017 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










@@ -290,26 +290,29 @@ 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;



























    if (crl) {













        obj->type = X509_LU_CRL;













        obj->data.crl = (X509_CRL *)x;













    } else {















        obj->type = X509_LU_X509;













    obj->data.x509 = x;













        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;









@@ -317,46 +320,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)