From c5ebfcab713a82a1d46a51c8c2668c419425b387 Mon Sep 17 00:00:00 2001 From: FdaSilvaYY Date: Mon, 7 Mar 2016 22:45:58 +0100 Subject: [PATCH] Unify _up_ref methods signature and behaviour. Add a status return value instead of void. Add some sanity checks on reference counter value. Update the docs. Reviewed-by: Rich Salz Reviewed-by: Matt Caswell --- CHANGES | 10 +++++++++- crypto/evp/p_lib.c | 10 ++++++++-- crypto/x509/x509_lu.c | 9 ++++----- crypto/x509/x509_set.c | 10 ++++++++-- crypto/x509/x509cset.c | 10 ++++++++-- doc/crypto/EVP_PKEY_new.pod | 4 ++-- doc/crypto/X509_new.pod | 4 ++-- doc/ssl/SSL_CTX_new.pod | 4 +++- doc/ssl/SSL_new.pod | 4 +++- doc/ssl/ssl.pod | 4 ++-- include/openssl/evp.h | 2 +- include/openssl/ssl.h | 4 ++-- include/openssl/x509.h | 4 ++-- include/openssl/x509_vfy.h | 2 +- ssl/ssl_lib.c | 20 ++++++++++++++++---- 15 files changed, 71 insertions(+), 30 deletions(-) diff --git a/CHANGES b/CHANGES index b096ec6f0f..10ec93fc17 100644 --- a/CHANGES +++ b/CHANGES @@ -2,7 +2,15 @@ OpenSSL CHANGES _______________ - Changes between 1.0.2g and 1.1.0 [xx XXX xxxx] + Changes between 1.0.2h and 1.1.0 [xx XXX 2016] + + *) Unify TYPE_up_ref(obj) methods signature. + SSL_CTX_up_ref(), SSL_up_ref(), X509_up_ref(), EVP_PKEY_up_ref(), + X509_CRL_up_ref(), X509_OBJECT_up_ref_count() methods are now returning an + int (instead of void) like all others TYPE_up_ref() methods. + So now these methods also check the return value of CRYPTO_atomic_add(), + and the validity of object reference counter. + [fdasilvayy@gmail.com] *) With Windows Visual Studio builds, the .pdb files are installed alongside the installed libraries and executables. For a static diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c index a8fa301b31..94b311fa90 100644 --- a/crypto/evp/p_lib.c +++ b/crypto/evp/p_lib.c @@ -196,10 +196,16 @@ EVP_PKEY *EVP_PKEY_new(void) return ret; } -void EVP_PKEY_up_ref(EVP_PKEY *pkey) +int EVP_PKEY_up_ref(EVP_PKEY *pkey) { int i; - CRYPTO_atomic_add(&pkey->references, 1, &i, pkey->lock); + + if (CRYPTO_atomic_add(&pkey->references, 1, &i, pkey->lock) <= 0) + return 0; + + REF_PRINT_COUNT("EVP_PKEY", pkey); + REF_ASSERT_ISNT(i < 2); + return ((i > 1) ? 1 : 0); } /* diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index c4ca619d8c..3a8c657b85 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -417,18 +417,17 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x) return ret; } -void X509_OBJECT_up_ref_count(X509_OBJECT *a) +int X509_OBJECT_up_ref_count(X509_OBJECT *a) { switch (a->type) { default: break; case X509_LU_X509: - X509_up_ref(a->data.x509); - break; + return X509_up_ref(a->data.x509); case X509_LU_CRL: - X509_CRL_up_ref(a->data.crl); - break; + return X509_CRL_up_ref(a->data.crl); } + return 1; } X509 *X509_OBJECT_get0_X509(X509_OBJECT *a) diff --git a/crypto/x509/x509_set.c b/crypto/x509/x509_set.c index 360ead87d3..59a6b29f89 100644 --- a/crypto/x509/x509_set.c +++ b/crypto/x509/x509_set.c @@ -146,10 +146,16 @@ int X509_set_pubkey(X509 *x, EVP_PKEY *pkey) return (X509_PUBKEY_set(&(x->cert_info.key), pkey)); } -void X509_up_ref(X509 *x) +int X509_up_ref(X509 *x) { int i; - CRYPTO_atomic_add(&x->references, 1, &i, x->lock); + + if (CRYPTO_atomic_add(&x->references, 1, &i, x->lock) <= 0) + return 0; + + REF_PRINT_COUNT("X509", x); + REF_ASSERT_ISNT(i < 2); + return ((i > 1) ? 1 : 0); } long X509_get_version(X509 *x) diff --git a/crypto/x509/x509cset.c b/crypto/x509/x509cset.c index ab5f192a15..d8259254f0 100644 --- a/crypto/x509/x509cset.c +++ b/crypto/x509/x509cset.c @@ -132,10 +132,16 @@ int X509_CRL_sort(X509_CRL *c) return 1; } -void X509_CRL_up_ref(X509_CRL *crl) +int X509_CRL_up_ref(X509_CRL *crl) { int i; - CRYPTO_atomic_add(&crl->references, 1, &i, crl->lock); + + if (CRYPTO_atomic_add(&crl->references, 1, &i, crl->lock) <= 0) + return 0; + + REF_PRINT_COUNT("X509_CRL", crl); + REF_ASSERT_ISNT(i < 2); + return ((i > 1) ? 1 : 0); } long X509_CRL_get_version(X509_CRL *crl) diff --git a/doc/crypto/EVP_PKEY_new.pod b/doc/crypto/EVP_PKEY_new.pod index 05ac08795e..b639c66454 100644 --- a/doc/crypto/EVP_PKEY_new.pod +++ b/doc/crypto/EVP_PKEY_new.pod @@ -9,7 +9,7 @@ EVP_PKEY_new, EVP_PKEY_up_ref, EVP_PKEY_free - private key allocation functions. #include EVP_PKEY *EVP_PKEY_new(void); - void EVP_PKEY_up_ref(EVP_PKEY *key); + int EVP_PKEY_up_ref(EVP_PKEY *key); void EVP_PKEY_free(EVP_PKEY *key); @@ -37,7 +37,7 @@ used. EVP_PKEY_new() returns either the newly allocated B structure or B if an error occurred. -EVP_PKEY_up_ref() and EVP_PKEY_free() do not return a value. +EVP_PKEY_up_ref() returns 1 for success and 0 for failure. =head1 SEE ALSO diff --git a/doc/crypto/X509_new.pod b/doc/crypto/X509_new.pod index 8db6cdb245..484408c4b7 100644 --- a/doc/crypto/X509_new.pod +++ b/doc/crypto/X509_new.pod @@ -10,7 +10,7 @@ X509_new, X509_free, X509_up_ref - X509 certificate ASN1 allocation functions X509 *X509_new(void); void X509_free(X509 *a); - void X509_up_ref(X509 *a); + int X509_up_ref(X509 *a); STACK_OF(X509) *X509_chain_up_ref(STACK_OF(X509) *x); =head1 DESCRIPTION @@ -46,7 +46,7 @@ If the allocation fails, X509_new() returns B and sets an error code that can be obtained by L. Otherwise it returns a pointer to the newly allocated structure. -X509_free() and X509_up_ref() do not return a value. +X509_up_ref() returns 1 for success and 0 for failure. X509_chain_up_ref() returns a copy of the stack or B if an error occurred. diff --git a/doc/ssl/SSL_CTX_new.pod b/doc/ssl/SSL_CTX_new.pod index f2cdc717ef..8f232a0c6f 100644 --- a/doc/ssl/SSL_CTX_new.pod +++ b/doc/ssl/SSL_CTX_new.pod @@ -17,7 +17,7 @@ functions #include SSL_CTX *SSL_CTX_new(const SSL_METHOD *method); - void SSL_CTX_up_ref(SSL_CTX *ctx); + int SSL_CTX_up_ref(SSL_CTX *ctx); const SSL_METHOD *TLS_method(void); const SSL_METHOD *TLS_server_method(void); @@ -184,6 +184,8 @@ the reason. The return value points to an allocated SSL_CTX object. +SSL_CTX_up_ref() returns 1 for success and 0 for failure. + =back =head1 HISTORY diff --git a/doc/ssl/SSL_new.pod b/doc/ssl/SSL_new.pod index f0e07951e3..cee6b24858 100644 --- a/doc/ssl/SSL_new.pod +++ b/doc/ssl/SSL_new.pod @@ -9,7 +9,7 @@ SSL_new, SSL_up_ref - create a new SSL structure for a connection #include SSL *SSL_new(SSL_CTX *ctx); - void SSL_up_ref(SSL *s); + int SSL_up_ref(SSL *s); =head1 DESCRIPTION @@ -38,6 +38,8 @@ find out the reason. The return value points to an allocated SSL structure. +SSL_up_ref() returns 1 for success and 0 for failure. + =back =head1 SEE ALSO diff --git a/doc/ssl/ssl.pod b/doc/ssl/ssl.pod index 1ade6acda1..88198d1be7 100644 --- a/doc/ssl/ssl.pod +++ b/doc/ssl/ssl.pod @@ -271,7 +271,7 @@ protocol context defined in the B structure. =item SSL_CTX *B(const SSL_METHOD *meth); -=item void SSL_CTX_up_ref(SSL_CTX *ctx); +=item int SSL_CTX_up_ref(SSL_CTX *ctx); =item int B(SSL_CTX *ctx, SSL_SESSION *c); @@ -599,7 +599,7 @@ fresh handle for each connection. =item SSL *B(SSL_CTX *ctx); -=item void SSL_up_ref(SSL *s); +=item int SSL_up_ref(SSL *s); =item long B(SSL *ssl); diff --git a/include/openssl/evp.h b/include/openssl/evp.h index 250730f4a0..bfb5dc8988 100644 --- a/include/openssl/evp.h +++ b/include/openssl/evp.h @@ -976,7 +976,7 @@ struct ec_key_st *EVP_PKEY_get1_EC_KEY(EVP_PKEY *pkey); # endif EVP_PKEY *EVP_PKEY_new(void); -void EVP_PKEY_up_ref(EVP_PKEY *pkey); +int EVP_PKEY_up_ref(EVP_PKEY *pkey); void EVP_PKEY_free(EVP_PKEY *pkey); EVP_PKEY *d2i_PublicKey(int type, EVP_PKEY **a, const unsigned char **pp, diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 0ab0df2749..cf02047d67 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1379,7 +1379,7 @@ void BIO_ssl_shutdown(BIO *ssl_bio); __owur int SSL_CTX_set_cipher_list(SSL_CTX *, const char *str); __owur SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth); -void SSL_CTX_up_ref(SSL_CTX *ctx); +int SSL_CTX_up_ref(SSL_CTX *ctx); void SSL_CTX_free(SSL_CTX *); __owur long SSL_CTX_set_timeout(SSL_CTX *ctx, long t); __owur long SSL_CTX_get_timeout(const SSL_CTX *ctx); @@ -1554,7 +1554,7 @@ __owur int SSL_CTX_set_session_id_context(SSL_CTX *ctx, const unsigned char *sid unsigned int sid_ctx_len); SSL *SSL_new(SSL_CTX *ctx); -void SSL_up_ref(SSL *s); +int SSL_up_ref(SSL *s); __owur int SSL_set_session_id_context(SSL *ssl, const unsigned char *sid_ctx, unsigned int sid_ctx_len); diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 009ee6aa4c..febe621b5b 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -674,7 +674,7 @@ int X509_set_notBefore(X509 *x, const ASN1_TIME *tm); ASN1_TIME *X509_get_notAfter(X509 *x); int X509_set_notAfter(X509 *x, const ASN1_TIME *tm); int X509_set_pubkey(X509 *x, EVP_PKEY *pkey); -void X509_up_ref(X509 *x); +int X509_up_ref(X509 *x); int X509_get_signature_type(const X509 *x); /* * This one is only used so that a binary form can output, as in @@ -731,7 +731,7 @@ int X509_CRL_set_issuer_name(X509_CRL *x, X509_NAME *name); int X509_CRL_set_lastUpdate(X509_CRL *x, const ASN1_TIME *tm); int X509_CRL_set_nextUpdate(X509_CRL *x, const ASN1_TIME *tm); int X509_CRL_sort(X509_CRL *crl); -void X509_CRL_up_ref(X509_CRL *crl); +int X509_CRL_up_ref(X509_CRL *crl); long X509_CRL_get_version(X509_CRL *crl); ASN1_TIME *X509_CRL_get_lastUpdate(X509_CRL *crl); diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h index 1c86d3148c..ae93c7b33c 100644 --- a/include/openssl/x509_vfy.h +++ b/include/openssl/x509_vfy.h @@ -277,7 +277,7 @@ X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h, int type, X509_NAME *name); X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h, X509_OBJECT *x); -void X509_OBJECT_up_ref_count(X509_OBJECT *a); +int X509_OBJECT_up_ref_count(X509_OBJECT *a); void X509_OBJECT_free(X509_OBJECT *a); int X509_OBJECT_get_type(X509_OBJECT *a); X509 *X509_OBJECT_get0_X509(X509_OBJECT *a); diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 9d189aa51c..e7eb3028b4 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -774,10 +774,16 @@ SSL *SSL_new(SSL_CTX *ctx) return NULL; } -void SSL_up_ref(SSL *s) +int SSL_up_ref(SSL *s) { int i; - CRYPTO_atomic_add(&s->references, 1, &i, s->lock); + + if (CRYPTO_atomic_add(&s->references, 1, &i, s->lock) <= 0) + return 0; + + REF_PRINT_COUNT("SSL", s); + REF_ASSERT_ISNT(i < 2); + return ((i > 1) ? 1 : 0); } int SSL_CTX_set_session_id_context(SSL_CTX *ctx, const unsigned char *sid_ctx, @@ -2504,10 +2510,16 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) return NULL; } -void SSL_CTX_up_ref(SSL_CTX *ctx) +int SSL_CTX_up_ref(SSL_CTX *ctx) { int i; - CRYPTO_atomic_add(&ctx->references, 1, &i, ctx->lock); + + if (CRYPTO_atomic_add(&ctx->references, 1, &i, ctx->lock) <= 0) + return 0; + + REF_PRINT_COUNT("SSL_CTX", ctx); + REF_ASSERT_ISNT(i < 2); + return ((i > 1) ? 1 : 0); } void SSL_CTX_free(SSL_CTX *a) -- 2.25.1