Retire impaired protocol logic

- Superseded by tactics

- Often ineffective (e.g. user restarts
  before impaired classification; or
  case where tunnel repeatedly connects
  with fast protocol but disrupted before
  handshake, thus never returning to
  first round to be excluded)

- Sometimes has a negative effect (e.g.,
  case where only a single protocol can
  connect, and is useful for a short time
  before being disrupted and having to
  reconnect; impaired exclusion is
  counterproductive)

psiphon/common/parameters/clientParameters.go

@@ -142,8 +142,6 @@ const (
 	FetchUpgradeStalePeriod                    = "FetchUpgradeStalePeriod"
 	UpgradeDownloadURLs                        = "UpgradeDownloadURLs"
 	UpgradeDownloadClientVersionHeader         = "UpgradeDownloadClientVersionHeader"
-	ImpairedProtocolClassificationDuration     = "ImpairedProtocolClassificationDuration"
-	ImpairedProtocolClassificationThreshold    = "ImpairedProtocolClassificationThreshold"
 	TotalBytesTransferredNoticePeriod          = "TotalBytesTransferredNoticePeriod"
 	MeekDialDomainsOnly                        = "MeekDialDomainsOnly"
 	MeekLimitBufferSizes                       = "MeekLimitBufferSizes"
@@ -301,9 +299,6 @@ var defaultClientParameters = map[string]struct {
 	UpgradeDownloadURLs:                {value: DownloadURLs{}},
 	UpgradeDownloadClientVersionHeader: {value: ""},
 
-	ImpairedProtocolClassificationDuration:  {value: 2 * time.Minute, minimum: 1 * time.Millisecond, flags: useNetworkLatencyMultiplier},
-	ImpairedProtocolClassificationThreshold: {value: 3, minimum: 1},
-
 	TotalBytesTransferredNoticePeriod: {value: 5 * time.Minute, minimum: 1 * time.Second},
 
 	// The meek server times out inactive sessions after 45 seconds, so this

psiphon/common/protocol/serverEntry.go

@@ -139,7 +139,6 @@ func (serverEntry *ServerEntry) SupportsProtocol(protocol string) bool {
 func (serverEntry *ServerEntry) GetSupportedProtocols(
 	useUpstreamProxy bool,
 	limitTunnelProtocols []string,
-	impairedTunnelProtocols []string,
 	excludeIntensive bool) []string {
 
 	supportedProtocols := make([]string, 0)
@@ -160,11 +159,6 @@ func (serverEntry *ServerEntry) GetSupportedProtocols(
 			}
 		}
 
-		if len(impairedTunnelProtocols) > 0 &&
-			common.Contains(impairedTunnelProtocols, protocol) {
-			continue
-		}
-
 		if excludeIntensive && TunnelProtocolIsResourceIntensive(protocol) {
 			continue
 		}

psiphon/controller.go

@@ -71,7 +71,6 @@ type Controller struct {
 	signalFetchCommonRemoteServerList       chan struct{}
 	signalFetchObfuscatedServerLists        chan struct{}
 	signalDownloadUpgrade                   chan string
-	impairedProtocolClassification          map[string]int
 	signalReportConnected                   chan struct{}
 	serverAffinityDoneBroadcast             chan struct{}
 	packetTunnelClient                      *tun.Client
@@ -82,7 +81,6 @@ type candidateServerEntry struct {
 	serverEntry                *protocol.ServerEntry
 	isServerAffinityCandidate  bool
 	usePriorityProtocol        bool
-	impairedProtocols          []string
 	adjustedEstablishStartTime monotime.Time
 }
 
@@ -115,14 +113,13 @@ func NewController(config *Config) (controller *Controller, err error) {
 		runWaitGroup: new(sync.WaitGroup),
 		// connectedTunnels and failedTunnels buffer sizes are large enough to
 		// receive full pools of tunnels without blocking. Senders should not block.
-		connectedTunnels:               make(chan *Tunnel, config.TunnelPoolSize),
-		failedTunnels:                  make(chan *Tunnel, config.TunnelPoolSize),
-		tunnels:                        make([]*Tunnel, 0),
-		establishedOnce:                false,
-		startedConnectedReporter:       false,
-		isEstablishing:                 false,
-		untunneledDialConfig:           untunneledDialConfig,
-		impairedProtocolClassification: make(map[string]int),
+		connectedTunnels:         make(chan *Tunnel, config.TunnelPoolSize),
+		failedTunnels:            make(chan *Tunnel, config.TunnelPoolSize),
+		tunnels:                  make([]*Tunnel, 0),
+		establishedOnce:          false,
+		startedConnectedReporter: false,
+		isEstablishing:           false,
+		untunneledDialConfig:     untunneledDialConfig,
 		// TODO: Add a buffer of 1 so we don't miss a signal while receiver is
 		// starting? Trade-off is potential back-to-back fetch remotes. As-is,
 		// establish will eventually signal another fetch remote.
@@ -592,8 +589,6 @@ loop:
 			NoticeAlert("tunnel failed: %s", failedTunnel.serverEntry.IpAddress)
 			controller.terminateTunnel(failedTunnel)
 
-			controller.classifyImpairedProtocol(failedTunnel)
-
 			// Clear the reference to this tunnel before calling startEstablishing,
 			// which will invoke a garbage collection.
 			failedTunnel = nil
@@ -604,19 +599,6 @@ loop:
 
 		case connectedTunnel := <-controller.connectedTunnels:
 
-			if controller.isImpairedProtocol(connectedTunnel.protocol) {
-
-				// Protocol was classified as impaired while this tunnel established.
-				// This is most likely to occur with TunnelPoolSize > 0. We log the
-				// event but take no action. Discarding the tunnel would break the
-				// impaired logic unless we did that (a) only if there are other
-				// unimpaired protocols; (b) only during the first iteration of the
-				// ESTABLISH_TUNNEL_WORK_TIME loop. By not discarding here, a true
-				// impaired protocol may require an extra reconnect.
-
-				NoticeAlert("connected tunnel with impaired protocol: %s", connectedTunnel.protocol)
-			}
-
 			// Tunnel establishment has two phases: connection and activation.
 			//
 			// Connection is run concurrently by the establishTunnelWorkers, to minimize
@@ -659,13 +641,6 @@ loop:
 				err := connectedTunnel.Activate(controller.runCtx, controller)
 
 				if err != nil {
-
-					// Assume the Activate failed due to a broken tunnel connection,
-					// currently the most likely case, and classify as impaired, as in
-					// the failed tunnel case above.
-					// TODO: distinguish between network and other errors
-					controller.classifyImpairedProtocol(connectedTunnel)
-
 					NoticeAlert("failed to activate %s: %s", connectedTunnel.serverEntry.IpAddress, err)
 					discardTunnel = true
 				} else {
@@ -780,79 +755,6 @@ loop:
 	NoticeInfo("exiting run tunnels")
 }
 
-// classifyImpairedProtocol tracks "impaired" protocol classifications for failed
-// tunnels. A protocol is classified as impaired if a tunnel using that protocol
-// fails, repeatedly, shortly after the start of the connection. During tunnel
-// establishment, impaired protocols are briefly skipped.
-//
-// One purpose of this measure is to defend against an attack where the adversary,
-// for example, tags an OSSH TCP connection as an "unidentified" protocol; allows
-// it to connect; but then kills the underlying TCP connection after a short time.
-// Since OSSH has less latency than other protocols that may bypass an "unidentified"
-// filter, these other protocols might never be selected for use.
-//
-// Concurrency note: only the runTunnels() goroutine may call classifyImpairedProtocol
-func (controller *Controller) classifyImpairedProtocol(failedTunnel *Tunnel) {
-
-	// If the tunnel failed while activating, its establishedTime will be 0.
-
-	duration := controller.config.clientParameters.Get().Duration(
-		parameters.ImpairedProtocolClassificationDuration)
-
-	if failedTunnel.establishedTime == 0 ||
-		failedTunnel.establishedTime.Add(duration).After(monotime.Now()) {
-		controller.impairedProtocolClassification[failedTunnel.protocol] += 1
-	} else {
-		controller.impairedProtocolClassification[failedTunnel.protocol] = 0
-	}
-
-	// 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.
-
-	if CountNonImpairedProtocols(
-		controller.config.EgressRegion,
-		controller.config.clientParameters.Get().TunnelProtocols(
-			parameters.LimitTunnelProtocols),
-		controller.getImpairedProtocols()) == 0 {
-
-		controller.impairedProtocolClassification = make(map[string]int)
-	}
-}
-
-// getImpairedProtocols returns a list of protocols that have sufficient
-// classifications to be considered impaired protocols.
-//
-// Concurrency note: only the runTunnels() goroutine may call getImpairedProtocols
-func (controller *Controller) getImpairedProtocols() []string {
-
-	NoticeImpairedProtocolClassification(controller.impairedProtocolClassification)
-
-	threshold := controller.config.clientParameters.Get().Int(
-		parameters.ImpairedProtocolClassificationThreshold)
-
-	impairedProtocols := make([]string, 0)
-	for protocol, count := range controller.impairedProtocolClassification {
-		if count >= threshold {
-			impairedProtocols = append(impairedProtocols, protocol)
-		}
-	}
-	return impairedProtocols
-}
-
-// isImpairedProtocol checks if the specified protocol is classified as impaired.
-//
-// Concurrency note: only the runTunnels() goroutine may call isImpairedProtocol
-func (controller *Controller) isImpairedProtocol(protocol string) bool {
-
-	threshold := controller.config.clientParameters.Get().Int(
-		parameters.ImpairedProtocolClassificationThreshold)
-
-	count, ok := controller.impairedProtocolClassification[protocol]
-
-	return ok && count >= threshold
-}
-
 // SignalSeededNewSLOK implements the TunnelOwner interface. This function
 // is called by Tunnel.operateTunnel when the tunnel has received a new,
 // previously unknown SLOK from the server. The Controller triggers an OSL
@@ -1118,10 +1020,10 @@ func (controller *Controller) startEstablishing() {
 	controller.serverAffinityDoneBroadcast = make(chan struct{})
 
 	controller.establishWaitGroup.Add(1)
-	go controller.launchEstablishing(controller.getImpairedProtocols())
+	go controller.launchEstablishing()
 }
 
-func (controller *Controller) launchEstablishing(impairedProtocols []string) {
+func (controller *Controller) launchEstablishing() {
 
 	defer controller.establishWaitGroup.Done()
 
@@ -1196,7 +1098,7 @@ func (controller *Controller) launchEstablishing(impairedProtocols []string) {
 	}
 
 	controller.establishWaitGroup.Add(1)
-	go controller.establishCandidateGenerator(impairedProtocols)
+	go controller.establishCandidateGenerator()
 }
 
 // stopEstablishing signals the establish goroutines to stop and waits
@@ -1436,7 +1338,7 @@ func (controller *Controller) doFetchTactics(
 // establishCandidateGenerator populates the candidate queue with server entries
 // from the data store. Server entries are iterated in rank order, so that promoted
 // servers with higher rank are priority candidates.
-func (controller *Controller) establishCandidateGenerator(impairedProtocols []string) {
+func (controller *Controller) establishCandidateGenerator() {
 	defer controller.establishWaitGroup.Done()
 	defer close(controller.candidateServerEntries)
 
@@ -1512,18 +1414,6 @@ loop:
 				parameters.PrioritizeTunnelProtocolsCandidateCount)
 			usePriorityProtocol := candidateCount < prioritizeCandidateCount
 
-			// Disable impaired protocols. This is only done for the
-			// first iteration of the EstablishTunnelWorkTime
-			// loop since (a) one iteration should be sufficient to
-			// 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).
-
-			var candidateImpairedProtocols []string
-			if i == 0 {
-				candidateImpairedProtocols = impairedProtocols
-			}
-
 			// adjustedEstablishStartTime is establishStartTime shifted
 			// to exclude time spent waiting for network connectivity.
 
@@ -1533,7 +1423,6 @@ loop:
 				serverEntry:                serverEntry,
 				isServerAffinityCandidate:  isServerAffinityCandidate,
 				usePriorityProtocol:        usePriorityProtocol,
-				impairedProtocols:          candidateImpairedProtocols,
 				adjustedEstablishStartTime: adjustedEstablishStartTime,
 			}
 
@@ -1715,15 +1604,13 @@ loop:
 		selectedProtocol, err := selectProtocol(
 			controller.config,
 			candidateServerEntry.serverEntry,
-			candidateServerEntry.impairedProtocols,
 			excludeIntensive,
 			candidateServerEntry.usePriorityProtocol)
 
 		if err == errNoProtocolSupported {
 			// selectProtocol returns errNoProtocolSupported when the server
 			// does not support any protocol that remains after applying the
-			// LimitTunnelProtocols parameter, the impaired protocol filter,
-			// and the excludeMeek flag.
+			// LimitTunnelProtocols parameter and the excludeMeek flag.
 			// Skip this candidate.
 
 			// Unblock other candidates immediately when

psiphon/controller_test.go

@@ -38,10 +38,8 @@ import (
 	"testing"
 	"time"
 
-	"github.com/Psiphon-Labs/goarista/monotime"
 	socks "github.com/Psiphon-Labs/goptlib"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
-	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/parameters"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/protocol"
 	"github.com/elazarl/goproxy"
 )
@@ -107,7 +105,6 @@ func TestUntunneledUpgradeDownload(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -125,7 +122,6 @@ func TestUntunneledResumableUpgradeDownload(t *testing.T) {
 			disruptNetwork:           true,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -143,7 +139,6 @@ func TestUntunneledUpgradeClientIsLatestVersion(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -161,7 +156,6 @@ func TestUntunneledResumableFetchRemoteServerList(t *testing.T) {
 			disruptNetwork:           true,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -179,33 +173,6 @@ func TestTunneledUpgradeClientIsLatestVersion(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
-		})
-}
-
-func TestImpairedProtocols(t *testing.T) {
-
-	// This test sets a tunnelPoolSize of 40 and runs
-	// the session for 1 minute with network disruption
-	// on. All 40 tunnels being disrupted every 10
-	// seconds (followed by ssh keep alive probe timeout)
-	// should be sufficient to trigger at least one
-	// impaired protocol classification.
-
-	controllerRun(t,
-		&controllerRunConfig{
-			expectNoServerEntries:    false,
-			protocol:                 "",
-			clientIsLatestVersion:    true,
-			disableUntunneledUpgrade: true,
-			disableEstablishing:      false,
-			disableApi:               false,
-			tunnelPoolSize:           40,
-			useUpstreamProxy:         false,
-			disruptNetwork:           true,
-			transformHostNames:       false,
-			useFragmentor:            false,
-			runDuration:              1 * time.Minute,
 		})
 }
 
@@ -223,7 +190,6 @@ func TestSSH(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -241,7 +207,6 @@ func TestObfuscatedSSH(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -259,7 +224,6 @@ func TestUnfrontedMeek(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -277,7 +241,6 @@ func TestUnfrontedMeekWithTransformer(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       true,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -295,7 +258,6 @@ func TestFrontedMeek(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -313,7 +275,6 @@ func TestFrontedMeekWithTransformer(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       true,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -331,7 +292,6 @@ func TestFrontedMeekHTTP(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -349,7 +309,6 @@ func TestUnfrontedMeekHTTPS(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -367,7 +326,6 @@ func TestUnfrontedMeekHTTPSWithTransformer(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       true,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -385,7 +343,6 @@ func TestDisabledApi(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -403,7 +360,6 @@ func TestObfuscatedSSHWithUpstreamProxy(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -421,7 +377,6 @@ func TestUnfrontedMeekWithUpstreamProxy(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -439,7 +394,6 @@ func TestUnfrontedMeekHTTPSWithUpstreamProxy(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            false,
-			runDuration:              0,
 		})
 }
 
@@ -457,7 +411,6 @@ func TestObfuscatedSSHFragmentor(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            true,
-			runDuration:              0,
 		})
 }
 
@@ -475,7 +428,6 @@ func TestFrontedMeekFragmentor(t *testing.T) {
 			disruptNetwork:           false,
 			transformHostNames:       false,
 			useFragmentor:            true,
-			runDuration:              0,
 		})
 }
 
@@ -491,7 +443,6 @@ type controllerRunConfig struct {
 	disruptNetwork           bool
 	transformHostNames       bool
 	useFragmentor            bool
-	runDuration              time.Duration
 }
 
 func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
@@ -627,11 +578,6 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 
 	var clientUpgradeDownloadedBytesCount int32
 	var remoteServerListDownloadedBytesCount int32
-	var impairedProtocolCount int32
-	var impairedProtocolClassification = struct {
-		sync.RWMutex
-		classification map[string]int
-	}{classification: make(map[string]int)}
 
 	SetNoticeWriter(NewNoticeReceiver(
 		func(notice []byte) {
@@ -708,47 +654,6 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 					default:
 					}
 				}
-
-			case "ImpairedProtocolClassification":
-
-				classification := payload["classification"].(map[string]interface{})
-
-				impairedProtocolClassification.Lock()
-				impairedProtocolClassification.classification = make(map[string]int)
-				for k, v := range classification {
-					count := int(v.(float64))
-					if count >= config.clientParameters.Get().Int(parameters.ImpairedProtocolClassificationThreshold) {
-						atomic.AddInt32(&impairedProtocolCount, 1)
-					}
-					impairedProtocolClassification.classification[k] = count
-				}
-				impairedProtocolClassification.Unlock()
-
-			case "ActiveTunnel":
-
-				serverProtocol := payload["protocol"].(string)
-
-				classification := make(map[string]int)
-				impairedProtocolClassification.RLock()
-				for k, v := range impairedProtocolClassification.classification {
-					classification[k] = v
-				}
-				impairedProtocolClassification.RUnlock()
-
-				count, ok := classification[serverProtocol]
-				if ok && count >= config.clientParameters.Get().Int(parameters.ImpairedProtocolClassificationThreshold) {
-
-					// TODO: Fix this test case. Use of TunnelPoolSize breaks this
-					// case, as many tunnel establishments are occurring in parallel,
-					// and it can happen that a protocol is classified as impaired
-					// while a tunnel with that protocol is established and set
-					// active.
-
-					// *not* t.Fatalf
-					t.Logf("unexpected tunnel using impaired protocol: %s, %+v",
-						serverProtocol, classification)
-				}
-
 			}
 		}))
 
@@ -828,36 +733,6 @@ func controllerRun(t *testing.T, runConfig *controllerRunConfig) {
 			if !runConfig.disruptNetwork {
 				fetchAndVerifyWebsite(t, httpProxyPort)
 			}
-
-			// Test: run for duration, periodically using the tunnel to
-			// ensure failed tunnel detection, and ultimately hitting
-			// impaired protocol checks.
-
-			startTime := monotime.Now()
-
-			for {
-
-				time.Sleep(1 * time.Second)
-				useTunnel(t, httpProxyPort)
-
-				if startTime.Add(runConfig.runDuration).Before(monotime.Now()) {
-					break
-				}
-			}
-
-			// Test: with disruptNetwork, impaired protocols should be exercised
-
-			if runConfig.runDuration > 0 && runConfig.disruptNetwork {
-				count := atomic.LoadInt32(&impairedProtocolCount)
-				if count <= 0 {
-					t.Fatalf("unexpected impaired protocol count: %d", count)
-				} else {
-					impairedProtocolClassification.RLock()
-					t.Logf("impaired protocol classification: %+v",
-						impairedProtocolClassification.classification)
-					impairedProtocolClassification.RUnlock()
-				}
-			}
 		}
 	}
 
@@ -1067,29 +942,6 @@ func fetchAndVerifyWebsite(t *testing.T, httpProxyPort int) error {
 	return nil
 }
 
-func useTunnel(t *testing.T, httpProxyPort int) {
-
-	// No action on errors as the tunnel is expected to fail sometimes
-
-	testUrl := "https://psiphon3.com"
-	roundTripTimeout := 1 * time.Second
-	proxyUrl, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", httpProxyPort))
-	if err != nil {
-		return
-	}
-	httpClient := &http.Client{
-		Transport: &http.Transport{
-			Proxy: http.ProxyURL(proxyUrl),
-		},
-		Timeout: roundTripTimeout,
-	}
-	response, err := httpClient.Get(testUrl)
-	if err != nil {
-		return
-	}
-	response.Body.Close()
-}
-
 // Note: Valid values for disruptorMaxConnectionBytes depend on the production
 // network; for example, the size of the remote server list resource must exceed
 // disruptorMaxConnectionBytes or else TestUntunneledResumableFetchRemoteServerList

psiphon/dataStore.go

@@ -604,9 +604,9 @@ func newTargetServerEntryIterator(config *Config, isTactics bool) (bool, *Server
 		limitTunnelProtocols := config.clientParameters.Get().TunnelProtocols(parameters.LimitTunnelProtocols)
 		if len(limitTunnelProtocols) > 0 {
 			// At the ServerEntryIterator level, only limitTunnelProtocols is applied;
-			// impairedTunnelProtocols and excludeMeek are handled higher up.
+			// excludeIntensive is handled higher up.
 			if len(serverEntry.GetSupportedProtocols(
-				config.UseUpstreamProxy(), limitTunnelProtocols, nil, false)) == 0 {
+				config.UseUpstreamProxy(), limitTunnelProtocols, false)) == 0 {
 				return false, nil, common.ContextError(errors.New("TargetServerEntry does not support LimitTunnelProtocols"))
 			}
 		}
@@ -636,9 +636,7 @@ func (iterator *ServerEntryIterator) Reset() error {
 
 	// For diagnostics, it's useful to count the number of known server
 	// entries that satisfy both the egress region and tunnel protocol
-	// requirements. The tunnel protocol filter is not applied by the iterator
-	// as protocol filtering, including impaire protocol and exclude-meek
-	// logic, is all handled higher up.
+	// requirements (excluding excludeIntensive logic).
 
 	// TODO: for isTacticsServerEntryIterator, emit tactics candidate count.
 
@@ -850,14 +848,14 @@ func scanServerEntries(scanner func(*protocol.ServerEntry)) error {
 
 // CountServerEntries returns a count of stored servers for the
 // specified region and tunnel protocols.
-func CountServerEntries(useUpstreamProxy bool, region string, tunnelProtocols []string) int {
+func CountServerEntries(useUpstreamProxy bool, region string, limitTunnelProtocols []string) int {
 	count := 0
 	err := scanServerEntries(func(serverEntry *protocol.ServerEntry) {
 		if (region == "" || serverEntry.Region == region) &&
-			(len(tunnelProtocols) == 0 ||
-				// When CountServerEntries is called only limitTunnelProtocols is known;
-				// impairedTunnelProtocols and excludeMeek may not apply.
-				len(serverEntry.GetSupportedProtocols(useUpstreamProxy, tunnelProtocols, nil, false)) > 0) {
+			(len(limitTunnelProtocols) == 0 ||
+				// When CountServerEntries is called only limitTunnelProtocols
+				// is known; excludeIntensive may not apply.
+				len(serverEntry.GetSupportedProtocols(useUpstreamProxy, limitTunnelProtocols, false)) > 0) {
 			count += 1
 		}
 	})
@@ -870,40 +868,6 @@ func CountServerEntries(useUpstreamProxy bool, region string, tunnelProtocols []
 	return count
 }
 
-// CountNonImpairedProtocols returns the number of distinct tunnel
-// protocols supported by stored server entries, excluding the
-// specified impaired protocols.
-func CountNonImpairedProtocols(
-	region string,
-	limitTunnelProtocols, impairedProtocols []string) int {
-
-	distinctProtocols := make(map[string]bool)
-
-	err := scanServerEntries(func(serverEntry *protocol.ServerEntry) {
-		if region == "" || serverEntry.Region == region {
-			for _, protocol := range protocol.SupportedTunnelProtocols {
-				if serverEntry.SupportsProtocol(protocol) {
-					if len(limitTunnelProtocols) == 0 ||
-						common.Contains(limitTunnelProtocols, protocol) {
-						distinctProtocols[protocol] = true
-					}
-				}
-			}
-		}
-	})
-
-	for _, protocol := range impairedProtocols {
-		delete(distinctProtocols, protocol)
-	}
-
-	if err != nil {
-		NoticeAlert("CountNonImpairedProtocols failed: %s", err)
-		return 0
-	}
-
-	return len(distinctProtocols)
-}
-
 // ReportAvailableRegions prints a notice with the available egress regions.
 func ReportAvailableRegions(config *Config) {
 
@@ -913,10 +877,10 @@ func ReportAvailableRegions(config *Config) {
 	regions := make(map[string]bool)
 	err := scanServerEntries(func(serverEntry *protocol.ServerEntry) {
 		if len(limitTunnelProtocols) == 0 ||
-			// When ReportAvailableRegions is called only limitTunnelProtocols is known;
-			// impairedTunnelProtocols and excludeMeek may not apply.
+			// When ReportAvailableRegions is called only limitTunnelProtocols
+			// is known; excludeIntensive may not apply.
 			len(serverEntry.GetSupportedProtocols(
-				config.UseUpstreamProxy(), limitTunnelProtocols, nil, false)) > 0 {
+				config.UseUpstreamProxy(), limitTunnelProtocols, false)) > 0 {
 
 			regions[serverEntry.Region] = true
 		}

psiphon/meekConn.go

@@ -232,7 +232,7 @@ func DialMeek(
 		// classify Psiphon traffic on some CDNs but not others) may throttle non-MiM CDNs so that our server
 		// selection always chooses tunnels to the MiM CDN (without any server cert verification, we won't
 		// exclusively connect to non-MiM CDNs); then the adversary kills the underlying TCP connection after
-		// some short period. This is mitigated by the "impaired" protocol classification mechanism.
+		// some short period. This is partially mitigated by tactics mechanisms.
 
 		scheme = "https"
 

psiphon/notice.go

@@ -566,12 +566,6 @@ func NoticeSessionId(sessionId string) {
 		"sessionId", sessionId)
 }
 
-func NoticeImpairedProtocolClassification(impairedProtocolClassification map[string]int) {
-	singletonNoticeLogger.outputNotice(
-		"ImpairedProtocolClassification", noticeIsDiagnostic,
-		"classification", impairedProtocolClassification)
-}
-
 // NoticeUntunneled indicates than an address has been classified as untunneled and is being
 // accessed directly.
 //

psiphon/tunnel.go

@@ -532,14 +532,12 @@ var errNoProtocolSupported = errors.New("server does not support any required pr
 func selectProtocol(
 	config *Config,
 	serverEntry *protocol.ServerEntry,
-	impairedProtocols []string,
 	excludeIntensive bool,
 	usePriorityProtocol bool) (selectedProtocol string, err error) {
 
 	candidateProtocols := serverEntry.GetSupportedProtocols(
 		config.UseUpstreamProxy(),
 		config.clientParameters.Get().TunnelProtocols(parameters.LimitTunnelProtocols),
-		impairedProtocols,
 		excludeIntensive)
 	if len(candidateProtocols) == 0 {
 		return "", errNoProtocolSupported