Make httpd stricter with respect to TLS configuration - in particular, do

not allow TLS and non-TLS to be configured on the same port, do not allow
TLS options to be specified without a TLS listener and ensure that the TLS
options are the same when a server is specified on the same address/port.
Currently, these configurations are permitted but do not work as intended.

Also factor out and reuse the server matching code, which was previously
duplicated.

ok reyk@

diff --git a/usr.sbin/httpd/httpd.h b/usr.sbin/httpd/httpd.h
index aa9df94..cbe5dcb 100644 (file)
--- a/usr.sbin/httpd/httpd.h
+++ b/usr.sbin/httpd/httpd.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: httpd.h,v 1.104 2016/07/13 16:35:47 jsing Exp $       */
+/*     $OpenBSD: httpd.h,v 1.105 2016/08/15 13:48:24 jsing Exp $       */
 
 /*
  * Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -526,6 +526,7 @@ int  cmdline_symset(char *);
 
 /* server.c */
 pid_t   server(struct privsep *, struct privsep_proc *);
+int     server_tls_cmp(struct server *, struct server *);
 int     server_tls_load_keypair(struct server *);
 int     server_privinit(struct server *);
 void    server_purge(struct server *);

diff --git a/usr.sbin/httpd/parse.y b/usr.sbin/httpd/parse.y
index 2995f89..f7433f3 100644 (file)
--- a/usr.sbin/httpd/parse.y
+++ b/usr.sbin/httpd/parse.y
@@ -1,4 +1,4 @@
-/*     $OpenBSD: parse.y,v 1.78 2016/06/21 21:35:24 benno Exp $        */
+/*     $OpenBSD: parse.y,v 1.79 2016/08/15 13:48:24 jsing Exp $        */
 
 /*
  * Copyright (c) 2007 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -107,6 +107,7 @@ int          host_if(const char *, struct addresslist *,
 int             host(const char *, struct addresslist *,
                    int, struct portrange *, const char *, int);
 void            host_free(struct addresslist *);
+struct server  *server_match(struct server *, int);
 struct server  *server_inherit(struct server *, struct server_config *,
                    struct server_config *);
 int             getservice(char *);
@@ -283,24 +284,13 @@ server            : SERVER optmatch STRING        {
 
                        TAILQ_INSERT_TAIL(&srv->srv_hosts, srv_conf, entry);
                } '{' optnl serveropts_l '}'    {
-                       struct server           *s = NULL, *sn;
+                       struct server           *s, *sn;
                        struct server_config    *a, *b;
 
                        srv_conf = &srv->srv_conf;
 
-                       TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
-                               if ((s->srv_conf.flags &
-                                   SRVFLAG_LOCATION) == 0 &&
-                                   strcmp(s->srv_conf.name,
-                                   srv->srv_conf.name) == 0 &&
-                                   s->srv_conf.port == srv->srv_conf.port &&
-                                   sockaddr_cmp(
-                                   (struct sockaddr *)&s->srv_conf.ss,
-                                   (struct sockaddr *)&srv->srv_conf.ss,
-                                   s->srv_conf.prefixlen) == 0)
-                                       break;
-                       }
-                       if (s != NULL) {
+                       /* Check if the new server already exists. */
+                       if (server_match(srv, 1) != NULL) {
                                yyerror("server \"%s\" defined twice",
                                    srv->srv_conf.name);
                                serverconfig_free(srv_conf);
@@ -315,16 +305,39 @@ server            : SERVER optmatch STRING        {
                                YYERROR;
                        }
 
+                       if ((s = server_match(srv, 0)) != NULL) {
+                               if ((s->srv_conf.flags & SRVFLAG_TLS) != 
+                                   (srv->srv_conf.flags & SRVFLAG_TLS)) {
+                                       yyerror("server \"%s\": tls and "
+                                           "non-tls on same address/port",
+                                           srv->srv_conf.name);
+                                       serverconfig_free(srv_conf);
+                                       free(srv);
+                                       YYERROR;
+                               }
+                               if (server_tls_cmp(s, srv) != 0) {
+                                       yyerror("server \"%s\": tls "
+                                           "configuration mismatch on same "
+                                           "address/port",
+                                           srv->srv_conf.name);
+                                       serverconfig_free(srv_conf);
+                                       free(srv);
+                                       YYERROR;
+                               }
+                       }
+
                        if ((srv->srv_conf.flags & SRVFLAG_TLS) &&
                            srv->srv_conf.tls_protocols == 0) {
-                               yyerror("no TLS protocols");
+                               yyerror("server \"%s\": no tls protocols",
+                                   srv->srv_conf.name);
+                               serverconfig_free(srv_conf);
                                free(srv);
                                YYERROR;
                        }
 
                        if (server_tls_load_keypair(srv) == -1) {
-                               yyerror("failed to load public/private keys "
-                                   "for server %s", srv->srv_conf.name);
+                               yyerror("server \"%s\": failed to load "
+                                   "public/private keys", srv->srv_conf.name);
                                serverconfig_free(srv_conf);
                                free(srv);
                                YYERROR;
@@ -398,7 +411,7 @@ serveroptsl : LISTEN ON STRING opttls port {
                                    sizeof(*alias))) == NULL)
                                        fatal("out of memory");
 
-                               /* Add as an alias */
+                               /* Add as an IP-based alias. */
                                s_conf = alias;
                        } else
                                s_conf = &srv->srv_conf;
@@ -416,9 +429,8 @@ serveroptsl : LISTEN ON STRING opttls port {
                        s_conf->prefixlen = h->prefixlen;
                        host_free(&al);
 
-                       if ($4) {
+                       if ($4)
                                s_conf->flags |= SRVFLAG_TLS;
-                       }
 
                        if (alias != NULL) {
                                /* IP-based; use name match flags from parent */
@@ -468,10 +480,22 @@ serveroptsl       : LISTEN ON STRING opttls port {
                        }
                }
                | tls                   {
+                       struct server_config    *sc;
+                       int                      tls_flag = 0;
+
                        if (parentsrv != NULL) {
                                yyerror("tls configuration inside location");
                                YYERROR;
                        }
+
+                       /* Ensure that at least one server has TLS enabled. */
+                       TAILQ_FOREACH(sc, &srv->srv_hosts, entry) {
+                               tls_flag |= (sc->flags & SRVFLAG_TLS);
+                       }
+                       if (tls_flag == 0) {
+                               yyerror("tls options without tls listener");
+                               YYERROR;
+                       }
                }
                | root
                | directory
@@ -716,7 +740,7 @@ tlsopts             : CERTIFICATE STRING            {
                | PROTOCOLS STRING              {
                        if (tls_config_parse_protocols(
                            &srv_conf->tls_protocols, $2) != 0) {
-                               yyerror("invalid TLS protocols");
+                               yyerror("invalid tls protocols");
                                free($2);
                                YYERROR;
                        }
@@ -1967,6 +1991,33 @@ host_free(struct addresslist *al)
        }
 }
 
+struct server *
+server_match(struct server *s2, int match_name)
+{
+       struct server   *s1;
+
+       /* Attempt to find matching server. */
+       TAILQ_FOREACH(s1, conf->sc_servers, srv_entry) {
+               if ((s1->srv_conf.flags & SRVFLAG_LOCATION) != 0)
+                       continue;
+               if (match_name) {
+                       if (strcmp(s1->srv_conf.name, s2->srv_conf.name) != 0)
+                               continue;
+               }
+               if (s1->srv_conf.port != s2->srv_conf.port)
+                       continue;
+               if (sockaddr_cmp(
+                   (struct sockaddr *)&s1->srv_conf.ss,
+                   (struct sockaddr *)&s2->srv_conf.ss,
+                   s1->srv_conf.prefixlen) != 0)
+                       continue;
+
+               return (s1);
+       }
+
+       return (NULL);
+}
+
 struct server *
 server_inherit(struct server *src, struct server_config *alias,
     struct server_config *addr)
@@ -2028,19 +2079,7 @@ server_inherit(struct server *src, struct server_config *alias,
        }
 
        /* Check if the new server already exists */
-       TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
-               if ((s->srv_conf.flags &
-                   SRVFLAG_LOCATION) == 0 &&
-                   strcmp(s->srv_conf.name,
-                   dst->srv_conf.name) == 0 &&
-                   s->srv_conf.port == dst->srv_conf.port &&
-                   sockaddr_cmp(
-                   (struct sockaddr *)&s->srv_conf.ss,
-                   (struct sockaddr *)&dst->srv_conf.ss,
-                   s->srv_conf.prefixlen) == 0)
-                       break;
-       }
-       if (s != NULL) {
+       if (server_match(dst, 1) != NULL) {
                yyerror("server \"%s\" defined twice",
                    dst->srv_conf.name);
                serverconfig_free(&dst->srv_conf);

diff --git a/usr.sbin/httpd/server.c b/usr.sbin/httpd/server.c
index db3b16d..7404d49 100644 (file)
--- a/usr.sbin/httpd/server.c
+++ b/usr.sbin/httpd/server.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: server.c,v 1.85 2016/04/28 17:18:06 jsing Exp $       */
+/*     $OpenBSD: server.c,v 1.86 2016/08/15 13:48:24 jsing Exp $       */
 
 /*
  * Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -132,6 +132,30 @@ server_privinit(struct server *srv)
        return (0);
 }
 
+int
+server_tls_cmp(struct server *s1, struct server *s2)
+{
+       struct server_config    *sc1, *sc2;
+
+       sc1 = &s1->srv_conf;
+       sc2 = &s2->srv_conf;
+
+       if (sc1->tls_protocols != sc2->tls_protocols)
+               return (-1);
+       if (strcmp(sc1->tls_cert_file, sc2->tls_cert_file) != 0)
+               return (-1);
+       if (strcmp(sc1->tls_key_file, sc2->tls_key_file) != 0)
+               return (-1);
+       if (strcmp(sc1->tls_ciphers, sc2->tls_ciphers) != 0)
+               return (-1);
+       if (strcmp(sc1->tls_dhe_params, sc2->tls_dhe_params) != 0)
+               return (-1);
+       if (strcmp(sc1->tls_ecdhe_curve, sc2->tls_ecdhe_curve) != 0)
+               return (-1);
+
+       return (0);
+}
+
 int
 server_tls_load_keypair(struct server *srv)
 {