Changes based on feedback

psiphon/common/parameters/parameters.go

@@ -372,10 +372,10 @@ const (
 	SteeringIPCacheMaxEntries                          = "SteeringIPCacheMaxEntries"
 	SteeringIPProbability                              = "SteeringIPProbability"
 	ServerDiscoveryStrategy                            = "ServerDiscoveryStrategy"
-	TransferURLReplayDialParametersTTL                 = "TransferURLReplayDialParametersTTL"
-	TransferURLReplayUpdateFrequency                   = "TransferURLReplayUpdateFrequency"
-	TransferURLReplayDialParametersProbability         = "TransferURLReplayDialParametersProbability"
-	TransferURLReplayRetainFailedProbability           = "TransferURLReplayRetainFailedProbability"
+	FrontedHTTPClientReplayDialParametersTTL           = "FrontedHTTPClientReplayDialParametersTTL"
+	FrontedHTTPClientReplayUpdateFrequency             = "FrontedHTTPClientReplayUpdateFrequency"
+	FrontedHTTPClientReplayDialParametersProbability   = "FrontedHTTPClientReplayDialParametersProbability"
+	FrontedHTTPClientReplayRetainFailedProbability     = "FrontedHTTPClientReplayRetainFailedProbability"
 	InproxyAllowProxy                                  = "InproxyAllowProxy"
 	InproxyAllowClient                                 = "InproxyAllowClient"
 	InproxyAllowDomainFrontedDestinations              = "InproxyAllowDomainFrontedDestinations"
@@ -880,10 +880,10 @@ var defaultParameters = map[string]struct {
 
 	ServerDiscoveryStrategy: {value: "", flags: serverSideOnly},
 
-	TransferURLReplayDialParametersTTL:         {value: 24 * time.Hour, minimum: time.Duration(0)},
-	TransferURLReplayUpdateFrequency:           {value: 5 * time.Minute, minimum: time.Duration(0)},
-	TransferURLReplayDialParametersProbability: {value: 1.0, minimum: 0.0},
-	TransferURLReplayRetainFailedProbability:   {value: 0.5, minimum: 0.0},
+	FrontedHTTPClientReplayDialParametersTTL:         {value: 24 * time.Hour, minimum: time.Duration(0)},
+	FrontedHTTPClientReplayUpdateFrequency:           {value: 5 * time.Minute, minimum: time.Duration(0)},
+	FrontedHTTPClientReplayDialParametersProbability: {value: 1.0, minimum: 0.0},
+	FrontedHTTPClientReplayRetainFailedProbability:   {value: 0.5, minimum: 0.0},
 
 	// For inproxy tactics, there is no proxyOnly flag, since Psiphon apps may
 	// run both clients and inproxy proxies.

psiphon/config.go

@@ -971,6 +971,13 @@ type Config struct {
 	SteeringIPCacheMaxEntries *int
 	SteeringIPProbability     *float64
 
+	// FrontedHTTPClientReplayDialParametersTTL and other TransferURL fields are for
+	// testing purposes only.
+	FrontedHTTPClientReplayDialParametersTTLSeconds  *int
+	FrontedHTTPClientReplayUpdateFrequencySeconds    *int
+	FrontedHTTPClientReplayDialParametersProbability *float64
+	FrontedHTTPClientReplayRetainFailedProbability   *float64
+
 	// The following in-proxy fields are for testing purposes only.
 	InproxyAllowProxy                                       *bool
 	InproxyAllowClient                                      *bool
@@ -2425,6 +2432,22 @@ func (config *Config) makeConfigParameters() map[string]interface{} {
 		applyParameters[parameters.SteeringIPProbability] = *config.SteeringIPProbability
 	}
 
+	if config.FrontedHTTPClientReplayDialParametersTTLSeconds != nil {
+		applyParameters[parameters.FrontedHTTPClientReplayDialParametersTTL] = fmt.Sprintf("%ds", *config.FrontedHTTPClientReplayDialParametersTTLSeconds)
+	}
+
+	if config.FrontedHTTPClientReplayUpdateFrequencySeconds != nil {
+		applyParameters[parameters.FrontedHTTPClientReplayUpdateFrequency] = fmt.Sprintf("%ds", *config.FrontedHTTPClientReplayUpdateFrequencySeconds)
+	}
+
+	if config.FrontedHTTPClientReplayDialParametersProbability != nil {
+		applyParameters[parameters.FrontedHTTPClientReplayDialParametersProbability] = *config.FrontedHTTPClientReplayDialParametersProbability
+	}
+
+	if config.FrontedHTTPClientReplayRetainFailedProbability != nil {
+		applyParameters[parameters.FrontedHTTPClientReplayRetainFailedProbability] = *config.FrontedHTTPClientReplayRetainFailedProbability
+	}
+
 	if config.InproxyPersonalPairingConnectionWorkerPoolSize != 0 {
 		applyParameters[parameters.InproxyPersonalPairingConnectionWorkerPoolSize] = config.InproxyPersonalPairingConnectionWorkerPoolSize
 	}

psiphon/frontedHTTPClientInstance.go → psiphon/frontedHTTP.go

@@ -10,6 +10,7 @@ import (
 	"sync"
 	"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"
@@ -67,17 +68,17 @@ func newFrontedHTTPClientInstance(
 	var spec *parameters.FrontingSpec
 	var dialParams *frontedHTTPDialParameters
 
-	// Replay is disabled when the TTL, TransferURLReplayDialParametersTTL,
+	// Replay is disabled when the TTL, FrontedHTTPClientReplayDialParametersTTL,
 	// is 0.
 	now := time.Now()
-	ttl := p.Duration(parameters.TransferURLReplayDialParametersTTL)
+	ttl := p.Duration(parameters.FrontedHTTPClientReplayDialParametersTTL)
 	networkID := config.GetNetworkID()
 
 	// Replay is disabled if there is an active tunnel.
 	replayEnabled := tunnel == nil &&
 		ttl > 0 &&
 		!config.DisableReplay &&
-		prng.FlipWeightedCoin(p.Float(parameters.TransferURLReplayDialParametersProbability))
+		prng.FlipWeightedCoin(p.Float(parameters.FrontedHTTPClientReplayDialParametersProbability))
 
 	if replayEnabled {
 		selectFirstCandidate := false
@@ -146,8 +147,8 @@ func newFrontedHTTPClientInstance(
 		frontedHTTPDialParameters: dialParams,
 		replayEnabled:             replayEnabled,
 
-		replayRetainFailedProbability: p.Float(parameters.TransferURLReplayRetainFailedProbability),
-		replayUpdateFrequency:         p.Duration(parameters.TransferURLReplayUpdateFrequency),
+		replayRetainFailedProbability: p.Float(parameters.FrontedHTTPClientReplayRetainFailedProbability),
+		replayUpdateFrequency:         p.Duration(parameters.FrontedHTTPClientReplayUpdateFrequency),
 	}, nil
 }
 
@@ -196,7 +197,19 @@ func (f *frontedHTTPClientInstance) RoundTrip(request *http.Request) (*http.Resp
 	// response body.
 	response.Body = newFrontedHTTPClientResponseReadCloser(f, body)
 
-	if response.StatusCode == http.StatusOK {
+	// HTTP status codes other than 200 may indicate success depending on the
+	// semantics of the operation. E.g., resumeable downloads are considered
+	// successful if the HTTP server returns 200, 206, 304, 412, or 416.
+	//
+	// TODO: have the caller determine success and failure cases because this
+	// is not always determined by the HTTP status code; e.g., HTTP server
+	// returns 200 but payload signature check fails.
+	if response.StatusCode == http.StatusOK ||
+		response.StatusCode == http.StatusPartialContent ||
+		response.StatusCode == http.StatusRequestedRangeNotSatisfiable ||
+		response.StatusCode == http.StatusPreconditionFailed ||
+		response.StatusCode == http.StatusNotModified {
+
 		f.frontedHTTPClientRoundTripperSucceeded()
 	} else {
 		// TODO: do not clear replay parameters on temporary round tripper
@@ -309,3 +322,124 @@ func hashFrontingSpec(spec *parameters.FrontingSpec) []byte {
 		uint64(xxhash.Sum64String(fmt.Sprintf("%+v", spec))))
 	return hash[:]
 }
+
+// frontedHTTPDialParameters represents a selected fronting transport and dial
+// parameters.
+//
+// frontedHTTPDialParameters is used to configure dialers; as a persistent
+// record to store successful dial parameters for replay; and to report dial
+// stats in notices and Psiphon API calls.
+//
+// frontedHTTPDialParameters is similar to tunnel DialParameters, but is
+// specific to fronted HTTP. It should be used for all fronted HTTP dials,
+// apart from the tunnel DialParameters cases.
+type frontedHTTPDialParameters struct {
+	isReplay bool `json:"-"`
+
+	LastUsedTimestamp        time.Time
+	LastUsedFrontingSpecHash []byte
+
+	FrontedMeekDialParameters *FrontedMeekDialParameters
+}
+
+// makeFrontedHTTPDialParameters creates a new frontedHTTPDialParameters for
+// configuring a fronted HTTP client, including selecting a fronting transport
+// and all the various protocol attributes.
+//
+// payloadSecure must only be set if all HTTP plaintext payloads sent through
+// the returned net/http.Client will be wrapped in their own transport security
+// layer, which permits skipping of server certificate verification.
+func makeFrontedHTTPDialParameters(
+	config *Config,
+	p parameters.ParametersAccessor,
+	tunnel *Tunnel,
+	frontingSpec *parameters.FrontingSpec,
+	selectedFrontingProviderID func(string),
+	useDeviceBinder,
+	skipVerify,
+	disableSystemRootCAs,
+	payloadSecure bool) (*frontedHTTPDialParameters, error) {
+
+	currentTimestamp := time.Now()
+
+	dialParams := &frontedHTTPDialParameters{
+		LastUsedTimestamp:        currentTimestamp,
+		LastUsedFrontingSpecHash: hashFrontingSpec(frontingSpec),
+	}
+
+	var err error
+	dialParams.FrontedMeekDialParameters, err = makeFrontedMeekDialParameters(
+		config,
+		p,
+		tunnel,
+		parameters.FrontingSpecs{frontingSpec},
+		selectedFrontingProviderID,
+		useDeviceBinder,
+		skipVerify,
+		disableSystemRootCAs,
+		payloadSecure,
+	)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	// Initialize Dial/MeekConfigs to be passed to the corresponding dialers.
+
+	err = dialParams.prepareDialConfigs(
+		config,
+		p,
+		false,
+		tunnel,
+		skipVerify,
+		disableSystemRootCAs,
+		useDeviceBinder,
+		payloadSecure)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	return dialParams, nil
+}
+
+// prepareDialConfigs is called for both new and replayed dial parameters.
+func (dialParams *frontedHTTPDialParameters) prepareDialConfigs(
+	config *Config,
+	p parameters.ParametersAccessor,
+	isReplay bool,
+	tunnel *Tunnel,
+	useDeviceBinder,
+	skipVerify,
+	disableSystemRootCAs,
+	payloadSecure bool) error {
+
+	dialParams.isReplay = isReplay
+
+	if isReplay {
+
+		// Initialize Dial/MeekConfigs to be passed to the corresponding dialers.
+
+		err := dialParams.FrontedMeekDialParameters.prepareDialConfigs(
+			config, p, tunnel, nil, useDeviceBinder, skipVerify,
+			disableSystemRootCAs, payloadSecure)
+		if err != nil {
+			return errors.Trace(err)
+		}
+	}
+
+	return nil
+}
+
+// GetMetrics implements the common.MetricsSource interface and returns log
+// fields detailing the fronted HTTP dial parameters.
+func (dialParams *frontedHTTPDialParameters) GetMetrics() common.LogFields {
+
+	logFields := dialParams.FrontedMeekDialParameters.GetMetrics("")
+
+	isReplay := "0"
+	if dialParams.isReplay {
+		isReplay = "1"
+	}
+	logFields["is_replay"] = isReplay
+
+	return logFields
+}

psiphon/frontingDialParameters.go

@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"strconv"
 	"sync/atomic"
-	"time"
 
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/errors"
@@ -19,119 +18,29 @@ import (
 	"golang.org/x/net/bpf"
 )
 
-// frontedHTTPDialParameters represents a selected fronting transport and dial
-// parameters.
+// FrontedMeekDialParameters represents a selected fronting transport and all
+// the related protocol attributes, many chosen at random, for a fronted dial
+// attempt.
 //
-// frontedHTTPDialParameters is used to configure dialers; as a persistent
-// record to store successful dial parameters for replay; and to report dial
-// stats in notices and Psiphon API calls.
+// FrontedMeekDialParameters is used:
+// - to configure dialers
+// - as a persistent record to store successful dial parameters for replay
+// - to report dial stats in notices and Psiphon API calls.
 //
-// frontedHTTPDialParameters is similar to tunnel DialParameters, but is
-// specific to TransferURLs.
-type frontedHTTPDialParameters struct {
-	isReplay bool `json:"-"`
-
-	LastUsedTimestamp        time.Time
-	LastUsedFrontingSpecHash []byte
-
-	FrontedMeekDialParameters *FrontedMeekDialParameters
-}
-
-// makeFrontedHTTPDialParameters creates a new frontedHTTPDialParameters for
-// configuring a fronted HTTP client, including selecting a fronting transport
-// and all the various protocol attributes.
+// FrontedMeekDialParameters is similar to tunnel DialParameters, but is
+// specific to fronted meek. It should be used for all fronted meek dials,
+// apart from the tunnel DialParameters cases.
 //
-// payloadSecure must only be set if all HTTP plaintext payloads sent through
-// the returned net/http.Client will be wrapped in their own transport security
-// layer, which permits skipping of server certificate verification.
-func makeFrontedHTTPDialParameters(
-	config *Config,
-	p parameters.ParametersAccessor,
-	tunnel *Tunnel,
-	frontingSpec *parameters.FrontingSpec,
-	selectedFrontingProviderID func(string),
-	useDeviceBinder,
-	skipVerify,
-	disableSystemRootCAs,
-	payloadSecure bool) (*frontedHTTPDialParameters, error) {
-
-	currentTimestamp := time.Now()
-
-	dialParams := &frontedHTTPDialParameters{
-		LastUsedTimestamp:        currentTimestamp,
-		LastUsedFrontingSpecHash: hashFrontingSpec(frontingSpec),
-	}
-
-	var err error
-	dialParams.FrontedMeekDialParameters, err = makeFrontedMeekDialParameters(
-		config,
-		p,
-		tunnel,
-		parameters.FrontingSpecs{frontingSpec},
-		selectedFrontingProviderID,
-		useDeviceBinder,
-		skipVerify,
-		disableSystemRootCAs,
-		payloadSecure,
-	)
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	// Initialize Dial/MeekConfigs to be passed to the corresponding dialers.
-
-	err = dialParams.prepareDialConfigs(
-		config,
-		p,
-		false,
-		tunnel,
-		skipVerify,
-		disableSystemRootCAs,
-		useDeviceBinder,
-		payloadSecure)
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	return dialParams, nil
-}
-
-// prepareDialConfigs is called for both new and replayed dial parameters.
-func (dialParams *frontedHTTPDialParameters) prepareDialConfigs(
-	config *Config,
-	p parameters.ParametersAccessor,
-	isReplay bool,
-	tunnel *Tunnel,
-	useDeviceBinder,
-	skipVerify,
-	disableSystemRootCAs,
-	payloadSecure bool) error {
-
-	dialParams.isReplay = isReplay
-
-	if isReplay {
-
-		// Initialize Dial/MeekConfigs to be passed to the corresponding dialers.
-
-		err := dialParams.FrontedMeekDialParameters.prepareDialConfigs(
-			config, p, tunnel, nil, useDeviceBinder, skipVerify,
-			disableSystemRootCAs, payloadSecure)
-		if err != nil {
-			return errors.Trace(err)
-		}
-	}
-
-	return nil
-}
-
-// FrontedMeekDialParameters represents a selected fronting transport and dial
-// parameters.
+// prepareDialConfigs must be called on any unmarshaled
+// FrontedMeekDialParameters. For example, when unmarshaled from a replay
+// record.
 //
-// FrontedMeekDialParameters is used to configure dialers; and to report dial
-// stats in notices and Psiphon API calls.
+// resolvedIPAddress is set asynchronously, as it is not known until the dial
+// process has begun. The atomic.Value will contain a string, initialized to
+// "", and set to the resolved IP address once that part of the dial process
+// has completed.
 //
-// FrontedMeekDialParameters is similar to tunnel DialParameters, but is
-// specific to the in-proxy broker dial phase and TransferURLs.
+// FrontedMeekDialParameters is not safe for concurrent use.
 type FrontedMeekDialParameters struct {
 	NetworkLatencyMultiplier float64
 
@@ -523,49 +432,58 @@ func (f *FrontedMeekDialParameters) prepareDialConfigs(
 	return nil
 }
 
-// GetMetrics implements the common.MetricsSource interface and returns log
-// fields detailing the fronted meek dial parameters.
-func (meekDialParameters *FrontedMeekDialParameters) GetMetrics() common.LogFields {
+// GetMetrics returns log fields detailing the fronted meek dial parameters.
+// All log field names are prefixed with overridePrefix, when specified, which
+// also overrides any default prefixes.
+func (meekDialParameters *FrontedMeekDialParameters) GetMetrics(overridePrefix string) common.LogFields {
+
+	prefix := ""
+	meekPrefix := "meek_"
+
+	if overridePrefix != "" {
+		prefix = overridePrefix
+		meekPrefix = overridePrefix
+	}
 
 	logFields := make(common.LogFields)
 
-	logFields["fronting_provider_id"] = meekDialParameters.FrontingProviderID
+	logFields[prefix+"fronting_provider_id"] = meekDialParameters.FrontingProviderID
 
 	if meekDialParameters.DialAddress != "" {
-		logFields["meek_dial_address"] = meekDialParameters.DialAddress
+		logFields[meekPrefix+"dial_address"] = meekDialParameters.DialAddress
 	}
 
 	meekResolvedIPAddress := meekDialParameters.resolvedIPAddress.Load().(string)
 	if meekResolvedIPAddress != "" {
-		logFields["meek_resolved_ip_address"] = meekResolvedIPAddress
+		logFields[meekPrefix+"resolved_ip_address"] = meekResolvedIPAddress
 	}
 
 	if meekDialParameters.SNIServerName != "" {
-		logFields["meek_sni_server_name"] = meekDialParameters.SNIServerName
+		logFields[meekPrefix+"sni_server_name"] = meekDialParameters.SNIServerName
 	}
 
 	if meekDialParameters.HostHeader != "" {
-		logFields["meek_host_header"] = meekDialParameters.HostHeader
+		logFields[meekPrefix+"host_header"] = meekDialParameters.HostHeader
 	}
 
 	transformedHostName := "0"
 	if meekDialParameters.TransformedHostName {
 		transformedHostName = "1"
 	}
-	logFields["meek_transformed_host_name"] = transformedHostName
+	logFields[meekPrefix+"transformed_host_name"] = transformedHostName
 
 	if meekDialParameters.SelectedUserAgent {
-		logFields["user_agent"] = meekDialParameters.UserAgent
+		logFields[prefix+"user_agent"] = meekDialParameters.UserAgent
 	}
 
 	if meekDialParameters.FrontingTransport == protocol.FRONTING_TRANSPORT_HTTPS {
 
 		if meekDialParameters.TLSProfile != "" {
-			logFields["tls_profile"] = meekDialParameters.TLSProfile
+			logFields[prefix+"tls_profile"] = meekDialParameters.TLSProfile
 		}
 
 		if meekDialParameters.TLSVersion != "" {
-			logFields["tls_version"] =
+			logFields[prefix+"tls_version"] =
 				getTLSVersionForMetrics(meekDialParameters.TLSVersion, meekDialParameters.NoDefaultTLSSessionID)
 		}
 
@@ -573,11 +491,11 @@ func (meekDialParameters *FrontedMeekDialParameters) GetMetrics() common.LogFiel
 		if meekDialParameters.TLSFragmentClientHello {
 			tlsFragmented = "1"
 		}
-		logFields["tls_fragmented"] = tlsFragmented
+		logFields[prefix+"tls_fragmented"] = tlsFragmented
 	}
 
 	if meekDialParameters.BPFProgramName != "" {
-		logFields["client_bpf"] = meekDialParameters.BPFProgramName
+		logFields[prefix+"client_bpf"] = meekDialParameters.BPFProgramName
 	}
 
 	if meekDialParameters.ResolveParameters != nil {
@@ -588,19 +506,19 @@ func (meekDialParameters *FrontedMeekDialParameters) GetMetrics() common.LogFiel
 		if meekDialParameters.ResolveParameters.PreresolvedIPAddress != "" {
 			dialDomain, _, _ := net.SplitHostPort(meekDialParameters.meekConfig.DialAddress)
 			if meekDialParameters.ResolveParameters.PreresolvedDomain == dialDomain {
-				logFields["dns_preresolved"] = meekDialParameters.ResolveParameters.PreresolvedIPAddress
+				logFields[prefix+"dns_preresolved"] = meekDialParameters.ResolveParameters.PreresolvedIPAddress
 			}
 		}
 
 		if meekDialParameters.ResolveParameters.PreferAlternateDNSServer {
-			logFields["dns_preferred"] = meekDialParameters.ResolveParameters.AlternateDNSServer
+			logFields[prefix+"dns_preferred"] = meekDialParameters.ResolveParameters.AlternateDNSServer
 		}
 
 		if meekDialParameters.ResolveParameters.ProtocolTransformName != "" {
-			logFields["dns_transform"] = meekDialParameters.ResolveParameters.ProtocolTransformName
+			logFields[prefix+"dns_transform"] = meekDialParameters.ResolveParameters.ProtocolTransformName
 		}
 
-		logFields["dns_attempt"] = strconv.Itoa(
+		logFields[prefix+"dns_attempt"] = strconv.Itoa(
 			meekDialParameters.ResolveParameters.GetFirstAttemptWithAnswer())
 	}
 

psiphon/inproxy.go

@@ -30,7 +30,6 @@ import (
 	"net/http"
 	"net/netip"
 	"strconv"
-	"strings"
 	"sync"
 	"sync/atomic"
 	"syscall"
@@ -1169,13 +1168,7 @@ func (brokerDialParams *InproxyBrokerDialParameters) GetMetrics() common.LogFiel
 
 	// Add underlying log fields, which must be renamed to be scoped to the
 	// broker.
-	for k, v := range brokerDialParams.FrontedHTTPDialParameters.GetMetrics() {
-		// 1. Remove meek prefix, if any
-		name, _ := strings.CutPrefix(k, "meek_")
-		// 2. Add inproxy broker prefix
-		name = "inproxy_broker_" + name
-		logFields[name] = v
-	}
+	logFields.Add(brokerDialParams.FrontedHTTPDialParameters.GetMetrics("inproxy_broker_"))
 
 	logFields["inproxy_broker_transport"] = brokerDialParams.FrontedHTTPDialParameters.FrontingTransport
 

psiphon/net.go

@@ -426,11 +426,7 @@ func makeFrontedHTTPClient(
 	}
 
 	getParams := func() common.APIParameters {
-		params := make(common.APIParameters)
-		for k, v := range frontedHTTPClient.frontedHTTPDialParameters.FrontedMeekDialParameters.GetMetrics() {
-			params[k] = v
-		}
-		return params
+		return common.APIParameters(frontedHTTPClient.frontedHTTPDialParameters.GetMetrics())
 	}
 
 	return &http.Client{