From 8e981051ceecd10754f8f6d1291414a7453c8fac Mon Sep 17 00:00:00 2001 From: Ionut Mihalcea Date: Wed, 6 Feb 2019 21:09:15 +0000 Subject: [PATCH] Don't set SNI by default if hostname is not dNS name Reviewed-by: Ben Kaduk Reviewed-by: Viktor Dukhovni Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/8175) --- apps/s_client.c | 76 ++++++++++++++++++++++++++++++++++++++++--- doc/man1/s_client.pod | 19 ++++++----- 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/apps/s_client.c b/apps/s_client.c index a30dff44cf..687e755380 100644 --- 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 e16450b515..c4a98435b4 100644 --- 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> -- 2.25.1