[PATCH] ALPN validation fix

[Achim Gratz]

The ALPN validation was broken and would always return "bad".  Why NTS
works anyway I don't know, but the ALPN negotiated protocol is a counted
string (without an added '\0'), so you can't use strcmp to check you've
got the expected protocol.  I've also shortened a way too long (probably
entirely expendable) buffer and tweaked the logging messages a bit.

--8<---------------cut here---------------start------------->8---
Date:   Sat Dec 7 13:17:17 2019 +0100

    fix bug in ALPN validation and shorten buffer

	Modified   ntpd/nts_client.c
diff --git a/ntpd/nts_client.c b/ntpd/nts_client.c
index 5563e757b..0734c9919 100644
--- a/ntpd/nts_client.c
+++ b/ntpd/nts_client.c
@@ -399,7 +399,7 @@ bool check_aead(SSL *ssl, struct peer* peer, const char *hostname) {
 	const unsigned char *data;
 	unsigned int len;
 	unsigned int i;
-	char buff [100];
+	char buff [16];
 	SSL_get0_alpn_selected(ssl, &data, &len);
 	if (0 == len) {
 		/* This happens when talking to old/TLSv1.2 systems. */
@@ -407,27 +407,22 @@ bool check_aead(SSL *ssl, struct peer* peer, const char *hostname) {
 			hostname, SSL_get_version(ssl));
 		return bad;
 	}
-	if (sizeof(buff) <= len) {
-		/* Broken or malicious server */
-		msyslog(LOG_DEBUG, "NTSc: Very long ALPN from %s (%u)",
-			hostname, len);
-		return bad;
-	}
-	memcpy(buff, data, len);
-	buff[len] = '\0';
-	for (i=0; i<len; i++) {
-		if (!isgraph(buff[i])) {
-			buff[i] = '*'; /* fix non-printing crap */
-		}
-	}
 	/* For now, we only support one version.
 	 * This gets more complicated when version 2 arrives. */
-	if (0 != strcmp((const char*)data, "ntske/1")) {
-		msyslog(LOG_DEBUG, "NTSc: Strange ALPN returned: %s (%u) from %s",
+	if (len != 7 ||
+	    0   != strncmp((const char*)data, "ntske/1", len)) {
+		strlcpy(buff, (const char *)data, sizeof(buff));
+		buff[sizeof(buff)] = '\0';
+		for (i=0; i<sizeof(buff); i++) {
+			if (!isgraph(buff[i])) {
+			  buff[i] = '*'; /* fix non-printing crap */
+			}
+		}
+		msyslog(LOG_DEBUG, "NTSc: Strange or too long ALPN %s (%u) from %s",
 			buff, len, hostname);
 		return bad;
 	}
-        msyslog(LOG_DEBUG, "NTSc: Good ALPN from: %s", hostname);
+	msyslog(LOG_DEBUG, "NTSc: Good ALPN ntske/1 (%u) from %s", len, hostname);
 
 #else
 	UNUSED_ARG(ssl);

--8<---------------cut here---------------end--------------->8---



Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs