Don't attempt to duplicate the BIO state in SSL_dup
authorMatt Caswell <matt@openssl.org>
Tue, 16 Jun 2020 16:40:40 +0000 (17:40 +0100)
committerMatt Caswell <matt@openssl.org>
Tue, 23 Jun 2020 11:46:47 +0000 (12:46 +0100)
SSL_dup attempted to duplicate the BIO state if the source SSL had BIOs
configured for it. This did not work.

Firstly the SSL_dup code was passing a BIO ** as the destination
argument for BIO_dup_state. However BIO_dup_state expects a BIO * for that
parameter. Any attempt to use this will either (1) fail silently, (2) crash
or fail in some other strange way.

Secondly many BIOs do not implement the BIO_CTRL_DUP ctrl required to make
this work.

Thirdly, if rbio == wbio in the original SSL object, then an attempt is made
to up-ref the BIO in the new SSL object - even though it hasn't been set
yet and is NULL. This results in a crash.

This appears to have been broken for a very long time with at least some of
the problems described above coming from SSLeay. The simplest approach is
to just remove this capability from the function.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/12180)

doc/man3/SSL_new.pod
ssl/ssl_lib.c

index 4d963d10d8b21518d698b9613674c13d5c48d8a4..659b6d6738d0e20bf59cc30c78c790bfb6027866 100644 (file)
@@ -73,9 +73,6 @@ L<SSL_set_info_callback(3)>
 
 =item any configured Cipher List
 
-=item any BIOs configured on I<s> will have new BIO's created and the BIO state
-duplicated via BIO_dup_state().
-
 =item initial accept (server) or connect (client) state
 
 =item the max cert list value set via L<SSL_set_max_cert_list(3)>
index f7544ab402026a81eb788a4b6b9f17bce9f69be5..fea040289b1ed6a7ceef636530c62a7d4feef6a3 100644 (file)
@@ -4023,21 +4023,6 @@ SSL *SSL_dup(SSL *s)
     if (!CRYPTO_dup_ex_data(CRYPTO_EX_INDEX_SSL, &ret->ex_data, &s->ex_data))
         goto err;
 
-    /* setup rbio, and wbio */
-    if (s->rbio != NULL) {
-        if (!BIO_dup_state(s->rbio, (char *)&ret->rbio))
-            goto err;
-    }
-    if (s->wbio != NULL) {
-        if (s->wbio != s->rbio) {
-            if (!BIO_dup_state(s->wbio, (char *)&ret->wbio))
-                goto err;
-        } else {
-            BIO_up_ref(ret->rbio);
-            ret->wbio = ret->rbio;
-        }
-    }
-
     ret->server = s->server;
     if (s->handshake_func) {
         if (s->server)