generic-library/openssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix X509_STORE_CTX_cleanup()

Browse Source 
Reviewed-by: Dr. Stephen Henson <steve@openssl.org>
This commit is contained in:
Viktor Dukhovni 2016-01-01 00:51:12 -05:00
parent 4d9c6fa01b
commit a3d74afcae
4 changed files with 43 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
apps/pkcs12.c
View File

@ -79,7 +79,8 @@ const EVP_CIPHER *enc;
# define CLCERTS 0x8 # define CLCERTS 0x8
# define CACERTS 0x10 # define CACERTS 0x10
int get_cert_chain(X509 *cert, X509_STORE *store, STACK_OF(X509) **chain); static int get_cert_chain(X509 *cert, X509_STORE *store,
STACK_OF(X509) **chain);
int dump_certs_keys_p12(BIO *out, PKCS12 *p12, char *pass, int passlen, int dump_certs_keys_p12(BIO *out, PKCS12 *p12, char *pass, int passlen,
int options, char *pempass); int options, char *pempass);
int dump_certs_pkeys_bags(BIO *out, STACK_OF(PKCS12_SAFEBAG) *bags, int dump_certs_pkeys_bags(BIO *out, STACK_OF(PKCS12_SAFEBAG) *bags,
@ -594,7 +595,7 @@ int MAIN(int argc, char **argv)
vret = get_cert_chain(ucert, store, &chain2); vret = get_cert_chain(ucert, store, &chain2);
X509_STORE_free(store); X509_STORE_free(store);
if (!vret) { if (vret == X509_V_OK) {
/* Exclude verified certificate */ /* Exclude verified certificate */
for (i = 1; i < sk_X509_num(chain2); i++) for (i = 1; i < sk_X509_num(chain2); i++)
sk_X509_push(certs, sk_X509_value(chain2, i)); sk_X509_push(certs, sk_X509_value(chain2, i));
@ -602,7 +603,7 @@ int MAIN(int argc, char **argv)
X509_free(sk_X509_value(chain2, 0)); X509_free(sk_X509_value(chain2, 0));
sk_X509_free(chain2); sk_X509_free(chain2);
} else { } else {
if (vret >= 0) if (vret != X509_V_ERR_UNSPECIFIED)
BIO_printf(bio_err, "Error %s getting chain.\n", BIO_printf(bio_err, "Error %s getting chain.\n",
X509_verify_cert_error_string(vret)); X509_verify_cert_error_string(vret));
else else
@ -906,36 +907,25 @@ int dump_certs_pkeys_bag(BIO *out, PKCS12_SAFEBAG *bag, char *pass,
/* Given a single certificate return a verified chain or NULL if error */ /* Given a single certificate return a verified chain or NULL if error */
/* Hope this is OK .... */ static int get_cert_chain(X509 *cert, X509_STORE *store,
STACK_OF(X509) **chain)
int get_cert_chain(X509 *cert, X509_STORE *store, STACK_OF(X509) **chain)
{ {
X509_STORE_CTX store_ctx; X509_STORE_CTX store_ctx;
STACK_OF(X509) *chn; STACK_OF(X509) *chn = NULL;
int i = 0; int i = 0;
/* if (!X509_STORE_CTX_init(&store_ctx, store, cert, NULL)) {
* FIXME: Should really check the return status of X509_STORE_CTX_init *chain = NULL;
* for an error, but how that fits into the return value of this function return X509_V_ERR_UNSPECIFIED;
* is less obvious. }
*/
X509_STORE_CTX_init(&store_ctx, store, cert, NULL); if (X509_verify_cert(&store_ctx) > 0)
if (X509_verify_cert(&store_ctx) <= 0) {
i = X509_STORE_CTX_get_error(&store_ctx);
if (i == 0)
/*
* avoid returning 0 if X509_verify_cert() did not set an
* appropriate error value in the context
*/
i = -1;
chn = NULL;
goto err;
} else
chn = X509_STORE_CTX_get1_chain(&store_ctx); chn = X509_STORE_CTX_get1_chain(&store_ctx);
err: else if ((i = X509_STORE_CTX_get_error(&store_ctx)) == 0)
i = X509_V_ERR_UNSPECIFIED;
X509_STORE_CTX_cleanup(&store_ctx); X509_STORE_CTX_cleanup(&store_ctx);
*chain = chn; *chain = chn;
return i; return i;
} }

3
crypto/ts/ts_rsp_verify.c
View File

@ -255,7 +255,8 @@ static int TS_verify_cert(X509_STORE *store, STACK_OF(X509) *untrusted,
/* chain is an out argument. */ /* chain is an out argument. */
*chain = NULL; *chain = NULL;
X509_STORE_CTX_init(&cert_ctx, store, signer, untrusted); if (!X509_STORE_CTX_init(&cert_ctx, store, signer, untrusted))
return 0;
X509_STORE_CTX_set_purpose(&cert_ctx, X509_PURPOSE_TIMESTAMP_SIGN); X509_STORE_CTX_set_purpose(&cert_ctx, X509_PURPOSE_TIMESTAMP_SIGN);
i = X509_verify_cert(&cert_ctx); i = X509_verify_cert(&cert_ctx);
if (i <= 0) { if (i <= 0) {

39
crypto/x509/x509_vfy.c
View File

@ -2283,9 +2283,10 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
ctx->current_reasons = 0; ctx->current_reasons = 0;
ctx->tree = NULL; ctx->tree = NULL;
ctx->parent = NULL; ctx->parent = NULL;
/* Zero ex_data to make sure we're cleanup-safe */
memset(&ctx->ex_data, 0, sizeof(ctx->ex_data));
ctx->param = X509_VERIFY_PARAM_new(); ctx->param = X509_VERIFY_PARAM_new();
if (!ctx->param) { if (!ctx->param) {
X509err(X509_F_X509_STORE_CTX_INIT, ERR_R_MALLOC_FAILURE); X509err(X509_F_X509_STORE_CTX_INIT, ERR_R_MALLOC_FAILURE);
return 0; return 0;
@ -2294,7 +2295,6 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
/* /*
* Inherit callbacks and flags from X509_STORE if not set use defaults. * Inherit callbacks and flags from X509_STORE if not set use defaults.
*/ */
if (store) if (store)
ret = X509_VERIFY_PARAM_inherit(ctx->param, store->param); ret = X509_VERIFY_PARAM_inherit(ctx->param, store->param);
else else
@ -2302,6 +2302,7 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
if (store) { if (store) {
ctx->verify_cb = store->verify_cb; ctx->verify_cb = store->verify_cb;
/* Seems to always be 0 in OpenSSL, else must be idempotent */
ctx->cleanup = store->cleanup; ctx->cleanup = store->cleanup;
} else } else
ctx->cleanup = 0; ctx->cleanup = 0;
@ -2312,7 +2313,7 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
if (ret == 0) { if (ret == 0) {
X509err(X509_F_X509_STORE_CTX_INIT, ERR_R_MALLOC_FAILURE); X509err(X509_F_X509_STORE_CTX_INIT, ERR_R_MALLOC_FAILURE);
return 0; goto err;
} }
if (store && store->check_issued) if (store && store->check_issued)
@ -2367,19 +2368,18 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
ctx->check_policy = check_policy; ctx->check_policy = check_policy;
if (CRYPTO_new_ex_data(CRYPTO_EX_INDEX_X509_STORE_CTX, ctx,
&ctx->ex_data))
return 1;
X509err(X509_F_X509_STORE_CTX_INIT, ERR_R_MALLOC_FAILURE);
err:
/* /*
* This memset() can't make any sense anyway, so it's removed. As * On error clean up allocated storage, if the store context was not
* X509_STORE_CTX_cleanup does a proper "free" on the ex_data, we put a * allocated with X509_STORE_CTX_new() this is our last chance to do so.
* corresponding "new" here and remove this bogus initialisation.
*/ */
/* memset(&(ctx->ex_data),0,sizeof(CRYPTO_EX_DATA)); */ X509_STORE_CTX_cleanup(ctx);
if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_X509_STORE_CTX, ctx, return 0;
&(ctx->ex_data))) {
OPENSSL_free(ctx);
X509err(X509_F_X509_STORE_CTX_INIT, ERR_R_MALLOC_FAILURE);
return 0;
}
return 1;
} }
/* /*
@ -2395,8 +2395,17 @@ void X509_STORE_CTX_trusted_stack(X509_STORE_CTX *ctx, STACK_OF(X509) *sk)
void X509_STORE_CTX_cleanup(X509_STORE_CTX *ctx) void X509_STORE_CTX_cleanup(X509_STORE_CTX *ctx)
{ {
if (ctx->cleanup) /*
* We need to be idempotent because, unfortunately, free() also calls
* cleanup(), so the natural call sequence new(), init(), cleanup(), free()
* calls cleanup() for the same object twice! Thus we must zero the
* pointers below after they're freed!
*/
/* Seems to always be 0 in OpenSSL, do this at most once. */
if (ctx->cleanup != NULL) {
ctx->cleanup(ctx); ctx->cleanup(ctx);
ctx->cleanup = NULL;
}
if (ctx->param != NULL) { if (ctx->param != NULL) {
if (ctx->parent == NULL) if (ctx->parent == NULL)
X509_VERIFY_PARAM_free(ctx->param); X509_VERIFY_PARAM_free(ctx->param);

2
crypto/x509/x509_vfy.h
View File

@ -313,7 +313,7 @@ void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth);
X509_LOOKUP_ctrl((x),X509_L_ADD_DIR,(name),(long)(type),NULL) X509_LOOKUP_ctrl((x),X509_L_ADD_DIR,(name),(long)(type),NULL)
# define X509_V_OK 0 # define X509_V_OK 0
/* illegal error (for uninitialized values, to avoid X509_V_OK): 1 */ # define X509_V_ERR_UNSPECIFIED 1
# define X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT 2 # define X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT 2
# define X509_V_ERR_UNABLE_TO_GET_CRL 3 # define X509_V_ERR_UNABLE_TO_GET_CRL 3