On Sat, Jan 16, 2016 at 09:42:18AM -0800, Claus Assmann wrote:
> While playing around with the DANE suppport in OpenSSL 1.1
Thanks for the test drive.
> I noticed that the TLS handshake will fail if I specify an empty name:
> SSL_dane_enable(ssl, "")
Good catch.
> (AFAICT no name is needed for DANE-TA(2) RRs).
DANE requires the client to send SNI in all cases. The server will
often ignore the SNI name if it has just one certificate.
> This can also be reproduced using
> openssl s_client -servername "" ...
Please try the two attached patches.
--
Viktor.
>From c9f8d6dc5c37988ec5c721356b3e287a6d491c55 Mon Sep 17 00:00:00 2001
From: Viktor Dukhovni <openssl-us...@dukhovni.org>
Date: Sat, 16 Jan 2016 12:57:24 -0500
Subject: [PATCH 1/2] Empty SNI names are not valid
While empty inputs to SSL_set1_host() clear the reference identifier
list.
---
crypto/x509/x509_vpm.c | 4 ++--
ssl/s3_lib.c | 5 ++++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/crypto/x509/x509_vpm.c b/crypto/x509/x509_vpm.c
index 827360d..b596d84 100644
--- a/crypto/x509/x509_vpm.c
+++ b/crypto/x509/x509_vpm.c
@@ -92,11 +92,11 @@ static int int_x509_param_set_hosts(X509_VERIFY_PARAM *vpm,
int mode,
* Refuse names with embedded NUL bytes, except perhaps as final byte.
* XXX: Do we need to push an error onto the error stack?
*/
- if (namelen == 0)
+ if (namelen == 0 || name == NULL)
namelen = name ? strlen(name) : 0;
else if (name && memchr(name, '\0', namelen > 1 ? namelen - 1 : namelen))
return 0;
- if (name && name[namelen - 1] == '\0')
+ if (name != NULL && name[namelen - 1] == '\0')
--namelen;
if (mode == SET_HOST) {
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index de8dae2..54b8eba 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3534,13 +3534,16 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
#endif /* !OPENSSL_NO_EC */
case SSL_CTRL_SET_TLSEXT_HOSTNAME:
if (larg == TLSEXT_NAMETYPE_host_name) {
+ size_t len;
+
OPENSSL_free(s->tlsext_hostname);
s->tlsext_hostname = NULL;
ret = 1;
if (parg == NULL)
break;
- if (strlen((char *)parg) > TLSEXT_MAXLEN_host_name) {
+ len = strlen((char *)parg);
+ if (len == 0 || len > TLSEXT_MAXLEN_host_name) {
SSLerr(SSL_F_SSL3_CTRL, SSL_R_SSL3_EXT_INVALID_SERVERNAME);
return 0;
}
--
2.5.4 (Apple Git-61)
>From 8fc1c14db18d000437664d81e4e250d44179bdc0 Mon Sep 17 00:00:00 2001
From: Viktor Dukhovni <openssl-us...@dukhovni.org>
Date: Sat, 16 Jan 2016 13:25:17 -0500
Subject: [PATCH 2/2] Better invalid SNI name error handling
Also report an SSL_dane_enable error when the basedomain is an
invalid SNI name. Avoid side-effects when such a name is valid
with X509_VERIFY_PARAM_set1_host(), as e.g. with an empty name, by
setting the SNI name first.
---
ssl/ssl_lib.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 90de747..e922e3f 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -872,18 +872,24 @@ int SSL_dane_enable(SSL *s, const char *basedomain)
return 0;
}
+ /*
+ * Default SNI name. This rejects empty names, while set1_host below
+ * accepts them and disables host name checks. To avoid side-effects with
+ * invalid input, set the SNI name first.
+ */
+ if (s->tlsext_hostname == NULL) {
+ if (!SSL_set_tlsext_host_name(s, basedomain)) {
+ SSLerr(SSL_F_SSL_DANE_ENABLE,
SSL_R_ERROR_SETTING_TLSA_BASE_DOMAIN);
+ return -1;
+ }
+ }
+
/* Primary RFC6125 reference identifier */
if (!X509_VERIFY_PARAM_set1_host(s->param, basedomain, 0)) {
SSLerr(SSL_F_SSL_DANE_ENABLE, SSL_R_ERROR_SETTING_TLSA_BASE_DOMAIN);
return -1;
}
- /* Default SNI name */
- if (s->tlsext_hostname == NULL) {
- if (!SSL_set_tlsext_host_name(s, basedomain))
- return -1;
- }
-
dane->mdpth = -1;
dane->pdpth = -1;
dane->dctx = &s->ctx->dane;
--
2.5.4 (Apple Git-61)
_______________________________________________
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
