Reset impaired classification sooner

- Previously, only reset when all possible
  protocols are impaired.
- Now resets when all distinct protocols, for
  all stored server entries, for selected egress
  region are impaired.
- This means the reset happens earlier, when
  it should, for clients that don't have
  server entries for all protocols, especially
  when "EgressRegion" is configured. (However,
  in practice, what most likely happened was
  that establishCandidateGenerator quickly
  dropped down into its next iteration.)
- Correct treatment when "TunnelProtocol" is set
  is now handled by the new code, so the special
  case is removed from establishCandidateGenerator.
- Replace serverEntrySupportsProtocol() with
  ServerEntry.SupportsProtocol(), which is identical.

psiphon/controller.go

@@ -683,16 +683,22 @@ loop:
 //
 // Concurrency note: only the runTunnels() goroutine may call classifyImpairedProtocol
 func (controller *Controller) classifyImpairedProtocol(failedTunnel *Tunnel) {
+
 	if failedTunnel.establishedTime.Add(IMPAIRED_PROTOCOL_CLASSIFICATION_DURATION).After(monotime.Now()) {
 		controller.impairedProtocolClassification[failedTunnel.protocol] += 1
 	} else {
 		controller.impairedProtocolClassification[failedTunnel.protocol] = 0
 	}
-	if len(controller.getImpairedProtocols()) == len(protocol.SupportedTunnelProtocols) {
-		// Reset classification if all protocols are classified as impaired as
-		// the network situation (or attack) may not be protocol-specific.
-		// TODO: compare against count of distinct supported protocols for
-		// current known server entries.
+
+	// Reset classification once all known protocols are classified as impaired, as
+	// there is now no way to proceed with only unimpaired protocols. The network
+	// situation (or attack) resulting in classification may not be protocol-specific.
+	//
+	// Note: with controller.config.TunnelProtocol set, this will always reset once
+	// that protocol has reached IMPAIRED_PROTOCOL_CLASSIFICATION_THRESHOLD.
+	if len(controller.getImpairedProtocols()) ==
+		CountSupportedProtocols(
+			controller.config.EgressRegion, controller.config.TunnelProtocol) {
 		controller.impairedProtocolClassification = make(map[string]int)
 	}
 }
@@ -1080,11 +1086,9 @@ loop:
 			// evade the attack; (b) there's a good chance of false
 			// positives (such as short tunnel durations due to network
 			// hopping on a mobile device).
-			// Impaired protocols logic is not applied when
-			// config.TunnelProtocol is specified.
 			// The edited serverEntry is temporary copy which is not
 			// stored or reused.
-			if i == 0 && controller.config.TunnelProtocol == "" {
+			if i == 0 {
 				serverEntry.DisableImpairedProtocols(impairedProtocols)
 				if len(serverEntry.GetSupportedProtocols()) == 0 {
 					// Skip this server entry, as it has no supported

psiphon/dataStore.go

@@ -365,13 +365,6 @@ func insertRankedServerEntry(tx *bolt.Tx, serverEntryId string, position int) er
 	return nil
 }
 
-func serverEntrySupportsProtocol(serverEntry *protocol.ServerEntry, protocol string) bool {
-	// Note: for meek, the capabilities are FRONTED-MEEK and UNFRONTED-MEEK
-	// and the additonal OSSH service is assumed to be available internally.
-	requiredCapability := strings.TrimSuffix(protocol, "-OSSH")
-	return common.Contains(serverEntry.Capabilities, requiredCapability)
-}
-
 // ServerEntryIterator is used to iterate over
 // stored server entries in rank order.
 type ServerEntryIterator struct {
@@ -573,7 +566,7 @@ func (iterator *ServerEntryIterator) Next() (serverEntry *protocol.ServerEntry,
 
 		// Check filter requirements
 		if (iterator.region == "" || serverEntry.Region == iterator.region) &&
-			(iterator.protocol == "" || serverEntrySupportsProtocol(serverEntry, iterator.protocol)) {
+			(iterator.protocol == "" || serverEntry.SupportsProtocol(iterator.protocol)) {
 
 			break
 		}
@@ -630,7 +623,7 @@ func CountServerEntries(region, tunnelProtocol string) int {
 	count := 0
 	err := scanServerEntries(func(serverEntry *protocol.ServerEntry) {
 		if (region == "" || serverEntry.Region == region) &&
-			(tunnelProtocol == "" || serverEntrySupportsProtocol(serverEntry, tunnelProtocol)) {
+			(tunnelProtocol == "" || serverEntry.SupportsProtocol(tunnelProtocol)) {
 			count += 1
 		}
 	})
@@ -643,6 +636,39 @@ func CountServerEntries(region, tunnelProtocol string) int {
 	return count
 }
 
+// CountSupportedProtocols returns the number of distinct tunnel
+// protocols supported by stored server entries.
+func CountSupportedProtocols(region, tunnelProtocol string) int {
+	checkInitDataStore()
+
+	distinctProtocols := make(map[string]bool)
+
+	err := scanServerEntries(func(serverEntry *protocol.ServerEntry) {
+		if region == "" || serverEntry.Region == region {
+			if tunnelProtocol != "" {
+				if serverEntry.SupportsProtocol(tunnelProtocol) {
+					distinctProtocols[tunnelProtocol] = true
+					// Exit early, since only one protocol is enabled
+					return
+				}
+			} else {
+				for _, protocol := range protocol.SupportedTunnelProtocols {
+					if serverEntry.SupportsProtocol(protocol) {
+						distinctProtocols[protocol] = true
+					}
+				}
+			}
+		}
+	})
+
+	if err != nil {
+		NoticeAlert("CountSupportedProtocols failed: %s", err)
+		return 0
+	}
+
+	return len(distinctProtocols)
+}
+
 // ReportAvailableRegions prints a notice with the available egress regions.
 // Note that this report ignores config.TunnelProtocol.
 func ReportAvailableRegions() {