Retain replay if in-proxy dial phase fails

psiphon/common/parameters/parameters.go

@@ -487,6 +487,7 @@ const (
 	InproxyProxyOnBrokerClientFailedRetryPeriod        = "InproxyProxyOnBrokerClientFailedRetryPeriod"
 	InproxyProxyIncompatibleNetworkTypes               = "InproxyProxyIncompatibleNetworkTypes"
 	InproxyClientIncompatibleNetworkTypes              = "InproxyClientIncompatibleNetworkTypes"
+	InproxyReplayRetainFailedProbability               = "InproxyReplayRetainFailedProbability"
 	InproxyEnableProxyQuality                          = "InproxyEnableProxyQuality"
 	InproxyEnableProxyQualityClientRegions             = "InproxyEnableProxyQualityClientRegions"
 	InproxyProxyQualityTargetUpstreamBytes             = "InproxyProxyQualityTargetUpstreamBytes"
@@ -1055,6 +1056,7 @@ var defaultParameters = map[string]struct {
 	InproxyProxyOnBrokerClientFailedRetryPeriod:        {value: 30 * time.Second, minimum: time.Duration(0)},
 	InproxyProxyIncompatibleNetworkTypes:               {value: []string{}},
 	InproxyClientIncompatibleNetworkTypes:              {value: []string{}},
+	InproxyReplayRetainFailedProbability:               {value: 1.0, minimum: 0.0},
 
 	InproxyEnableProxyQuality:                        {value: false, flags: serverSideOnly},
 	InproxyEnableProxyQualityClientRegions:           {value: []string{}, flags: serverSideOnly},

psiphon/dialParameters.go

@@ -24,6 +24,7 @@ import (
 	"context"
 	"crypto/md5"
 	"encoding/binary"
+	std_errors "errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -1820,7 +1821,7 @@ func (dialParams *DialParameters) Succeeded() {
 	}
 }
 
-func (dialParams *DialParameters) Failed(config *Config) {
+func (dialParams *DialParameters) Failed(config *Config, dialErr error) {
 
 	// When a tunnel fails, and the dial is a replay, clear the stored dial
 	// parameters which are now presumed to be blocked, impaired or otherwise
@@ -1834,9 +1835,30 @@ func (dialParams *DialParameters) Failed(config *Config) {
 	// probability; this is intended to help mitigate false positive failures due
 	// to, e.g., temporary network disruptions or server load limiting.
 
+	// When dialing in-proxy tunnel protocols, replay is retained when the
+	// dial fails in the inproxyDial phase. This phase includes the broker
+	// request and the 1st hop WebRTC connection. The broker client has its
+	// own replay layer. The WebRTC peer is an ephemeral proxy which cannot
+	// be replayed and clearing replay for individual proxy failures unfairly
+	// deprioritizes in-proxy protocol selection overall. Any replay TTL
+	// remains in effect and will eventually clear the replay if there is a
+	// persistent issue with the in-proxy protocol. Replay is still cleared
+	// immediately for post-inproxyDial failures, as these can be due to more
+	// permanent conditions, such as a retired Psiphon server.
+	//
+	// Limitation: with this retention logic, InproxySTUNDialParameters and
+	// InproxyWebRTCDialParameters are retained and replayed, although it may
+	// be more optimal to replay in-proxy protocols while still reselecting
+	// different STUN servers and WebRTC properties.
+
+	p := config.GetParameters().Get()
+
+	var inproxyDialErr *inproxyDialFailedError
+	isInproxyDialErr := std_errors.As(dialErr, &inproxyDialErr)
+
 	if dialParams.IsReplay &&
-		!config.GetParameters().Get().WeightedCoinFlip(
-			parameters.ReplayRetainFailedProbability) {
+		!p.WeightedCoinFlip(parameters.ReplayRetainFailedProbability) &&
+		(!isInproxyDialErr || !p.WeightedCoinFlip(parameters.InproxyReplayRetainFailedProbability)) {
 
 		NoticeInfo("Delete dial parameters for %s", dialParams.ServerEntry.GetDiagnosticID())
 		err := DeleteDialParameters(dialParams.ServerEntry.IpAddress, dialParams.NetworkID)

psiphon/dialParameters_test.go

@@ -31,6 +31,7 @@ import (
 	"time"
 
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
+	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/errors"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/parameters"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/prng"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/protocol"
@@ -293,7 +294,9 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 	// Test: no replay after dial reported to fail
 
-	dialParams.Failed(clientConfig)
+	dialErr := errors.TraceNew("dial error")
+
+	dialParams.Failed(clientConfig, dialErr)
 
 	dialParams, err = MakeDialParameters(
 		clientConfig, steeringIPCache, nil, nil, nil, canReplay, selectProtocol, serverEntries[0], nil, nil, false, 0, 0)
@@ -659,7 +662,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 			t.Fatalf("MakeDialParameters failed: %s", err)
 		}
 
-		dialParams.Failed(clientConfig)
+		dialParams.Failed(clientConfig, dialErr)
 
 		getCacheKey := func() string {
 			return fmt.Sprintf("%s %s %s", testNetworkID, frontingProviderID, tunnelProtocol)
@@ -755,7 +758,7 @@ func runDialParametersAndReplay(t *testing.T, tunnelProtocol string) {
 
 		// Test: steering IP cleared from cache after failure
 
-		dialParams.Failed(clientConfig)
+		dialParams.Failed(clientConfig, dialErr)
 
 		_, ok := steeringIPCache.Get(getCacheKey())
 		if ok {

psiphon/tunnel.go

@@ -185,7 +185,7 @@ func (tunnel *Tunnel) Activate(
 	baseCtx := ctx
 	defer func() {
 		if !activationSucceeded && baseCtx.Err() != context.Canceled {
-			tunnel.dialParams.Failed(tunnel.config)
+			tunnel.dialParams.Failed(tunnel.config, retErr)
 			if tunnel.extraFailureAction != nil {
 				tunnel.extraFailureAction()
 			}
@@ -830,7 +830,7 @@ func dialTunnel(
 	var extraFailureAction func()
 	defer func() {
 		if !dialSucceeded && baseCtx.Err() != context.Canceled {
-			dialParams.Failed(config)
+			dialParams.Failed(config, retErr)
 			if extraFailureAction != nil {
 				extraFailureAction()
 			}
@@ -1518,6 +1518,14 @@ func makeInproxyTCPDialer(
 	}
 }
 
+type inproxyDialFailedError struct {
+	err error
+}
+
+func (e inproxyDialFailedError) Error() string {
+	return e.err.Error()
+}
+
 // dialInproxy performs the in-proxy dial and returns the resulting conn for
 // use as an underlying conn for the 2nd hop protocol. The in-proxy dial
 // first connects to the broker (or reuses an existing connection) to match
@@ -1525,7 +1533,15 @@ func makeInproxyTCPDialer(
 func dialInproxy(
 	ctx context.Context,
 	config *Config,
-	dialParams *DialParameters) (*inproxy.ClientConn, error) {
+	dialParams *DialParameters) (retConn *inproxy.ClientConn, retErr error) {
+
+	defer func() {
+		// Wrap all returned errors with inproxyDialFailedError so callers can
+		// check for dialInproxy failures within nested errors.
+		if retErr != nil {
+			retErr = &inproxyDialFailedError{err: retErr}
+		}
+	}()
 
 	isProxy := false
 	webRTCDialInstance, err := NewInproxyWebRTCDialInstance(