[openssl] master update
dev at ddvo.net
dev at ddvo.net
Thu Feb 4 06:28:34 UTC 2021
The branch master has been updated
via d53b437f9992f974c1623e9b9b9bdf053aefbcc3 (commit)
from b91a13f429570512bfee290e8ec50096b0667e45 (commit)
- Log -----------------------------------------------------------------
commit d53b437f9992f974c1623e9b9b9bdf053aefbcc3
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date: Wed Dec 23 19:33:03 2020 +0100
Allow NULL arg to OPENSSL_sk_{dup,deep_copy} returning empty stack
This simplifies many usages
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14040)
-----------------------------------------------------------------------
Summary of changes:
crypto/cmp/cmp_ctx.c | 27 +++++++---------------
crypto/ocsp/ocsp_vfy.c | 10 ++++-----
crypto/stack/stack.c | 53 +++++++++++++++++++++++++++-----------------
crypto/ts/ts_rsp_sign.c | 9 +-------
crypto/x509/x509_cmp.c | 7 ++++--
crypto/x509/x509_vfy.c | 2 +-
doc/man3/DEFINE_STACK_OF.pod | 12 +++++-----
doc/man3/X509_new.pod | 13 +++++------
test/stack_test.c | 8 +++++++
9 files changed, 73 insertions(+), 68 deletions(-)
diff --git a/crypto/cmp/cmp_ctx.c b/crypto/cmp/cmp_ctx.c
index e1b4e50ea9..ccca282721 100644
--- a/crypto/cmp/cmp_ctx.c
+++ b/crypto/cmp/cmp_ctx.c
@@ -462,8 +462,6 @@ STACK_OF(X509) *OSSL_CMP_CTX_get1_newChain(const OSSL_CMP_CTX *ctx)
ERR_raise(ERR_LIB_CMP, CMP_R_NULL_ARGUMENT);
return NULL;
}
- if (ctx->newChain == NULL)
- return sk_X509_new_null();
return X509_chain_up_ref(ctx->newChain);
}
@@ -477,10 +475,9 @@ int ossl_cmp_ctx_set1_newChain(OSSL_CMP_CTX *ctx, STACK_OF(X509) *newChain)
return 0;
sk_X509_pop_free(ctx->newChain, X509_free);
- ctx->newChain= NULL;
- if (newChain == NULL)
- return 1;
- return (ctx->newChain = X509_chain_up_ref(newChain)) != NULL;
+ ctx->newChain = NULL;
+ return newChain == NULL ||
+ (ctx->newChain = X509_chain_up_ref(newChain)) != NULL;
}
/* Returns the stack of extraCerts received in CertRepMessage, NULL on error */
@@ -490,8 +487,6 @@ STACK_OF(X509) *OSSL_CMP_CTX_get1_extraCertsIn(const OSSL_CMP_CTX *ctx)
ERR_raise(ERR_LIB_CMP, CMP_R_NULL_ARGUMENT);
return NULL;
}
- if (ctx->extraCertsIn == NULL)
- return sk_X509_new_null();
return X509_chain_up_ref(ctx->extraCertsIn);
}
@@ -507,9 +502,8 @@ int ossl_cmp_ctx_set1_extraCertsIn(OSSL_CMP_CTX *ctx,
sk_X509_pop_free(ctx->extraCertsIn, X509_free);
ctx->extraCertsIn = NULL;
- if (extraCertsIn == NULL)
- return 1;
- return (ctx->extraCertsIn = X509_chain_up_ref(extraCertsIn)) != NULL;
+ return extraCertsIn == NULL
+ || (ctx->extraCertsIn = X509_chain_up_ref(extraCertsIn)) != NULL;
}
/*
@@ -526,9 +520,8 @@ int OSSL_CMP_CTX_set1_extraCertsOut(OSSL_CMP_CTX *ctx,
sk_X509_pop_free(ctx->extraCertsOut, X509_free);
ctx->extraCertsOut = NULL;
- if (extraCertsOut == NULL)
- return 1;
- return (ctx->extraCertsOut = X509_chain_up_ref(extraCertsOut)) != NULL;
+ return extraCertsOut == NULL
+ || (ctx->extraCertsOut = X509_chain_up_ref(extraCertsOut)) != NULL;
}
/*
@@ -580,8 +573,6 @@ STACK_OF(X509) *OSSL_CMP_CTX_get1_caPubs(const OSSL_CMP_CTX *ctx)
ERR_raise(ERR_LIB_CMP, CMP_R_NULL_ARGUMENT);
return NULL;
}
- if (ctx->caPubs == NULL)
- return sk_X509_new_null();
return X509_chain_up_ref(ctx->caPubs);
}
@@ -596,9 +587,7 @@ int ossl_cmp_ctx_set1_caPubs(OSSL_CMP_CTX *ctx, STACK_OF(X509) *caPubs)
sk_X509_pop_free(ctx->caPubs, X509_free);
ctx->caPubs = NULL;
- if (caPubs == NULL)
- return 1;
- return (ctx->caPubs = X509_chain_up_ref(caPubs)) != NULL;
+ return caPubs == NULL || (ctx->caPubs = X509_chain_up_ref(caPubs)) != NULL;
}
#define char_dup OPENSSL_strdup
diff --git a/crypto/ocsp/ocsp_vfy.c b/crypto/ocsp/ocsp_vfy.c
index f49f651007..56b9261640 100644
--- a/crypto/ocsp/ocsp_vfy.c
+++ b/crypto/ocsp/ocsp_vfy.c
@@ -113,10 +113,9 @@ int OCSP_basic_verify(OCSP_BASICRESP *bs, STACK_OF(X509) *certs,
goto end;
if ((flags & OCSP_NOVERIFY) == 0) {
ret = -1;
- if ((flags & OCSP_NOCHAIN) != 0) {
- untrusted = NULL;
- } else if (bs->certs != NULL && certs != NULL) {
- untrusted = sk_X509_dup(bs->certs);
+ if ((flags & OCSP_NOCHAIN) == 0) {
+ if ((untrusted = sk_X509_dup(bs->certs)) == NULL)
+ goto end;
if (!X509_add_certs(untrusted, certs, X509_ADD_FLAG_DEFAULT))
goto end;
} else if (certs != NULL) {
@@ -159,8 +158,7 @@ int OCSP_basic_verify(OCSP_BASICRESP *bs, STACK_OF(X509) *certs,
end:
sk_X509_pop_free(chain, X509_free);
- if (bs->certs && certs)
- sk_X509_free(untrusted);
+ sk_X509_free(untrusted);
return ret;
}
diff --git a/crypto/stack/stack.c b/crypto/stack/stack.c
index e38efad022..c50a55da14 100644
--- a/crypto/stack/stack.c
+++ b/crypto/stack/stack.c
@@ -45,26 +45,33 @@ OPENSSL_STACK *OPENSSL_sk_dup(const OPENSSL_STACK *sk)
{
OPENSSL_STACK *ret;
- if ((ret = OPENSSL_malloc(sizeof(*ret))) == NULL) {
- ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
- return NULL;
- }
+ if ((ret = OPENSSL_malloc(sizeof(*ret))) == NULL)
+ goto err;
- /* direct structure assignment */
- *ret = *sk;
+ if (sk == NULL) {
+ ret->num = 0;
+ ret->sorted = 0;
+ ret->comp = NULL;
+ } else {
+ /* direct structure assignment */
+ *ret = *sk;
+ }
- if (sk->num == 0) {
+ if (sk == NULL || sk->num == 0) {
/* postpone |ret->data| allocation */
ret->data = NULL;
ret->num_alloc = 0;
return ret;
}
+
/* duplicate |sk->data| content */
if ((ret->data = OPENSSL_malloc(sizeof(*ret->data) * sk->num_alloc)) == NULL)
goto err;
memcpy(ret->data, sk->data, sizeof(void *) * sk->num);
return ret;
+
err:
+ ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
OPENSSL_sk_free(ret);
return NULL;
}
@@ -76,15 +83,19 @@ OPENSSL_STACK *OPENSSL_sk_deep_copy(const OPENSSL_STACK *sk,
OPENSSL_STACK *ret;
int i;
- if ((ret = OPENSSL_malloc(sizeof(*ret))) == NULL) {
- ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
- return NULL;
- }
+ if ((ret = OPENSSL_malloc(sizeof(*ret))) == NULL)
+ goto err;
- /* direct structure assignment */
- *ret = *sk;
+ if (sk == NULL) {
+ ret->num = 0;
+ ret->sorted = 0;
+ ret->comp = NULL;
+ } else {
+ /* direct structure assignment */
+ *ret = *sk;
+ }
- if (sk->num == 0) {
+ if (sk == NULL || sk->num == 0) {
/* postpone |ret| data allocation */
ret->data = NULL;
ret->num_alloc = 0;
@@ -93,10 +104,8 @@ OPENSSL_STACK *OPENSSL_sk_deep_copy(const OPENSSL_STACK *sk,
ret->num_alloc = sk->num > min_nodes ? sk->num : min_nodes;
ret->data = OPENSSL_zalloc(sizeof(*ret->data) * ret->num_alloc);
- if (ret->data == NULL) {
- OPENSSL_free(ret);
- return NULL;
- }
+ if (ret->data == NULL)
+ goto err;
for (i = 0; i < ret->num; ++i) {
if (sk->data[i] == NULL)
@@ -105,11 +114,15 @@ OPENSSL_STACK *OPENSSL_sk_deep_copy(const OPENSSL_STACK *sk,
while (--i >= 0)
if (ret->data[i] != NULL)
free_func((void *)ret->data[i]);
- OPENSSL_sk_free(ret);
- return NULL;
+ goto err;
}
}
return ret;
+
+ err:
+ ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
+ OPENSSL_sk_free(ret);
+ return NULL;
}
OPENSSL_STACK *OPENSSL_sk_new_null(void)
diff --git a/crypto/ts/ts_rsp_sign.c b/crypto/ts/ts_rsp_sign.c
index 9ae584ff12..17024ea7bb 100644
--- a/crypto/ts/ts_rsp_sign.c
+++ b/crypto/ts/ts_rsp_sign.c
@@ -183,17 +183,10 @@ int TS_RESP_CTX_set_def_policy(TS_RESP_CTX *ctx, const ASN1_OBJECT *def_policy)
int TS_RESP_CTX_set_certs(TS_RESP_CTX *ctx, STACK_OF(X509) *certs)
{
-
sk_X509_pop_free(ctx->certs, X509_free);
ctx->certs = NULL;
- if (!certs)
- return 1;
- if ((ctx->certs = X509_chain_up_ref(certs)) == NULL) {
- ERR_raise(ERR_LIB_TS, ERR_R_MALLOC_FAILURE);
- return 0;
- }
- return 1;
+ return certs == NULL || (ctx->certs = X509_chain_up_ref(certs)) != NULL;
}
int TS_RESP_CTX_add_policy(TS_RESP_CTX *ctx, const ASN1_OBJECT *policy)
diff --git a/crypto/x509/x509_cmp.c b/crypto/x509/x509_cmp.c
index 1192527125..8e525a3815 100644
--- a/crypto/x509/x509_cmp.c
+++ b/crypto/x509/x509_cmp.c
@@ -531,6 +531,7 @@ int X509_CRL_check_suiteb(X509_CRL *crl, EVP_PKEY *pk, unsigned long flags)
}
#endif
+
/*
* Not strictly speaking an "up_ref" as a STACK doesn't have a reference
* count but it has the same effect by duping the STACK and upping the ref of
@@ -538,17 +539,19 @@ int X509_CRL_check_suiteb(X509_CRL *crl, EVP_PKEY *pk, unsigned long flags)
*/
STACK_OF(X509) *X509_chain_up_ref(STACK_OF(X509) *chain)
{
- STACK_OF(X509) *ret;
+ STACK_OF(X509) *ret = sk_X509_dup(chain);
int i;
- ret = sk_X509_dup(chain);
+
if (ret == NULL)
return NULL;
for (i = 0; i < sk_X509_num(ret); i++) {
X509 *x = sk_X509_value(ret, i);
+
if (!X509_up_ref(x))
goto err;
}
return ret;
+
err:
while (i-- > 0)
X509_free(sk_X509_value(ret, i));
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 29ccc0ecb1..8e78c13b8e 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -3004,7 +3004,7 @@ static int build_chain(X509_STORE_CTX *ctx)
* typically the content of the peer's certificate message) so can make
* multiple passes over it, while free to remove elements as we go.
*/
- if (ctx->untrusted && (sktmp = sk_X509_dup(ctx->untrusted)) == NULL) {
+ if ((sktmp = sk_X509_dup(ctx->untrusted)) == NULL) {
ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE);
ctx->error = X509_V_ERR_OUT_OF_MEM;
return 0;
diff --git a/doc/man3/DEFINE_STACK_OF.pod b/doc/man3/DEFINE_STACK_OF.pod
index 9088dc040b..b5908fead5 100644
--- a/doc/man3/DEFINE_STACK_OF.pod
+++ b/doc/man3/DEFINE_STACK_OF.pod
@@ -182,12 +182,14 @@ B<sk_I<TYPE>_sort>() sorts I<sk> using the supplied comparison function.
B<sk_I<TYPE>_is_sorted>() returns B<1> if I<sk> is sorted and B<0> otherwise.
-B<sk_I<TYPE>_dup>() returns a copy of I<sk>. Note the pointers in the copy
-are identical to the original.
+B<sk_I<TYPE>_dup>() returns a shallow copy of I<sk>
+or an empty stack if the passed stack is NULL.
+Note the pointers in the copy are identical to the original.
B<sk_I<TYPE>_deep_copy>() returns a new stack where each element has been
-copied. Copying is performed by the supplied copyfunc() and freeing by
-freefunc(). The function freefunc() is only called if an error occurs.
+copied or an empty stack if the passed stack is NULL.
+Copying is performed by the supplied copyfunc() and freeing by freefunc().
+The function freefunc() is only called if an error occurs.
=head1 NOTES
@@ -258,7 +260,7 @@ B<sk_I<TYPE>_is_sorted>() returns B<1> if the stack is sorted and B<0> if it is
not.
B<sk_I<TYPE>_dup>() and B<sk_I<TYPE>_deep_copy>() return a pointer to the copy
-of the stack.
+of the stack or NULL on error.
=head1 HISTORY
diff --git a/doc/man3/X509_new.pod b/doc/man3/X509_new.pod
index b40715bddf..ab310bff57 100644
--- a/doc/man3/X509_new.pod
+++ b/doc/man3/X509_new.pod
@@ -2,9 +2,9 @@
=head1 NAME
-X509_chain_up_ref,
X509_new, X509_new_ex,
-X509_free, X509_up_ref - X509 certificate ASN1 allocation functions
+X509_free, X509_up_ref,
+X509_chain_up_ref - X509 certificate ASN1 allocation functions
=head1 SYNOPSIS
@@ -37,7 +37,7 @@ frees it up if the reference count is zero. If B<a> is NULL nothing is done.
X509_up_ref() increments the reference count of B<a>.
X509_chain_up_ref() increases the reference count of all certificates in
-chain B<x> and returns a copy of the stack.
+chain B<x> and returns a copy of the stack, or an empty stack if B<a> is NULL.
=head1 NOTES
@@ -46,20 +46,19 @@ used by several different operations each of which will free it up after
use: this avoids the need to duplicate the entire certificate structure.
The function X509_chain_up_ref() doesn't just up the reference count of
-each certificate it also returns a copy of the stack, using sk_X509_dup(),
+each certificate. It also returns a copy of the stack, using sk_X509_dup(),
but it serves a similar purpose: the returned chain persists after the
original has been freed.
=head1 RETURN VALUES
-If the allocation fails, X509_new() returns B<NULL> and sets an error
+If the allocation fails, X509_new() returns NULL and sets an error
code that can be obtained by L<ERR_get_error(3)>.
Otherwise it returns a pointer to the newly allocated structure.
X509_up_ref() returns 1 for success and 0 for failure.
-X509_chain_up_ref() returns a copy of the stack or B<NULL> if an error
-occurred.
+X509_chain_up_ref() returns a copy of the stack or NULL if an error occurred.
=head1 SEE ALSO
diff --git a/test/stack_test.c b/test/stack_test.c
index 0c1648da77..e59acd353b 100644
--- a/test/stack_test.c
+++ b/test/stack_test.c
@@ -195,6 +195,10 @@ static int test_uchar_stack(int reserve)
goto end;
/* dup */
+ r = sk_uchar_dup(NULL);
+ if (sk_uchar_num(r) != 0)
+ goto end;
+ sk_uchar_free(r);
r = sk_uchar_dup(s);
if (!TEST_int_eq(sk_uchar_num(r), n))
goto end;
@@ -291,6 +295,10 @@ static int test_SS_stack(void)
goto end;
/* deepcopy */
+ r = sk_SS_deep_copy(NULL, &SS_copy, &SS_free);
+ if (sk_SS_num(r) != 0)
+ goto end;
+ sk_SS_free(r);
r = sk_SS_deep_copy(s, &SS_copy, &SS_free);
if (!TEST_ptr(r))
goto end;
More information about the openssl-commits
mailing list