Add DisallowTCP/UDPPorts traffic rules

psiphon/server/server_test.go

@@ -1583,12 +1583,19 @@ func paveTrafficRulesFile(
 		t.Fatalf("unexpected intLookupThreshold")
 	}
 
-	allowTCPPorts := fmt.Sprintf("%d", mockWebServerPort)
-	allowUDPPorts := "53, 123, 10001, 10002, 10003, 10004, 10005, 10006, 10007, 10008, 10009, 10010"
+	TCPPorts := fmt.Sprintf("%d", mockWebServerPort)
+	UDPPorts := "53, 123, 10001, 10002, 10003, 10004, 10005, 10006, 10007, 10008, 10009, 10010"
+
+	allowTCPPorts := TCPPorts
+	allowUDPPorts := UDPPorts
+	disallowTCPPorts := "0"
+	disallowUDPPorts := "0"
 
 	if deny {
 		allowTCPPorts = "0"
 		allowUDPPorts = "0"
+		disallowTCPPorts = TCPPorts
+		disallowUDPPorts = UDPPorts
 	}
 
 	authorizationFilterFormat := `,
@@ -1600,6 +1607,11 @@ func paveTrafficRulesFile(
 		authorizationFilter = fmt.Sprintf(authorizationFilterFormat, accessType)
 	}
 
+	// Supports two traffic rule test cases:
+	//
+	// 1. no ports are allowed until after the filtered rule is applied
+	// 2. no required ports are allowed (deny = true)
+
 	trafficRulesJSONFormat := `
     {
         "DefaultRules" :  {
@@ -1630,7 +1642,9 @@ func paveTrafficRulesFile(
                         "WriteBytesPerSecond": 2097152
                     },
                     "AllowTCPPorts" : [%s],
-                    "AllowUDPPorts" : [%s]
+                    "AllowUDPPorts" : [%s],
+                    "DisallowTCPPorts" : [%s],
+                    "DisallowUDPPorts" : [%s]
                 }
             }
         ]
@@ -1640,7 +1654,8 @@ func paveTrafficRulesFile(
 	trafficRulesJSON := fmt.Sprintf(
 		trafficRulesJSONFormat,
 		livenessTestSize, livenessTestSize,
-		propagationChannelID, authorizationFilter, allowTCPPorts, allowUDPPorts)
+		propagationChannelID, authorizationFilter,
+		allowTCPPorts, allowUDPPorts, disallowTCPPorts, disallowUDPPorts)
 
 	err := ioutil.WriteFile(trafficRulesFilename, []byte(trafficRulesJSON), 0600)
 	if err != nil {

psiphon/server/trafficRules.go

@@ -204,28 +204,38 @@ type TrafficRules struct {
 	// DefaultRules, DEFAULT_MAX_UDP_PORT_FORWARD_COUNT is used.
 	MaxUDPPortForwardCount *int
 
-	// AllowTCPPorts specifies a whitelist of TCP ports that
-	// are permitted for port forwarding. When set, only ports
-	// in the list are accessible to clients.
+	// AllowTCPPorts specifies a list of TCP ports that are permitted for port
+	// forwarding. When set, only ports in the list are accessible to clients.
 	AllowTCPPorts []int
 
-	// AllowUDPPorts specifies a whitelist of UDP ports that
-	// are permitted for port forwarding. When set, only ports
-	// in the list are accessible to clients.
+	// AllowUDPPorts specifies a list of UDP ports that are permitted for port
+	// forwarding. When set, only ports in the list are accessible to clients.
 	AllowUDPPorts []int
 
-	// AllowSubnets specifies a list of IP address subnets for
-	// which all TCP and UDP ports are allowed. This list is
-	// consulted if a port is disallowed by the AllowTCPPorts
-	// or AllowUDPPorts configuration. Each entry is a IP subnet
-	// in CIDR notation.
-	// Limitation: currently, AllowSubnets only matches port
-	// forwards where the client sends an IP address. Domain
-	// names are not resolved before checking AllowSubnets.
+	// DisallowTCPPorts specifies a list of TCP ports that are not permitted for
+	// port forwarding. DisallowTCPPorts takes priority over AllowTCPPorts and
+	// AllowSubnets.
+	DisallowTCPPorts []int
+
+	// DisallowUDPPorts specifies a list of UDP ports that are not permitted for
+	// port forwarding. DisallowUDPPorts takes priority over AllowUDPPorts and
+	// AllowSubnets.
+	DisallowUDPPorts []int
+
+	// AllowSubnets specifies a list of IP address subnets for which all TCP and
+	// UDP ports are allowed. This list is consulted if a port is disallowed by
+	// the AllowTCPPorts or AllowUDPPorts configuration. Each entry is a IP
+	// subnet in CIDR notation.
+	//
+	// Limitation: currently, AllowSubnets only matches port forwards where the
+	// client sends an IP address. Domain names are not resolved before checking
+	// AllowSubnets.
 	AllowSubnets []string
 
-	allowTCPPortsLookup map[int]bool
-	allowUDPPortsLookup map[int]bool
+	allowTCPPortsLookup    map[int]bool
+	allowUDPPortsLookup    map[int]bool
+	disallowTCPPortsLookup map[int]bool
+	disallowUDPPortsLookup map[int]bool
 }
 
 // RateLimits is a clone of common.RateLimits with pointers
@@ -397,6 +407,20 @@ func (set *TrafficRulesSet) initLookups() {
 				rules.allowUDPPortsLookup[port] = true
 			}
 		}
+
+		if len(rules.DisallowTCPPorts) >= intLookupThreshold {
+			rules.disallowTCPPortsLookup = make(map[int]bool)
+			for _, port := range rules.DisallowTCPPorts {
+				rules.disallowTCPPortsLookup[port] = true
+			}
+		}
+
+		if len(rules.DisallowUDPPorts) >= intLookupThreshold {
+			rules.disallowUDPPortsLookup = make(map[int]bool)
+			for _, port := range rules.DisallowUDPPorts {
+				rules.disallowUDPPortsLookup[port] = true
+			}
+		}
 	}
 
 	initTrafficRulesFilterLookups := func(filter *TrafficRulesFilter) {
@@ -673,6 +697,16 @@ func (set *TrafficRulesSet) GetTrafficRules(
 			trafficRules.allowUDPPortsLookup = filteredRules.Rules.allowUDPPortsLookup
 		}
 
+		if filteredRules.Rules.DisallowTCPPorts != nil {
+			trafficRules.DisallowTCPPorts = filteredRules.Rules.DisallowTCPPorts
+			trafficRules.disallowTCPPortsLookup = filteredRules.Rules.disallowTCPPortsLookup
+		}
+
+		if filteredRules.Rules.DisallowUDPPorts != nil {
+			trafficRules.DisallowUDPPorts = filteredRules.Rules.DisallowUDPPorts
+			trafficRules.disallowUDPPortsLookup = filteredRules.Rules.disallowUDPPortsLookup
+		}
+
 		if filteredRules.Rules.AllowSubnets != nil {
 			trafficRules.AllowSubnets = filteredRules.Rules.AllowSubnets
 		}
@@ -692,6 +726,20 @@ func (set *TrafficRulesSet) GetTrafficRules(
 
 func (rules *TrafficRules) AllowTCPPort(remoteIP net.IP, port int) bool {
 
+	if len(rules.DisallowTCPPorts) > 0 {
+		if rules.disallowTCPPortsLookup != nil {
+			if rules.disallowTCPPortsLookup[port] == true {
+				return false
+			}
+		} else {
+			for _, disallowPort := range rules.DisallowTCPPorts {
+				if port == disallowPort {
+					return false
+				}
+			}
+		}
+	}
+
 	if len(rules.AllowTCPPorts) == 0 {
 		return true
 	}
@@ -713,6 +761,20 @@ func (rules *TrafficRules) AllowTCPPort(remoteIP net.IP, port int) bool {
 
 func (rules *TrafficRules) AllowUDPPort(remoteIP net.IP, port int) bool {
 
+	if len(rules.DisallowUDPPorts) > 0 {
+		if rules.disallowUDPPortsLookup != nil {
+			if rules.disallowUDPPortsLookup[port] == true {
+				return false
+			}
+		} else {
+			for _, disallowPort := range rules.DisallowUDPPorts {
+				if port == disallowPort {
+					return false
+				}
+			}
+		}
+	}
+
 	if len(rules.AllowUDPPorts) == 0 {
 		return true
 	}