From f819d841198c5bce66e0a7f080decb033efcf610 Mon Sep 17 00:00:00 2001
From: jsing <>
Date: Sat, 21 May 2016 13:46:10 +0000
Subject: [PATCH] Harden TLS for ntpd constraints - stop disabling server name
 verification, ensure that we load the CA certificates and use
 tls_connect_servername() so that we can verify the server we are connecting
 to (even though we've already resolved the hostname). Also add additional
 warnings for TLS connect and TLS write failures so that we know what is
 happening and why.

Lack of server name verification also reported by Luis M. Merino
<luismiguelmerino at gmail dot com> - thanks!

ok deraadt@ reyk@
---
 src/usr.sbin/ntpd/constraint.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/usr.sbin/ntpd/constraint.c b/src/usr.sbin/ntpd/constraint.c
index b6ba364e..5bf8c4ea 100644
--- a/src/usr.sbin/ntpd/constraint.c
+++ b/src/usr.sbin/ntpd/constraint.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: constraint.c,v 1.27 2016/05/06 16:49:46 jsing Exp $	*/
+/*	$OpenBSD: constraint.c,v 1.28 2016/05/21 13:46:10 jsing Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -292,12 +292,12 @@ priv_constraint_child(struct constraint *cstr, struct ntp_addr_msg *am,
 	if (setpriority(PRIO_PROCESS, 0, 0) == -1)
 		log_warn("could not set priority");
 
-	/* Init TLS and load cert before chroot() */
+	/* Init TLS and load CA certs before chroot() */
 	if (tls_init() == -1)
 		fatalx("tls_init");
 	if ((conf->ca = tls_load_file(CONSTRAINT_CA,
 	    &conf->ca_len, NULL)) == NULL)
-		log_warnx("constraint certificate verification turned off");
+		fatalx("failed to load constraint ca");
 
 	if (chroot(pw_dir) == -1)
 		fatal("chroot");
@@ -821,9 +821,6 @@ httpsdate_init(const char *addr, const char *port, const char *hostname,
 	if (tls_config_set_ciphers(httpsdate->tls_config, "compat") != 0)
 		goto fail;
 
-	/* XXX we have to pre-resolve, so name and host are not equal */
-	tls_config_insecure_noverifyname(httpsdate->tls_config);
-
 	if (ca == NULL || ca_len == 0)
 		tls_config_insecure_noverifycert(httpsdate->tls_config);
 	else
@@ -867,9 +864,10 @@ httpsdate_request(struct httpsdate *httpsdate, struct timeval *when)
 	if (tls_configure(httpsdate->tls_ctx, httpsdate->tls_config) == -1)
 		goto fail;
 
-	if (tls_connect(httpsdate->tls_ctx,
-	    httpsdate->tls_addr, httpsdate->tls_port) == -1) {
-		log_debug("tls failed: %s: %s", httpsdate->tls_addr,
+	if (tls_connect_servername(httpsdate->tls_ctx, httpsdate->tls_addr,
+	    httpsdate->tls_port, httpsdate->tls_hostname) == -1) {
+		log_warnx("tls connect failed: %s (%s): %s",
+		    httpsdate->tls_addr, httpsdate->tls_hostname,
 		    tls_error(httpsdate->tls_ctx));
 		goto fail;
 	}
@@ -880,8 +878,12 @@ httpsdate_request(struct httpsdate *httpsdate, struct timeval *when)
 		ret = tls_write(httpsdate->tls_ctx, buf, len);
 		if (ret == TLS_WANT_POLLIN || ret == TLS_WANT_POLLOUT)
 			continue;
-		if (ret < 0)
+		if (ret < 0) {
+			log_warnx("tls write failed: %s (%s): %s",
+			    httpsdate->tls_addr, httpsdate->tls_hostname,
+			    tls_error(httpsdate->tls_ctx));
 			goto fail;
+		}
 		buf += ret;
 		len -= ret;
 	}
@@ -916,8 +918,8 @@ httpsdate_request(struct httpsdate *httpsdate, struct timeval *when)
 		free(line);
 	}
 
-
 	return (0);
+
  fail:
 	httpsdate_free(httpsdate);
 	return (-1);