More scopers.
Note the legacy client cert callback case fixes a leak.
Change-Id: I2772167bd03d308676d9e00885c751207002b31e
Reviewed-on: https://boringssl-review.googlesource.com/18824
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 2413a1c..b427df6 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -2254,10 +2254,15 @@
OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
return NULL;
}
- OPENSSL_memset(ctx, 0, sizeof(X509_STORE_CTX));
+ X509_STORE_CTX_zero(ctx);
return ctx;
}
+void X509_STORE_CTX_zero(X509_STORE_CTX *ctx)
+{
+ OPENSSL_memset(ctx, 0, sizeof(X509_STORE_CTX));
+}
+
void X509_STORE_CTX_free(X509_STORE_CTX *ctx)
{
if (ctx == NULL) {
@@ -2272,7 +2277,7 @@
{
int ret = 1;
- OPENSSL_memset(ctx, 0, sizeof(X509_STORE_CTX));
+ X509_STORE_CTX_zero(ctx);
ctx->ctx = store;
ctx->cert = x509;
ctx->untrusted = chain;
diff --git a/include/openssl/base.h b/include/openssl/base.h
index e575069..6b43c76 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -413,6 +413,9 @@
T *get() { return &ctx_; }
const T *get() const { return &ctx_; }
+ T *operator->() { return &ctx_; }
+ const T *operator->() const { return &ctx_; }
+
void Reset() {
cleanup(&ctx_);
init(&ctx_);
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 40b2332..138c060 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -1094,7 +1094,9 @@
#ifdef __cplusplus
}
+#endif
+#if !defined(BORINGSSL_NO_CXX)
extern "C++" {
namespace bssl {
@@ -1118,11 +1120,14 @@
BORINGSSL_MAKE_DELETER(X509_STORE_CTX, X509_STORE_CTX_free)
BORINGSSL_MAKE_DELETER(X509_VERIFY_PARAM, X509_VERIFY_PARAM_free)
+using ScopedX509_STORE_CTX =
+ internal::StackAllocated<X509_STORE_CTX, void, X509_STORE_CTX_zero,
+ X509_STORE_CTX_cleanup>;
+
} // namespace bssl
} /* extern C++ */
-
-#endif
+#endif /* !BORINGSSL_NO_CXX */
#define X509_R_AKID_MISMATCH 100
#define X509_R_BAD_PKCS7_VERSION 101
diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h
index ac739ea..4abd9cd 100644
--- a/include/openssl/x509_vfy.h
+++ b/include/openssl/x509_vfy.h
@@ -449,6 +449,7 @@
OPENSSL_EXPORT int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x);
+OPENSSL_EXPORT void X509_STORE_CTX_zero(X509_STORE_CTX *ctx);
OPENSSL_EXPORT void X509_STORE_CTX_free(X509_STORE_CTX *ctx);
OPENSSL_EXPORT int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store,
X509 *x509, STACK_OF(X509) *chain);
diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc
index 3e3fa94..ecdf48f 100644
--- a/ssl/ssl_privkey.cc
+++ b/ssl/ssl_privkey.cc
@@ -320,27 +320,19 @@
using namespace bssl;
int SSL_use_RSAPrivateKey(SSL *ssl, RSA *rsa) {
- EVP_PKEY *pkey;
- int ret;
-
if (rsa == NULL) {
OPENSSL_PUT_ERROR(SSL, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
- pkey = EVP_PKEY_new();
- if (pkey == NULL) {
+ UniquePtr<EVP_PKEY> pkey(EVP_PKEY_new());
+ if (!pkey ||
+ !EVP_PKEY_set1_RSA(pkey.get(), rsa)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_EVP_LIB);
return 0;
}
- RSA_up_ref(rsa);
- EVP_PKEY_assign_RSA(pkey, rsa);
-
- ret = ssl_set_pkey(ssl->cert, pkey);
- EVP_PKEY_free(pkey);
-
- return ret;
+ return ssl_set_pkey(ssl->cert, pkey.get());
}
int SSL_use_RSAPrivateKey_ASN1(SSL *ssl, const uint8_t *der, size_t der_len) {
@@ -370,52 +362,40 @@
}
const uint8_t *p = der;
- EVP_PKEY *pkey = d2i_PrivateKey(type, NULL, &p, (long)der_len);
- if (pkey == NULL || p != der + der_len) {
+ UniquePtr<EVP_PKEY> pkey(d2i_PrivateKey(type, NULL, &p, (long)der_len));
+ if (!pkey || p != der + der_len) {
OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB);
- EVP_PKEY_free(pkey);
return 0;
}
- int ret = SSL_use_PrivateKey(ssl, pkey);
- EVP_PKEY_free(pkey);
- return ret;
+ return SSL_use_PrivateKey(ssl, pkey.get());
}
int SSL_CTX_use_RSAPrivateKey(SSL_CTX *ctx, RSA *rsa) {
- int ret;
- EVP_PKEY *pkey;
-
if (rsa == NULL) {
OPENSSL_PUT_ERROR(SSL, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
- pkey = EVP_PKEY_new();
- if (pkey == NULL) {
+ UniquePtr<EVP_PKEY> pkey(EVP_PKEY_new());
+ if (!pkey ||
+ !EVP_PKEY_set1_RSA(pkey.get(), rsa)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_EVP_LIB);
return 0;
}
- RSA_up_ref(rsa);
- EVP_PKEY_assign_RSA(pkey, rsa);
-
- ret = ssl_set_pkey(ctx->cert, pkey);
- EVP_PKEY_free(pkey);
- return ret;
+ return ssl_set_pkey(ctx->cert, pkey.get());
}
int SSL_CTX_use_RSAPrivateKey_ASN1(SSL_CTX *ctx, const uint8_t *der,
size_t der_len) {
- RSA *rsa = RSA_private_key_from_bytes(der, der_len);
- if (rsa == NULL) {
+ UniquePtr<RSA> rsa(RSA_private_key_from_bytes(der, der_len));
+ if (!rsa) {
OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB);
return 0;
}
- int ret = SSL_CTX_use_RSAPrivateKey(ctx, rsa);
- RSA_free(rsa);
- return ret;
+ return SSL_CTX_use_RSAPrivateKey(ctx, rsa.get());
}
int SSL_CTX_use_PrivateKey(SSL_CTX *ctx, EVP_PKEY *pkey) {
@@ -435,16 +415,13 @@
}
const uint8_t *p = der;
- EVP_PKEY *pkey = d2i_PrivateKey(type, NULL, &p, (long)der_len);
- if (pkey == NULL || p != der + der_len) {
+ UniquePtr<EVP_PKEY> pkey(d2i_PrivateKey(type, NULL, &p, (long)der_len));
+ if (!pkey || p != der + der_len) {
OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB);
- EVP_PKEY_free(pkey);
return 0;
}
- int ret = SSL_CTX_use_PrivateKey(ctx, pkey);
- EVP_PKEY_free(pkey);
- return ret;
+ return SSL_CTX_use_PrivateKey(ctx, pkey.get());
}
void SSL_set_private_key_method(SSL *ssl,
diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc
index 22f4fb4..98a5b8c 100644
--- a/ssl/ssl_x509.cc
+++ b/ssl/ssl_x509.cc
@@ -429,50 +429,47 @@
}
X509 *leaf = sk_X509_value(cert_chain, 0);
- int ret = 0;
- X509_STORE_CTX ctx;
- if (!X509_STORE_CTX_init(&ctx, verify_store, leaf, cert_chain)) {
+ ScopedX509_STORE_CTX ctx;
+ if (!X509_STORE_CTX_init(ctx.get(), verify_store, leaf, cert_chain)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_X509_LIB);
return 0;
}
- if (!X509_STORE_CTX_set_ex_data(&ctx, SSL_get_ex_data_X509_STORE_CTX_idx(),
- ssl)) {
- goto err;
+ if (!X509_STORE_CTX_set_ex_data(ctx.get(),
+ SSL_get_ex_data_X509_STORE_CTX_idx(), ssl)) {
+ return 0;
}
/* We need to inherit the verify parameters. These can be determined by the
* context: if its a server it will verify SSL client certificates or vice
* versa. */
- X509_STORE_CTX_set_default(&ctx, ssl->server ? "ssl_client" : "ssl_server");
+ X509_STORE_CTX_set_default(ctx.get(),
+ ssl->server ? "ssl_client" : "ssl_server");
/* Anything non-default in "param" should overwrite anything in the ctx. */
- X509_VERIFY_PARAM_set1(X509_STORE_CTX_get0_param(&ctx), ssl->param);
+ X509_VERIFY_PARAM_set1(X509_STORE_CTX_get0_param(ctx.get()), ssl->param);
if (ssl->verify_callback) {
- X509_STORE_CTX_set_verify_cb(&ctx, ssl->verify_callback);
+ X509_STORE_CTX_set_verify_cb(ctx.get(), ssl->verify_callback);
}
int verify_ret;
if (ssl->ctx->app_verify_callback != NULL) {
- verify_ret = ssl->ctx->app_verify_callback(&ctx, ssl->ctx->app_verify_arg);
+ verify_ret =
+ ssl->ctx->app_verify_callback(ctx.get(), ssl->ctx->app_verify_arg);
} else {
- verify_ret = X509_verify_cert(&ctx);
+ verify_ret = X509_verify_cert(ctx.get());
}
- session->verify_result = ctx.error;
+ session->verify_result = ctx->error;
/* If |SSL_VERIFY_NONE|, the error is non-fatal, but we keep the result. */
if (verify_ret <= 0 && ssl->verify_mode != SSL_VERIFY_NONE) {
- *out_alert = ssl_verify_alarm_type(ctx.error);
- goto err;
+ *out_alert = ssl_verify_alarm_type(ctx->error);
+ return 0;
}
ERR_clear_error();
- ret = 1;
-
-err:
- X509_STORE_CTX_cleanup(&ctx);
- return ret;
+ return 1;
}
static void ssl_crypto_x509_hs_flush_cached_ca_names(SSL_HANDSHAKE *hs) {
@@ -509,31 +506,27 @@
return 1;
}
- X509 *leaf =
- X509_parse_from_buffer(sk_CRYPTO_BUFFER_value(ssl->cert->chain, 0));
+ UniquePtr<X509> leaf(
+ X509_parse_from_buffer(sk_CRYPTO_BUFFER_value(ssl->cert->chain, 0)));
if (!leaf) {
OPENSSL_PUT_ERROR(SSL, ERR_R_X509_LIB);
return 0;
}
- X509_STORE_CTX ctx;
- if (!X509_STORE_CTX_init(&ctx, ssl->ctx->cert_store, leaf, NULL)) {
- X509_free(leaf);
+ ScopedX509_STORE_CTX ctx;
+ if (!X509_STORE_CTX_init(ctx.get(), ssl->ctx->cert_store, leaf.get(), NULL)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_X509_LIB);
return 0;
}
/* Attempt to build a chain, ignoring the result. */
- X509_verify_cert(&ctx);
- X509_free(leaf);
+ X509_verify_cert(ctx.get());
ERR_clear_error();
/* Remove the leaf from the generated chain. */
- X509_free(sk_X509_shift(ctx.chain));
+ X509_free(sk_X509_shift(ctx->chain));
- const int ok = ssl_cert_set_chain(ssl->cert, ctx.chain);
- X509_STORE_CTX_cleanup(&ctx);
- if (!ok) {
+ if (!ssl_cert_set_chain(ssl->cert, ctx->chain)) {
return 0;
}
@@ -1248,6 +1241,8 @@
if (ret < 0) {
return -1;
}
+ UniquePtr<X509> free_x509(x509);
+ UniquePtr<EVP_PKEY> free_pkey(pkey);
if (ret != 0) {
if (!SSL_use_certificate(ssl, x509) ||
@@ -1256,8 +1251,6 @@
}
}
- X509_free(x509);
- EVP_PKEY_free(pkey);
return 1;
}
