Add ISP to TrafficRules and remove EnforceLimitsServerSide

- Support filtering by ISP for traffic rules and meek rate limiting.

- Remove tactics server-side tunnel protocol limiting: enforcing before
  handshake is too coarse grained for tactics targetting; had bug: meek
  "client protocol" wasn't used; same effect can be achieved in traffic
  rules.

psiphon/common/tactics/tactics.go

@@ -233,10 +233,6 @@ type Server struct {
 	// RequestObfuscatedKey is the tactics request obfuscation key.
 	RequestObfuscatedKey []byte
 
-	// EnforceLimitsServerSide enables server-side enforcement of certain limit
-	// tactics parameters via Listeners.
-	EnforceLimitsServerSide bool
-
 	// DefaultTactics is the baseline tactics for all clients. It must include a
 	// TTL and Probability.
 	DefaultTactics Tactics
@@ -268,6 +264,7 @@ type Filter struct {
 	// Regions specifies a list of GeoIP regions/countries the client
 	// must match.
 	Regions []string
+
 	// ISPs specifies a list of GeoIP ISPs the client must match.
 	ISPs []string
 
@@ -456,7 +453,6 @@ func NewServer(
 			server.RequestPublicKey = newServer.RequestPublicKey
 			server.RequestPrivateKey = newServer.RequestPrivateKey
 			server.RequestObfuscatedKey = newServer.RequestObfuscatedKey
-			server.EnforceLimitsServerSide = newServer.EnforceLimitsServerSide
 			server.DefaultTactics = newServer.DefaultTactics
 			server.FilteredTactics = newServer.FilteredTactics
 
@@ -1145,17 +1141,6 @@ func (listener *Listener) Accept() (net.Conn, error) {
 
 		p := clientParameters.Get()
 
-		if listener.server.EnforceLimitsServerSide {
-			tunnelProtocols := p.TunnelProtocols(parameters.LimitTunnelProtocols)
-			if len(tunnelProtocols) > 0 &&
-				!common.Contains(tunnelProtocols, listener.tunnelProtocol) {
-				// Don't accept this connection as its tactics prohibits the
-				// listener's tunnel protocol.
-				conn.Close()
-				continue
-			}
-		}
-
 		// Wrap the conn in a fragmentor.Conn, subject to tactics parameters.
 		//
 		// Limitation: this server-side fragmentation is not synchronized with

psiphon/common/tactics/tactics_test.go

@@ -33,6 +33,7 @@ import (
 	"time"
 
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
+	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/fragmentor"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/parameters"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/protocol"
 )
@@ -49,7 +50,6 @@ func TestTactics(t *testing.T) {
       "RequestPublicKey" : "%s",
       "RequestPrivateKey" : "%s",
       "RequestObfuscatedKey" : "%s",
-      "EnforceLimitsServerSide" : true,
       "DefaultTactics" : {
         "TTL" : "1s",
         "Probability" : %0.1f,
@@ -104,7 +104,11 @@ func TestTactics(t *testing.T) {
           },
           "Tactics" : {
             "Parameters" : {
-              "LimitTunnelProtocols" : ["SSH"]
+              "FragmentorDownstreamMinTotalBytes" : 1,
+              "FragmentorDownstreamMaxTotalBytes" : 1,
+              "FragmentorDownstreamMinWriteBytes" : 1,
+              "FragmentorDownstreamMaxWriteBytes" : 1,
+              "FragmentorDownstreamLimitProtocols" : ["OSSH"]
             }
           }
         }
@@ -129,8 +133,8 @@ func TestTactics(t *testing.T) {
 	expectedApplyCount := 3
 
 	listenerProtocol := "OSSH"
-	listenerProhibitedGeoIP := func(string) common.GeoIPData { return common.GeoIPData{Country: "R7"} }
-	listenerAllowedGeoIP := func(string) common.GeoIPData { return common.GeoIPData{Country: "R8"} }
+	listenerFragmentedGeoIP := func(string) common.GeoIPData { return common.GeoIPData{Country: "R7"} }
+	listenerUnfragmentedGeoIP := func(string) common.GeoIPData { return common.GeoIPData{Country: "R8"} }
 
 	tacticsConfig := fmt.Sprintf(
 		tacticsConfigTemplate,
@@ -736,17 +740,17 @@ func TestTactics(t *testing.T) {
 	listenerTestCases := []struct {
 		description      string
 		geoIPLookup      func(string) common.GeoIPData
-		expectConnection bool
+		expectFragmentor bool
 	}{
 		{
-			"connection prohibited",
-			listenerProhibitedGeoIP,
-			false,
+			"fragmented",
+			listenerFragmentedGeoIP,
+			true,
 		},
 		{
-			"connection allowed",
-			listenerAllowedGeoIP,
-			true,
+			"unfragmented",
+			listenerUnfragmentedGeoIP,
+			false,
 		},
 	}
 
@@ -770,13 +774,12 @@ func TestTactics(t *testing.T) {
 				return
 			}
 
-			result := make(chan struct{}, 1)
+			result := make(chan net.Conn, 1)
 
 			go func() {
 				serverConn, err := tacticsListener.Accept()
 				if err == nil {
-					result <- *new(struct{})
-					serverConn.Close()
+					result <- serverConn
 				}
 			}()
 
@@ -784,14 +787,16 @@ func TestTactics(t *testing.T) {
 			defer timer.Stop()
 
 			select {
-			case <-result:
-				if !testCase.expectConnection {
-					t.Fatalf("unexpected accepted connection")
+			case serverConn := <-result:
+				_, isFragmentor := serverConn.(*fragmentor.Conn)
+				if testCase.expectFragmentor && !isFragmentor {
+					t.Fatalf("unexpected non-fragmentor: %T", serverConn)
+				} else if !testCase.expectFragmentor && isFragmentor {
+					t.Fatalf("unexpected fragmentor:  %T", serverConn)
 				}
+				serverConn.Close()
 			case <-timer.C:
-				if testCase.expectConnection {
-					t.Fatalf("timeout before expected accepted connection")
-				}
+				t.Fatalf("timeout before expected accepted connection")
 			}
 
 			clientConn.Close()

psiphon/server/meek.go

@@ -653,17 +653,28 @@ func (server *MeekServer) getSessionOrEndpoint(
 
 func (server *MeekServer) rateLimit(clientIP string) bool {
 
-	historySize, thresholdSeconds, regions, GCTriggerCount, _ :=
+	historySize, thresholdSeconds, regions, ISPs, GCTriggerCount, _ :=
 		server.support.TrafficRulesSet.GetMeekRateLimiterConfig()
 
 	if historySize == 0 {
 		return false
 	}
 
-	if len(regions) > 0 {
+	if len(regions) > 0 || len(ISPs) > 0 {
+
 		// TODO: avoid redundant GeoIP lookups?
-		if !common.Contains(regions, server.support.GeoIPService.Lookup(clientIP).Country) {
-			return false
+		geoIPData := server.support.GeoIPService.Lookup(clientIP)
+
+		if len(regions) > 0 {
+			if !common.Contains(regions, geoIPData.Country) {
+				return false
+			}
+		}
+
+		if len(ISPs) > 0 {
+			if !common.Contains(ISPs, geoIPData.ISP) {
+				return false
+			}
 		}
 	}
 
@@ -716,7 +727,7 @@ func (server *MeekServer) rateLimit(clientIP string) bool {
 
 func (server *MeekServer) rateLimitWorker() {
 
-	_, _, _, _, reapFrequencySeconds :=
+	_, _, _, _, _, reapFrequencySeconds :=
 		server.support.TrafficRulesSet.GetMeekRateLimiterConfig()
 
 	timer := time.NewTimer(time.Duration(reapFrequencySeconds) * time.Second)
@@ -726,7 +737,7 @@ func (server *MeekServer) rateLimitWorker() {
 		select {
 		case <-timer.C:
 
-			_, thresholdSeconds, _, _, reapFrequencySeconds :=
+			_, thresholdSeconds, _, _, _, reapFrequencySeconds :=
 				server.support.TrafficRulesSet.GetMeekRateLimiterConfig()
 
 			server.rateLimitLock.Lock()

psiphon/server/trafficRules.go

@@ -71,7 +71,7 @@ type TrafficRulesSet struct {
 	// not any meek request for an existing session, if the
 	// MeekRateLimiterHistorySize requests occur in
 	// MeekRateLimiterThresholdSeconds. The scope of rate limiting may be
-	// limited using LimitMeekRateLimiterRegions.
+	// limited using LimitMeekRateLimiterRegions and LimitMeekRateLimiterISPs.
 	//
 	// Hot reloading a new history size will result in existing history being
 	// truncated.
@@ -84,9 +84,15 @@ type TrafficRulesSet struct {
 	// MeekRateLimiterRegions, if set, limits application of the meek
 	// late-stage rate limiter to clients in the specified list of GeoIP
 	// countries. When omitted or empty, meek rate limiting, if configured,
-	// is applied to all clients.
+	// is applied to all client countries.
 	MeekRateLimiterRegions []string
 
+	// MeekRateLimiterISPs, if set, limits application of the meek
+	// late-stage rate limiter to clients in the specified list of GeoIP
+	// ISPs. When omitted or empty, meek rate limiting, if configured,
+	// is applied to all client ISPs.
+	MeekRateLimiterISPs []string
+
 	// MeekRateLimiterGarbageCollectionTriggerCount specifies the number of
 	// rate limit events after which garbage collection is manually triggered
 	// in order to reclaim memory used by rate limited and other rejected
@@ -110,11 +116,15 @@ type TrafficRulesFilter struct {
 	// matches.
 	TunnelProtocols []string
 
-	// Regions is a list of client GeoIP countries that the client must
-	// reolve to to match this filter. When omitted or empty, any client
-	// region matches.
+	// Regions is a list of countries that the client must geolocate to in
+	// order to match this filter. When omitted or empty, any client country
+	// matches.
 	Regions []string
 
+	// ISPs is a list of ISPs that the client must geolocate to in order to
+	// match this filter. When omitted or empty, any client ISP matches.
+	ISPs []string
+
 	// APIProtocol specifies whether the client must use the SSH
 	// API protocol (when "ssh") or the web API protocol (when "web").
 	// When omitted or blank, any API protocol matches.
@@ -262,6 +272,7 @@ func NewTrafficRulesSet(filename string) (*TrafficRulesSet, error) {
 			set.MeekRateLimiterHistorySize = newSet.MeekRateLimiterHistorySize
 			set.MeekRateLimiterThresholdSeconds = newSet.MeekRateLimiterThresholdSeconds
 			set.MeekRateLimiterRegions = newSet.MeekRateLimiterRegions
+			set.MeekRateLimiterISPs = newSet.MeekRateLimiterISPs
 			set.MeekRateLimiterGarbageCollectionTriggerCount = newSet.MeekRateLimiterGarbageCollectionTriggerCount
 			set.MeekRateLimiterReapHistoryFrequencySeconds = newSet.MeekRateLimiterReapHistoryFrequencySeconds
 			set.DefaultRules = newSet.DefaultRules
@@ -470,6 +481,12 @@ func (set *TrafficRulesSet) GetTrafficRules(
 			}
 		}
 
+		if len(filteredRules.Filter.ISPs) > 0 {
+			if !common.Contains(filteredRules.Filter.ISPs, geoIPData.ISP) {
+				continue
+			}
+		}
+
 		if filteredRules.Filter.APIProtocol != "" {
 			if !state.completed {
 				continue
@@ -599,7 +616,7 @@ func (set *TrafficRulesSet) GetTrafficRules(
 
 // GetMeekRateLimiterConfig gets a snapshot of the meek rate limiter
 // configuration values.
-func (set *TrafficRulesSet) GetMeekRateLimiterConfig() (int, int, []string, int, int) {
+func (set *TrafficRulesSet) GetMeekRateLimiterConfig() (int, int, []string, []string, int, int) {
 
 	set.ReloadableFile.RLock()
 	defer set.ReloadableFile.RUnlock()
@@ -618,6 +635,7 @@ func (set *TrafficRulesSet) GetMeekRateLimiterConfig() (int, int, []string, int,
 	return set.MeekRateLimiterHistorySize,
 		set.MeekRateLimiterThresholdSeconds,
 		set.MeekRateLimiterRegions,
+		set.MeekRateLimiterISPs,
 		GCTriggerCount,
 		reapFrequencySeconds
 }