Revise s_client and s_server verbiage re secure renegotiation.

Since TLS v1.3 eschews renegotiation entirely it’s misleading to have
these apps say it’s “not supported” when in fact the TLS version is
new enough not to need renegotiation at all.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16937)

diff --git a/CHANGES.md b/CHANGES.md
index 4902332206e6e9e9c06c33a82fd0f8eeeafcee45..940d450fdf104cf708e9e31cf30d8cfc79e2a939 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -24,6 +24,13 @@ OpenSSL 3.1
 
 ### Changes between 3.0 and 3.1 [xx XXX xxxx]
 
+ * s_client and s_server apps now explicitly say when the TLS version
+   does not include the renegotiation mechanism. This avoids confusion
+   between that scenario versus when the TLS version includes secure
+   renegotiation but the peer lacks support for it.
+
+   *Felipe Gasper*
+
  * The default SSL/TLS security level has been changed from 1 to 2. RSA,
    DSA and DH keys of 1024 bits and above and less than 2048 bits and ECC keys
    of 160 bits and above and less than 224 bits were previously accepted by

diff --git a/apps/include/s_apps.h b/apps/include/s_apps.h
index d610df40be3feb750a53bbddb8040d2e9da1c1a4..194ea746ed4ca66cc3eb62167583f5811f0772fb 100644 (file)
--- a/apps/include/s_apps.h
+++ b/apps/include/s_apps.h
@@ -15,6 +15,9 @@
 #define PORT            "4433"
 #define PROTOCOL        "tcp"
 
+#define SSL_VERSION_ALLOWS_RENEGOTIATION(s) \
+    (SSL_is_dtls(s) || (SSL_version(s) < TLS1_3_VERSION))
+
 typedef int (*do_server_cb)(int s, int stype, int prot, unsigned char *context);
 int report_server_accept(BIO *out, int asock, int with_address, int with_pid);
 int do_server(int *accept_sock, const char *host, const char *port,
@@ -79,6 +82,7 @@ int ssl_load_stores(SSL_CTX *ctx, const char *vfyCApath,
 void ssl_ctx_security_debug(SSL_CTX *ctx, int verbose);
 int set_keylog_file(SSL_CTX *ctx, const char *keylog_file);
 void print_ca_names(BIO *bio, SSL *s);
+void ssl_print_secure_renegotiation_notes(BIO *bio, SSL *s);
 
 #ifndef OPENSSL_NO_SRP
 /* The client side SRP context that we pass to all SRP related callbacks */

diff --git a/apps/lib/s_cb.c b/apps/lib/s_cb.c
index 1d318027d2ed315c6679f72b5ab63f4c48d3b02c..f721695e405e83a37e2072a4282395ae621ac88e 100644 (file)
--- a/apps/lib/s_cb.c
+++ b/apps/lib/s_cb.c
@@ -7,7 +7,10 @@
  * https://www.openssl.org/source/license.html
  */
 
-/* callback functions used by s_client, s_server, and s_time */
+/*
+ * callback functions used by s_client, s_server, and s_time,
+ * as well as other common logic for those apps
+ */
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h> /* for memcpy() and strcmp() */
@@ -1543,3 +1546,13 @@ void print_ca_names(BIO *bio, SSL *s)
         BIO_write(bio, "\n", 1);
     }
 }
+
+void ssl_print_secure_renegotiation_notes(BIO *bio, SSL *s)
+{
+    if (SSL_VERSION_ALLOWS_RENEGOTIATION(s)) {
+        BIO_printf(bio, "Secure Renegotiation IS%s supported\n",
+                   SSL_get_secure_renegotiation_support(s) ? "" : " NOT");
+    } else {
+        BIO_printf(bio, "This TLS version forbids renegotiation.\n");
+    }
+}

diff --git a/apps/s_client.c b/apps/s_client.c
index f23919bb8dafaaa3bd46987bf989a1cd350f0530..46cecb9a826f66b5199a75ab19fdf05f8526df03 100644 (file)
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -3196,8 +3196,9 @@ static void print_stuff(BIO *bio, SSL *s, int full)
         BIO_printf(bio, "Server public key is %d bit\n",
                    EVP_PKEY_get_bits(pktmp));
     }
-    BIO_printf(bio, "Secure Renegotiation IS%s supported\n",
-               SSL_get_secure_renegotiation_support(s) ? "" : " NOT");
+
+    ssl_print_secure_renegotiation_notes(bio, s);
+
 #ifndef OPENSSL_NO_COMP
     comp = SSL_get_current_compression(s);
     expansion = SSL_get_current_expansion(s);

diff --git a/apps/s_server.c b/apps/s_server.c
index 0003f7a2a6a1ad5be596cb2288e00f8882804854..27c7db80a759a4add72f25cb8a753e2f5fc3d852 100644 (file)
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -2948,8 +2948,9 @@ static void print_connection_info(SSL *con)
 #endif
     if (SSL_session_reused(con))
         BIO_printf(bio_s_out, "Reused session-id\n");
-    BIO_printf(bio_s_out, "Secure Renegotiation IS%s supported\n",
-               SSL_get_secure_renegotiation_support(con) ? "" : " NOT");
+
+    ssl_print_secure_renegotiation_notes(bio_s_out, con);
+
     if ((SSL_get_options(con) & SSL_OP_NO_RENEGOTIATION))
         BIO_printf(bio_s_out, "Renegotiation is DISABLED\n");
 
@@ -3160,10 +3161,7 @@ static int www_body(int s, int stype, int prot, unsigned char *context)
             }
             BIO_puts(io, "\n");
 
-            BIO_printf(io,
-                       "Secure Renegotiation IS%s supported\n",
-                       SSL_get_secure_renegotiation_support(con) ?
-                       "" : " NOT");
+            ssl_print_secure_renegotiation_notes(io, con);
 
             /*
              * The following is evil and should not really be done