Log irregularities during initial network connection establishment

- For Tapdance, this includes absence of the PROXY protocol header.

psiphon/common/logger.go

@@ -50,3 +50,9 @@ type MetricsSource interface {
 	// metrics from the MetricsSource
 	GetMetrics() LogFields
 }
+
+func SetIrregularTunnelErrorLogField(
+	logFields LogFields, tunnelError error) {
+
+	logFields["tunnel_error"] = tunnelError
+}

psiphon/common/net.go

@@ -39,18 +39,25 @@ type NetDialer interface {
 	DialContext(ctx context.Context, network, address string) (net.Conn, error)
 }
 
-// Closer defines the interface to a type, typically
-// a net.Conn, that can be closed.
+// Closer defines the interface to a type, typically a net.Conn, that can be
+// closed.
 type Closer interface {
 	IsClosed() bool
 }
 
-// CloseWriter defines the interface to a type, typically
-// a net.TCPConn, that implements CloseWrite.
+// CloseWriter defines the interface to a type, typically a net.TCPConn, that
+// implements CloseWrite.
 type CloseWriter interface {
 	CloseWrite() error
 }
 
+// IrregularIndicator defines the interface for a type, typically a net.Conn,
+// that detects and reports irregular conditions during initial network
+// connection establishment.
+type IrregularIndicator interface {
+	IrregularTunnelError() error
+}
+
 // TerminateHTTPConnection sends a 404 response to a client and also closes
 // the persistent connection.
 func TerminateHTTPConnection(

psiphon/common/obfuscator/obfuscator.go

@@ -304,7 +304,7 @@ func readSeedMessage(
 		errStr := "duplicate obfuscation seed"
 		if duplicateLogFields != nil {
 			if config.IrregularLogger != nil {
-				setIrregularTunnelErrorLogField(
+				common.SetIrregularTunnelErrorLogField(
 					*duplicateLogFields, errors.BackTraceNew(errBackTrace, errStr))
 				config.IrregularLogger(clientIP, *duplicateLogFields)
 			}
@@ -357,7 +357,7 @@ func readSeedMessage(
 	if errStr != "" {
 		if config.IrregularLogger != nil {
 			errLogFields := make(common.LogFields)
-			setIrregularTunnelErrorLogField(
+			common.SetIrregularTunnelErrorLogField(
 				errLogFields, errors.BackTraceNew(errBackTrace, errStr))
 			config.IrregularLogger(clientIP, errLogFields)
 		}
@@ -393,9 +393,3 @@ func readSeedMessage(
 
 	return clientToServerCipher, serverToClientCipher, paddingPRNGSeed, nil
 }
-
-func setIrregularTunnelErrorLogField(
-	logFields common.LogFields, tunnelError error) {
-
-	logFields["tunnel_error"] = tunnelError
-}

psiphon/common/tapdance/tapdance.go

@@ -117,8 +117,8 @@ func (l *stationListener) Accept() (net.Conn, error) {
 		return nil, errors.TraceNew("missing station address")
 	}
 	return &stationConn{
-		Conn:              conn,
-		stationRemoteAddr: stationRemoteAddr,
+		Conn:             conn,
+		stationIPAddress: common.IPAddressFromAddr(stationRemoteAddr),
 	}, nil
 }
 
@@ -132,13 +132,38 @@ func (l *stationListener) Addr() net.Addr {
 
 type stationConn struct {
 	net.Conn
-	stationRemoteAddr net.Addr
+	stationIPAddress string
+}
+
+// IrregularTunnelError implements the common.IrregularIndicator interface.
+func (c *stationConn) IrregularTunnelError() error {
+
+	// We expect a PROXY protocol header, but go-proxyproto does not produce an
+	// error if the "PROXY " prefix is absent; instead the connection will
+	// proceed. To detect this case, check if the go-proxyproto RemoteAddr IP
+	// address matches the underlying connection IP address. When these values
+	// match, there was no PROXY protocol header.
+	//
+	// Limitation: the values will match if there is a PROXY protocol header
+	// containing the same IP address as the underlying connection. This is not
+	// an expected case.
+
+	if common.IPAddressFromAddr(c.RemoteAddr()) == c.stationIPAddress {
+		return errors.TraceNew("unexpected station IP address")
+	}
+	return nil
 }
 
 // GetMetrics implements the common.MetricsSource interface.
 func (c *stationConn) GetMetrics() common.LogFields {
+
 	logFields := make(common.LogFields)
-	logFields["station_ip_address"] = common.IPAddressFromAddr(c.stationRemoteAddr)
+
+	// Ensure we don't log a potential non-station IP address.
+	if c.IrregularTunnelError() == nil {
+		logFields["station_ip_address"] = c.stationIPAddress
+	}
+
 	return logFields
 }
 

psiphon/server/tunnelServer.go

@@ -961,8 +961,46 @@ func (sshServer *sshServer) handleClient(
 	// Calling clientConn.RemoteAddr at this point, before any Read calls,
 	// satisfies the constraint documented in tapdance.Listen.
 
+	clientAddr := clientConn.RemoteAddr()
+
+	// Check if there were irregularities during the network connection
+	// establishment. When present, log and then behave as Obfuscated SSH does
+	// when the client fails to provide a valid seed message.
+	//
+	// One concrete irregular case is failure to send a PROXY protocol header for
+	// TAPDANCE-OSSH.
+
+	if indicator, ok := clientConn.(common.IrregularIndicator); ok {
+
+		tunnelErr := indicator.IrregularTunnelError()
+
+		if tunnelErr != nil {
+
+			logFields := make(common.LogFields)
+			common.SetIrregularTunnelErrorLogField(
+				logFields, errors.Trace(tunnelErr))
+			logIrregularTunnel(
+				sshServer.support,
+				listenerTunnelProtocol,
+				listenerPort,
+				common.IPAddressFromAddr(clientAddr),
+				LogFields(logFields))
+
+			var afterFunc *time.Timer
+			if sshServer.support.Config.sshHandshakeTimeout > 0 {
+				afterFunc = time.AfterFunc(sshServer.support.Config.sshHandshakeTimeout, func() {
+					clientConn.Close()
+				})
+			}
+			io.Copy(ioutil.Discard, clientConn)
+			afterFunc.Stop()
+		}
+
+		return
+	}
+
 	geoIPData := sshServer.support.GeoIPService.Lookup(
-		common.IPAddressFromAddr(clientConn.RemoteAddr()))
+		common.IPAddressFromAddr(clientAddr))
 
 	sshServer.registerAcceptedClient(tunnelProtocol, geoIPData.Country)
 	defer sshServer.unregisterAcceptedClient(tunnelProtocol, geoIPData.Country)