Allow NULL arg to OPENSSL_sk_{dup,deep_copy} returning empty stack

This simplifies many usages

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14040)
This commit is contained in:
Dr. David von Oheimb 2020-12-23 19:33:03 +01:00 committed by Dr. David von Oheimb
parent b91a13f429
commit d53b437f99
9 changed files with 73 additions and 68 deletions

View File

@ -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

View File

@ -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;
}

View File

@ -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;
if (sk == NULL) {
ret->num = 0;
ret->sorted = 0;
ret->comp = NULL;
} else {
/* direct structure assignment */
*ret = *sk;
}
/* 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;
if (sk == NULL) {
ret->num = 0;
ret->sorted = 0;
ret->comp = NULL;
} else {
/* direct structure assignment */
*ret = *sk;
}
/* 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)

View File

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

View File

@ -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));

View File

@ -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;

View File

@ -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

View File

@ -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

View File

@ -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;