Remove a hack from ssl_test_old

ssl_test_old was reaching inside the SSL structure and changing the internal
BIO values. This is completely unneccessary, and was causing an abort in the
test when enabling TLSv1.3.

I also removed the need for ssl_test_old to include ssl_locl.h. This
required the addition of some missing accessors for SSL_COMP name and id
fields.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit e304d3e20f45243f9e643607edfe4db49c329596)

diff --git a/doc/ssl/SSL_COMP_add_compression_method.pod b/doc/ssl/SSL_COMP_add_compression_method.pod
index c455832078be15f5ba9d440377c1bb73e7bbf5d8..15929df32b1e6b5f44aeb78ab8a5306e0e29028f 100644 (file)
--- a/doc/ssl/SSL_COMP_add_compression_method.pod
+++ b/doc/ssl/SSL_COMP_add_compression_method.pod
@@ -2,13 +2,18 @@
 
 =head1 NAME
 
-SSL_COMP_add_compression_method, SSL_COMP_free_compression_methods - handle SSL/TLS integrated compression methods
+SSL_COMP_add_compression_method, SSL_COMP_get_compression_methods,
+SSL_COMP_get0_name, SSL_COMP_get_id, SSL_COMP_free_compression_methods
+- handle SSL/TLS integrated compression methods
 
 =head1 SYNOPSIS
 
  #include <openssl/ssl.h>
 
  int SSL_COMP_add_compression_method(int id, COMP_METHOD *cm);
+ STACK_OF(SSL_COMP) *SSL_COMP_get_compression_methods(void);
+ const char *SSL_COMP_get0_name(const SSL_COMP *comp);
+ int SSL_COMP_get_id(const SSL_COMP *comp);
 
 Deprecated:
 
@@ -23,6 +28,13 @@ the identifier B<id> to the list of available compression methods. This
 list is globally maintained for all SSL operations within this application.
 It cannot be set for specific SSL_CTX or SSL objects.
 
+SSL_COMP_get_compression_methods() returns a stack of all of the available
+compression methods or NULL on error.
+
+SSL_COMP_get0_name() returns the name of the compression method B<comp>.
+
+SSL_COMP_get_id() returns the id of the compression method B<comp>.
+
 In versions of OpenSSL prior to 1.1.0 SSL_COMP_free_compression_methods() freed
 the internal table of compression methods that were built internally, and
 possibly augmented by adding SSL_COMP_add_compression_method(). However this is
@@ -76,6 +88,13 @@ The operation failed. Check the error queue to find out the reason.
 
 =back
 
+SSL_COMP_get_compression_methods() returns the stack of compressions methods or
+NULL on error.
+
+SSL_COMP_get0_name() returns the name of the compression method or NULL on error.
+
+SSL_COMP_get_id() returns the name of the compression method or -1 on error.
+
 =head1 SEE ALSO
 
 L<ssl(3)>
@@ -83,6 +102,7 @@ L<ssl(3)>
 =head1 HISTORY
 
 SSL_COMP_free_compression_methods() was deprecated in OpenSSL 1.1.0.
+SSL_COMP_get0_name() and SSL_comp_get_id() were added in OpenSSL 1.1.0d.
 
 =head1 COPYRIGHT
 

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 86ab9125de8c5d70168fb948ee27f021403ad6b7..ccb2d3553331febcb683991ea040fe2cb77b4764 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1777,6 +1777,8 @@ void SSL_set_tmp_dh_callback(SSL *ssl,
 __owur const COMP_METHOD *SSL_get_current_compression(SSL *s);
 __owur const COMP_METHOD *SSL_get_current_expansion(SSL *s);
 __owur const char *SSL_COMP_get_name(const COMP_METHOD *comp);
+__owur const char *SSL_COMP_get0_name(const SSL_COMP *comp);
+__owur int SSL_COMP_get_id(const SSL_COMP *comp);
 STACK_OF(SSL_COMP) *SSL_COMP_get_compression_methods(void);
 __owur STACK_OF(SSL_COMP) *SSL_COMP_set0_compression_methods(STACK_OF(SSL_COMP)
                                                       *meths);

diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c
index 0d46509b190cf3e4884d1c34ea85a72ea5e1586b..99b64bb967c7171e21cfb4c90d1f023bb3edba33 100644 (file)
--- a/ssl/ssl_ciph.c
+++ b/ssl/ssl_ciph.c
@@ -1868,6 +1868,24 @@ const char *SSL_COMP_get_name(const COMP_METHOD *comp)
 #endif
 }
 
+const char *SSL_COMP_get0_name(const SSL_COMP *comp)
+{
+#ifndef OPENSSL_NO_COMP
+    return comp->name;
+#else
+    return NULL;
+#endif
+}
+
+int SSL_COMP_get_id(const SSL_COMP *comp)
+{
+#ifndef OPENSSL_NO_COMP
+    return comp->id;
+#else
+    return -1;
+#endif
+}
+
 /* For a cipher return the index corresponding to the certificate type */
 int ssl_cipher_get_cert_index(const SSL_CIPHER *c)
 {

diff --git a/test/ssltest_old.c b/test/ssltest_old.c
index 6a5cd7038d9a3d4e39c54ef519c80f59653a10e5..ccb2edb52d3432903e0570cce529b45d50999f54 100644 (file)
--- a/test/ssltest_old.c
+++ b/test/ssltest_old.c
@@ -92,8 +92,6 @@
 # include <openssl/ct.h>
 #endif
 
-#include "../ssl/ssl_locl.h"
-
 /*
  * Or gethostname won't be declared properly
  * on Compaq platforms (at least with DEC C).
@@ -1421,7 +1419,7 @@ int main(int argc, char *argv[])
         printf("Available compression methods:");
         for (j = 0; j < n; j++) {
             SSL_COMP *c = sk_SSL_COMP_value(ssl_comp_methods, j);
-            printf("  %s:%d", c->name, c->id);
+            printf("  %s:%d", SSL_COMP_get0_name(c), SSL_COMP_get_id(c));
         }
         printf("\n");
     }
@@ -2664,8 +2662,29 @@ int doit(SSL *s_ssl, SSL *c_ssl, long count)
     SSL_set_max_send_fragment(c_ssl, max_frag);
     BIO_set_ssl(c_bio, c_ssl, BIO_NOCLOSE);
 
+    /*
+     * We've just given our ref to these BIOs to c_ssl. We need another one to
+     * give to s_ssl
+     */
+    if (!BIO_up_ref(c_to_s)) {
+        /* c_to_s and s_to_c will get freed when we free c_ssl */
+        c_to_s = NULL;
+        s_to_c = NULL;
+        goto err;
+    }
+    if (!BIO_up_ref(s_to_c)) {
+        /* s_to_c will get freed when we free c_ssl */
+        s_to_c = NULL;
+        goto err;
+    }
+
     SSL_set_accept_state(s_ssl);
     SSL_set_bio(s_ssl, c_to_s, s_to_c);
+
+    /* We've used up all our refs to these now */
+    c_to_s = NULL;
+    s_to_c = NULL;
+
     SSL_set_max_send_fragment(s_ssl, max_frag);
     BIO_set_ssl(s_bio, s_ssl, BIO_NOCLOSE);
 
@@ -2878,23 +2897,6 @@ int doit(SSL *s_ssl, SSL *c_ssl, long count)
     }
     ret = 0;
  err:
-    /*
-     * We have to set the BIO's to NULL otherwise they will be
-     * OPENSSL_free()ed twice.  Once when th s_ssl is SSL_free()ed and again
-     * when c_ssl is SSL_free()ed. This is a hack required because s_ssl and
-     * c_ssl are sharing the same BIO structure and SSL_set_bio() and
-     * SSL_free() automatically BIO_free non NULL entries. You should not
-     * normally do this or be required to do this
-     */
-    if (s_ssl != NULL) {
-        s_ssl->rbio = NULL;
-        s_ssl->wbio = NULL;
-    }
-    if (c_ssl != NULL) {
-        c_ssl->rbio = NULL;
-        c_ssl->wbio = NULL;
-    }
-
     BIO_free(c_to_s);
     BIO_free(s_to_c);
     BIO_free_all(c_bio);

diff --git a/util/libssl.num b/util/libssl.num
index 200629f74bb727cdb64bce790cf265b142e2e8ab..7b9b3c251ced53bcb50e2f1291552ff3176f4fb8 100644 (file)
--- a/util/libssl.num
+++ b/util/libssl.num
@@ -403,3 +403,5 @@ SSL_dane_clear_flags                    403 1_1_0   EXIST::FUNCTION:
 SSL_SESSION_get0_cipher                 404    1_1_0   EXIST::FUNCTION:
 SSL_SESSION_get0_id_context             405    1_1_0   EXIST::FUNCTION:
 SSL_SESSION_set1_id                     406    1_1_0   EXIST::FUNCTION:
+SSL_COMP_get_id                         412    1_1_0d  EXIST::FUNCTION:
+SSL_COMP_get0_name                      413    1_1_0d  EXIST::FUNCTION: