Explorar o código

Add RemoteAddrOverride for inproxy.ClientConn

- Fixes nil pointer panic when utls calls RemoteAddr
- Facilitates configuring a more appropriate TLS session cache key (for non-SNI
  cases only; and will be superseded by overall TLS session cache improvements
  in https://github.com/Psiphon-Labs/psiphon-tunnel-core/pull/679
Rod Hynes hai 1 ano
pai
achega
f92ad9eb92
Modificáronse 3 ficheiros con 82 adicións e 1 borrados
  1. 41 1
      psiphon/common/inproxy/client.go
  2. 27 0
      psiphon/common/protocol/serverEntry.go
  3. 14 0
      psiphon/tunnel.go

+ 41 - 1
psiphon/common/inproxy/client.go

@@ -22,6 +22,7 @@ package inproxy
 import (
 	"context"
 	"net"
+	"net/netip"
 	"sync"
 	"time"
 
@@ -46,6 +47,7 @@ type ClientConn struct {
 	brokerClient *BrokerClient
 	webRTCConn   *webRTCConn
 	connectionID ID
+	remoteAddr   net.Addr
 
 	relayMutex         sync.Mutex
 	initialRelayPacket []byte
@@ -91,6 +93,11 @@ type ClientConfig struct {
 	// will relay traffic to.
 	DialAddress string
 
+	// RemoteAddrOverride, when specified, is the address to be returned by
+	// ClientConn.RemoteAddr. When not specified, ClientConn.RemoteAddr
+	// returns a zero-value address.
+	RemoteAddrOverride string
+
 	// PackedDestinationServerEntry is a signed Psiphon server entry
 	// corresponding to the destination dial address. This signed server
 	// entry is sent to the broker, which will use it to validate that the
@@ -110,6 +117,33 @@ func DialClient(
 	ctx context.Context,
 	config *ClientConfig) (retConn *ClientConn, retErr error) {
 
+	// Configure the value returned by ClientConn.RemoteAddr. If no
+	// config.RemoteAddrOverride is specified, RemoteAddr will return a
+	// zero-value, non-nil net.Addr. The underlying webRTCConn.RemoteAddr
+	// returns only nil.
+
+	var remoteAddr net.Addr
+	var addrPort netip.AddrPort
+	if config.RemoteAddrOverride != "" {
+
+		// ParseAddrPort does not perform any domain resolution. The addr
+		// portion must be an IP address.
+		var err error
+		addrPort, err = netip.ParseAddrPort(config.RemoteAddrOverride)
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
+	}
+
+	switch config.DialNetworkProtocol {
+	case NetworkProtocolTCP:
+		remoteAddr = net.TCPAddrFromAddrPort(addrPort)
+	case NetworkProtocolUDP:
+		remoteAddr = net.UDPAddrFromAddrPort(addrPort)
+	default:
+		return nil, errors.TraceNew("unexpected DialNetworkProtocol")
+	}
+
 	// Reset and configure port mapper component, as required. See
 	// initPortMapper comment.
 	initPortMapper(config.WebRTCDialCoordinator)
@@ -207,6 +241,7 @@ func DialClient(
 		webRTCConn:         result.conn,
 		connectionID:       result.connectionID,
 		initialRelayPacket: result.relayPacket,
+		remoteAddr:         remoteAddr,
 	}, nil
 }
 
@@ -420,7 +455,12 @@ func (conn *ClientConn) LocalAddr() net.Addr {
 }
 
 func (conn *ClientConn) RemoteAddr() net.Addr {
-	return conn.webRTCConn.RemoteAddr()
+
+	// *TEMP*
+	return nil
+
+	// Do not return conn.webRTCConn.RemoteAddr(), which is always nil.
+	return conn.remoteAddr
 }
 
 func (conn *ClientConn) SetDeadline(t time.Time) error {

+ 27 - 0
psiphon/common/protocol/serverEntry.go

@@ -33,6 +33,7 @@ import (
 	"io"
 	"net"
 	"regexp"
+	"strconv"
 	"strings"
 	"sync/atomic"
 	"time"
@@ -792,6 +793,32 @@ func (serverEntry *ServerEntry) GetDialPortNumber(tunnelProtocol string) (int, e
 	return 0, errors.TraceNew("unknown protocol")
 }
 
+// GetTLSSessionCacheKeyAddress returns a network address (IP:port) that is
+// suitable to use as a TLS session cache key.
+//
+// By default, TLS implementations, including crypto/tls and utls use SNI as a
+// session cache key, but this is not a suitable key when SNI is manipulated.
+// When SNI is not present, these implementations fall back to using the peer
+// remote address, which is also not a suitable key in cases where there is a
+// non-TLS-terminating temporary intermediary, such as an in-proxy proxy.
+//
+// The key is unique to the Psiphon server and tunnel protocol listener. For
+// direct tunnel protocols, the key precisely maps TLS sessions to the
+// corresponding TLS server. For indirect tunnel protocols, with an
+// intermediate TLS server, the key is an approximate map which assumes the
+// redials will mostly use the same intermediate TLS server.
+//
+// Do not use the GetTLSSessionCacheKeyAddress value for dialing.
+func (serverEntry *ServerEntry) GetTLSSessionCacheKeyAddress(tunnelProtocol string) (string, error) {
+
+	port, err := serverEntry.GetDialPortNumber(tunnelProtocol)
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+
+	return net.JoinHostPort(serverEntry.IpAddress, strconv.Itoa(port)), nil
+}
+
 // IsValidInproxyDialAddress indicates whether the dial destination
 // network/host/port matches the dial parameters for any of the tunnel
 // protocols supported by the server entry.

+ 14 - 0
psiphon/tunnel.go

@@ -1519,6 +1519,19 @@ func dialInproxy(
 		dialAddress = dialParams.MeekDialAddress
 	}
 
+	// Specify the value to be returned by inproxy.ClientConn.RemoteAddr.
+	// Currently, the one caller of RemoteAddr is utls, which uses the
+	// RemoteAddr as a TLS session cache key when there is no SNI.
+	// GetTLSSessionCacheKeyAddress returns a cache key value that is a valid
+	// address and that is also a more appropriate TLS session cache key than
+	// the proxy address.
+
+	remoteAddrOverride, err := dialParams.ServerEntry.GetTLSSessionCacheKeyAddress(
+		dialParams.TunnelProtocol)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
 	// Unlike the proxy broker case, clients already actively fetch tactics
 	// during tunnel estalishment, so no tactics tags are sent to the broker
 	// and no tactics are returned by the broker.
@@ -1545,6 +1558,7 @@ func dialInproxy(
 		ReliableTransport:            reliableTransport,
 		DialNetworkProtocol:          networkProtocol,
 		DialAddress:                  dialAddress,
+		RemoteAddrOverride:           remoteAddrOverride,
 		PackedDestinationServerEntry: dialParams.inproxyPackedSignedServerEntry,
 	}