Browse Source

Fix: omit empty server_name extension

Rod Hynes 6 years ago
parent
commit
f6274f2244
1 changed files with 33 additions and 6 deletions
  1. 33 6
      psiphon/tlsDialer.go

+ 33 - 6
psiphon/tlsDialer.go

@@ -531,9 +531,12 @@ func CustomTLSDial(
 		}
 	}
 
+	// Perform at most one remarshal for the following ClientHello
+	// modifications.
+	needRemarshal := false
+
 	// Either pre-TLS 1.3 ClientHellos or any randomized ClientHello is a
 	// candidate for NoDefaultSessionID logic.
-
 	if len(conn.HandshakeState.Hello.SessionTicket) == 0 &&
 		(!isTLS13 || utlsClientHelloID.Client == "Randomized") {
 
@@ -546,15 +549,39 @@ func CustomTLSDial(
 		}
 
 		if noDefaultSessionID {
-
 			conn.HandshakeState.Hello.SessionId = nil
+			needRemarshal = true
+		}
+	}
 
-			// Apply changes to utls
-			err = conn.MarshalClientHello()
-			if err != nil {
-				return nil, common.ContextError(err)
+	// utls doesn't omit the server_name extension when the SNI value is empty
+	// (including both the case where we set the SNI value to "" and the case
+	// where the SNI address is an IP address, which is internally changed to
+	// ""). To avoid a fingerprintable invalid/unusual server_name extension,
+	// remove it in these cases.
+	if tlsConfigServerName == "" || net.ParseIP(tlsConfigServerName) != nil {
+
+		// Assumes only one SNIExtension.
+		deleteIndex := -1
+		for index, extension := range conn.Extensions {
+			if _, ok := extension.(*utls.SNIExtension); ok {
+				deleteIndex = index
+				break
 			}
 		}
+		if deleteIndex != -1 {
+			conn.Extensions = append(
+				conn.Extensions[:deleteIndex], conn.Extensions[deleteIndex+1:]...)
+		}
+		needRemarshal = true
+	}
+
+	if needRemarshal {
+		// Apply changes to utls
+		err = conn.MarshalClientHello()
+		if err != nil {
+			return nil, common.ContextError(err)
+		}
 	}
 
 	// Perform the TLS Handshake.