Move network_type and applied_tactics_tag to base API params

psiphon/controller.go

@@ -2648,9 +2648,9 @@ func (controller *Controller) runInproxyProxy() {
 		EnableWebRTCDebugLogging:      debugLogging,
 		WaitForNetworkConnectivity:    controller.inproxyWaitForNetworkConnectivity,
 		GetBrokerClient:               controller.inproxyGetProxyBrokerClient,
-		GetBaseAPIParameters:          controller.inproxyGetAPIParameters,
-		MakeWebRTCDialCoordinator:     controller.inproxyMakeWebRTCDialCoordinator,
-		HandleTacticsPayload:          controller.inproxyHandleTacticsPayload,
+		GetBaseAPIParameters:          controller.inproxyGetProxyAPIParameters,
+		MakeWebRTCDialCoordinator:     controller.inproxyMakeProxyWebRTCDialCoordinator,
+		HandleTacticsPayload:          controller.inproxyHandleProxyTacticsPayload,
 		MaxClients:                    controller.config.InproxyMaxClients,
 		LimitUpstreamBytesPerSecond:   controller.config.InproxyLimitUpstreamBytesPerSecond,
 		LimitDownstreamBytesPerSecond: controller.config.InproxyLimitDownstreamBytesPerSecond,
@@ -2729,27 +2729,28 @@ func (controller *Controller) inproxyGetProxyBrokerClient() (*inproxy.BrokerClie
 	return brokerClient, nil
 }
 
-func (controller *Controller) inproxyGetAPIParameters() (common.APIParameters, string, error) {
+func (controller *Controller) inproxyGetProxyAPIParameters() (
+	common.APIParameters, string, error) {
 
 	// TODO: include broker fronting dial parameters to be logged by the
 	// broker.
 	params := getBaseAPIParameters(baseParametersNoDialParameters, controller.config, nil)
 
-	networkID := controller.config.GetNetworkID()
-	params["network_type"] = GetNetworkType(networkID)
-
 	if controller.config.DisableTactics {
 		return params, "", nil
 	}
 
-	// Add the known tactics tag, so that the broker can return new tactics if
+	// Add the stored tactics tag, so that the broker can return new tactics if
 	// available.
 	//
 	// The active network ID is recorded returned and rechecked for
 	// consistency when storing any new tactics returned from the broker;
 	// other tactics fetches have this same check.
 
-	err := tactics.SetTacticsAPIParameters(GetTacticsStorer(controller.config), networkID, params)
+	networkID := controller.config.GetNetworkID()
+
+	err := tactics.SetTacticsAPIParameters(
+		GetTacticsStorer(controller.config), networkID, params)
 	if err != nil {
 		return nil, "", errors.Trace(err)
 	}
@@ -2757,7 +2758,8 @@ func (controller *Controller) inproxyGetAPIParameters() (common.APIParameters, s
 	return params, networkID, nil
 }
 
-func (controller *Controller) inproxyMakeWebRTCDialCoordinator() (inproxy.WebRTCDialCoordinator, error) {
+func (controller *Controller) inproxyMakeProxyWebRTCDialCoordinator() (
+	inproxy.WebRTCDialCoordinator, error) {
 
 	// nil is passed in for both InproxySTUNDialParameters and
 	// InproxyWebRTCDialParameters, so those parameters will be newly
@@ -2780,12 +2782,12 @@ func (controller *Controller) inproxyMakeWebRTCDialCoordinator() (inproxy.WebRTC
 	return webRTCDialInstance, nil
 }
 
-// inproxyHandleTacticsPayload handles new tactics returned from the proxy and
-// returns when tactics have changed.
+// inproxyHandleProxyTacticsPayload handles new tactics returned from the
+// proxy and returns when tactics have changed.
 //
 // inproxyHandleTacticsPayload duplicates some tactics-handling code from
 // doHandshakeRequest.
-func (controller *Controller) inproxyHandleTacticsPayload(
+func (controller *Controller) inproxyHandleProxyTacticsPayload(
 	networkID string, tacticsPayload []byte) bool {
 
 	if controller.config.DisableTactics {

psiphon/server/api.go

@@ -981,8 +981,7 @@ func getTacticsAPIParameterLogFieldFormatter() common.APIParameterLogFieldFormat
 
 var inproxyBrokerRequestParams = append(
 	append(
-		[]requestParamSpec{
-			{"network_type", isAnyString, requestParamOptional}},
+		[]requestParamSpec{},
 		tacticsParams...),
 	baseSessionParams...)
 
@@ -1041,6 +1040,8 @@ var baseParams = []requestParamSpec{
 	{"client_build_rev", isHexDigits, requestParamOptional},
 	{"device_region", isAnyString, requestParamOptional},
 	{"device_location", isGeoHashString, requestParamOptional},
+	{"network_type", isAnyString, requestParamOptional},
+	{tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME, isAnyString, requestParamOptional},
 }
 
 // baseSessionParams adds to baseParams the required session_id parameter. For
@@ -1071,7 +1072,6 @@ var baseDialParams = []requestParamSpec{
 	{"server_entry_region", isRegionCode, requestParamOptional},
 	{"server_entry_source", isServerEntrySource, requestParamOptional},
 	{"server_entry_timestamp", isISO8601Date, requestParamOptional},
-	{tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME, isAnyString, requestParamOptional},
 	{"dial_port_number", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"quic_version", isAnyString, requestParamOptional},
 	{"quic_dial_sni_address", isAnyString, requestParamOptional},
@@ -1095,7 +1095,6 @@ var baseDialParams = []requestParamSpec{
 	{"meek_tls_padding", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"network_latency_multiplier", isFloatString, requestParamOptional | requestParamLogStringAsFloat},
 	{"client_bpf", isAnyString, requestParamOptional},
-	{"network_type", isAnyString, requestParamOptional},
 	{"conjure_cached", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 	{"conjure_delay", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"conjure_transport", isAnyString, requestParamOptional},

psiphon/server/server_test.go

@@ -1762,6 +1762,7 @@ func runServer(t *testing.T, runConfig *runServerConfig) {
 	case logFields := <-serverTunnelLog:
 		err := checkExpectedServerTunnelLogFields(
 			runConfig,
+			doClientTactics,
 			expectClientBPFField,
 			expectServerBPFField,
 			expectServerPacketManipulationField,
@@ -2009,6 +2010,7 @@ func waitOnNotification(t *testing.T, c, timeoutSignal <-chan struct{}, timeoutM
 
 func checkExpectedServerTunnelLogFields(
 	runConfig *runServerConfig,
+	expectAppliedTacticsTag bool,
 	expectClientBPFField bool,
 	expectServerBPFField bool,
 	expectServerPacketManipulationField bool,
@@ -2066,6 +2068,11 @@ func checkExpectedServerTunnelLogFields(
 		}
 	}
 
+	appliedTacticsTag := len(fields[tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME].(string)) > 0
+	if expectAppliedTacticsTag != appliedTacticsTag {
+		return fmt.Errorf("unexpected applied_tactics_tag")
+	}
+
 	if fields["host_id"].(string) != "example-host-id" {
 		return fmt.Errorf("unexpected host_id '%s'", fields["host_id"])
 	}
@@ -2216,7 +2223,6 @@ func checkExpectedServerTunnelLogFields(
 			"meek_limit_request",
 			"meek_underlying_connection_count",
 			"meek_server_http_version",
-			tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME,
 		} {
 			if fields[name] == nil || fmt.Sprintf("%s", fields[name]) == "" {
 				return fmt.Errorf("missing expected field '%s'", name)

psiphon/serverApi.go

@@ -1029,6 +1029,15 @@ func getBaseAPIParameters(
 	params["client_platform"] = config.ClientPlatform
 	params["client_features"] = config.clientFeatures
 	params["client_build_rev"] = buildinfo.GetBuildInfo().BuildRev
+	if dialParams != nil {
+		// Prefer the dialParams network ID snapshot if available.
+		params["network_type"] = dialParams.GetNetworkType()
+	} else {
+		params["network_type"] = GetNetworkType(config.GetNetworkID())
+	}
+	// TODO: snapshot tactics tag used when dialParams initialized.
+	params[tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME] =
+		config.GetParameters().Get().Tag()
 
 	// The server secret is deprecated and included only in legacy JSON
 	// encoded API messages for backwards compatibility. SSH login proves
@@ -1065,7 +1074,6 @@ func getBaseAPIParameters(
 		}
 
 		params["relay_protocol"] = dialParams.TunnelProtocol
-		params["network_type"] = dialParams.GetNetworkType()
 
 		if dialParams.BPFProgramName != "" {
 			params["client_bpf"] = dialParams.BPFProgramName
@@ -1156,9 +1164,6 @@ func getBaseAPIParameters(
 			params["server_entry_timestamp"] = localServerEntryTimestamp
 		}
 
-		params[tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME] =
-			config.GetParameters().Get().Tag()
-
 		if dialParams.DialPortNumber != "" {
 			params["dial_port_number"] = dialParams.DialPortNumber
 		}

psiphon/tunnel.go

@@ -1535,16 +1535,14 @@ func dialInproxy(
 	}
 
 	// Unlike the proxy broker case, clients already actively fetch tactics
-	// during tunnel estalishment, so no tactics tags are sent to the broker
-	// and no tactics are returned by the broker.
+	// during tunnel estalishment, so tactics.SetTacticsAPIParameters are not
+	// sent to the broker and no tactics are returned by the broker.
 	//
 	// TODO: include broker fronting dial parameters to be logged by the
 	// broker -- as successful parameters might not otherwise by logged via
 	// server_tunnel if the subsequent WebRTC dials fail.
 	params := getBaseAPIParameters(baseParametersNoDialParameters, config, nil)
 
-	params["network_type"] = dialParams.GetNetworkType()
-
 	// The debugLogging flag is passed to both NoticeCommonLogger and to the
 	// inproxy package as well; skipping debug logs in the inproxy package,
 	// before calling into the notice logger, avoids unnecessary allocations