Merge pull request #173 from rod-hynes/master

Fix: missing nil check caused crash when DisableApi set

psiphon/controller_test.go

@@ -77,6 +77,7 @@ func TestUntunneledUpgradeDownload(t *testing.T) {
 			clientIsLatestVersion:    false,
 			disableUntunneledUpgrade: false,
 			disableEstablishing:      true,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -92,6 +93,7 @@ func TestUntunneledResumableUpgradeDownload(t *testing.T) {
 			clientIsLatestVersion:    false,
 			disableUntunneledUpgrade: false,
 			disableEstablishing:      true,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           true,
 			useHostNameTransformer:   false,
@@ -107,6 +109,7 @@ func TestUntunneledUpgradeClientIsLatestVersion(t *testing.T) {
 			clientIsLatestVersion:    true,
 			disableUntunneledUpgrade: false,
 			disableEstablishing:      true,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -122,6 +125,7 @@ func TestUntunneledResumableFetchRemoveServerList(t *testing.T) {
 			clientIsLatestVersion:    true,
 			disableUntunneledUpgrade: false,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           true,
 			useHostNameTransformer:   false,
@@ -137,6 +141,7 @@ func TestTunneledUpgradeClientIsLatestVersion(t *testing.T) {
 			clientIsLatestVersion:    true,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -160,6 +165,7 @@ func TestImpairedProtocols(t *testing.T) {
 			clientIsLatestVersion:    true,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           40,
 			disruptNetwork:           true,
 			useHostNameTransformer:   false,
@@ -175,6 +181,7 @@ func TestSSH(t *testing.T) {
 			clientIsLatestVersion:    false,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -190,6 +197,7 @@ func TestObfuscatedSSH(t *testing.T) {
 			clientIsLatestVersion:    false,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -205,6 +213,7 @@ func TestUnfrontedMeek(t *testing.T) {
 			clientIsLatestVersion:    false,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -220,6 +229,7 @@ func TestUnfrontedMeekWithTransformer(t *testing.T) {
 			clientIsLatestVersion:    true,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   true,
@@ -235,6 +245,7 @@ func TestFrontedMeek(t *testing.T) {
 			clientIsLatestVersion:    false,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -250,6 +261,7 @@ func TestFrontedMeekWithTransformer(t *testing.T) {
 			clientIsLatestVersion:    true,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   true,
@@ -265,6 +277,7 @@ func TestFrontedMeekHTTP(t *testing.T) {
 			clientIsLatestVersion:    true,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -280,6 +293,7 @@ func TestUnfrontedMeekHTTPS(t *testing.T) {
 			clientIsLatestVersion:    false,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   false,
@@ -295,6 +309,7 @@ func TestUnfrontedMeekHTTPSWithTransformer(t *testing.T) {
 			clientIsLatestVersion:    true,
 			disableUntunneledUpgrade: true,
 			disableEstablishing:      false,
+			disableApi:               false,
 			tunnelPoolSize:           1,
 			disruptNetwork:           false,
 			useHostNameTransformer:   true,
@@ -302,6 +317,22 @@ func TestUnfrontedMeekHTTPSWithTransformer(t *testing.T) {
 		})
 }
 
+func TestDisabledApi(t *testing.T) {
+	controllerRun(t,
+		&controllerRunConfig{
+			expectNoServerEntries:    false,
+			protocol:                 "",
+			clientIsLatestVersion:    true,
+			disableUntunneledUpgrade: true,
+			disableEstablishing:      false,
+			disableApi:               true,
+			tunnelPoolSize:           1,
+			disruptNetwork:           false,
+			useHostNameTransformer:   false,
+			runDuration:              0,
+		})
+}
+
 type controllerRunConfig struct {
 	expectNoServerEntries    bool
 	protocol                 string
@@ -312,6 +343,7 @@ type controllerRunConfig struct {
 	disruptNetwork           bool
 	useHostNameTransformer   bool
 	runDuration              time.Duration
+	disableApi               bool
 }
 
 func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
@@ -336,6 +368,10 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 		config.RemoteServerListUrl = ""
 	}
 
+	if runConfig.disableApi {
+		config.DisableApi = true
+	}
+
 	config.TunnelPoolSize = runConfig.tunnelPoolSize
 
 	if runConfig.disableUntunneledUpgrade {
@@ -599,31 +635,35 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 
 	// Test: upgrade check/download must be downloaded within 180 seconds
 
-	upgradeTimeout := time.NewTimer(180 * time.Second)
+	expectUpgrade := !runConfig.disableApi && !runConfig.disableUntunneledUpgrade
 
-	select {
-	case <-upgradeDownloaded:
-		// TODO: verify downloaded file
-		if runConfig.clientIsLatestVersion {
-			t.Fatalf("upgrade downloaded unexpectedly")
-		}
+	if expectUpgrade {
+		upgradeTimeout := time.NewTimer(180 * time.Second)
 
-		// Test: with disruptNetwork, must be multiple download progress notices
+		select {
+		case <-upgradeDownloaded:
+			// TODO: verify downloaded file
+			if runConfig.clientIsLatestVersion {
+				t.Fatalf("upgrade downloaded unexpectedly")
+			}
+
+			// Test: with disruptNetwork, must be multiple download progress notices
 
-		if runConfig.disruptNetwork {
-			count := atomic.LoadInt32(&clientUpgradeDownloadedBytesCount)
-			if count <= 1 {
-				t.Fatalf("unexpected upgrade download progress: %d", count)
+			if runConfig.disruptNetwork {
+				count := atomic.LoadInt32(&clientUpgradeDownloadedBytesCount)
+				if count <= 1 {
+					t.Fatalf("unexpected upgrade download progress: %d", count)
+				}
 			}
-		}
 
-	case <-confirmedLatestVersion:
-		if !runConfig.clientIsLatestVersion {
-			t.Fatalf("confirmed latest version unexpectedly")
-		}
+		case <-confirmedLatestVersion:
+			if !runConfig.clientIsLatestVersion {
+				t.Fatalf("confirmed latest version unexpectedly")
+			}
 
-	case <-upgradeTimeout.C:
-		t.Fatalf("upgrade download timeout exceeded")
+		case <-upgradeTimeout.C:
+			t.Fatalf("upgrade download timeout exceeded")
+		}
 	}
 }
 

psiphon/notice.go

@@ -195,7 +195,7 @@ func NoticeClientIsLatestVersion(availableVersion string) {
 	outputNotice("ClientIsLatestVersion", false, false, "availableVersion", availableVersion)
 }
 
-// NoticeClientUpgradeAvailable is a sponsor homepage, as per the handshake. The client
+// NoticeHomepage is a sponsor homepage, as per the handshake. The client
 // should display the sponsor's homepage.
 func NoticeHomepage(url string) {
 	outputNotice("Homepage", false, false, "url", url)

psiphon/tunnel.go

@@ -898,7 +898,8 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 	// timestamp in the handshake response as the tunnel start time. This time
 	// will be slightly earlier than the actual tunnel activation time, as the
 	// client has to receive and parse the response and activate the tunnel.
-	if !tunnel.IsDiscarded() {
+	// Tunnel does not have a serverContext when DisableApi is set.
+	if tunnel.serverContext != nil && !tunnel.IsDiscarded() {
 		err := RecordTunnelStats(
 			tunnel.serverContext.sessionId,
 			tunnel.serverContext.tunnelNumber,