From 11ba87f2ff8e2455c6627a83aa458384fe7de70a Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 13 Feb 2017 13:26:37 +0000 Subject: [PATCH] Ensure s_client sends an SNI extension by default Enforcement of an SNI extension in the initial ClientHello is becoming increasingly common (e.g. see GitHub issue #2580). This commit changes s_client so that it adds SNI be default, unless explicitly told not to via the new "-noservername" option. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2614) --- CHANGES | 6 ++++++ apps/s_client.c | 30 ++++++++++++++++++++++++---- doc/man1/s_client.pod | 13 +++++++++++- test/recipes/70-test_sslmessages.t | 12 +++++------ test/recipes/70-test_tls13messages.t | 11 +++++----- test/testlib/checkhandshake.pm | 4 ++-- 6 files changed, 58 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index b1476d8ba6..7bd0f924f1 100644 --- a/CHANGES +++ b/CHANGES @@ -49,6 +49,12 @@ *) Add support for ARIA [Paul Dale] + *) s_client will now send the Server Name Indication (SNI) extension by + default unless the new "-noservername" option is used. The server name is + based on the host provided to the "-connect" option unless overridden by + using "-servername". + [Matt Caswell] + *) Add support for SipHash [Todd Short] diff --git a/apps/s_client.c b/apps/s_client.c index efdc8e3ef3..c544d49b6d 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -544,7 +544,7 @@ typedef enum OPTION_choice { OPT_VERIFYCAPATH, OPT_KEY, OPT_RECONNECT, OPT_BUILD_CHAIN, OPT_CAFILE, OPT_NOCAFILE, OPT_CHAINCAFILE, OPT_VERIFYCAFILE, OPT_NEXTPROTONEG, OPT_ALPN, - OPT_SERVERINFO, OPT_STARTTLS, OPT_SERVERNAME, + OPT_SERVERINFO, OPT_STARTTLS, OPT_SERVERNAME, OPT_NOSERVERNAME, OPT_USE_SRTP, OPT_KEYMATEXPORT, OPT_KEYMATEXPORTLEN, OPT_SMTPHOST, OPT_ASYNC, OPT_SPLIT_SEND_FRAG, OPT_MAX_PIPELINES, OPT_READ_BUF, OPT_KEYLOG_FILE, OPT_EARLY_DATA, OPT_REQCAFILE, @@ -652,6 +652,8 @@ const OPTIONS s_client_options[] = { {"nocommands", OPT_NOCMDS, '-', "Do not use interactive command letters"}, {"servername", OPT_SERVERNAME, 's', "Set TLS extension servername in ClientHello"}, + {"noservername", OPT_NOSERVERNAME, '-', + "Do not send the server name (SNI) extension in the ClientHello"}, {"tlsextdebug", OPT_TLSEXTDEBUG, '-', "Hex dump of all TLS extensions received"}, #ifndef OPENSSL_NO_OCSP @@ -872,6 +874,7 @@ int s_client_main(int argc, char **argv) struct timeval tv; #endif char *servername = NULL; + int noservername = 0; const char *alpn_in = NULL; tlsextctx tlsextcbp = { NULL, 0 }; const char *ssl_config = NULL; @@ -1359,6 +1362,9 @@ int s_client_main(int argc, char **argv) case OPT_SERVERNAME: servername = opt_arg(); break; + case OPT_NOSERVERNAME: + noservername = 1; + break; case OPT_USE_SRTP: srtp_profiles = opt_arg(); break; @@ -1399,6 +1405,20 @@ int s_client_main(int argc, char **argv) BIO_printf(bio_err, "%s: Can't use both -4 and -6\n", prog); goto opthelp; } + if (noservername) { + if (servername != NULL) { + BIO_printf(bio_err, + "%s: Can't use -servername and -noservername together\n", + prog); + goto opthelp; + } + if (dane_tlsa_domain != NULL) { + BIO_printf(bio_err, + "%s: Can't use -dane_tlsa_domain and -noservername together\n", + prog); + goto opthelp; + } + } argc = opt_num_rest(); if (argc != 0) goto opthelp; @@ -1720,7 +1740,7 @@ int s_client_main(int argc, char **argv) if (!set_cert_key_stuff(ctx, cert, key, chain, build_chain)) goto end; - if (servername != NULL) { + if (!noservername) { tlsextcbp.biodebug = bio_err; SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_cb); SSL_CTX_set_tlsext_servername_arg(ctx, &tlsextcbp); @@ -1793,7 +1813,9 @@ int s_client_main(int argc, char **argv) if (fallback_scsv) SSL_set_mode(con, SSL_MODE_SEND_FALLBACK_SCSV); - if (servername != NULL) { + if (!noservername && (servername != NULL || dane_tlsa_domain == NULL)) { + if (servername == NULL) + servername = (host == NULL) ? "localhost" : host; if (!SSL_set_tlsext_host_name(con, servername)) { BIO_printf(bio_err, "Unable to set TLS servername extension.\n"); ERR_print_errors(bio_err); @@ -2459,7 +2481,7 @@ int s_client_main(int argc, char **argv) if (in_init) { in_init = 0; - if (servername != NULL && !SSL_session_reused(con)) { + if (!noservername && !SSL_session_reused(con)) { BIO_printf(bio_c_out, "Server did %sacknowledge servername extension.\n", tlsextcbp.ack ? "" : "not "); diff --git a/doc/man1/s_client.pod b/doc/man1/s_client.pod index cc2ad4f1f2..16538567bc 100644 --- a/doc/man1/s_client.pod +++ b/doc/man1/s_client.pod @@ -14,6 +14,7 @@ B B [B<-4>] [B<-6>] [B<-servername name>] +[B<-noservername>] [B<-verify depth>] [B<-verify_return_error>] [B<-cert filename>] @@ -156,7 +157,17 @@ Use IPv6 only. =item B<-servername name> -Set the TLS SNI (Server Name Indication) extension in the ClientHello message. +Set the TLS SNI (Server Name Indication) extension in the ClientHello message to +the given value. + +=item B<-noservername> + +Suppresses sending of the SNI (Server Name Indication) extension in the +ClientHello message. Cannot be used in conjunction with the B<-servername> or +<-dane_tlsa_domain> options. If this option is not given then the hostname +provided to the B<-connect> option is used in the SNI extension, or "localhost" +if B<-connect> has not been supplied. Note that an SNI name should normally be a +DNS name and not an IP address. =item B<-cert certname> diff --git a/test/recipes/70-test_sslmessages.t b/test/recipes/70-test_sslmessages.t index a6278dc630..a763486f5b 100644 --- a/test/recipes/70-test_sslmessages.t +++ b/test/recipes/70-test_sslmessages.t @@ -222,22 +222,23 @@ checkhandshake($proxy, checkhandshake::RENEG_HANDSHAKE, checkhandshake::DEFAULT_EXTENSIONS, "Rengotiation handshake test"); -#Test 8: Server name handshake (client request only) +#Test 8: Server name handshake (no client request) $proxy->clear(); -$proxy->clientflags("-no_tls1_3 -servername testhost"); +$proxy->clientflags("-no_tls1_3 -noservername"); $proxy->start(); checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE, checkhandshake::DEFAULT_EXTENSIONS - | checkhandshake::SERVER_NAME_CLI_EXTENSION, + & ~checkhandshake::SERVER_NAME_CLI_EXTENSION, "Server name handshake test (client)"); #Test 9: Server name handshake (server support only) $proxy->clear(); -$proxy->clientflags("-no_tls1_3"); +$proxy->clientflags("-no_tls1_3 -noservername"); $proxy->serverflags("-servername testhost"); $proxy->start(); checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE, - checkhandshake::DEFAULT_EXTENSIONS, + checkhandshake::DEFAULT_EXTENSIONS + & ~checkhandshake::SERVER_NAME_CLI_EXTENSION, "Server name handshake test (server)"); #Test 10: Server name handshake (client and server) @@ -247,7 +248,6 @@ $proxy->serverflags("-servername testhost"); $proxy->start(); checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE, checkhandshake::DEFAULT_EXTENSIONS - | checkhandshake::SERVER_NAME_CLI_EXTENSION | checkhandshake::SERVER_NAME_SRV_EXTENSION, "Server name handshake test"); diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t index 0d25beffa9..c4e20b7614 100644 --- a/test/recipes/70-test_tls13messages.t +++ b/test/recipes/70-test_tls13messages.t @@ -200,21 +200,23 @@ checkhandshake($proxy, checkhandshake::CLIENT_AUTH_HANDSHAKE, checkhandshake::DEFAULT_EXTENSIONS, "Client auth handshake test"); -#Test 7: Server name handshake (client request only) +#Test 7: Server name handshake (no client request) $proxy->clear(); -$proxy->clientflags("-servername testhost"); +$proxy->clientflags("-noservername"); $proxy->start(); checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE, checkhandshake::DEFAULT_EXTENSIONS - | checkhandshake::SERVER_NAME_CLI_EXTENSION, + & ~checkhandshake::SERVER_NAME_CLI_EXTENSION, "Server name handshake test (client)"); #Test 8: Server name handshake (server support only) $proxy->clear(); +$proxy->clientflags("-noservername"); $proxy->serverflags("-servername testhost"); $proxy->start(); checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE, - checkhandshake::DEFAULT_EXTENSIONS, + checkhandshake::DEFAULT_EXTENSIONS + & ~checkhandshake::SERVER_NAME_CLI_EXTENSION, "Server name handshake test (server)"); #Test 9: Server name handshake (client and server) @@ -224,7 +226,6 @@ $proxy->serverflags("-servername testhost"); $proxy->start(); checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE, checkhandshake::DEFAULT_EXTENSIONS - | checkhandshake::SERVER_NAME_CLI_EXTENSION | checkhandshake::SERVER_NAME_SRV_EXTENSION, "Server name handshake test"); diff --git a/test/testlib/checkhandshake.pm b/test/testlib/checkhandshake.pm index 43efe81327..d5d0e29ee5 100644 --- a/test/testlib/checkhandshake.pm +++ b/test/testlib/checkhandshake.pm @@ -31,8 +31,8 @@ use constant { }; use constant { - #DEFAULT ALSO INCLUDES SESSION_TICKET_SRV_EXTENSION - DEFAULT_EXTENSIONS => 0x00000003, + #DEFAULT also includes SESSION_TICKET_SRV_EXTENSION and SERVER_NAME_CLI + DEFAULT_EXTENSIONS => 0x00000007, SESSION_TICKET_SRV_EXTENSION => 0x00000002, SERVER_NAME_CLI_EXTENSION => 0x00000004, SERVER_NAME_SRV_EXTENSION => 0x00000008, -- 2.25.1