Merge pull request #689 from rod-hynes/master

In-proxy enhancements

README.md

@@ -165,55 +165,5 @@ go: added github.com/Psiphon-Labs/psiphon-tunnel-core v1.0.11-0.20240424194431-3
 Acknowledgements
 --------------------------------------------------------------------------------
 
-Psiphon Tunnel Core uses:
-
-* [Go](https://golang.org)
-* [agl/ed25519](https://github.com/agl/ed25519)
-* [AndreasBriese/bbloom](https://github.com/AndreasBriese/bbloom)
-* [aristanetworks/goarista/monotime](https://github.com/aristanetworks/goarista)
-* [armon/go-proxyproto](https://github.com/armon/go-proxyproto)
-* [armon/go-socks](https://github.com/armon/go-socks5)
-* [bifurcation/mint](https://github.com/bifurcation/mint)
-* [boltdb/bolt](https://github.com/boltdb/bolt)
-* [cheekybits/genny/generic](https://github.com/cheekybits/genny/generic)
-* [codahale/sss](https://github.com/codahale/sss)
-* [cognusion/go-cache-lru](https://github.com/cognusion/go-cache-lru)
-* [creack/goselect](https://github.com/creack/goselect)
-* [davecgh/go-spew/spew](https://github.com/davecgh/go-spew/spew)
-* [deckarep/golang-set](https://github.com/deckarep/golang-set)
-* [dgraph-io/badger](https://github.com/dgraph-io/badger)
-* [dgryski/go-farm](https://github.com/dgryski/go-farm)
-* [elazarl/goproxy](https://github.com/elazarl/goproxy)
-* [florianl/go-nfqueue](https://github.com/florianl/go-nfqueue)
-* [gobwas/glob](https://github.com/gobwas/glob)
-* [golang/protobuf](https://github.com/golang/protobuf)
-* [google/gopacket](https://github.com/google/gopacket)
-* [grafov/m3u8](https://github.com/grafov/m3u8)
-* [hashicorp/golang-lru](https://github.com/hashicorp/golang-lru)
-* [juju/ratelimit](https://github.com/juju/ratelimit)
-* [kardianos/osext](https://github.com/kardianos/osext)
-* [groupcache/lru]("github.com/golang/groupcache/lru")
-* [lucas-clemente/quic-go](https://github.com/lucas-clemente/quic-go)
-* [marusama/semaphore](https://github.com/marusama/semaphore)
-* [mdlayher/netlink)](https://github.com/mdlayher/netlink)
-* [miekg/dns](https://github.com/miekg/dns)
-* [mitchellh/panicwrap](https://github.com/mitchellh/panicwrap)
-* [oschwald/maxminddb-golang](https://github.com/oschwald/maxminddb-golang)
-* [patrickmn/go-cache](https://github.com/patrickmn/go-cache)
-* [pkg/errors](https://github.com/pkg/errors)
-* [pmezard/go-difflib](https://github.com/pmezard/go-difflib)
-* [refraction-networking/gotapdance](https://github.com/refraction-networking/gotapdance)
-* [refraction-networking/utls](https://github.com/refraction-networking/utls)
-* [ryanuber/go-glob](https://github.com/ryanuber/go-glob)
-* [sergeyfrolov/bsbuffer](https://github.com/sergeyfrolov/bsbuffer)
-* [sirupsen/logrus](https://github.com/sirupsen/logrus)
-* [stretchr/testify](https://github.com/stretchr/testify)
-* [syndtr/gocapability/capability](https://github.com/syndtr/gocapability/capability)
-* [ThomsonReutersEikon/go-ntlm](https://github.com/ThomsonReutersEikon/go-ntlm)
-* [wader/filtertransport](https://github.com/wader/filtertransport)
-* [Yawning/chacha20](https://github.com/Yawning/chacha20)
-* [Yawning/goptlib](https://github.com/Yawning/goptlib)
-* [yawning/obfs4](https://gitlab.com/yawning/obfs4)
-* [zach-klippenstein/goregen](https://github.com/zach-klippenstein/goregen)
-* [zap](https://go.uber.org/zap)
+Psiphon Tunnel Core uses the following Go modules: https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/master/go.mod
 

psiphon/common/inproxy/api.go

@@ -575,25 +575,27 @@ func (request *ClientOfferRequest) ValidateAndGetLogFields(
 	lookupGeoIP LookupGeoIP,
 	baseAPIParameterValidator common.APIParameterValidator,
 	formatter common.APIParameterLogFieldFormatter,
-	geoIPData common.GeoIPData) (common.LogFields, error) {
+	geoIPData common.GeoIPData) ([]byte, common.LogFields, error) {
 
 	if len(request.CommonCompartmentIDs) > maxCompartmentIDs {
-		return nil, errors.Tracef("invalid compartment IDs length: %d", len(request.CommonCompartmentIDs))
+		return nil, nil, errors.Tracef(
+			"invalid compartment IDs length: %d", len(request.CommonCompartmentIDs))
 	}
 
 	if len(request.PersonalCompartmentIDs) > maxCompartmentIDs {
-		return nil, errors.Tracef("invalid compartment IDs length: %d", len(request.PersonalCompartmentIDs))
+		return nil, nil, errors.Tracef(
+			"invalid compartment IDs length: %d", len(request.PersonalCompartmentIDs))
 	}
 
 	// The client offer SDP may contain no ICE candidates.
 	errorOnNoCandidates := false
 
 	// Client offer SDP candidate addresses must match the country and ASN of
 	// the client. Don't facilitate connections to arbitrary destinations.
-	sdpMetrics, err := validateSDPAddresses(
+	filteredSDP, sdpMetrics, err := filterSDPAddresses(
 		[]byte(request.ClientOfferSDP.SDP), errorOnNoCandidates, lookupGeoIP, geoIPData)
 	if err != nil {
-		return nil, errors.Trace(err)
+		return nil, nil, errors.Trace(err)
 	}
 
 	// The client's self-reported ICECandidateTypes are used instead of the
@@ -602,23 +604,24 @@ func (request *ClientOfferRequest) ValidateAndGetLogFields(
 	// indistinguishable from host candidate types.
 
 	if !request.ICECandidateTypes.IsValid() {
-		return nil, errors.Tracef("invalid ICE candidate types: %v", request.ICECandidateTypes)
+		return nil, nil, errors.Tracef(
+			"invalid ICE candidate types: %v", request.ICECandidateTypes)
 	}
 
 	if request.Metrics == nil {
-		return nil, errors.TraceNew("missing metrics")
+		return nil, nil, errors.TraceNew("missing metrics")
 	}
 
 	logFields, err := request.Metrics.ValidateAndGetLogFields(
 		baseAPIParameterValidator, formatter, geoIPData)
 	if err != nil {
-		return nil, errors.Trace(err)
+		return nil, nil, errors.Trace(err)
 	}
 
 	if request.TrafficShapingParameters != nil {
 		err := request.TrafficShapingParameters.Validate()
 		if err != nil {
-			return nil, errors.Trace(err)
+			return nil, nil, errors.Trace(err)
 		}
 	}
 
@@ -634,8 +637,9 @@ func (request *ClientOfferRequest) ValidateAndGetLogFields(
 	logFields["has_personal_compartment_ids"] = hasPersonalCompartmentIDs
 	logFields["ice_candidate_types"] = request.ICECandidateTypes
 	logFields["has_IPv6"] = sdpMetrics.hasIPv6
+	logFields["filtered_ice_candidates"] = sdpMetrics.filteredICECandidates
 
-	return logFields, nil
+	return filteredSDP, logFields, nil
 }
 
 // Validate validates the that client has not specified excess traffic shaping
@@ -675,17 +679,17 @@ func (request *ProxyAnswerRequest) ValidateAndGetLogFields(
 	lookupGeoIP LookupGeoIP,
 	baseAPIParameterValidator common.APIParameterValidator,
 	formatter common.APIParameterLogFieldFormatter,
-	geoIPData common.GeoIPData) (common.LogFields, error) {
+	geoIPData common.GeoIPData) ([]byte, common.LogFields, error) {
 
 	// The proxy answer SDP must contain at least one ICE candidate.
 	errorOnNoCandidates := true
 
 	// Proxy answer SDP candidate addresses must match the country and ASN of
 	// the proxy. Don't facilitate connections to arbitrary destinations.
-	sdpMetrics, err := validateSDPAddresses(
+	filteredSDP, sdpMetrics, err := filterSDPAddresses(
 		[]byte(request.ProxyAnswerSDP.SDP), errorOnNoCandidates, lookupGeoIP, geoIPData)
 	if err != nil {
-		return nil, errors.Trace(err)
+		return nil, nil, errors.Trace(err)
 	}
 
 	// The proxy's self-reported ICECandidateTypes are used instead of the
@@ -694,21 +698,24 @@ func (request *ProxyAnswerRequest) ValidateAndGetLogFields(
 	// indistinguishable from host candidate types.
 
 	if !request.ICECandidateTypes.IsValid() {
-		return nil, errors.Tracef("invalid ICE candidate types: %v", request.ICECandidateTypes)
+		return nil, nil, errors.Tracef(
+			"invalid ICE candidate types: %v", request.ICECandidateTypes)
 	}
 
 	if request.SelectedProxyProtocolVersion != ProxyProtocolVersion1 {
-		return nil, errors.Tracef("invalid select proxy protocol version: %v", request.SelectedProxyProtocolVersion)
+		return nil, nil, errors.Tracef(
+			"invalid select proxy protocol version: %v", request.SelectedProxyProtocolVersion)
 	}
 
 	logFields := formatter(geoIPData, common.APIParameters{})
 
 	logFields["connection_id"] = request.ConnectionID
 	logFields["ice_candidate_types"] = request.ICECandidateTypes
 	logFields["has_IPv6"] = sdpMetrics.hasIPv6
+	logFields["filtered_ice_candidates"] = sdpMetrics.filteredICECandidates
 	logFields["answer_error"] = request.AnswerError
 
-	return logFields, nil
+	return filteredSDP, logFields, nil
 }
 
 // ValidateAndGetLogFields validates the ClientRelayedPacketRequest and returns

psiphon/common/inproxy/broker.go

@@ -349,6 +349,9 @@ func (b *Broker) HandleSessionPacket(
 	handleUnwrappedRequest := func(initiatorID ID, unwrappedRequestPayload []byte) ([]byte, error) {
 
 		recordType, err := peekRecordPreambleType(unwrappedRequestPayload)
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
 
 		var responsePayload []byte
 
@@ -759,7 +762,15 @@ func (b *Broker) handleClientOffer(
 		return nil, errors.Trace(err)
 	}
 
-	logFields, err = offerRequest.ValidateAndGetLogFields(
+	// The filtered SDP is the request SDP with any invalid (bogon, unexpected
+	// GeoIP) ICE candidates filtered out. In some cases, clients cannot
+	// avoid submitting invalid candidates (see comment in
+	// processSDPAddresses), so all invalid candidates are removed and the
+	// remaining SDP is used. Filtered candidate information is logged in
+	// logFields.
+
+	var filteredSDP []byte
+	filteredSDP, logFields, err = offerRequest.ValidateAndGetLogFields(
 		int(atomic.LoadInt64(&b.maxCompartmentIDs)),
 		b.config.LookupGeoIP,
 		b.config.APIParameterValidator,
@@ -769,6 +780,9 @@ func (b *Broker) handleClientOffer(
 		return nil, errors.Trace(err)
 	}
 
+	offerSDP := offerRequest.ClientOfferSDP
+	offerSDP.SDP = string(filteredSDP)
+
 	// AllowClient may be used to disallow clients from certain geolocations
 	// from offering. Clients are always allowed to match proxies with shared
 	// personal compartment IDs.
@@ -818,7 +832,7 @@ func (b *Broker) handleClientOffer(
 			PortMappingTypes:       offerRequest.Metrics.PortMappingTypes,
 		},
 		ClientProxyProtocolVersion:  offerRequest.Metrics.ProxyProtocolVersion,
-		ClientOfferSDP:              offerRequest.ClientOfferSDP,
+		ClientOfferSDP:              offerSDP,
 		ClientRootObfuscationSecret: offerRequest.ClientRootObfuscationSecret,
 		DoDTLSRandomization:         offerRequest.DoDTLSRandomization,
 		TrafficShapingParameters:    offerRequest.TrafficShapingParameters,
@@ -1000,7 +1014,15 @@ func (b *Broker) handleProxyAnswer(
 		return nil, errors.Trace(err)
 	}
 
-	logFields, err = answerRequest.ValidateAndGetLogFields(
+	// The filtered SDP is the request SDP with any invalid (bogon, unexpected
+	// GeoIP) ICE candidates filtered out. In some cases, proxies cannot
+	// avoid submitting invalid candidates (see comment in
+	// processSDPAddresses), so all invalid candidates are removed and the
+	// remaining SDP is used. Filtered candidate information is logged in
+	// logFields.
+
+	var filteredSDP []byte
+	filteredSDP, logFields, err = answerRequest.ValidateAndGetLogFields(
 		b.config.LookupGeoIP,
 		b.config.APIParameterValidator,
 		b.config.APIParameterLogFieldFormatter,
@@ -1009,6 +1031,9 @@ func (b *Broker) handleProxyAnswer(
 		return nil, errors.Trace(err)
 	}
 
+	answerSDP := answerRequest.ProxyAnswerSDP
+	answerSDP.SDP = string(filteredSDP)
+
 	if answerRequest.AnswerError != "" {
 
 		// The proxy failed to create an answer.
@@ -1029,7 +1054,7 @@ func (b *Broker) handleProxyAnswer(
 			ProxyID:                      initiatorID,
 			ConnectionID:                 answerRequest.ConnectionID,
 			SelectedProxyProtocolVersion: answerRequest.SelectedProxyProtocolVersion,
-			ProxyAnswerSDP:               answerRequest.ProxyAnswerSDP,
+			ProxyAnswerSDP:               answerSDP,
 		}
 
 		err = b.matcher.Answer(proxyAnswer)

psiphon/common/inproxy/inproxy_disabled.go

@@ -119,8 +119,9 @@ func (conn *webRTCConn) GetMetrics() common.LogFields {
 }
 
 type webRTCSDPMetrics struct {
-	iceCandidateTypes []ICECandidateType
-	hasIPv6           bool
+	iceCandidateTypes     []ICECandidateType
+	hasIPv6               bool
+	filteredICECandidates []string
 }
 
 func newWebRTCConnWithOffer(
@@ -139,12 +140,12 @@ func newWebRTCConnWithAnswer(
 	return nil, WebRTCSessionDescription{}, nil, errors.Trace(errNotEnabled)
 }
 
-func validateSDPAddresses(
+func filterSDPAddresses(
 	encodedSDP []byte,
 	errorOnNoCandidates bool,
 	lookupGeoIP LookupGeoIP,
-	expectedGeoIPData common.GeoIPData) (*webRTCSDPMetrics, error) {
-	return nil, errors.Trace(errNotEnabled)
+	expectedGeoIPData common.GeoIPData) ([]byte, *webRTCSDPMetrics, error) {
+	return nil, nil, errors.Trace(errNotEnabled)
 }
 
 func initPortMapper(coordinator WebRTCDialCoordinator) {

psiphon/common/inproxy/inproxy_test.go

@@ -102,6 +102,7 @@ func runTestInproxy() error {
 
 	// Enable test to run without requiring host firewall exceptions
 	SetAllowBogonWebRTCConnections(true)
+	defer SetAllowBogonWebRTCConnections(false)
 
 	// Init logging and profiling