[PATCH RFC 0/5] Address security review comments

[PATCH RFC 0/5] Address security review comments

[Chuck Lever]

I requested some internal code-practices review of ktls-utils, and
got the results last week. These patches attempt to address the
concerns that were raised by the review. We can discuss these during
tomorrow's meeting, or here on the list.

---

Chuck Lever (5):
      tlshd: Check mode bit settings on certificate material
      tlshd: Fix client's use of Server Name Indication
      tlshd: Pass tlshd_parameters to the verification functions
      tlshd: Fix server-side peer hostname validation
      tlshd: Disable Nagle during handshakes on TCP sockets


 src/tlshd/client.c       | 67 ++++++++++++++++++++++++---------------
 src/tlshd/config.c       | 67 +++++++++++++++++++++++++++++++++------
 src/tlshd/handshake.c    | 30 ++++++++++++++++++
 src/tlshd/log.c          | 14 +++++++++
 src/tlshd/server.c       | 68 ++++++++++++++++++++++++++--------------
 src/tlshd/tlshd.conf     |  1 +
 src/tlshd/tlshd.conf.man | 31 ++++++++++++++++--
 src/tlshd/tlshd.h        |  4 ++-
 8 files changed, 221 insertions(+), 61 deletions(-)

--
Chuck Lever

[PATCH RFC 1/5] tlshd: Check mode bit settings on certificate material

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Log a conventional warning if certificate material is not permitted
defensively.

Suggested-by: John Haxby <john.haxby@oracle.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/tlshd/config.c |   36 +++++++++++++++++++++++++++---------
 src/tlshd/log.c    |   14 ++++++++++++++
 src/tlshd/tlshd.h  |    1 +
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/src/tlshd/config.c b/src/tlshd/config.c
index fa2da64bfbdd..464635ed56e0 100644
--- a/src/tlshd/config.c
+++ b/src/tlshd/config.c
@@ -123,10 +123,18 @@ static int tlshd_file_open(const char *pathname)
 }
 #endif
 
+/*
+ * Expected file attributes
+ */
+#define TLSHD_OWNER		0	/* root */
+#define TLSHD_CERT_MODE		(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)
+#define TLSHD_PRIVKEY_MODE	(S_IRUSR|S_IWUSR)
+
 /*
  * On success, caller must release buffer returned in @data by calling free(3)
  */
-static bool tlshd_config_read_datum(const char *pathname, gnutls_datum_t *data)
+static bool tlshd_config_read_datum(const char *pathname, gnutls_datum_t *data,
+				    uid_t owner, mode_t mode)
 {
 	struct stat statbuf;
 	void *buf;
@@ -137,21 +145,27 @@ static bool tlshd_config_read_datum(const char *pathname, gnutls_datum_t *data)
 
 	fd = tlshd_file_open(pathname);
 	if (fd == -1) {
-		tlshd_log_perror("Failed to open file");
+		tlshd_log_perror("open");
 		goto out;
 	}
 	if (fstat(fd, &statbuf)) {
-		tlshd_log_perror("Failed to stat file");
+		tlshd_log_perror("stat");
 		goto out_close;
 	}
+	if (statbuf.st_uid != owner)
+		tlshd_log_notice("File %s: expected owner %u",
+				 pathname, owner);
+	if ((statbuf.st_mode & ALLPERMS) != mode)
+		tlshd_log_notice("File %s: expected mode %o",
+				 pathname, mode);
 	buf = malloc(statbuf.st_size);
 	if (!buf) {
 		errno = ENOMEM;
-		tlshd_log_perror("Failed to allocate buffer");
+		tlshd_log_perror("malloc");
 		goto out_close;
 	}
 	if (read(fd, buf, statbuf.st_size) == -1) {
-		tlshd_log_perror("Failed to read file");
+		tlshd_log_perror("read");
 		free(buf);
 		goto out_close;
 	}
@@ -188,7 +202,8 @@ bool tlshd_config_get_client_cert(gnutls_pcert_st *cert)
 		return false;
 	}
 
-	if (!tlshd_config_read_datum(pathname, &data))
+	if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER,
+				     TLSHD_CERT_MODE))
 		return false;
 
 	/* Config file supports only PEM-encoded certificates */
@@ -227,7 +242,8 @@ bool tlshd_config_get_client_privkey(gnutls_privkey_t *privkey)
 		return false;
 	}
 
-	if (!tlshd_config_read_datum(pathname, &data))
+	if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER,
+				     TLSHD_PRIVKEY_MODE))
 		return false;
 
 	ret = gnutls_privkey_init(privkey);
@@ -273,7 +289,8 @@ bool tlshd_config_get_server_cert(gnutls_pcert_st *cert)
 		return false;
 	}
 
-	if (!tlshd_config_read_datum(pathname, &data))
+	if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER,
+				     TLSHD_CERT_MODE))
 		return false;
 
 	/* Config file supports only PEM-encoded certificates */
@@ -312,7 +329,8 @@ bool tlshd_config_get_server_privkey(gnutls_privkey_t *privkey)
 		return false;
 	}
 
-	if (!tlshd_config_read_datum(pathname, &data))
+	if (!tlshd_config_read_datum(pathname, &data, TLSHD_OWNER,
+				     TLSHD_PRIVKEY_MODE))
 		return false;
 
 	ret = gnutls_privkey_init(privkey);
diff --git a/src/tlshd/log.c b/src/tlshd/log.c
index 01e248660607..3594d529ad74 100644
--- a/src/tlshd/log.c
+++ b/src/tlshd/log.c
@@ -112,6 +112,20 @@ void tlshd_log_error(const char *fmt, ...)
 	va_end(args);
 }
 
+/**
+ * tlshd_log_notice - Emit a generic warning
+ * @fmt - printf-style format string
+ *
+ */
+void tlshd_log_notice(const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	vsyslog(LOG_NOTICE, fmt, args);
+	va_end(args);
+}
+
 /**
  * tlshd_log_perror - Emit "system call failed" notification
  * @sap: remote address to log
diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h
index f598beeb65fd..d8cb3c4b4baa 100644
--- a/src/tlshd/tlshd.h
+++ b/src/tlshd/tlshd.h
@@ -87,6 +87,7 @@ extern void tlshd_log_failure(const char *hostname,
 			      const struct sockaddr *sap, socklen_t salen);
 
 extern void tlshd_log_debug(const char *fmt, ...);
+extern void tlshd_log_notice(const char *fmt, ...);
 extern void tlshd_log_error(const char *fmt, ...);
 extern void tlshd_log_perror(const char *prefix);
 extern void tlshd_log_gai_error(int error);

[PATCH RFC 2/5] tlshd: Fix client's use of Server Name Indication

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

The man page says gnutls_server_name_set(3) is used to send the
local hostname to the remote peer as part of ClientHello. It appears
that tlshd is incorrectly sending the remote's hostname instead.

To ensure that an appropriate hostname is used, enable the client's
admin to set the client's hostname in /etc/tlshd.conf. If one isn't
set there, the result of gethostname(3) is used instead. The SNI
hostname needs to match one of the Subject names in the provided
client certificate.

I'll note that RFC 9289 should RECOMMEND or REQUIRE the use of SNI,
but it currently does not.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/tlshd/client.c       |   32 +++++++++++++++++++++++++++-----
 src/tlshd/config.c       |   31 +++++++++++++++++++++++++++++++
 src/tlshd/tlshd.conf     |    1 +
 src/tlshd/tlshd.conf.man |   31 +++++++++++++++++++++++++++++--
 src/tlshd/tlshd.h        |    1 +
 5 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/src/tlshd/client.c b/src/tlshd/client.c
index f40e19882827..46a973f4393c 100644
--- a/src/tlshd/client.c
+++ b/src/tlshd/client.c
@@ -31,6 +31,7 @@
 #include <stdio.h>
 #include <errno.h>
 #include <keyutils.h>
+#include <netdb.h>
 
 #include <gnutls/gnutls.h>
 #include <gnutls/abstract.h>
@@ -45,6 +46,30 @@
 static unsigned int tlshd_num_remote_peerids;
 static key_serial_t tlshd_remote_peerid[10];
 
+static void tlshd_client_set_sni_hostname(gnutls_session_t session)
+{
+	char buf[NI_MAXHOST];
+	size_t len = sizeof(buf);
+	int ret;
+
+	if (!tlshd_config_get_sni_hostname(buf, &len)) {
+		ret = gethostname(buf, len);
+		if (ret < 0) {
+			tlshd_log_perror("gethostname");
+			return;
+		}
+		len = strlen(buf);
+	}
+
+	ret = gnutls_server_name_set(session, GNUTLS_NAME_DNS, buf, len);
+	if (ret != GNUTLS_E_SUCCESS) {
+		tlshd_log_gnutls_error(ret);
+		return;
+	}
+
+	tlshd_log_debug("Using SNI hostname: %s", buf);
+}
+
 static void tlshd_client_anon_handshake(struct tlshd_handshake_parms *parms)
 {
 	gnutls_certificate_credentials_t xcred;
@@ -81,9 +106,7 @@ static void tlshd_client_anon_handshake(struct tlshd_handshake_parms *parms)
 	}
 	gnutls_transport_set_int(session, parms->sockfd);
 
-	gnutls_server_name_set(session, GNUTLS_NAME_DNS,
-			       parms->peername, strlen(parms->peername));
-
+	tlshd_client_set_sni_hostname(session);
 	gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, xcred);
 
 	ret = gnutls_set_default_priority(session);
@@ -277,8 +300,7 @@ static void tlshd_client_x509_handshake(struct tlshd_handshake_parms *parms)
 	}
 	gnutls_transport_set_int(session, parms->sockfd);
 
-	gnutls_server_name_set(session, GNUTLS_NAME_DNS,
-			       parms->peername, strlen(parms->peername));
+	tlshd_client_set_sni_hostname(session);
 	gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, xcred);
 	gnutls_certificate_set_verify_function(xcred,
 					       tlshd_client_x509_verify_function);
diff --git a/src/tlshd/config.c b/src/tlshd/config.c
index 464635ed56e0..cbc522927088 100644
--- a/src/tlshd/config.c
+++ b/src/tlshd/config.c
@@ -352,3 +352,34 @@ bool tlshd_config_get_server_privkey(gnutls_privkey_t *privkey)
 	tlshd_log_debug("Retrieved private key from %s", pathname);
 	return true;
 }
+
+/**
+ * tlshd_config_get_sni_hostname - Get the SNI hostname from .conf
+ * @hostname: buffer to contain hostname string
+ * @len: OUT: length of hostname string
+ *
+ * Return values:
+ *   %true: hostname retrieved successfully
+ *   %false: hostname not retrieved
+ */
+bool tlshd_config_get_sni_hostname(char *hostname, size_t *len)
+{
+	GError *error = NULL;
+	gchar *result;
+
+	result = g_key_file_get_string(tlshd_configuration, "authenticate.client",
+					"sni-hostname", &error);
+	if (!result) {
+		tlshd_log_gerror("Missing key", error);
+		g_error_free(error);
+		return false;
+	}
+	if (strlen(result) > *len - 1) {
+		tlshd_log_error("SNI hostname too large");
+		return false;
+	}
+
+	strcpy(hostname, result);
+	*len = strlen(result);
+	return true;
+}
diff --git a/src/tlshd/tlshd.conf b/src/tlshd/tlshd.conf
index 2f301dd6a42c..a16a96206c12 100644
--- a/src/tlshd/tlshd.conf
+++ b/src/tlshd/tlshd.conf
@@ -30,6 +30,7 @@ nl_debug=0
 [authenticate.client]
 #x509.certificate= <pathname>
 #x509.private_key= <pathname>
+#sni-hostname= <client hostname>
 
 [authenticate.server]
 #x509.certificate= <pathname>
diff --git a/src/tlshd/tlshd.conf.man b/src/tlshd/tlshd.conf.man
index 2fb361845950..bae689e2a387 100644
--- a/src/tlshd/tlshd.conf.man
+++ b/src/tlshd/tlshd.conf.man
@@ -80,8 +80,35 @@ The
 section specifies default authentication material when establishing
 TLS sessions.
 There are two subsections:
-.IR [client] and [server] .
-In each of these subsections, there are two available options:
+.I [client]
+and
+.IR [server] .
+.P
+In the
+.I [client]
+subsection, there are three available options:
+.TP
+.B x509.certificate
+This option specifies the pathname of a file containing
+a PEM-encoded x.509 certificate that is to be presented during
+a ClientHello request when no other certificate is available.
+.TP
+.B x509.private_key
+This option specifies the pathname of a file containing
+a PEM-encoded private key associated with the above certificate.
+.TP
+.B sni-hostname
+This option specifies the hostname to present in the ClientHello
+Server Name Indication extension.
+If not provided, the default is to use the result of the
+.BR gethostname (3)
+library call.
+The SNI hostname should match one of the SubjectNames in the
+client's x.509 certificate.
+.P
+In the
+.I [server]
+subsection, there are two available options:
 .TP
 .B x509.certificate
 This option specifies the pathname of a file containing
diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h
index d8cb3c4b4baa..9b7bd522bc12 100644
--- a/src/tlshd/tlshd.h
+++ b/src/tlshd/tlshd.h
@@ -54,6 +54,7 @@ bool tlshd_config_get_client_cert(gnutls_pcert_st *cert);
 bool tlshd_config_get_client_privkey(gnutls_privkey_t *privkey);
 bool tlshd_config_get_server_cert(gnutls_pcert_st *cert);
 bool tlshd_config_get_server_privkey(gnutls_privkey_t *privkey);
+bool tlshd_config_get_sni_hostname(char *hostname, size_t *len);
 
 /* handshake.c */
 extern void tlshd_start_tls_handshake(gnutls_session_t session,

[PATCH RFC 3/5] tlshd: Pass tlshd_parameters to the verification functions

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

I misunderstood the gnutls_session_get_ptr(3) API, possibly because
many code examples I've audited (ab)use it without context or
explanation.

When used it properly, the full set of handshake parameters can be
passed into the verification functions so the global variables can
be replaced.

Note that, because tlshd never called gnutls_session_set_ptr(3),
@hostname was always set to NULL. That means tlshd in ktls-utils-0.8
always skipped the hostname part of certificate verification.

The peername returned by DNS is fairly likely not to match the
SubjectName in the certificate in many circumstances. To prevent
breakage, we'll pass NULL explicitly to
gnutls_certificate_verify_peers3() for the moment until we can get a
real non-DNS-based hostname for certificate verification.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/tlshd/client.c |   37 ++++++++++++++++---------------------
 src/tlshd/server.c |   40 ++++++++++++++++++++--------------------
 src/tlshd/tlshd.h  |    2 +-
 3 files changed, 37 insertions(+), 42 deletions(-)

diff --git a/src/tlshd/client.c b/src/tlshd/client.c
index 46a973f4393c..1a9916d763c5 100644
--- a/src/tlshd/client.c
+++ b/src/tlshd/client.c
@@ -43,9 +43,6 @@
 #include "tlshd.h"
 #include "netlink.h"
 
-static unsigned int tlshd_num_remote_peerids;
-static key_serial_t tlshd_remote_peerid[10];
-
 static void tlshd_client_set_sni_hostname(gnutls_session_t session)
 {
 	char buf[NI_MAXHOST];
@@ -215,15 +212,15 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
  */
 static int tlshd_client_x509_verify_function(gnutls_session_t session)
 {
+	struct tlshd_handshake_parms *parms;
 	const gnutls_datum_t *peercerts;
 	unsigned int i, status;
-	const char *hostname;
 	gnutls_datum_t out;
 	int type, ret;
 
-	hostname = gnutls_session_get_ptr(session);
+	parms = gnutls_session_get_ptr(session);
 
-	ret = gnutls_certificate_verify_peers3(session, hostname, &status);
+	ret = gnutls_certificate_verify_peers3(session, NULL, &status);
 	if (ret != GNUTLS_E_SUCCESS) {
 		tlshd_log_gnutls_error(ret);
 		return GNUTLS_E_CERTIFICATE_ERROR;
@@ -242,23 +239,24 @@ static int tlshd_client_x509_verify_function(gnutls_session_t session)
 	 * via a netlink attribute. */
 
 	peercerts = gnutls_certificate_get_peers(session,
-						 &tlshd_num_remote_peerids);
-	if (!peercerts || tlshd_num_remote_peerids == 0) {
+						 &parms->num_remote_peerids);
+	if (!peercerts || parms->num_remote_peerids == 0) {
 		tlshd_log_debug("The peer cert list is empty.\n");
                 return GNUTLS_E_CERTIFICATE_ERROR;
 	}
 
-	tlshd_log_debug("The peer offered %d certificate(s).\n",
-			tlshd_num_remote_peerids);
+	tlshd_log_debug("The peer offered %u certificate(s).\n",
+			parms->num_remote_peerids);
 
-	if (tlshd_num_remote_peerids > ARRAY_SIZE(tlshd_remote_peerid))
-		tlshd_num_remote_peerids = ARRAY_SIZE(tlshd_remote_peerid);
-	for (i = 0; i < tlshd_num_remote_peerids; i++) {
+	if (parms->num_remote_peerids > ARRAY_SIZE(parms->remote_peerid))
+		parms->num_remote_peerids = ARRAY_SIZE(parms->remote_peerid);
+	for (i = 0; i < parms->num_remote_peerids; i++) {
 		gnutls_x509_crt_t cert;
 
 		gnutls_x509_crt_init(&cert);
 		gnutls_x509_crt_import(cert, &peercerts[i], GNUTLS_X509_FMT_DER);
-		tlshd_remote_peerid[i] = tlshd_keyring_create_cert(cert, hostname);
+		parms->remote_peerid[i] =
+			tlshd_keyring_create_cert(cert, parms->peername);
 		gnutls_x509_crt_deinit(cert);
 	}
 
@@ -299,12 +297,12 @@ static void tlshd_client_x509_handshake(struct tlshd_handshake_parms *parms)
 		goto out_free_creds;
 	}
 	gnutls_transport_set_int(session, parms->sockfd);
+	gnutls_session_set_ptr(session, parms);
 
 	tlshd_client_set_sni_hostname(session);
 	gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, xcred);
 	gnutls_certificate_set_verify_function(xcred,
 					       tlshd_client_x509_verify_function);
-	gnutls_session_set_verify_cert(session, parms->peername, 0);
 
 	tlshd_start_tls_handshake(session, parms);
 
@@ -356,14 +354,15 @@ static void tlshd_client_psk_handshake_one(struct tlshd_handshake_parms *parms,
 		goto out_free_creds;
 	}
 	gnutls_transport_set_int(session, parms->sockfd);
+	gnutls_session_set_ptr(session, parms);
 	gnutls_credentials_set(session, GNUTLS_CRD_PSK, psk_cred);
 
 	tlshd_log_debug("start ClientHello handshake");
 	tlshd_start_tls_handshake(session, parms);
 	if (!parms->session_status) {
 		/* PSK uses the same identity for both client and server */
-		tlshd_remote_peerid[0] = peerid;
-		tlshd_num_remote_peerids = 1;
+		parms->num_remote_peerids = 1;
+		parms->remote_peerid[0] = peerid;
 	}
 
 	gnutls_deinit(session);
@@ -429,10 +428,6 @@ void tlshd_clienthello_handshake(struct tlshd_handshake_parms *parms)
 		tlshd_log_debug("Unrecognized auth mode (%d)",
 				parms->auth_mode);
 	}
-	if (tlshd_num_remote_peerids) {
-		parms->num_remote_peerids = tlshd_num_remote_peerids;
-		parms->remote_peerid = tlshd_remote_peerid;
-	}
 
 	gnutls_global_deinit();
 }
diff --git a/src/tlshd/server.c b/src/tlshd/server.c
index 267856f968ce..3e2cd6379b8d 100644
--- a/src/tlshd/server.c
+++ b/src/tlshd/server.c
@@ -43,8 +43,6 @@
 
 static gnutls_privkey_t tlshd_server_privkey;
 static gnutls_pcert_st tlshd_server_cert;
-static unsigned int tlshd_num_remote_peerids;
-static key_serial_t tlshd_remote_peerid[10];
 
 /*
  * XXX: After this point, tlshd_server_cert should be deinited on error.
@@ -133,15 +131,15 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
  */
 static int tlshd_server_x509_verify_function(gnutls_session_t session)
 {
+	struct tlshd_handshake_parms *parms;
 	const gnutls_datum_t *peercerts;
 	unsigned int i, status;
-	const char *hostname;
 	gnutls_datum_t out;
 	int type, ret;
 
-	hostname = gnutls_session_get_ptr(session);
+	parms = gnutls_session_get_ptr(session);
 
-	ret = gnutls_certificate_verify_peers3(session, hostname, &status);
+	ret = gnutls_certificate_verify_peers3(session, NULL, &status);
 	switch (ret) {
 	case GNUTLS_E_SUCCESS:
 		break;
@@ -166,23 +164,24 @@ static int tlshd_server_x509_verify_function(gnutls_session_t session)
 	 * via a netlink attribute. */
 
 	peercerts = gnutls_certificate_get_peers(session,
-						 &tlshd_num_remote_peerids);
-	if (!peercerts || tlshd_num_remote_peerids == 0) {
+						 &parms->num_remote_peerids);
+	if (!peercerts || parms->num_remote_peerids == 0) {
 		tlshd_log_debug("The peer cert list is empty.\n");
                 return GNUTLS_E_CERTIFICATE_ERROR;
 	}
 
-	tlshd_log_debug("The peer offered %d certificate(s).\n",
-			tlshd_num_remote_peerids);
+	tlshd_log_debug("The peer offered %u certificate(s).\n",
+			parms->num_remote_peerids);
 
-	if (tlshd_num_remote_peerids > ARRAY_SIZE(tlshd_remote_peerid))
-		tlshd_num_remote_peerids= ARRAY_SIZE(tlshd_remote_peerid);
-	for (i = 0; i < tlshd_num_remote_peerids; i++) {
+	if (parms->num_remote_peerids > ARRAY_SIZE(parms->remote_peerid))
+		parms->num_remote_peerids = ARRAY_SIZE(parms->remote_peerid);
+	for (i = 0; i < parms->num_remote_peerids; i++) {
 		gnutls_x509_crt_t cert;
 
 		gnutls_x509_crt_init(&cert);
 		gnutls_x509_crt_import(cert, &peercerts[i], GNUTLS_X509_FMT_DER);
-		tlshd_remote_peerid[i] = tlshd_keyring_create_cert(cert, hostname);
+		parms->remote_peerid[i] =
+			tlshd_keyring_create_cert(cert, parms->peername);
 		gnutls_x509_crt_deinit(cert);
 	}
 
@@ -223,6 +222,7 @@ static void tlshd_server_x509_handshake(struct tlshd_handshake_parms *parms)
 		goto out_free_creds;
 	}
 	gnutls_transport_set_int(session, parms->sockfd);
+	gnutls_session_set_ptr(session, parms);
 
 	gnutls_server_name_set(session, GNUTLS_NAME_DNS,
 			       parms->peername, strlen(parms->peername));
@@ -252,11 +252,14 @@ out_free_creds:
  *   %0: Matching key has been stored in @key
  *   %-1: Error during lookup, @key is not updated
  */
-static int tlshd_server_psk_cb(__attribute__ ((unused))gnutls_session_t session,
+static int tlshd_server_psk_cb(gnutls_session_t session,
 			       const char *username, gnutls_datum_t *key)
 {
+	struct tlshd_handshake_parms *parms;
 	key_serial_t psk;
 
+	parms = gnutls_session_get_ptr(session);
+
 	psk = keyctl_search(KEY_SPEC_SESSION_KEYRING, "psk", username, 0);
 	if (psk < 0) {
 		tlshd_log_error("failed to search key");
@@ -267,8 +270,8 @@ static int tlshd_server_psk_cb(__attribute__ ((unused))gnutls_session_t session,
 		return -1;
 	}
 	/* PSK uses the same identity for both client and server */
-	tlshd_remote_peerid[0] = psk;
-	tlshd_num_remote_peerids = 1;
+	parms->remote_peerid[0] = psk;
+	parms->num_remote_peerids = 1;
 	return 0;
 }
 
@@ -293,6 +296,7 @@ static void tlshd_server_psk_handshake(struct tlshd_handshake_parms *parms)
 		goto out_free_creds;
 	}
 	gnutls_transport_set_int(session, parms->sockfd);
+	gnutls_session_set_ptr(session, parms);
 
 	gnutls_credentials_set(session, GNUTLS_CRD_PSK, psk_cred);
 
@@ -337,10 +341,6 @@ void tlshd_serverhello_handshake(struct tlshd_handshake_parms *parms)
 		tlshd_log_debug("Unrecognized auth mode (%d)",
 				parms->auth_mode);
 	}
-	if (tlshd_num_remote_peerids) {
-		parms->num_remote_peerids = tlshd_num_remote_peerids;
-		parms->remote_peerid = tlshd_remote_peerid;
-	}
 
 	gnutls_global_deinit();
 }
diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h
index 9b7bd522bc12..1c764787c28a 100644
--- a/src/tlshd/tlshd.h
+++ b/src/tlshd/tlshd.h
@@ -41,7 +41,7 @@ struct tlshd_handshake_parms {
 	unsigned int	session_status;
 
 	unsigned int	num_remote_peerids;
-	key_serial_t	*remote_peerid;
+	key_serial_t	remote_peerid[10];
 };
 
 /* client.c */

[PATCH RFC 4/5] tlshd: Fix server-side peer hostname validation

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

John says:
> ... you're getting the peer's name from its IP address. That's not
> going to work. What one commonly does is get the peer's name from
> the peer and check that the cert is valid for that name _and_ that
> an IP address for that name address matches the peer's IP address.

In other words, tlshd shouldn't use a DNS-acquired peername for
certificate validation. Instead, rely on SNI.

Suggested-by: John Haxby <john.haxby@oracle.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/tlshd/server.c |   34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/tlshd/server.c b/src/tlshd/server.c
index 3e2cd6379b8d..3532dfc4c369 100644
--- a/src/tlshd/server.c
+++ b/src/tlshd/server.c
@@ -30,6 +30,7 @@
 #include <stdio.h>
 #include <errno.h>
 #include <keyutils.h>
+#include <netdb.h>
 
 #include <gnutls/gnutls.h>
 #include <gnutls/abstract.h>
@@ -125,21 +126,42 @@ tlshd_x509_retrieve_key_cb(gnutls_session_t session,
  * tlshd_server_x509_verify_function - Verify remote's x.509 certificate
  * @session: session in the midst of a handshake
  *
- * Return values:
- *   %0: Incoming certificate has been successfully verified
- *   %GNUTLS_E_CERTIFICATE_ERROR: certificate verification failed
+ * A return value of %GNUTLS_E_SUCCESS indicates that the TLS session
+ * has been allowed to continue. tlshd either sets the peerid array if
+ * the presented certificate has been successfully verified, or the
+ * peerid array is left empty if no peer information was available to
+ * perform verification. The kernel consumer can apply a security policy
+ * based on this information.
+ *
+ * A return value of %GNUTLS_E_CERTIFICATE_ERROR means that certificate
+ * verification failed.
  */
 static int tlshd_server_x509_verify_function(gnutls_session_t session)
 {
 	struct tlshd_handshake_parms *parms;
 	const gnutls_datum_t *peercerts;
 	unsigned int i, status;
+	char hostname[NI_MAXHOST];
 	gnutls_datum_t out;
 	int type, ret;
+	size_t len;
 
 	parms = gnutls_session_get_ptr(session);
 
-	ret = gnutls_certificate_verify_peers3(session, NULL, &status);
+	len = sizeof(hostname);
+	ret = gnutls_server_name_get(session, hostname, &len, &i, 0);
+	switch (ret) {
+	case GNUTLS_E_SUCCESS:
+		break;
+	case GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE:
+		tlshd_log_debug("The peer offered no SNI hostname.");
+		return GNUTLS_E_SUCCESS;
+	default:
+		tlshd_log_gnutls_error(ret);
+		return GNUTLS_E_CERTIFICATE_ERROR;
+	}
+
+	ret = gnutls_certificate_verify_peers3(session, hostname, &status);
 	switch (ret) {
 	case GNUTLS_E_SUCCESS:
 		break;
@@ -181,7 +203,7 @@ static int tlshd_server_x509_verify_function(gnutls_session_t session)
 		gnutls_x509_crt_init(&cert);
 		gnutls_x509_crt_import(cert, &peercerts[i], GNUTLS_X509_FMT_DER);
 		parms->remote_peerid[i] =
-			tlshd_keyring_create_cert(cert, parms->peername);
+			tlshd_keyring_create_cert(cert, hostname);
 		gnutls_x509_crt_deinit(cert);
 	}
 
@@ -224,8 +246,6 @@ static void tlshd_server_x509_handshake(struct tlshd_handshake_parms *parms)
 	gnutls_transport_set_int(session, parms->sockfd);
 	gnutls_session_set_ptr(session, parms);
 
-	gnutls_server_name_set(session, GNUTLS_NAME_DNS,
-			       parms->peername, strlen(parms->peername));
 	gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, xcred);
 	gnutls_certificate_set_verify_function(xcred,
 					       tlshd_server_x509_verify_function);

[PATCH RFC 5/5] tlshd: Disable Nagle during handshakes on TCP sockets

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Fedora Docs suggest:
> The TLS handshake has very poor performance if the TCP Nagle
> algorithm is active. You should switch on the TCP_NODELAY socket
> option (at least for the duration of the handshake), or use the
> Linux-specific TCP_CORK option.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/tlshd/handshake.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c
index 5bc96cb5d5aa..25940d099274 100644
--- a/src/tlshd/handshake.c
+++ b/src/tlshd/handshake.c
@@ -43,6 +43,34 @@
 #include "tlshd.h"
 #include "netlink.h"
 
+static void tlshd_set_nagle(gnutls_session_t session, int val)
+{
+	int ret;
+
+	ret = setsockopt(gnutls_transport_get_int(session),
+			 IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val));
+	if (ret < 0)
+		tlshd_log_perror("setsockopt (NODELAY)");
+}
+
+static void tlshd_save_nagle(gnutls_session_t session, int *saved)
+{
+	socklen_t len;
+	int ret;
+
+
+	len = sizeof(saved);
+	ret = getsockopt(gnutls_transport_get_int(session),
+			 IPPROTO_TCP, TCP_NODELAY, saved, &len);
+	if (ret < 0) {
+		tlshd_log_perror("getsockopt (NODELAY)");
+		saved = 0;
+		return;
+	}
+
+	tlshd_set_nagle(session, 1);
+}
+
 /**
  * tlshd_start_tls_handshake - Drive the handshake interaction
  * @session: TLS session to initialize
@@ -77,9 +105,11 @@ void tlshd_start_tls_handshake(gnutls_session_t session,
 	}
 
 	gnutls_handshake_set_timeout(session, parms->timeout_ms);
+	tlshd_save_nagle(session, &saved);
 	do {
 		ret = gnutls_handshake(session);
 	} while (ret < 0 && !gnutls_error_is_fatal(ret));
+	tlshd_set_nagle(session, saved);
 	if (ret < 0) {
 		switch (ret) {
 		case GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR: