Fixes from initial device testing

- Android: use anet.InterfaceAddrsByInterface as anet.Interfaces.Addrs is
  still subject to OS restrictions
- Android: wire up UDPConn callback for BindToDevice behavior in
  pionNetwork.Interfaces
- Android: don't ignore FlagPointToPoint network interfaces; mobile interfaces
  have this property
- Don't back-off proxy announcements after a no-match
- Allow client offer SDP to have no ICE candidates; proxy answer SDP must
  still have a candidate
- Increase default client/proxy-side timeouts to exceed broker timeouts
- Fix marshaling of error message fields in notices
- Fix nil dialParams logic in getBaseAPIParameters
- Fix proxyOneClient error path nil panics
- Rename inproxy-broker log to inproxy_broker
- Remove redundant inproxy.WebRTCConn.setDataChannel callback inits

psiphon/common/inproxy/api.go

@@ -561,9 +561,13 @@ func (request *ClientOfferRequest) ValidateAndGetLogFields(
 		return 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([]byte(request.ClientOfferSDP.SDP), lookupGeoIP, geoIPData)
+	sdpMetrics, err := ValidateSDPAddresses(
+		[]byte(request.ClientOfferSDP.SDP), errorOnNoCandidates, lookupGeoIP, geoIPData)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
@@ -649,9 +653,13 @@ func (request *ProxyAnswerRequest) ValidateAndGetLogFields(
 	formatter common.APIParameterLogFieldFormatter,
 	geoIPData common.GeoIPData) (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([]byte(request.ProxyAnswerSDP.SDP), lookupGeoIP, geoIPData)
+	sdpMetrics, err := ValidateSDPAddresses(
+		[]byte(request.ProxyAnswerSDP.SDP), errorOnNoCandidates, lookupGeoIP, geoIPData)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}

psiphon/common/inproxy/broker.go

@@ -50,7 +50,7 @@ const (
 	brokerClientOfferTimeout          = 10 * time.Second
 	brokerPendingServerReportsTTL     = 60 * time.Second
 	brokerPendingServerReportsMaxSize = 100000
-	brokerMetricName                  = "inproxy-broker"
+	brokerMetricName                  = "inproxy_broker"
 )
 
 // LookupGeoIP is a callback for providing GeoIP lookup service.

psiphon/common/inproxy/coordinator.go

@@ -281,6 +281,14 @@ type WebRTCDialCoordinator interface {
 	// to mitigate the issue documented in psiphon/common.WriteTimeoutUDPConn.
 	UDPListen(ctx context.Context) (net.PacketConn, error)
 
+	// UDPConn creates a local UDP socket "connected" to the specified remote
+	// address. The socket should be excluded from VPN routing. This socket
+	// is used to determine the local address of the active interface the OS
+	// will select for the specified network ("udp4" for IPv4 or "udp6" for
+	// IPv6) and remote destination. For this use case, the socket will not
+	// be used to send network traffic.
+	UDPConn(ctx context.Context, network, remoteAddress string) (net.PacketConn, error)
+
 	// BindToDevice binds a socket, specified by the file descriptor, to an
 	// interface that isn't routed through a VPN when Psiphon is running in
 	// VPN mode. BindToDevice is used in cases where a custom dialer cannot

psiphon/common/inproxy/coordinator_test.go

@@ -322,6 +322,18 @@ func (t *testWebRTCDialCoordinator) UDPListen(_ context.Context) (net.PacketConn
 	return conn, errors.Trace(err)
 }
 
+func (t *testWebRTCDialCoordinator) UDPConn(_ context.Context, network, remoteAddress string) (net.PacketConn, error) {
+	t.mutex.Lock()
+	defer t.mutex.Unlock()
+	switch network {
+	case "udp", "udp4", "udp6":
+	default:
+		return nil, errors.TraceNew("invalid network")
+	}
+	conn, err := net.Dial(network, remoteAddress)
+	return conn.(*net.UDPConn), errors.Trace(err)
+}
+
 func (t *testWebRTCDialCoordinator) BindToDevice(fileDescriptor int) error {
 	t.mutex.Lock()
 	defer t.mutex.Unlock()

psiphon/common/inproxy/proxy.go

@@ -309,7 +309,7 @@ func (p *Proxy) proxyClients(ctx context.Context, delayFirstAnnounce bool) {
 		// This delay is distinct from the post-failure delay, although both
 		// use the same delay parameter settings.
 
-		relayedTraffic, err := p.proxyOneClient(ctx, delayFirstAnnounce && i == 0)
+		backOff, err := p.proxyOneClient(ctx, delayFirstAnnounce && i == 0)
 
 		if err != nil && ctx.Err() == nil {
 
@@ -319,19 +319,21 @@ func (p *Proxy) proxyClients(ctx context.Context, delayFirstAnnounce bool) {
 				}).Error("proxy client failed")
 
 			// Apply a simple exponential backoff base on whether
-			// proxyOneClient failed to relay client traffic. The
-			// proxyOneClient failure could range from local configuration
-			// (no broker clients) to network issues(failure to completely
-			// establish WebRTC connection) and this backoff prevents both
-			// excess local logging and churning in the former case and
-			// excessive bad service to clients or unintentionally
+			// proxyOneClient either relayed client traffic or got no match,
+			// or encountered a failure.
+			//
+			// The proxyOneClient failure could range from local
+			// configuration (no broker clients) to network issues(failure to
+			// completely establish WebRTC connection) and this backoff
+			// prevents both excess local logging and churning in the former
+			// case and excessive bad service to clients or unintentionally
 			// overloading the broker in the latter case.
 			//
 			// TODO: specific tactics parameters to control this logic.
 
 			delay, jitter := p.getAnnounceDelayParameters()
 
-			if relayedTraffic {
+			if !backOff {
 				failureDelayFactor = 1
 			}
 			delay = delay * failureDelayFactor
@@ -413,13 +415,13 @@ func (p *Proxy) doNetworkDiscovery(
 
 func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, error) {
 
-	relayedTraffic := false
+	backOff := true
 
 	// Get a new WebRTCDialCoordinator, which should be configured with the
 	// latest network tactics.
 	webRTCCoordinator, err := p.config.MakeWebRTCDialCoordinator()
 	if err != nil {
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 
 	// Perform network discovery, to determine NAT type and other network
@@ -476,7 +478,7 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 
 	brokerClient, err := p.config.GetBrokerClient()
 	if err != nil {
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 
 	brokerCoordinator := brokerClient.GetBrokerDialCoordinator()
@@ -493,7 +495,7 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 
 	metrics, tacticsNetworkID, err := p.getMetrics(webRTCCoordinator)
 	if err != nil {
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 
 	// A proxy ID is implicitly sent with requests; it's the proxy's session
@@ -508,7 +510,7 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 			Metrics:                metrics,
 		})
 	if err != nil {
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 
 	if announceResponse.OperatorMessageJSON != "" {
@@ -531,14 +533,16 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 
 	if announceResponse.NoMatch {
 
-		// While "no match" may be an expected outcome in many scenarios,
-		// still return an error so that the event is logged and the next
-		// announcement is delayed.
-		return relayedTraffic, errors.TraceNew("no match")
+		// Don't apply a back off delay to the next announcement since this
+		// iteration successfully received a no-match response for the broker,
+		// and a client may soon arrive at the broker.
+		backOff = false
+
+		return backOff, errors.TraceNew("no match")
 	}
 
 	if announceResponse.ClientProxyProtocolVersion != ProxyProtocolVersion1 {
-		return relayedTraffic, errors.Tracef(
+		return backOff, errors.Tracef(
 			"Unsupported proxy protocol version: %d",
 			announceResponse.ClientProxyProtocolVersion)
 	}
@@ -559,7 +563,7 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 		ctx, common.ValueOrDefault(webRTCCoordinator.WebRTCAnswerTimeout(), proxyWebRTCAnswerTimeout))
 	defer webRTCAnswerCancelFunc()
 
-	webRTCConn, SDP, SDPMetrics, webRTCErr := NewWebRTCConnWithAnswer(
+	webRTCConn, SDP, sdpMetrics, webRTCErr := NewWebRTCConnWithAnswer(
 		webRTCAnswerCtx,
 		&WebRTCConfig{
 			Logger:                      p.config.Logger,
@@ -574,10 +578,12 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 		webRTCErr = errors.Trace(webRTCErr)
 		webRTCRequestErr = webRTCErr.Error()
 		SDP = webrtc.SessionDescription{}
+		sdpMetrics = &SDPMetrics{}
 		// Continue to report the error to the broker. The broker will respond
 		// with failure to the client's offer request.
+	} else {
+		defer webRTCConn.Close()
 	}
-	defer webRTCConn.Close()
 
 	// Send answer request with SDP or error.
 
@@ -587,21 +593,21 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 			ConnectionID:                 announceResponse.ConnectionID,
 			SelectedProxyProtocolVersion: announceResponse.ClientProxyProtocolVersion,
 			ProxyAnswerSDP:               SDP,
-			ICECandidateTypes:            SDPMetrics.ICECandidateTypes,
+			ICECandidateTypes:            sdpMetrics.ICECandidateTypes,
 			AnswerError:                  webRTCRequestErr,
 		})
 	if err != nil {
 		if webRTCErr != nil {
 			// Prioritize returning any WebRTC error for logging.
-			return relayedTraffic, webRTCErr
+			return backOff, webRTCErr
 		}
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 
 	// Now that an answer is sent, stop if WebRTC initialization failed.
 
 	if webRTCErr != nil {
-		return relayedTraffic, webRTCErr
+		return backOff, webRTCErr
 	}
 
 	// Await the WebRTC connection.
@@ -622,7 +628,7 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 
 	err = webRTCConn.AwaitInitialDataChannel(awaitDataChannelCtx)
 	if err != nil {
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 
 	p.config.Logger.WithTraceFields(common.LogFields{
@@ -654,7 +660,7 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 	destinationAddress, err := webRTCCoordinator.ResolveAddress(
 		ctx, "ip", announceResponse.DestinationAddress)
 	if err != nil {
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 
 	var dialer net.Dialer
@@ -663,7 +669,7 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 		announceResponse.NetworkProtocol.String(),
 		destinationAddress)
 	if err != nil {
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 	defer destinationConn.Close()
 
@@ -701,7 +707,7 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 	destinationConn, err = common.NewActivityMonitoredConn(
 		destinationConn, 0, false, nil, p.activityUpdateWrapper)
 	if err != nil {
-		return relayedTraffic, errors.Trace(err)
+		return backOff, errors.Trace(err)
 	}
 
 	// Relay the client traffic to the destination. The client traffic is a
@@ -779,9 +785,13 @@ func (p *Proxy) proxyOneClient(ctx context.Context, delayAnnounce bool) (bool, e
 		"connectionID": announceResponse.ConnectionID,
 	}).Info("connection closed")
 
-	relayedTraffic = atomic.LoadInt32(&relayedUp) == 1 && atomic.LoadInt32(&relayedDown) == 1
+	// Don't apply a back off delay to the next announcement since this
+	// iteration successfully relayed bytes.
+	if atomic.LoadInt32(&relayedUp) == 1 || atomic.LoadInt32(&relayedDown) == 1 {
+		backOff = false
+	}
 
-	return relayedTraffic, err
+	return backOff, err
 }
 
 func (p *Proxy) getMetrics(webRTCCoordinator WebRTCDialCoordinator) (*ProxyMetrics, string, error) {

psiphon/common/inproxy/webrtc.go

@@ -619,10 +619,14 @@ func newWebRTCConn(
 	localDescription := conn.peerConnection.LocalDescription()
 
 	// Adjust the SDP, removing local network addresses and adding any
-	// port mapping candidate.
+	// port mapping candidate. Clients (offer) are permitted to have
+	// no ICE candidates but proxies (answer) must have at least one
+	//candidate.
+	errorOnNoCandidates := !isOffer
 
 	adjustedSDP, metrics, err := PrepareSDPAddresses(
 		[]byte(localDescription.SDP),
+		errorOnNoCandidates,
 		portMappingExternalAddr,
 		config.WebRTCDialCoordinator.DisableIPv6ICECandidates())
 	if err != nil {
@@ -670,9 +674,6 @@ func (conn *WebRTCConn) setDataChannel(dataChannel *webrtc.DataChannel) {
 	conn.dataChannel.OnOpen(conn.onDataChannelOpen)
 	conn.dataChannel.OnClose(conn.onDataChannelClose)
 
-	conn.dataChannel.OnOpen(conn.onDataChannelOpen)
-	conn.dataChannel.OnClose(conn.onDataChannelClose)
-
 	// Set up flow control (see comment in conn.Write)
 	conn.dataChannel.SetBufferedAmountLowThreshold(dataChannelBufferedAmountLowThreshold)
 	conn.dataChannel.OnBufferedAmountLow(func() {
@@ -1233,11 +1234,18 @@ func (conn *WebRTCConn) onDataChannelClose() {
 // adding any port mapping as a host candidate.
 func PrepareSDPAddresses(
 	encodedSDP []byte,
+	errorOnNoCandidates bool,
 	portMappingExternalAddr string,
 	disableIPv6Candidates bool) ([]byte, *SDPMetrics, error) {
 
 	modifiedSDP, metrics, err := processSDPAddresses(
-		encodedSDP, portMappingExternalAddr, disableIPv6Candidates, false, nil, common.GeoIPData{})
+		encodedSDP,
+		portMappingExternalAddr,
+		disableIPv6Candidates,
+		false, // bogons are expected, and stripped out
+		errorOnNoCandidates,
+		nil,
+		common.GeoIPData{})
 	return modifiedSDP, metrics, errors.Trace(err)
 }
 
@@ -1246,10 +1254,18 @@ func PrepareSDPAddresses(
 // for the specified expectedGeoIPData.
 func ValidateSDPAddresses(
 	encodedSDP []byte,
+	errorOnNoCandidates bool,
 	lookupGeoIP LookupGeoIP,
 	expectedGeoIPData common.GeoIPData) (*SDPMetrics, error) {
 
-	_, metrics, err := processSDPAddresses(encodedSDP, "", false, true, lookupGeoIP, expectedGeoIPData)
+	_, metrics, err := processSDPAddresses(
+		encodedSDP,
+		"",
+		false,
+		true, // bogons should already by stripped out
+		errorOnNoCandidates,
+		lookupGeoIP,
+		expectedGeoIPData)
 	return metrics, errors.Trace(err)
 }
 
@@ -1302,6 +1318,7 @@ func processSDPAddresses(
 	portMappingExternalAddr string,
 	disableIPv6Candidates bool,
 	errorOnBogon bool,
+	errorOnNoCandidates bool,
 	lookupGeoIP LookupGeoIP,
 	expectedGeoIPData common.GeoIPData) ([]byte, *SDPMetrics, error) {
 
@@ -1454,7 +1471,7 @@ func processSDPAddresses(
 		mediaDescription.Attributes = attributes
 	}
 
-	if candidateCount == 0 {
+	if errorOnNoCandidates && candidateCount == 0 {
 		return nil, nil, errors.TraceNew("no candidates")
 	}
 
@@ -1586,12 +1603,15 @@ func (p *pionNetwork) Interfaces() ([]*transport.Interface, error) {
 
 	var defaultIPv4, defaultIPv6 net.IP
 
-	udpConnIPv4, err := net.Dial("udp4", "93.184.216.34:3478")
+	udpConnIPv4, err := p.webRTCDialCoordinator.UDPConn(
+		context.Background(), "udp4", "93.184.216.34:3478")
 	if err == nil {
 		defaultIPv4 = udpConnIPv4.LocalAddr().(*net.UDPAddr).IP
 		udpConnIPv4.Close()
 	}
-	udpConnIPv6, err := net.Dial("udp6", "[2606:2800:220:1:248:1893:25c8:1946]:3478")
+
+	udpConnIPv6, err := p.webRTCDialCoordinator.UDPConn(
+		context.Background(), "udp6", "[2606:2800:220:1:248:1893:25c8:1946]:3478")
 	if err == nil {
 		defaultIPv6 = udpConnIPv6.LocalAddr().(*net.UDPAddr).IP
 		udpConnIPv6.Close()
@@ -1600,17 +1620,19 @@ func (p *pionNetwork) Interfaces() ([]*transport.Interface, error) {
 	transportInterfaces := []*transport.Interface{}
 
 	netInterfaces, err := anet.Interfaces()
+
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 
 	for _, netInterface := range netInterfaces {
+		// Note: don't exclude interfaces with the net.FlagPointToPoint flag,
+		// which is set for certain mobile networks
 		if (netInterface.Flags&net.FlagUp == 0) ||
-			(netInterface.Flags&net.FlagPointToPoint != 0) ||
 			(!GetAllowBogonWebRTCConnections() && (netInterface.Flags&net.FlagLoopback != 0)) {
 			continue
 		}
-		addrs, err := netInterface.Addrs()
+		addrs, err := anet.InterfaceAddrsByInterface(&netInterface)
 		if err != nil {
 			return nil, errors.Trace(err)
 		}

psiphon/common/parameters/parameters.go

@@ -865,11 +865,11 @@ var defaultParameters = map[string]struct {
 	InproxyBrokerProxyAnnounceTimeout:                  {value: 2 * time.Minute, minimum: time.Duration(0), flags: serverSideOnly},
 	InproxyBrokerClientOfferTimeout:                    {value: 10 * time.Second, minimum: time.Duration(0), flags: serverSideOnly},
 	InproxyBrokerPendingServerRequestsTTL:              {value: 60 * time.Second, minimum: time.Duration(0), flags: serverSideOnly},
-	InproxyProxyAnnounceRequestTimeout:                 {value: 2*time.Minute + 5*time.Second, minimum: time.Duration(0)},
+	InproxyProxyAnnounceRequestTimeout:                 {value: 2*time.Minute + 10*time.Second, minimum: time.Duration(0)},
 	InproxyProxyAnnounceRetryDelay:                     {value: 2 * time.Second, minimum: time.Duration(0)},
 	InproxyProxyAnnounceRetryJitter:                    {value: 0.3, minimum: 0.0},
-	InproxyProxyAnswerRequestTimeout:                   {value: 10 * time.Second, minimum: time.Duration(0)},
-	InproxyClientOfferRequestTimeout:                   {value: 10*time.Second + 5*time.Second, minimum: time.Duration(0)},
+	InproxyProxyAnswerRequestTimeout:                   {value: 10*time.Second + 10*time.Second, minimum: time.Duration(0)},
+	InproxyClientOfferRequestTimeout:                   {value: 10*time.Second + 10*time.Second, minimum: time.Duration(0)},
 	InproxyClientOfferRetryDelay:                       {value: 1 * time.Second, minimum: time.Duration(0)},
 	InproxyClientOfferRetryJitter:                      {value: 0.3, minimum: 0.0},
 	InproxyClientRelayedPacketRequestTimeout:           {value: 10 * time.Second, minimum: time.Duration(0)},

psiphon/common/resolver/resolver.go

@@ -1361,7 +1361,9 @@ func hasRoutableIPv6Interface() (bool, error) {
 	for _, in := range interfaces {
 
 		if (in.Flags&net.FlagUp == 0) ||
-			(in.Flags&(net.FlagLoopback|net.FlagPointToPoint)) != 0 {
+			// Note: don't exclude interfaces with the net.FlagPointToPoint
+			// flag, which is set for certain mobile networks
+			(in.Flags&net.FlagLoopback != 0) {
 			continue
 		}
 

psiphon/inproxy.go

@@ -1552,6 +1552,38 @@ func (w *InproxyWebRTCDialInstance) UDPListen(ctx context.Context) (net.PacketCo
 	return conn, nil
 }
 
+// Implements the inproxy.WebRTCDialCoordinator interface.
+func (w *InproxyWebRTCDialInstance) UDPConn(
+	ctx context.Context, network, remoteAddress string) (net.PacketConn, error) {
+
+	// Create a new UDPConn bound to the specified remote address. This UDP
+	// conn is used, by the inproxy package, to determine the local address
+	// of the active interface the OS will select for the specified remote
+	// destination.
+	//
+	// Only IP address destinations are supported. ResolveIP is wired up only
+	// because NewUDPConn requires a non-nil resolver.
+
+	dialConfig := &DialConfig{
+		DeviceBinder:    w.config.deviceBinder,
+		IPv6Synthesizer: w.config.IPv6Synthesizer,
+		ResolveIP: func(_ context.Context, hostname string) ([]net.IP, error) {
+			IP := net.ParseIP(hostname)
+			if IP == nil {
+				return nil, errors.TraceNew("not supported")
+			}
+			return []net.IP{IP}, nil
+		},
+	}
+
+	conn, _, err := NewUDPConn(ctx, network, true, "", remoteAddress, dialConfig)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	return conn, nil
+}
+
 // Implements the inproxy.WebRTCDialCoordinator interface.
 func (w *InproxyWebRTCDialInstance) BindToDevice(fileDescriptor int) error {
 

psiphon/notice.go

@@ -219,8 +219,16 @@ func (nl *noticeLogger) outputNotice(noticeType string, noticeFlags uint32, args
 	obj["timestamp"] = time.Now().UTC().Format(common.RFC3339Milli)
 	for i := 0; i < len(args)-1; i += 2 {
 		name, ok := args[i].(string)
-		value := args[i+1]
 		if ok {
+
+			value := args[i+1]
+
+			// encoding/json marshals error types as "{}", so convert to error
+			// message string.
+			if err, isError := value.(error); isError {
+				value = err.Error()
+			}
+
 			noticeData[name] = value
 		}
 	}

psiphon/serverApi.go

@@ -1028,12 +1028,15 @@ func getBaseAPIParameters(
 	// encoded API messages for backwards compatibility. SSH login proves
 	// client possession of the server entry; the server secret was for the
 	// legacy web API with no SSH login. Note that we can't check
-	// SupportsSSHAPIRequests in the baseParametersNoDialParameters case, but
+	// dialParams.ServerEntry in the baseParametersNoDialParameters case, but
 	// that case is used by in-proxy dials, which implies support.
 
-	if (dialParams != nil && !dialParams.ServerEntry.SupportsSSHAPIRequests()) ||
-		config.TargetAPIEncoding == protocol.PSIPHON_API_ENCODING_JSON {
-		params["server_secret"] = dialParams.ServerEntry.WebServerSecret
+	if dialParams != nil {
+		if !dialParams.ServerEntry.SupportsSSHAPIRequests() ||
+			config.TargetAPIEncoding == protocol.PSIPHON_API_ENCODING_JSON {
+
+			params["server_secret"] = dialParams.ServerEntry.WebServerSecret
+		}
 	}
 
 	// Blank parameters must be omitted.