Network ID changes

- Re-instate "UNKNOWN" network IDs, as we will
  proceed with tactics with any GetNetworkID
  return value.

- Check if the network ID is the same before
  and after tactics network operations to reduce
  (not close) the window for network ID/server
  geoIP mismatches.

- Add a WaitForNetworkConnectivity throttle in
  the the tactics request worker.

MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java

@@ -374,16 +374,19 @@ public class PsiphonTunnel extends Psi.PsiphonProvider.Stub {
         // The network ID contains potential PII. In tunnel-core, the network ID
         // is used only locally in the client and not sent to the server.
 
-        String networkID = "";
+        String networkID = "UNKNOWN";
 
         Context context = mHostService.getContext();
-        ConnectivityManager connectivityManager = (ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE);;
+        ConnectivityManager connectivityManager = (ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE);
         NetworkInfo activeNetworkInfo = null;
         try {
             activeNetworkInfo = connectivityManager.getActiveNetworkInfo();
 
         } catch (java.lang.Exception e) {
             // May get exceptions due to missing permissions like android.permission.ACCESS_NETWORK_STATE.
+
+            // Apps using the Psiphon Library and lacking android.permission.ACCESS_NETWORK_STATE will
+            // proceed and use tactics, but with "UNKNOWN" as the sole network ID.
         }
 
         if (activeNetworkInfo != null && activeNetworkInfo.getType() == ConnectivityManager.TYPE_WIFI) {

MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m

@@ -1095,7 +1095,7 @@
     // The network ID contains potential PII. In tunnel-core, the network ID
     // is used only locally in the client and not sent to the server.
 
-    NSMutableString *networkID = [NSMutableString stringWithString:@""];
+    NSMutableString *networkID = [NSMutableString stringWithString:@"UNKNOWN"];
     NetworkStatus status = [[Reachability reachabilityForInternetConnection] currentReachabilityStatus];
     if (status == ReachableViaWiFi) {
         [networkID setString:@"WIFI"];

psiphon/common/tactics/tactics.go

@@ -1150,12 +1150,19 @@ func UseStoredTactics(
 // when there is an unexpired stored tactics record available. The
 // caller is expected to set any overall timeout in the context input.
 //
+// Limitation: it is assumed that the network ID obtained from getNetworkID
+// is the one that is active when the tactics request is received by the
+// server. However, it is remotely possible to switch networks
+// immediately after invoking the GetNetworkID callback and initiating
+// the request. This is partially mitigated by rechecking the network ID
+// after the request and failing if it differs from the initial network ID.
+//
 // FetchTactics modifies the apiParams input.
 func FetchTactics(
 	ctx context.Context,
 	clientParameters *parameters.ClientParameters,
 	storer Storer,
-	networkID string,
+	getNetworkID func() string,
 	apiParams common.APIParameters,
 	endPointRegion string,
 	endPointProtocol string,
@@ -1163,6 +1170,8 @@ func FetchTactics(
 	encodedRequestObfuscatedKey string,
 	roundTripper RoundTripper) (*Record, error) {
 
+	networkID := getNetworkID()
+
 	record, err := getStoredTacticsRecord(storer, networkID)
 	if err != nil {
 		return nil, common.ContextError(err)
@@ -1196,6 +1205,10 @@ func FetchTactics(
 			return nil, common.ContextError(err)
 		}
 
+		if networkID != getNetworkID() {
+			return nil, common.ContextError(errors.New("network ID changed"))
+		}
+
 		err = AddSpeedTestSample(
 			clientParameters,
 			storer,
@@ -1251,6 +1264,10 @@ func FetchTactics(
 		return nil, common.ContextError(err)
 	}
 
+	if networkID != getNetworkID() {
+		return nil, common.ContextError(errors.New("network ID changed"))
+	}
+
 	// Process and store the response payload.
 
 	var payload *Payload

psiphon/common/tactics/tactics_test.go

@@ -219,6 +219,8 @@ func TestTactics(t *testing.T) {
 
 	networkID := "NETWORK1"
 
+	getNetworkID := func() string { return networkID }
+
 	apiParams := common.APIParameters{
 		"client_platform": "P1",
 		"client_version":  "V1"}
@@ -316,7 +318,7 @@ func TestTactics(t *testing.T) {
 		ctx,
 		clientParams,
 		storer,
-		networkID,
+		getNetworkID,
 		apiParams,
 		endPointProtocol,
 		endPointRegion,
@@ -388,7 +390,7 @@ func TestTactics(t *testing.T) {
 		context.Background(),
 		clientParams,
 		storer,
-		networkID,
+		getNetworkID,
 		apiParams,
 		endPointProtocol,
 		endPointRegion,
@@ -453,7 +455,7 @@ func TestTactics(t *testing.T) {
 		context.Background(),
 		clientParams,
 		storer,
-		networkID,
+		getNetworkID,
 		apiParams,
 		endPointProtocol,
 		endPointRegion,
@@ -609,7 +611,7 @@ func TestTactics(t *testing.T) {
 		context.Background(),
 		clientParams,
 		storer,
-		networkID,
+		getNetworkID,
 		apiParams,
 		endPointProtocol,
 		endPointRegion,
@@ -624,7 +626,7 @@ func TestTactics(t *testing.T) {
 		context.Background(),
 		clientParams,
 		storer,
-		networkID,
+		getNetworkID,
 		apiParams,
 		endPointProtocol,
 		endPointRegion,

psiphon/controller.go

@@ -1284,6 +1284,12 @@ func (controller *Controller) getTactics(done chan struct{}) {
 
 		for iteration := 0; ; iteration++ {
 
+			if !WaitForNetworkConnectivity(
+				controller.runCtx,
+				controller.config.NetworkConnectivityChecker) {
+				return
+			}
+
 			serverEntry, err := iterator.Next()
 			if err != nil {
 				NoticeAlert("tactics iterator failed: %s", err)
@@ -1403,22 +1409,15 @@ func (controller *Controller) doFetchTactics(
 		timeout)
 	defer cancelFunc()
 
-	// Limitation: it is assumed that the network ID obtained here is the
-	// one that is active when the tactics request is received by the
-	// server. However, it is remotely possible to switch networks
-	// immediately after invoking the GetNetworkID callback and initiating
-	// the request.
-	//
-	// TODO: ensure that meek in round trip mode will fail the request when
-	// the pre-dial connection is broken.
-
-	networkID := controller.config.NetworkIDGetter.GetNetworkID()
-
 	// DialMeek completes the TCP/TLS handshakes for HTTPS
 	// meek protocols but _not_ for HTTP meek protocols.
 	//
 	// TODO: pre-dial HTTP protocols to conform with speed
 	// test RTT spec.
+	//
+	// TODO: ensure that meek in round trip mode will fail
+	// the request when the pre-dial connection is broken,
+	// to minimize the possibility of network ID mismatches.
 
 	meekConn, err := DialMeek(ctx, meekConfig, dialConfig)
 	if err != nil {
@@ -1437,7 +1436,7 @@ func (controller *Controller) doFetchTactics(
 		ctx,
 		controller.config.clientParameters,
 		GetTacticsStorer(),
-		networkID,
+		controller.config.NetworkIDGetter.GetNetworkID,
 		apiParams,
 		serverEntry.Region,
 		tacticsProtocol,

psiphon/serverApi.go

@@ -130,10 +130,15 @@ func (serverContext *ServerContext) doHandshakeRequest(
 	if doTactics {
 
 		// Limitation: it is assumed that the network ID obtained here is the
-		// one that is active when the tactics request is received by the
+		// one that is active when the handshake request is received by the
 		// server. However, it is remotely possible to switch networks
 		// immediately after invoking the GetNetworkID callback and initiating
 		// the handshake, if the tunnel protocol is meek.
+		//
+		// The response handling code below calls GetNetworkID again and ignores
+		// any tactics payload if the network ID is not the same. While this
+		// doesn't detect all cases of changing networks, it reduces the already
+		// narrow window.
 
 		networkID = serverContext.tunnel.config.NetworkIDGetter.GetNetworkID()
 
@@ -254,7 +259,8 @@ func (serverContext *ServerContext) doHandshakeRequest(
 
 	NoticeActiveAuthorizationIDs(handshakeResponse.ActiveAuthorizationIDs)
 
-	if doTactics && handshakeResponse.TacticsPayload != nil {
+	if doTactics && handshakeResponse.TacticsPayload != nil &&
+		networkID == serverContext.tunnel.config.NetworkIDGetter.GetNetworkID() {
 
 		var payload *tactics.Payload
 		err = json.Unmarshal(handshakeResponse.TacticsPayload, &payload)