From 028391df5d84d9fae3433afdee9261d565900355 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 31 Jan 2017 16:05:33 -0800 Subject: [PATCH] openssl: Don't use certificate after transferring ownership SSL_CTX_add_extra_chain_cert takes ownership of the given certificate while, despite the similar name, SSL_CTX_add_client_CA does not. Thus it's best to call SSL_CTX_add_client_CA before SSL_CTX_add_extra_chain_cert, while the code still has ownership of the argument. Closes https://github.com/curl/curl/pull/1236 --- lib/vtls/openssl.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index 48a4c0b02..eb625fe93 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -493,23 +493,21 @@ int cert_stuff(struct connectdata *conn, /* * Note that sk_X509_pop() is used below to make sure the cert is * removed from the stack properly before getting passed to - * SSL_CTX_add_extra_chain_cert(). Previously we used - * sk_X509_value() instead, but then we'd clean it in the subsequent - * sk_X509_pop_free() call. + * SSL_CTX_add_extra_chain_cert(), which takes ownership. Previously + * we used sk_X509_value() instead, but then we'd clean it in the + * subsequent sk_X509_pop_free() call. */ X509 *x = sk_X509_pop(ca); + if(!SSL_CTX_add_client_CA(ctx, x)) { + X509_free(x); + failf(data, "cannot add certificate to client CA list"); + goto fail; + } if(!SSL_CTX_add_extra_chain_cert(ctx, x)) { X509_free(x); failf(data, "cannot add certificate to certificate chain"); goto fail; } - /* SSL_CTX_add_client_CA() seems to work with either sk_* function, - * presumably because it duplicates what we pass to it. - */ - if(!SSL_CTX_add_client_CA(ctx, x)) { - failf(data, "cannot add certificate to client CA list"); - goto fail; - } } }