5 files changed, 76 insertions, 47 deletions

diff --git a/src/lib/libtls/tls_client.c b/src/lib/libtls/tls_client.c
index a1e2caa717..0e519684ef 100644
--- a/src/lib/libtls/tls_client.c
+++ b/src/lib/libtls/tls_client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_client.c,v 1.40 2017/01/26 12:56:37 jsing Exp $ */
+/* $OpenBSD: tls_client.c,v 1.41 2017/04/10 17:11:13 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -289,7 +289,7 @@ int
 tls_handshake_client(struct tls *ctx)
 {
         X509 *cert = NULL;
-        int ssl_ret;
+        int match, ssl_ret;
         int rv = -1;
 
         if ((ctx->flags & TLS_CLIENT) == 0) {
@@ -311,11 +311,11 @@ tls_handshake_client(struct tls *ctx)
                         tls_set_errorx(ctx, "no server certificate");
                         goto err;
                 }
-                if ((rv = tls_check_name(ctx, cert,
-                    ctx->servername)) != 0) {
-                        if (rv != -2)
-                                tls_set_errorx(ctx, "name `%s' not present in"
-                                    " server certificate", ctx->servername);
+                if (tls_check_name(ctx, cert, ctx->servername, &match) == -1)
+                        goto err;
+                if (!match) {
+                        tls_set_errorx(ctx, "name `%s' not present in"
+                            " server certificate", ctx->servername);
                         goto err;
                 }
         }
diff --git a/src/lib/libtls/tls_internal.h b/src/lib/libtls/tls_internal.h
index 7bbc14ca86..bd23249e57 100644
--- a/src/lib/libtls/tls_internal.h
+++ b/src/lib/libtls/tls_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_internal.h,v 1.56 2017/04/07 08:48:30 jsing Exp $ */
+/* $OpenBSD: tls_internal.h,v 1.57 2017/04/10 17:11:13 jsing Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
@@ -186,7 +186,8 @@ void tls_sni_ctx_free(struct tls_sni_ctx *sni_ctx);
 struct tls *tls_new(void);
 struct tls *tls_server_conn(struct tls *ctx);
 
-int tls_check_name(struct tls *ctx, X509 *cert, const char *servername);
+int tls_check_name(struct tls *ctx, X509 *cert, const char *servername,
+    int *match);
 int tls_configure_server(struct tls *ctx);
 
 int tls_configure_ssl(struct tls *ctx, SSL_CTX *ssl_ctx);
diff --git a/src/lib/libtls/tls_peer.c b/src/lib/libtls/tls_peer.c
index 1a9065dfb1..ec97a30838 100644
--- a/src/lib/libtls/tls_peer.c
+++ b/src/lib/libtls/tls_peer.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_peer.c,v 1.7 2017/04/05 03:19:22 beck Exp $ */
+/* $OpenBSD: tls_peer.c,v 1.8 2017/04/10 17:11:13 jsing Exp $ */
 /*
  * Copyright (c) 2015 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2015 Bob Beck <beck@openbsd.org>
@@ -55,10 +55,15 @@ tls_peer_cert_provided(struct tls *ctx)
 int
 tls_peer_cert_contains_name(struct tls *ctx, const char *name)
 {
+        int match;
+
         if (ctx->ssl_peer_cert == NULL)
                 return (0);
 
-        return (tls_check_name(ctx, ctx->ssl_peer_cert, name) == 0);
+        if (tls_check_name(ctx, ctx->ssl_peer_cert, name, &match) == -1)
+                return (0);
+
+        return (match);
 }
 
 time_t
diff --git a/src/lib/libtls/tls_server.c b/src/lib/libtls/tls_server.c
index 51deff2510..39c6ca79e9 100644
--- a/src/lib/libtls/tls_server.c
+++ b/src/lib/libtls/tls_server.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_server.c,v 1.35 2017/01/31 15:57:43 jsing Exp $ */
+/* $OpenBSD: tls_server.c,v 1.36 2017/04/10 17:11:13 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -75,11 +75,13 @@ tls_servername_cb(SSL *ssl, int *al, void *arg)
         union tls_addr addrbuf;
         struct tls *conn_ctx;
         const char *name;
+        int match;
 
         if ((conn_ctx = SSL_get_app_data(ssl)) == NULL)
                 goto err;
 
-        if ((name = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name)) == NULL) {
+        if ((name = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name)) ==
+            NULL) {
                 /*
                  * The servername callback gets called even when there is no
                  * TLS servername extension provided by the client. Sigh!
@@ -98,7 +100,10 @@ tls_servername_cb(SSL *ssl, int *al, void *arg)
 
         /* Find appropriate SSL context for requested servername. */
         for (sni_ctx = ctx->sni_ctx; sni_ctx != NULL; sni_ctx = sni_ctx->next) {
-                if (tls_check_name(ctx, sni_ctx->ssl_cert, name) == 0) {
+                if (tls_check_name(ctx, sni_ctx->ssl_cert, name,
+                    &match) == -1)
+                        goto err;
+                if (match) {
                         SSL_set_SSL_CTX(conn_ctx->ssl_conn, sni_ctx->ssl_ctx);
                         return (SSL_TLSEXT_ERR_OK);
                 }
diff --git a/src/lib/libtls/tls_verify.c b/src/lib/libtls/tls_verify.c
index 23e58ebef7..3bd1057d0c 100644
--- a/src/lib/libtls/tls_verify.c
+++ b/src/lib/libtls/tls_verify.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_verify.c,v 1.18 2016/11/04 15:32:40 jsing Exp $ */
+/* $OpenBSD: tls_verify.c,v 1.19 2017/04/10 17:11:13 jsing Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  *
@@ -27,11 +27,6 @@
 #include <tls.h>
 #include "tls_internal.h"
 
-static int tls_match_name(const char *cert_name, const char *name);
-static int tls_check_subject_altname(struct tls *ctx, X509 *cert,
-    const char *name);
-static int tls_check_common_name(struct tls *ctx, X509 *cert, const char *name);
-
 static int
 tls_match_name(const char *cert_name, const char *name)
 {
@@ -84,20 +79,28 @@ tls_match_name(const char *cert_name, const char *name)
         return -1;
 }
 
-/* See RFC 5280 section 4.2.1.6 for SubjectAltName details. */
+/*
+ * See RFC 5280 section 4.2.1.6 for SubjectAltName details.
+ * alt_match is set to 1 if a matching alternate name is found.
+ * alt_exists is set to 1 if any known alternate name exists in the certificate.
+ */
 static int
-tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name)
+tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name,
+    int *alt_match, int *alt_exists)
 {
         STACK_OF(GENERAL_NAME) *altname_stack = NULL;
         union tls_addr addrbuf;
         int addrlen, type;
         int count, i;
-        int rv = -1;
+        int rv = 0;
+
+        *alt_match = 0;
+        *alt_exists = 0;
 
         altname_stack = X509_get_ext_d2i(cert, NID_subject_alt_name,
             NULL, NULL);
         if (altname_stack == NULL)
-                return -1;
+                return 0;
 
         if (inet_pton(AF_INET, name, &addrbuf) == 1) {
                 type = GEN_IPADD;
@@ -115,6 +118,10 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name)
                 GENERAL_NAME    *altname;
 
                 altname = sk_GENERAL_NAME_value(altname_stack, i);
+
+                if (altname->type == GEN_DNS || altname->type == GEN_IPADD)
+                        *alt_exists = 1;
+
                 if (altname->type != type)
                         continue;
 
@@ -133,7 +140,7 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name)
                                             "NUL byte in subjectAltName, "
                                             "probably a malicious certificate",
                                             name);
-                                        rv = -2;
+                                        rv = -1;
                                         break;
                                 }
 
@@ -143,16 +150,16 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name)
                                  * dNSName must be rejected.
                                  */
                                 if (strcmp(data, " ") == 0) {
-                                        tls_set_error(ctx,
+                                        tls_set_errorx(ctx,
                                             "error verifying name '%s': "
                                             "a dNSName of \" \" must not be "
                                             "used", name);
-                                        rv = -2;
+                                        rv = -1;
                                         break;
                                 }
 
                                 if (tls_match_name(data, name) == 0) {
-                                        rv = 0;
+                                        *alt_match = 1;
                                         break;
                                 }
                         } else {
@@ -174,7 +181,7 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name)
                                 tls_set_errorx(ctx,
                                     "Unexpected negative length for an "
                                     "IP address: %d", datalen);
-                                rv = -2;
+                                rv = -1;
                                 break;
                         }
 
@@ -184,7 +191,7 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name)
                          */
                         if (datalen == addrlen &&
                             memcmp(data, &addrbuf, addrlen) == 0) {
-                                rv = 0;
+                                *alt_match = 1;
                                 break;
                         }
                 }
@@ -195,13 +202,16 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name)
 }
 
 static int
-tls_check_common_name(struct tls *ctx, X509 *cert, const char *name)
+tls_check_common_name(struct tls *ctx, X509 *cert, const char *name,
+    int *cn_match)
 {
         X509_NAME *subject_name;
         char *common_name = NULL;
         union tls_addr addrbuf;
         int common_name_len;
-        int rv = -1;
+        int rv = 0;
+
+        *cn_match = 0;
 
         subject_name = X509_get_subject_name(cert);
         if (subject_name == NULL)
@@ -225,38 +235,46 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *name)
                 tls_set_errorx(ctx, "error verifying name '%s': "
                     "NUL byte in Common Name field, "
                     "probably a malicious certificate", name);
-                rv = -2;
+                rv = -1;
                 goto out;
         }
 
+        /*
+         * We don't want to attempt wildcard matching against IP addresses,
+         * so perform a simple comparison here.
+         */
         if (inet_pton(AF_INET,  name, &addrbuf) == 1 ||
             inet_pton(AF_INET6, name, &addrbuf) == 1) {
-                /*
-                 * We don't want to attempt wildcard matching against IP
-                 * addresses, so perform a simple comparison here.
-                 */
                 if (strcmp(common_name, name) == 0)
-                        rv = 0;
-                else
-                        rv = -1;
+                        *cn_match = 1;
                 goto out;
         }
 
         if (tls_match_name(common_name, name) == 0)
-                rv = 0;
+                *cn_match = 1;
+
  out:
         free(common_name);
         return rv;
 }
 
 int
-tls_check_name(struct tls *ctx, X509 *cert, const char *name)
+tls_check_name(struct tls *ctx, X509 *cert, const char *name, int *match)
 {
-        int     rv;
+        int alt_exists;
 
-        rv = tls_check_subject_altname(ctx, cert, name);
-        if (rv == 0 || rv == -2)
-                return rv;
+        *match = 0;
+
+        if (tls_check_subject_altname(ctx, cert, name, match,
+            &alt_exists) == -1)
+                return -1;
+
+        /*
+         * As per RFC 6125 section 6.4.4, if any known alternate name existed
+         * in the certificate, we do not attempt to match on the CN.
+         */
+        if (*match || alt_exists)
+                return 0;
 
-        return tls_check_common_name(ctx, cert, name);
+        return tls_check_common_name(ctx, cert, name, match);
 }