yggdrasil/ring
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove ex_data's dup hook.

Browse Source 
The only place it is used is EC_KEY_{dup,copy} and no one calls that
function on an EC_KEY with ex_data. This aligns with functions like
RSAPublicKey_dup which do not copy ex_data. The logic is also somewhat
subtle in the face of malloc errors (upstream's PR 3323).

In fact, we'd even changed the function pointer signature from upstream,
so BoringSSL-only code is needed to pass this pointer in anyway. (I
haven't switched it to CRYPTO_EX_unused because there are some callers
which pass in an implementation anyway.)

Note, in upstream, the dup hook is also used for SSL_SESSIONs when those
are duplicated (for TLS 1.2 ticket renewal or TLS 1.3 resumption). Our
interpretation is that callers should treat those SSL_SESSIONs
equivalently to newly-established ones. This avoids every consumer
providing a dup hook and simplifies the interface.

(I've gone ahead and removed the TODO(fork). I don't think we'll be able
to change this API. Maybe introduce a new one, but it may not be worth
it? Then again, this API is atrocious... I've never seen anyone use argl
and argp even.)

BUG=21

Change-Id: I6c9e9d5a02347cb229d4c084c1e85125bd741d2b
Reviewed-on: https://boringssl-review.googlesource.com/16344
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
David Benjamin 2017-05-14 17:10:18 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent 21cb0744bb
commit d94682dce5
18 changed files with 39 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crypto/dh/dh.c
View File

@ -465,9 +465,9 @@ DH *DHparams_dup(const DH *dh) {
}
int DH_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) {
CRYPTO_EX_dup *dup_unused, CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, dup_func,
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp,
free_func)) {
return -1;
}

4
crypto/dsa/dsa.c
View File

@ -908,9 +908,9 @@ err:
}
int DSA_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) {
CRYPTO_EX_dup *dup_unused, CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, dup_func,
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp,
free_func)) {
return -1;
}

44
crypto/ex_data.c
View File

@ -127,12 +127,10 @@ struct crypto_ex_data_func_st {
long argl; /* Arbitary long */
void *argp; /* Arbitary void pointer */
CRYPTO_EX_free *free_func;
CRYPTO_EX_dup *dup_func;
};
int CRYPTO_get_ex_new_index(CRYPTO_EX_DATA_CLASS *ex_data_class, int *out_index,
long argl, void *argp, CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func) {
long argl, void *argp, CRYPTO_EX_free *free_func) {
CRYPTO_EX_DATA_FUNCS *funcs;
int ret = 0;
@ -144,7 +142,6 @@ int CRYPTO_get_ex_new_index(CRYPTO_EX_DATA_CLASS *ex_data_class, int *out_index,
funcs->argl = argl;
funcs->argp = argp;
funcs->dup_func = dup_func;
funcs->free_func = free_func;
CRYPTO_STATIC_MUTEX_lock_write(&ex_data_class->lock);
@ -233,45 +230,6 @@ void CRYPTO_new_ex_data(CRYPTO_EX_DATA *ad) {
ad->sk = NULL;
}
int CRYPTO_dup_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class, CRYPTO_EX_DATA *to,
const CRYPTO_EX_DATA *from) {
if (from->sk == NULL) {
/* In this case, |from| is blank, which is also the initial state of |to|,
* so there's nothing to do. */
return 1;
}
for (size_t i = 0; i < ex_data_class->num_reserved; i++) {
void *ptr = CRYPTO_get_ex_data(from, i);
if (!CRYPTO_set_ex_data(to, i, ptr)) {
return 0;
}
}
STACK_OF(CRYPTO_EX_DATA_FUNCS) *func_pointers;
if (!get_func_pointers(&func_pointers, ex_data_class)) {
return 0;
}
for (size_t i = 0; i < sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers); i++) {
CRYPTO_EX_DATA_FUNCS *func_pointer =
sk_CRYPTO_EX_DATA_FUNCS_value(func_pointers, i);
void *ptr = CRYPTO_get_ex_data(from, i + ex_data_class->num_reserved);
if (func_pointer->dup_func) {
func_pointer->dup_func(to, from, &ptr, i + ex_data_class->num_reserved,
func_pointer->argl, func_pointer->argp);
}
if (!CRYPTO_set_ex_data(to, i + ex_data_class->num_reserved, ptr)) {
sk_CRYPTO_EX_DATA_FUNCS_free(func_pointers);
return 0;
}
}
sk_CRYPTO_EX_DATA_FUNCS_free(func_pointers);
return 1;
}
void CRYPTO_free_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class, void *obj,
CRYPTO_EX_DATA *ad) {
if (ad->sk == NULL) {

9
crypto/fipsmodule/ec/ec_key.c
View File

@ -201,11 +201,6 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src) {
dest->ecdsa_meth = src->ecdsa_meth;
METHOD_ref(dest->ecdsa_meth);
}
CRYPTO_free_ex_data(g_ec_ex_data_class_bss_get(), dest, &dest->ex_data);
if (!CRYPTO_dup_ex_data(g_ec_ex_data_class_bss_get(), &dest->ex_data,
&src->ex_data)) {
return NULL;
}
/* copy the rest */
dest->enc_flag = src->enc_flag;
@ -502,11 +497,11 @@ int EC_KEY_generate_key_fips(EC_KEY *eckey) {
}
int EC_KEY_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(g_ec_ex_data_class_bss_get(), &index, argl, argp,
dup_func, free_func)) {
free_func)) {
return -1;
}
return index;

4
crypto/fipsmodule/rsa/rsa.c
View File

@ -284,10 +284,10 @@ int RSA_is_opaque(const RSA *rsa) {
}
int RSA_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) {
CRYPTO_EX_dup *dup_unused, CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(g_rsa_ex_data_class_bss_get(), &index, argl,
argp, dup_func, free_func)) {
argp, free_func)) {
return -1;
}
return index;

9
crypto/internal.h
View File

@ -528,7 +528,7 @@ typedef struct {
* zero otherwise. */
OPENSSL_EXPORT int CRYPTO_get_ex_new_index(CRYPTO_EX_DATA_CLASS *ex_data_class,
int *out_index, long argl,
void *argp, CRYPTO_EX_dup *dup_func,
void *argp,
CRYPTO_EX_free *free_func);
/* CRYPTO_set_ex_data sets an extra data pointer on a given object. Each class
@ -543,13 +543,6 @@ OPENSSL_EXPORT void *CRYPTO_get_ex_data(const CRYPTO_EX_DATA *ad, int index);
/* CRYPTO_new_ex_data initialises a newly allocated |CRYPTO_EX_DATA|. */
OPENSSL_EXPORT void CRYPTO_new_ex_data(CRYPTO_EX_DATA *ad);
/* CRYPTO_dup_ex_data duplicates |from| into a freshly allocated
* |CRYPTO_EX_DATA|, |to|. Both of which are inside objects of the given
* class. It returns one on success and zero otherwise. */
OPENSSL_EXPORT int CRYPTO_dup_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class,
CRYPTO_EX_DATA *to,
const CRYPTO_EX_DATA *from);
/* CRYPTO_free_ex_data frees |ad|, which is embedded inside |obj|, which is an
* object of the given class. */
OPENSSL_EXPORT void CRYPTO_free_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class,

4
crypto/x509/x509_vfy.c
View File

@ -2101,7 +2101,7 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
int X509_STORE_CTX_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused * unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func)
{
/*
@ -2110,7 +2110,7 @@ int X509_STORE_CTX_get_ex_new_index(long argl, void *argp,
*/
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp,
dup_func, free_func)) {
free_func)) {
return -1;
}
return index;

4
crypto/x509/x_x509.c
View File

@ -188,11 +188,11 @@ int X509_up_ref(X509 *x)
}
int X509_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused * unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func)
CRYPTO_EX_dup *dup_unused, CRYPTO_EX_free *free_func)
{
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp,
dup_func, free_func)) {
free_func)) {
return -1;
}
return index;

2
include/openssl/dh.h
View File

@ -210,7 +210,7 @@ OPENSSL_EXPORT int DH_marshal_parameters(CBB *cbb, const DH *dh);
OPENSSL_EXPORT int DH_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int DH_set_ex_data(DH *d, int idx, void *arg);
OPENSSL_EXPORT void *DH_get_ex_data(DH *d, int idx);

2
include/openssl/dsa.h
View File

@ -296,7 +296,7 @@ OPENSSL_EXPORT DH *DSA_dup_DH(const DSA *dsa);
OPENSSL_EXPORT int DSA_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int DSA_set_ex_data(DSA *d, int idx, void *arg);
OPENSSL_EXPORT void *DSA_get_ex_data(const DSA *d, int idx);

2
include/openssl/ec_key.h
View File

@ -223,7 +223,7 @@ OPENSSL_EXPORT EC_GROUP *EC_KEY_parse_parameters(CBS *cbs);
OPENSSL_EXPORT int EC_KEY_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int EC_KEY_set_ex_data(EC_KEY *r, int idx, void *arg);
OPENSSL_EXPORT void *EC_KEY_get_ex_data(const EC_KEY *r, int idx);

30
include/openssl/ex_data.h
View File

@ -134,16 +134,14 @@ typedef struct crypto_ex_data_st CRYPTO_EX_DATA;
#if 0 /* Sample */
/* TYPE_get_ex_new_index allocates a new index for |TYPE|. See the
* descriptions of the callback typedefs for details of when they are
* called. Any of the callback arguments may be NULL. The |argl| and |argp|
* arguments are opaque values that are passed to the callbacks. It returns the
* new index or a negative number on error.
*
* TODO(fork): this should follow the standard calling convention. */
/* TYPE_get_ex_new_index allocates a new index for |TYPE|. An optional
* |free_func| argument may be provided which is called when the owning object
* is destroyed. See |CRYPTO_EX_free| for details. The |argl| and |argp|
* arguments are opaque values that are passed to the callback. It returns the
* new index or a negative number on error. */
OPENSSL_EXPORT int TYPE_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func);
/* TYPE_set_ex_data sets an extra data pointer on |t|. The |index| argument
@ -176,24 +174,16 @@ OPENSSL_EXPORT void *TYPE_get_ex_data(const TYPE *t, int index);
typedef void CRYPTO_EX_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
int index, long argl, void *argp);
/* CRYPTO_EX_dup is a callback function that is called when an object of the
* class is being copied and thus the ex_data linked to it also needs to be
* copied. On entry, |*from_d| points to the data for this index from the
* original object. When the callback returns, |*from_d| will be set as the
* data for this index in |to|.
*
* This callback may be called with a NULL value for |*from_d| if |from| has no
* value set for this index. However, the callbacks may also be skipped entirely
* if no extra data pointers are set on |from| at all. */
typedef int CRYPTO_EX_dup(CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from,
void **from_d, int index, long argl, void *argp);
/* Deprecated functions. */
/* CRYPTO_cleanup_all_ex_data does nothing. */
OPENSSL_EXPORT void CRYPTO_cleanup_all_ex_data(void);
/* CRYPTO_EX_dup is a legacy callback function type which is ignored. */
typedef int CRYPTO_EX_dup(CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from,
void **from_d, int index, long argl, void *argp);
/* Private structures. */

2
include/openssl/rsa.h
View File

@ -454,7 +454,7 @@ OPENSSL_EXPORT int RSA_private_key_to_bytes(uint8_t **out_bytes,
OPENSSL_EXPORT int RSA_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int RSA_set_ex_data(RSA *r, int idx, void *arg);
OPENSSL_EXPORT void *RSA_get_ex_data(const RSA *r, int idx);

6
include/openssl/ssl.h
View File

@ -2913,7 +2913,7 @@ OPENSSL_EXPORT int SSL_set_ex_data(SSL *ssl, int idx, void *data);
OPENSSL_EXPORT void *SSL_get_ex_data(const SSL *ssl, int idx);
OPENSSL_EXPORT int SSL_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int SSL_SESSION_set_ex_data(SSL_SESSION *session, int idx,
@ -2922,14 +2922,14 @@ OPENSSL_EXPORT void *SSL_SESSION_get_ex_data(const SSL_SESSION *session,
int idx);
OPENSSL_EXPORT int SSL_SESSION_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int SSL_CTX_set_ex_data(SSL_CTX *ctx, int idx, void *data);
OPENSSL_EXPORT void *SSL_CTX_get_ex_data(const SSL_CTX *ctx, int idx);
OPENSSL_EXPORT int SSL_CTX_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func);

2
include/openssl/x509.h
View File

@ -779,7 +779,7 @@ DECLARE_ASN1_FUNCTIONS(X509_CERT_PAIR)
OPENSSL_EXPORT int X509_up_ref(X509 *x);
OPENSSL_EXPORT int X509_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func);
CRYPTO_EX_dup *dup_unused, CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int X509_set_ex_data(X509 *r, int idx, void *arg);
OPENSSL_EXPORT void *X509_get_ex_data(X509 *r, int idx);
OPENSSL_EXPORT int i2d_X509_AUX(X509 *a,unsigned char **pp);

2
include/openssl/x509_vfy.h
View File

@ -513,7 +513,7 @@ OPENSSL_EXPORT int X509_STORE_set_default_paths(X509_STORE *ctx);
#endif
OPENSSL_EXPORT int X509_STORE_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func);
CRYPTO_EX_dup *dup_unused, CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int X509_STORE_CTX_set_ex_data(X509_STORE_CTX *ctx,int idx,void *data);
OPENSSL_EXPORT void * X509_STORE_CTX_get_ex_data(X509_STORE_CTX *ctx,int idx);
OPENSSL_EXPORT int X509_STORE_CTX_get_error(X509_STORE_CTX *ctx);

8
ssl/ssl_lib.c
View File

@ -2048,10 +2048,10 @@ char *SSL_get_shared_ciphers(const SSL *ssl, char *buf, int len) {
}
int SSL_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) {
CRYPTO_EX_dup *dup_unused, CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class_ssl, &index, argl, argp,
dup_func, free_func)) {
free_func)) {
return -1;
}
return index;
@ -2066,11 +2066,11 @@ void *SSL_get_ex_data(const SSL *ssl, int idx) {
}
int SSL_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class_ssl_ctx, &index, argl, argp,
dup_func, free_func)) {
free_func)) {
return -1;
}
return index;

4
ssl/ssl_session.c
View File

@ -483,10 +483,10 @@ SSL_SESSION *SSL_get1_session(SSL *ssl) {
int SSL_SESSION_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_dup *dup_unused,
CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, dup_func,
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp,
free_func)) {
return -1;
}