Bug fixes

- Ensure a nil DataChannelTrafficShapingParameters is sent indicating traffic
  shaping is not enabled.
- HandleTacticsPayload now returns a nil record when tactics haven't changed,
  skipping no-op tactics parameter applications in the API handshake and
  broker response handlers.
- Add missing parameters to psiphon.Config.setDialParametersHash.
- Avoid reflect overwrites in ParametersAccessor vararg getters.
- Log requested network type (IPv4/IPv6/both) in resolver "no IP for network"
  error message.

psiphon/common/parameters/parameters.go

@@ -1618,14 +1618,14 @@ func (p ParametersAccessor) String(name string) string {
 // Strings returns a []string parameter value. If multiple parameter names are
 // specified, the first name with a non-empty value is used.
 func (p ParametersAccessor) Strings(names ...string) []string {
-	value := []string{}
 	for _, name := range names {
+		value := []string{}
 		p.snapshot.getValue(name, &value)
 		if len(value) > 0 {
-			break
+			return value
 		}
 	}
-	return value
+	return []string{}
 }
 
 // Int returns an int parameter value.
@@ -1982,14 +1982,14 @@ func (p ParametersAccessor) ConjureTransports(name string) protocol.ConjureTrans
 // multiple parameter names are specified, the first name with a non-empty
 // value is used.
 func (p ParametersAccessor) InproxyBrokerSpecs(names ...string) InproxyBrokerSpecsValue {
-	value := InproxyBrokerSpecsValue{}
 	for _, name := range names {
+		value := InproxyBrokerSpecsValue{}
 		p.snapshot.getValue(name, &value)
 		if len(value) > 0 {
-			break
+			return value
 		}
 	}
-	return value
+	return InproxyBrokerSpecsValue{}
 }
 
 // InproxyBrokerSpecs returns a InproxyBrokerSpecs parameter value.

psiphon/common/resolver/resolver.go

@@ -500,7 +500,7 @@ func (r *Resolver) ResolveAddress(
 	}
 
 	if index == -1 {
-		return "", errors.TraceNew("no IP for network")
+		return "", errors.Tracef("no IP for network '%s'", network)
 	}
 
 	return net.JoinHostPort(IPs[index].String(), port), nil

psiphon/common/tactics/tactics.go

@@ -1305,12 +1305,17 @@ func SetTacticsAPIParameters(
 	return nil
 }
 
-// HandleTacticsPayload updates the stored tactics with the given payload.
-// If the payload has a new tag/tactics, this is stored and a new expiry
-// time is set. If the payload has the same tag, the existing tactics are
-// retained and the expiry is extended using the previous TTL.
-// HandleTacticsPayload is called by the Psiphon client to handle the
-// tactics payload in the handshake response.
+// HandleTacticsPayload updates the stored tactics with the given payload. If
+// the payload has a new tag/tactics, this is stored and a new expiry time is
+// set. If the payload has the same tag, the existing tactics are retained,
+// the expiry is extended using the previous TTL, and a nil record is
+// rerturned.
+//
+// HandleTacticsPayload is called by the Psiphon client to handle the tactics
+// payload in the API handshake and inproxy broker responses. As the Psiphon
+// client has already called UseStoredTactics/FetchTactics and applied
+// tactics, the nil record return value allows the caller to skip an
+// unnecessary tactics parameters application.
 func HandleTacticsPayload(
 	storer Storer,
 	networkID string,
@@ -1339,18 +1344,27 @@ func HandleTacticsPayload(
 		return nil, errors.Trace(err)
 	}
 
-	err = applyTacticsPayload(storer, networkID, record, payload)
+	newTactics, err := applyTacticsPayload(storer, networkID, record, payload)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 
-	// TODO: if tags match, just set an expiry record, not the whole tactics record?
+	// Store the tactics record, which may contain new tactics, and always
+	// contains an extended TTL.
+	//
+	// TODO: if tags match, just set an expiry record, not the whole tactics
+	// record?
 
 	err = setStoredTacticsRecord(storer, networkID, record)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 
+	if !newTactics {
+		// Don't return a tactics record when the tactics have not changed.
+		record = nil
+	}
+
 	return record, nil
 }
 
@@ -1516,7 +1530,7 @@ func FetchTactics(
 		return nil, errors.Trace(err)
 	}
 
-	err = applyTacticsPayload(storer, networkID, record, payload)
+	_, err = applyTacticsPayload(storer, networkID, record, payload)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
@@ -1680,10 +1694,12 @@ func applyTacticsPayload(
 	storer Storer,
 	networkID string,
 	record *Record,
-	payload *Payload) error {
+	payload *Payload) (bool, error) {
+
+	newTactics := false
 
 	if payload.Tag == "" {
-		return errors.TraceNew("invalid tag")
+		return newTactics, errors.TraceNew("invalid tag")
 	}
 
 	// Replace the tactics data when the tags differ.
@@ -1705,35 +1721,37 @@ func applyTacticsPayload(
 		// as the JSON "null".
 		if payload.Tactics == nil ||
 			bytes.Equal(payload.Tactics, []byte("null")) {
-			return errors.TraceNew("missing tactics")
+			return newTactics, errors.TraceNew("missing tactics")
 		}
 
 		record.Tag = payload.Tag
 		record.Tactics = Tactics{}
 		err := json.Unmarshal(payload.Tactics, &record.Tactics)
 		if err != nil {
-			return errors.Trace(err)
+			return newTactics, errors.Trace(err)
 		}
+
+		newTactics = true
 	}
 
 	// Note: record.Tactics.TTL is validated by server
 	ttl, err := time.ParseDuration(record.Tactics.TTL)
 	if err != nil {
-		return errors.Trace(err)
+		return newTactics, errors.Trace(err)
 	}
 
 	if ttl <= 0 {
-		return errors.TraceNew("invalid TTL")
+		return newTactics, errors.TraceNew("invalid TTL")
 	}
 	if record.Tactics.Probability <= 0.0 {
-		return errors.TraceNew("invalid probability")
+		return newTactics, errors.TraceNew("invalid probability")
 	}
 
 	// Set or extend the expiry.
 
 	record.Expiry = time.Now().UTC().Add(ttl)
 
-	return nil
+	return newTactics, nil
 }
 
 func setStoredTacticsRecord(

psiphon/config.go

@@ -3087,6 +3087,14 @@ func (config *Config) setDialParametersHash() {
 		hash.Write([]byte("InproxyBrokerSpecs"))
 		hash.Write([]byte(fmt.Sprintf("%+v", config.InproxyBrokerSpecs)))
 	}
+	if len(config.InproxyProxyBrokerSpecs) > 0 {
+		hash.Write([]byte("InproxyProxyBrokerSpecs"))
+		hash.Write([]byte(fmt.Sprintf("%+v", config.InproxyProxyBrokerSpecs)))
+	}
+	if len(config.InproxyClientBrokerSpecs) > 0 {
+		hash.Write([]byte("InproxyClientBrokerSpecs"))
+		hash.Write([]byte(fmt.Sprintf("%+v", config.InproxyClientBrokerSpecs)))
+	}
 	if config.InproxyReplayBrokerDialParametersTTLSeconds != nil {
 		hash.Write([]byte("InproxyReplayBrokerDialParametersTTLSeconds"))
 		binary.Write(hash, binary.LittleEndian, int64(*config.InproxyReplayBrokerDialParametersTTLSeconds))
@@ -3155,6 +3163,14 @@ func (config *Config) setDialParametersHash() {
 		hash.Write([]byte("InproxyDataChannelTrafficShapingParameters"))
 		hash.Write([]byte(fmt.Sprintf("%+v", config.InproxyDataChannelTrafficShapingParameters)))
 	}
+	if config.InproxySTUNServerAddresses != nil {
+		hash.Write([]byte("InproxySTUNServerAddresses"))
+		hash.Write([]byte(fmt.Sprintf("%+v", config.InproxyProxySTUNServerAddresses)))
+	}
+	if config.InproxySTUNServerAddressesRFC5780 != nil {
+		hash.Write([]byte("InproxySTUNServerAddressesRFC5780"))
+		hash.Write([]byte(fmt.Sprintf("%+v", config.InproxyProxySTUNServerAddressesRFC5780)))
+	}
 	if config.InproxyProxySTUNServerAddresses != nil {
 		hash.Write([]byte("InproxyProxySTUNServerAddresses"))
 		hash.Write([]byte(fmt.Sprintf("%+v", config.InproxyProxySTUNServerAddresses)))

psiphon/inproxy.go

@@ -1401,7 +1401,7 @@ func (w *InproxyWebRTCDialInstance) DoDTLSRandomization() bool {
 
 // Implements the inproxy.WebRTCDialCoordinator interface.
 func (w *InproxyWebRTCDialInstance) DataChannelTrafficShapingParameters() *inproxy.DataChannelTrafficShapingParameters {
-	return &w.webRTCDialParameters.DataChannelTrafficShapingParameters
+	return w.webRTCDialParameters.DataChannelTrafficShapingParameters
 }
 
 // Implements the inproxy.WebRTCDialCoordinator interface.
@@ -1807,7 +1807,7 @@ func (dialParams *InproxySTUNDialParameters) GetMetrics() common.LogFields {
 // WebRTC dial parameter selection and replay.
 type InproxyWebRTCDialParameters struct {
 	RootObfuscationSecret               inproxy.ObfuscationSecret
-	DataChannelTrafficShapingParameters inproxy.DataChannelTrafficShapingParameters
+	DataChannelTrafficShapingParameters *inproxy.DataChannelTrafficShapingParameters
 	DoDTLSRandomization                 bool
 }
 
@@ -1820,17 +1820,18 @@ func MakeInproxyWebRTCDialParameters(
 		return nil, errors.Trace(err)
 	}
 
-	var trafficShapingParameters parameters.InproxyDataChannelTrafficShapingParametersValue
+	var trafficSharingParams inproxy.DataChannelTrafficShapingParameters
 	if p.WeightedCoinFlip(parameters.InproxyDataChannelTrafficShapingProbability) {
-		trafficShapingParameters = p.InproxyDataChannelTrafficShapingParameters(
-			parameters.InproxyDataChannelTrafficShapingParameters)
+		trafficSharingParams = inproxy.DataChannelTrafficShapingParameters(
+			p.InproxyDataChannelTrafficShapingParameters(
+				parameters.InproxyDataChannelTrafficShapingParameters))
 	}
 
 	doDTLSRandomization := p.WeightedCoinFlip(parameters.InproxyDTLSRandomizationProbability)
 
 	return &InproxyWebRTCDialParameters{
 		RootObfuscationSecret:               rootObfuscationSecret,
-		DataChannelTrafficShapingParameters: inproxy.DataChannelTrafficShapingParameters(trafficShapingParameters),
+		DataChannelTrafficShapingParameters: &trafficSharingParams,
 		DoDTLSRandomization:                 doDTLSRandomization,
 	}, nil
 }