From 5c64f8855352bdd9650210f8589c97b084af08d3 Mon Sep 17 00:00:00 2001
From: Maxb <bittmanmax@gmail.com>
Date: Tue, 1 Feb 2022 21:01:29 -0800
Subject: [PATCH] Fix invalid SNI handling

SNIExtension was previously marshalling both ip addresses and empty
strings, which are not allowed. See RFC 6066, Section 3.

All of the utls specific testdata replays needed to be rebuilt to
properly accomodate this change since they had previously been including
empty server name extension values

Addresses https://github.com/refraction-networking/utls/issues/96
---
 u_conn_test.go      | 33 +++++++++++++++++++++++++++++++++
 u_tls_extensions.go | 32 +++++++++++++++++++++++---------
 2 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/u_conn_test.go b/u_conn_test.go
index daec15ed..2ce4b515 100644
--- a/u_conn_test.go
+++ b/u_conn_test.go
@@ -196,6 +196,38 @@ func TestUTLSRemoveSNIExtension(t *testing.T) {
 	runUTLSClientTestForVersion(t, test, "TLSv12-", "-tls1_2", hello, true)
 }
 
+func TestUTLSServerNameIP(t *testing.T) {
+	hello := &helloID{HelloChrome_70}
+
+	config := getUTLSTestConfig()
+	config.ServerName = "1.1.1.1"
+
+	opensslCipherName := "ECDHE-RSA-AES128-GCM-SHA256"
+	test := &clientTest{
+		name:   "UTLS-" + opensslCipherName + "-" + hello.helloName() + "-ServerNameIP",
+		args:   []string{"-cipher", opensslCipherName},
+		config: config,
+	}
+
+	runUTLSClientTestForVersion(t, test, "TLSv12-", "-tls1_2", hello, true)
+}
+
+func TestUTLSEmptyServerName(t *testing.T) {
+	hello := &helloID{HelloChrome_70}
+
+	config := getUTLSTestConfig()
+	config.ServerName = ""
+
+	opensslCipherName := "ECDHE-RSA-AES128-GCM-SHA256"
+	test := &clientTest{
+		name:   "UTLS-" + opensslCipherName + "-" + hello.helloName() + "-EmptyServerName",
+		args:   []string{"-cipher", opensslCipherName},
+		config: config,
+	}
+
+	runUTLSClientTestForVersion(t, test, "TLSv12-", "-tls1_2", hello, true)
+}
+
 /*
 *
  HELPER FUNCTIONS BELOW
@@ -212,6 +244,7 @@ func getUTLSTestConfig() *Config {
 		MinVersion:         VersionSSL30,
 		MaxVersion:         VersionTLS13,
 		CipherSuites:       allCipherSuites(),
+		ServerName:         "foobar.com",
 	}
 	return testUTLSConfig
 }
diff --git a/u_tls_extensions.go b/u_tls_extensions.go
index 2e4d0476..4330f36b 100644
--- a/u_tls_extensions.go
+++ b/u_tls_extensions.go
@@ -49,29 +49,43 @@ type SNIExtension struct {
 
 func (e *SNIExtension) writeToUConn(uc *UConn) error {
 	uc.config.ServerName = e.ServerName
-	uc.HandshakeState.Hello.ServerName = e.ServerName
+	hostName := hostnameInSNI(e.ServerName)
+	uc.HandshakeState.Hello.ServerName = hostName
+
 	return nil
 }
 
 func (e *SNIExtension) Len() int {
-	return 4 + 2 + 1 + 2 + len(e.ServerName)
+	// Literal IP addresses, absolute FQDNs, and empty strings are not permitted as SNI values.
+	// See RFC 6066, Section 3.
+	hostName := hostnameInSNI(e.ServerName)
+	if len(hostName) == 0 {
+		return 0
+	}
+	return 4 + 2 + 1 + 2 + len(hostName)
 }
 
 func (e *SNIExtension) Read(b []byte) (int, error) {
+	// Literal IP addresses, absolute FQDNs, and empty strings are not permitted as SNI values.
+	// See RFC 6066, Section 3.
+	hostName := hostnameInSNI(e.ServerName)
+	if len(hostName) == 0 {
+		return 0, io.EOF
+	}
 	if len(b) < e.Len() {
 		return 0, io.ErrShortBuffer
 	}
 	// RFC 3546, section 3.1
 	b[0] = byte(extensionServerName >> 8)
 	b[1] = byte(extensionServerName)
-	b[2] = byte((len(e.ServerName) + 5) >> 8)
-	b[3] = byte((len(e.ServerName) + 5))
-	b[4] = byte((len(e.ServerName) + 3) >> 8)
-	b[5] = byte(len(e.ServerName) + 3)
+	b[2] = byte((len(hostName) + 5) >> 8)
+	b[3] = byte((len(hostName) + 5))
+	b[4] = byte((len(hostName) + 3) >> 8)
+	b[5] = byte(len(hostName) + 3)
 	// b[6] Server Name Type: host_name (0)
-	b[7] = byte(len(e.ServerName) >> 8)
-	b[8] = byte(len(e.ServerName))
-	copy(b[9:], []byte(e.ServerName))
+	b[7] = byte(len(hostName) >> 8)
+	b[8] = byte(len(hostName))
+	copy(b[9:], []byte(hostName))
 	return e.Len(), io.EOF
 }