Don't set SNI by default if hostname is not dNS name

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8175)

diff --git a/apps/s_client.c b/apps/s_client.c
index a30dff44cfc22452c2d9f45fc61b176f797e11f3..687e75538036f780f9857e0329cb9d2c31a9b371 100644 (file)
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
  * Copyright 2005 Nokia. All rights reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
@@ -75,6 +75,7 @@ static int ocsp_resp_cb(SSL *s, void *arg);
 #endif
 static int ldap_ExtendedResponse_parse(const char *buf, long rem);
 static char *base64encode (const void *buf, size_t len);
+static int is_dNS_name(const char *host);
 
 static int saved_errno;
 
@@ -2013,9 +2014,11 @@ int s_client_main(int argc, char **argv)
         SSL_set_mode(con, SSL_MODE_SEND_FALLBACK_SCSV);
 
     if (!noservername && (servername != NULL || dane_tlsa_domain == NULL)) {
-        if (servername == NULL)
-            servername = (host == NULL) ? "localhost" : host;
-        if (!SSL_set_tlsext_host_name(con, servername)) {
+        if (servername == NULL) {
+            if(host == NULL || is_dNS_name(host)) 
+                servername = (host == NULL) ? "localhost" : host;
+        }
+        if (servername != NULL && !SSL_set_tlsext_host_name(con, servername)) {
             BIO_printf(bio_err, "Unable to set TLS servername extension.\n");
             ERR_print_errors(bio_err);
             goto end;
@@ -3554,4 +3557,69 @@ static char *base64encode (const void *buf, size_t len)
     return out;
 }
 
+/*
+ * Host dNS Name verifier: used for checking that the hostname is in dNS format 
+ * before setting it as SNI
+ */
+static int is_dNS_name(const char *host)
+{
+    const size_t MAX_LABEL_LENGTH = 63;
+    size_t i;
+    int isdnsname = 0;
+    size_t length = strlen(host);
+    size_t label_length = 0;
+    int all_numeric = 1;
+
+    /*
+     * Deviation from strict DNS name syntax, also check names with '_'
+     * Check DNS name syntax, any '-' or '.' must be internal,
+     * and on either side of each '.' we can't have a '-' or '.'.
+     *
+     * If the name has just one label, we don't consider it a DNS name.
+     */
+    for (i = 0; i < length && label_length < MAX_LABEL_LENGTH; ++i) {
+        char c = host[i];
+
+        if ((c >= 'a' && c <= 'z')
+            || (c >= 'A' && c <= 'Z')
+            || c == '_') {
+            label_length += 1;
+            all_numeric = 0;
+            continue;
+        }
+
+        if (c >= '0' && c <= '9') {
+            label_length += 1;
+            continue;
+        }
+
+        /* Dot and hyphen cannot be first or last. */
+        if (i > 0 && i < length - 1) {
+            if (c == '-') {
+                label_length += 1;
+                continue;
+            }
+            /*
+             * Next to a dot the preceding and following characters must not be
+             * another dot or a hyphen.  Otherwise, record that the name is
+             * plausible, since it has two or more labels.
+             */
+            if (c == '.'
+                && host[i + 1] != '.'
+                && host[i - 1] != '-'
+                && host[i + 1] != '-') {
+                label_length = 0;
+                isdnsname = 1;
+                continue;
+            }
+        }
+        isdnsname = 0;
+        break;
+    }
+
+    /* dNS name must not be all numeric and labels must be shorter than 64 characters. */
+    isdnsname &= !all_numeric && !(label_length == MAX_LABEL_LENGTH);
+
+    return isdnsname;
+}
 #endif                          /* OPENSSL_NO_SOCK */

diff --git a/doc/man1/s_client.pod b/doc/man1/s_client.pod
index e16450b515d8b689cd5c9f6afa103a19f50b9a4f..c4a98435b4f97c490a57599b414ac12c379f37cf 100644 (file)
--- a/doc/man1/s_client.pod
+++ b/doc/man1/s_client.pod
@@ -208,14 +208,17 @@ Use IPv6 only.
 =item B<-servername name>
 
 Set the TLS SNI (Server Name Indication) extension in the ClientHello message to
-the given value. If both this option and the B<-noservername> are not given, the
-TLS SNI extension is still set to the hostname provided to the B<-connect> option,
-or "localhost" if B<-connect> has not been supplied. This is default since OpenSSL
-1.1.1.
-
-Even though SNI name should normally be a DNS name and not an IP address, this
-option will not make the distinction when parsing B<-connect> and will send
-IP address if one passed.
+the given value. 
+If B<-servername> is not provided, the TLS SNI extension will be populated with 
+the name given to B<-connect> if it follows a DNS name format. If B<-connect> is 
+not provided either, the SNI is set to "localhost".
+This is the default since OpenSSL 1.1.1.
+
+Even though SNI should normally be a DNS name and not an IP address, if 
+B<-servername> is provided then that name will be sent, regardless of whether 
+it is a DNS name or not.
+
+This option cannot be used in conjuction with B<-noservername>.
 
 =item B<-noservername>