From 310d3f63f38cf8a82fa9ae5032e343ba5159eb4d Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 12 Jul 2016 10:39:20 -0700 Subject: [PATCH] Change |EVP_PKEY_up_ref| to return int. Upstream have added |EVP_PKEY_up_ref|, but their version returns an int. Having this function with a different signature like that is dangerous so this change aligns BoringSSL with upstream. Users of this function in Chromium and internally should already have been updated. Change-Id: I0a7aeaf1a1ca3b0f0c635e2ee3826aa100b18157 Reviewed-on: https://boringssl-review.googlesource.com/8736 Reviewed-by: David Benjamin --- crypto/evp/evp.c | 4 ++-- crypto/evp/evp_ctx.c | 9 ++++++--- crypto/x509/x_pubkey.c | 6 ++++-- include/openssl/evp.h | 4 ++-- ssl/ssl_cert.c | 3 ++- ssl/ssl_lib.c | 10 ++++++---- ssl/ssl_rsa.c | 3 ++- 7 files changed, 24 insertions(+), 15 deletions(-) diff --git a/crypto/evp/evp.c b/crypto/evp/evp.c index ee207f9b..3b4b05bb 100644 --- a/crypto/evp/evp.c +++ b/crypto/evp/evp.c @@ -108,9 +108,9 @@ void EVP_PKEY_free(EVP_PKEY *pkey) { OPENSSL_free(pkey); } -EVP_PKEY *EVP_PKEY_up_ref(EVP_PKEY *pkey) { +int EVP_PKEY_up_ref(EVP_PKEY *pkey) { CRYPTO_refcount_inc(&pkey->references); - return pkey; + return 1; } int EVP_PKEY_is_opaque(const EVP_PKEY *pkey) { diff --git a/crypto/evp/evp_ctx.c b/crypto/evp/evp_ctx.c index f510f6c7..f7d4b41b 100644 --- a/crypto/evp/evp_ctx.c +++ b/crypto/evp/evp_ctx.c @@ -112,7 +112,8 @@ static EVP_PKEY_CTX *evp_pkey_ctx_new(EVP_PKEY *pkey, ENGINE *e, int id) { ret->operation = EVP_PKEY_OP_UNDEFINED; if (pkey) { - ret->pkey = EVP_PKEY_up_ref(pkey); + EVP_PKEY_up_ref(pkey); + ret->pkey = pkey; } if (pmeth->init) { @@ -165,14 +166,16 @@ EVP_PKEY_CTX *EVP_PKEY_CTX_dup(EVP_PKEY_CTX *pctx) { rctx->operation = pctx->operation; if (pctx->pkey) { - rctx->pkey = EVP_PKEY_up_ref(pctx->pkey); + EVP_PKEY_up_ref(pctx->pkey); + rctx->pkey = pctx->pkey; if (rctx->pkey == NULL) { goto err; } } if (pctx->peerkey) { - rctx->peerkey = EVP_PKEY_up_ref(pctx->peerkey); + EVP_PKEY_up_ref(pctx->peerkey); + rctx->peerkey = pctx->peerkey; if (rctx->peerkey == NULL) { goto err; } diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c index 23534b2b..3d07d661 100644 --- a/crypto/x509/x_pubkey.c +++ b/crypto/x509/x_pubkey.c @@ -141,7 +141,8 @@ EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key) CRYPTO_STATIC_MUTEX_lock_read(&g_pubkey_lock); if (key->pkey != NULL) { CRYPTO_STATIC_MUTEX_unlock_read(&g_pubkey_lock); - return EVP_PKEY_up_ref(key->pkey); + EVP_PKEY_up_ref(key->pkey); + return key->pkey; } CRYPTO_STATIC_MUTEX_unlock_read(&g_pubkey_lock); @@ -170,7 +171,8 @@ EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key) } OPENSSL_free(spki); - return EVP_PKEY_up_ref(ret); + EVP_PKEY_up_ref(ret); + return ret; error: OPENSSL_free(spki); diff --git a/include/openssl/evp.h b/include/openssl/evp.h index 54074073..e4f812a3 100644 --- a/include/openssl/evp.h +++ b/include/openssl/evp.h @@ -89,8 +89,8 @@ OPENSSL_EXPORT EVP_PKEY *EVP_PKEY_new(void); * itself. */ OPENSSL_EXPORT void EVP_PKEY_free(EVP_PKEY *pkey); -/* EVP_PKEY_up_ref increments the reference count of |pkey| and returns it. */ -OPENSSL_EXPORT EVP_PKEY *EVP_PKEY_up_ref(EVP_PKEY *pkey); +/* EVP_PKEY_up_ref increments the reference count of |pkey| and returns one. */ +OPENSSL_EXPORT int EVP_PKEY_up_ref(EVP_PKEY *pkey); /* EVP_PKEY_is_opaque returns one if |pkey| is opaque. Opaque keys are backed by * custom implementations which do not expose key material and parameters. It is diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index d61f9f53..dead3ba7 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -174,7 +174,8 @@ CERT *ssl_cert_dup(CERT *cert) { } if (cert->privatekey != NULL) { - ret->privatekey = EVP_PKEY_up_ref(cert->privatekey); + EVP_PKEY_up_ref(cert->privatekey); + ret->privatekey = cert->privatekey; } if (cert->chain) { diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index e0cba62b..ca522335 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -456,8 +456,8 @@ SSL *SSL_new(SSL_CTX *ctx) { ssl->tlsext_channel_id_enabled = ctx->tlsext_channel_id_enabled; if (ctx->tlsext_channel_id_private) { - ssl->tlsext_channel_id_private = - EVP_PKEY_up_ref(ctx->tlsext_channel_id_private); + EVP_PKEY_up_ref(ctx->tlsext_channel_id_private); + ssl->tlsext_channel_id_private = ctx->tlsext_channel_id_private; } ssl->signed_cert_timestamps_enabled = @@ -1834,7 +1834,8 @@ int SSL_CTX_set1_tls_channel_id(SSL_CTX *ctx, EVP_PKEY *private_key) { } EVP_PKEY_free(ctx->tlsext_channel_id_private); - ctx->tlsext_channel_id_private = EVP_PKEY_up_ref(private_key); + EVP_PKEY_up_ref(private_key); + ctx->tlsext_channel_id_private = private_key; ctx->tlsext_channel_id_enabled = 1; return 1; @@ -1847,7 +1848,8 @@ int SSL_set1_tls_channel_id(SSL *ssl, EVP_PKEY *private_key) { } EVP_PKEY_free(ssl->tlsext_channel_id_private); - ssl->tlsext_channel_id_private = EVP_PKEY_up_ref(private_key); + EVP_PKEY_up_ref(private_key); + ssl->tlsext_channel_id_private = private_key; ssl->tlsext_channel_id_enabled = 1; return 1; diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index cfa4cda2..6dcbcc95 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c @@ -143,7 +143,8 @@ static int ssl_set_pkey(CERT *c, EVP_PKEY *pkey) { } EVP_PKEY_free(c->privatekey); - c->privatekey = EVP_PKEY_up_ref(pkey); + EVP_PKEY_up_ref(pkey); + c->privatekey = pkey; return 1; }