The branch OpenSSL_1_1_0-stable has been updated
via 8ab4fed9bdcc5b8498b3d59d2fa72dd045f63539 (commit)
from 819d18f6116e97845ebe453128f3c2a78e42a785 (commit)
- Log -----------------------------------------------------------------
commit 8ab4fed9bdcc5b8498b3d59d2fa72dd045f63539
Author: Todd Short <tsh...@akamai.com>
Date: Wed Apr 26 14:05:49 2017 -0400
Fix ex_data and session_dup issues
Code was added in commit b3c31a65 that overwrote the last ex_data value
using CRYPTO_dup_ex_data() causing a memory leak, and potentially
confusing the ex_data dup() callback.
In ssl_session_dup(), fix error handling (properly reference and up-ref
shared data) and new-up the ex_data before calling CRYPTO_dup_ex_data();
all other structures that dup ex_data have the destination ex_data new'd
before the dup.
Fix up some of the ex_data documentation.
Reviewed-by: Matt Caswell <m...@openssl.org>
Reviewed-by: Rich Salz <rs...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3625)
-----------------------------------------------------------------------
Summary of changes:
crypto/ex_data.c | 9 ++-
doc/crypto/CRYPTO_get_ex_new_index.pod | 7 +-
ssl/ssl_sess.c | 14 +++-
test/exdatatest.c | 131 ++++++++++++++++++++++++++++++---
4 files changed, 143 insertions(+), 18 deletions(-)
diff --git a/crypto/ex_data.c b/crypto/ex_data.c
index 4a3201a..22c4d3d 100644
--- a/crypto/ex_data.c
+++ b/crypto/ex_data.c
@@ -287,7 +287,14 @@ int CRYPTO_dup_ex_data(int class_index, CRYPTO_EX_DATA *to,
CRYPTOerr(CRYPTO_F_CRYPTO_DUP_EX_DATA, ERR_R_MALLOC_FAILURE);
return 0;
}
- if (!CRYPTO_set_ex_data(to, mx - 1, NULL))
+ /*
+ * Make sure the ex_data stack is at least |mx| elements long to avoid
+ * issues in the for loop that follows; so go get the |mx|'th element
+ * (if it does not exist CRYPTO_get_ex_data() returns NULL), and assign
+ * to itself. This is normally a no-op; but ensures the stack is the
+ * proper size
+ */
+ if (!CRYPTO_set_ex_data(to, mx - 1, CRYPTO_get_ex_data(to, mx - 1)))
goto err;
for (i = 0; i < mx; i++) {
diff --git a/doc/crypto/CRYPTO_get_ex_new_index.pod
b/doc/crypto/CRYPTO_get_ex_new_index.pod
index 0853ce5..a5bf620 100644
--- a/doc/crypto/CRYPTO_get_ex_new_index.pod
+++ b/doc/crypto/CRYPTO_get_ex_new_index.pod
@@ -17,8 +17,8 @@ CRYPTO_get_ex_data, CRYPTO_free_ex_data, CRYPTO_new_ex_data
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
- typedef int CRYPTO_EX_new(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
- int idx, long argl, void *argp);
+ typedef void CRYPTO_EX_new(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
+ int idx, long argl, void *argp);
typedef void CRYPTO_EX_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
int idx, long argl, void *argp);
typedef int CRYPTO_EX_dup(CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from,
@@ -128,7 +128,8 @@ initially registered via CRYPTO_get_ex_new_index() and can
be used if
the same callback handles different types of exdata.
dup_func() is called when a structure is being copied. This is only done
-for B<SSL> and B<SSL_SESSION> objects. The B<to> and B<from> parameters
+for B<SSL>, B<SSL_SESSION>, B<EC_KEY> objects and B<BIO> chains via
+BIO_dup_chain(). The B<to> and B<from> parameters
are pointers to the destination and source B<CRYPTO_EX_DATA> structures,
respectively. The B<from_d> parameter needs to be cast to a B<void **pptr>
as the API has currently the wrong signature; that will be changed in a
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 3f06884..92ba599 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -138,6 +138,8 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket)
#ifndef OPENSSL_NO_SRP
dest->srp_username = NULL;
#endif
+ dest->peer_chain = NULL;
+ dest->peer = NULL;
memset(&dest->ex_data, 0, sizeof(dest->ex_data));
/* We deliberately don't copy the prev and next pointers */
@@ -150,8 +152,14 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket)
if (dest->lock == NULL)
goto err;
- if (src->peer != NULL)
- X509_up_ref(src->peer);
+ if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL_SESSION, dest, &dest->ex_data))
+ goto err;
+
+ if (src->peer != NULL) {
+ if (!X509_up_ref(src->peer))
+ goto err;
+ dest->peer = src->peer;
+ }
if (src->peer_chain != NULL) {
dest->peer_chain = X509_chain_up_ref(src->peer_chain);
@@ -207,7 +215,7 @@ SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket)
}
#endif
- if (ticket != 0) {
+ if (ticket != 0 && src->tlsext_tick != NULL) {
dest->tlsext_tick =
OPENSSL_memdup(src->tlsext_tick, src->tlsext_ticklen);
if (dest->tlsext_tick == NULL)
diff --git a/test/exdatatest.c b/test/exdatatest.c
index e0eadd3..7998622 100644
--- a/test/exdatatest.c
+++ b/test/exdatatest.c
@@ -15,6 +15,12 @@
static long saved_argl;
static void *saved_argp;
static int saved_idx;
+static int saved_idx2;
+
+/*
+ * SIMPLE EX_DATA IMPLEMENTATION
+ * Apps explicitly set/get ex_data as needed
+ */
static void exnew(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
int idx, long argl, void *argp)
@@ -43,6 +49,75 @@ static void exfree(void *parent, void *ptr, CRYPTO_EX_DATA
*ad,
OPENSSL_assert(argp == saved_argp);
}
+/*
+ * PRE-ALLOCATED EX_DATA IMPLEMENTATION
+ * Extended data structure is allocated in exnew2/freed in exfree2
+ * Data is stored inside extended data structure
+ */
+
+typedef struct myobj_ex_data_st {
+ char *hello;
+ int new;
+ int dup;
+} MYOBJ_EX_DATA;
+
+static void exnew2(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
+ int idx, long argl, void *argp)
+{
+ int ret;
+ MYOBJ_EX_DATA *ex_data;
+
+ OPENSSL_assert(idx == saved_idx2);
+ OPENSSL_assert(argl == saved_argl);
+ OPENSSL_assert(argp == saved_argp);
+ OPENSSL_assert(ptr == NULL);
+
+ ex_data = OPENSSL_zalloc(sizeof(*ex_data));
+ OPENSSL_assert(ex_data != NULL);
+ ret = CRYPTO_set_ex_data(ad, saved_idx2, ex_data);
+ OPENSSL_assert(ret);
+
+ ex_data->new = 1;
+}
+
+static int exdup2(CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from,
+ void *from_d, int idx, long argl, void *argp)
+{
+ MYOBJ_EX_DATA **update_ex_data = (MYOBJ_EX_DATA**)from_d;
+ MYOBJ_EX_DATA *ex_data = CRYPTO_get_ex_data(to, saved_idx2);
+
+ OPENSSL_assert(idx == saved_idx2);
+ OPENSSL_assert(argl == saved_argl);
+ OPENSSL_assert(argp == saved_argp);
+ OPENSSL_assert(from_d != NULL);
+ OPENSSL_assert(*update_ex_data != NULL);
+ OPENSSL_assert(ex_data != NULL);
+ OPENSSL_assert(ex_data->new);
+
+ /* Copy hello over */
+ ex_data->hello = (*update_ex_data)->hello;
+ /* indicate this is a dup */
+ ex_data->dup = 1;
+ /* Keep my original ex_data */
+ *update_ex_data = ex_data;
+ return 1;
+}
+
+static void exfree2(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
+ int idx, long argl, void *argp)
+{
+ MYOBJ_EX_DATA *ex_data = CRYPTO_get_ex_data(ad, saved_idx2);
+ int ret;
+
+ OPENSSL_assert(ex_data != NULL);
+ OPENSSL_free(ex_data);
+ OPENSSL_assert(idx == saved_idx2);
+ OPENSSL_assert(argl == saved_argl);
+ OPENSSL_assert(argp == saved_argp);
+ ret = CRYPTO_set_ex_data(ad, saved_idx2, NULL);
+ OPENSSL_assert(ret);
+}
+
typedef struct myobj_st {
CRYPTO_EX_DATA ex_data;
int id;
@@ -71,6 +146,25 @@ static char *MYOBJ_gethello(MYOBJ *obj)
return CRYPTO_get_ex_data(&obj->ex_data, saved_idx);
}
+static void MYOBJ_sethello2(MYOBJ *obj, char *cp)
+{
+ MYOBJ_EX_DATA* ex_data = CRYPTO_get_ex_data(&obj->ex_data, saved_idx2);
+ if (ex_data != NULL)
+ ex_data->hello = cp;
+ else
+ obj->st = 0;
+}
+
+static char *MYOBJ_gethello2(MYOBJ *obj)
+{
+ MYOBJ_EX_DATA* ex_data = CRYPTO_get_ex_data(&obj->ex_data, saved_idx2);
+ if (ex_data != NULL)
+ return ex_data->hello;
+
+ obj->st = 0;
+ return NULL;
+}
+
static void MYOBJ_free(MYOBJ *obj)
{
CRYPTO_free_ex_data(CRYPTO_EX_INDEX_APP, obj, &obj->ex_data);
@@ -90,36 +184,51 @@ static MYOBJ *MYOBJ_dup(MYOBJ *in)
int main()
{
MYOBJ *t1, *t2, *t3;
+ MYOBJ_EX_DATA *ex_data;
const char *cp;
char *p;
- p = strdup("hello world");
+ p = OPENSSL_strdup("hello world");
saved_argl = 21;
- saved_argp = malloc(1);
+ saved_argp = OPENSSL_malloc(1);
saved_idx = CRYPTO_get_ex_new_index(CRYPTO_EX_INDEX_APP,
saved_argl, saved_argp,
exnew, exdup, exfree);
+ saved_idx2 = CRYPTO_get_ex_new_index(CRYPTO_EX_INDEX_APP,
+ saved_argl, saved_argp,
+ exnew2, exdup2, exfree2);
t1 = MYOBJ_new();
t2 = MYOBJ_new();
+ OPENSSL_assert(t1->st && t2->st);
+ ex_data = CRYPTO_get_ex_data(&t1->ex_data, saved_idx2);
+ OPENSSL_assert(ex_data != NULL);
+ ex_data = CRYPTO_get_ex_data(&t2->ex_data, saved_idx2);
+ OPENSSL_assert(ex_data != NULL);
MYOBJ_sethello(t1, p);
cp = MYOBJ_gethello(t1);
OPENSSL_assert(cp == p);
- if (cp != p)
- return 1;
cp = MYOBJ_gethello(t2);
OPENSSL_assert(cp == NULL);
- if (cp != NULL)
- return 1;
+ MYOBJ_sethello2(t1, p);
+ cp = MYOBJ_gethello2(t1);
+ OPENSSL_assert(cp == p);
+ OPENSSL_assert(t1->st);
+ cp = MYOBJ_gethello2(t2);
+ OPENSSL_assert(cp == NULL);
+ OPENSSL_assert(t2->st);
t3 = MYOBJ_dup(t1);
+ ex_data = CRYPTO_get_ex_data(&t3->ex_data, saved_idx2);
+ OPENSSL_assert(ex_data != NULL);
+ OPENSSL_assert(ex_data->dup);
cp = MYOBJ_gethello(t3);
OPENSSL_assert(cp == p);
- if (cp != p)
- return 1;
- cp = MYOBJ_gethello(t2);
+ cp = MYOBJ_gethello2(t3);
+ OPENSSL_assert(cp == p);
+ OPENSSL_assert(t3->st);
MYOBJ_free(t1);
MYOBJ_free(t2);
MYOBJ_free(t3);
- free(saved_argp);
- free(p);
+ OPENSSL_free(saved_argp);
+ OPENSSL_free(p);
return 0;
}
_____
openssl-commits mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits
