summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFelipe Gasper <felipe@felipegasper.com>2021-10-28 10:13:47 -0400
committerTomas Mraz <tomas@openssl.org>2021-11-02 16:30:10 +0100
commitaf5e63e1e3300f784f302a5d3309bf673cc08894 (patch)
tree80b29cb3085b48eb17744aa74d5247fa572b7677
parente81c81c9af8a5d22658110d2dc753582eb87a58e (diff)
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)
-rw-r--r--CHANGES.md7
-rw-r--r--apps/include/s_apps.h4
-rw-r--r--apps/lib/s_cb.c15
-rw-r--r--apps/s_client.c5
-rw-r--r--apps/s_server.c10
5 files changed, 32 insertions, 9 deletions
diff --git a/CHANGES.md b/CHANGES.md
index 4902332206..940d450fdf 100644
--- 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 d610df40be..194ea746ed 100644
--- 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 1d318027d2..f721695e40 100644
--- 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 f23919bb8d..46cecb9a82 100644
--- 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 0003f7a2a6..27c7db80a7 100644
--- 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