hardening(proxy): don't proxy private IP addresses

...by default.

This is useful when `RelayDomainNamePattern` is lax (e.g. just "$")
(which is not the case by default, so this is simply
a hardening measure).

proxy/README.md

@@ -31,6 +31,9 @@ The Snowflake proxy can be run with the following options:
 Usage of ./proxy:
   -allow-non-tls-relay
         allow relay without tls encryption
+  -allow-proxying-to-private-addresses
+        allow forwarding client connections to private IP addresses.
+        Useful when a Snowflake server (relay) is hosted on the same private network as this proxy.
   -allowed-relay-hostname-pattern string
         a pattern to specify allowed hostname pattern for relay URL. (default "snowflake.torproject.net$")
   -broker string

proxy/lib/proxy-go_test.go

@@ -495,10 +495,11 @@ func TestUtilityFuncs(t *testing.T) {
 	})
 	Convey("isRelayURLAcceptable", t, func() {
 		testingVector := []struct {
-			pattern     string
-			allowNonTLS bool
-			targetURL   string
-			expects     error
+			pattern               string
+			allowPrivateAddresses bool
+			allowNonTLS           bool
+			targetURL             string
+			expects               error
 		}{
 			// These are copied from `TestMatchMember`.
 			{pattern: "^snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://snowflake.torproject.net", expects: nil},
@@ -525,6 +526,20 @@ func TestUtilityFuncs(t *testing.T) {
 			{pattern: "^1.1.1.1$", allowNonTLS: true, targetURL: "ws://1.1.1.1/test?test=test#test", expects: nil},
 			{pattern: "^1.1.1.1$", allowNonTLS: true, targetURL: "ws://231.1.1.1/test?test=test#test", expects: fmt.Errorf("")},
 			{pattern: "1.1.1.1$", allowNonTLS: true, targetURL: "ws://231.1.1.1/test?test=test#test", expects: nil},
+			// Private IP address
+			{pattern: "$", allowNonTLS: true, targetURL: "ws://192.168.1.1", expects: fmt.Errorf("")},
+			{pattern: "$", allowNonTLS: true, targetURL: "ws://127.0.0.1", expects: fmt.Errorf("")},
+			{pattern: "$", allowNonTLS: true, targetURL: "ws://[fc00::]/", expects: fmt.Errorf("")},
+			{pattern: "$", allowNonTLS: true, targetURL: "ws://[::1]/", expects: fmt.Errorf("")},
+			{pattern: "$", allowNonTLS: true, targetURL: "ws://0.0.0.0/", expects: fmt.Errorf("")},
+			{pattern: "$", allowNonTLS: true, targetURL: "ws://169.254.1.1/", expects: fmt.Errorf("")},
+			{pattern: "$", allowNonTLS: true, targetURL: "ws://100.111.1.1/", expects: fmt.Errorf("")},
+			{pattern: "192.168.1.100$", allowPrivateAddresses: true, allowNonTLS: true, targetURL: "ws://192.168.1.100/test?test=test", expects: nil},
+			{pattern: "localhost$", allowPrivateAddresses: true, allowNonTLS: true, targetURL: "ws://localhost/test?test=test", expects: nil},
+			{pattern: "::1$", allowPrivateAddresses: true, allowNonTLS: true, targetURL: "ws://[::1]/test?test=test", expects: nil},
+			// Multicast IP address. `checkIsRelayURLAcceptable` allows it,
+			// but it's not valid in the context of WebSocket
+			{pattern: "255.255.255.255$", allowPrivateAddresses: true, allowNonTLS: true, targetURL: "ws://255.255.255.255/test?test=test", expects: nil},
 
 			// Port
 			{pattern: "^snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://snowflake.torproject.net:8080/test?test=test#test", expects: nil},
@@ -551,7 +566,7 @@ func TestUtilityFuncs(t *testing.T) {
 			{pattern: "snowflake.torproject.net$", allowNonTLS: true, targetURL: "ftp://snowflake.torproject.net", expects: fmt.Errorf("")},
 		}
 		for _, v := range testingVector {
-			err := checkIsRelayURLAcceptable(v.pattern, v.allowNonTLS, v.targetURL)
+			err := checkIsRelayURLAcceptable(v.pattern, v.allowPrivateAddresses, v.allowNonTLS, v.targetURL)
 			if v.expects != nil {
 				So(err, ShouldNotBeNil)
 			} else {

proxy/lib/snowflake.go

@@ -138,7 +138,12 @@ type SnowflakeProxy struct {
 	// There is no look ahead assertion when matching domain name suffix,
 	// thus the string prepend the suffix does not need to be empty or ends with a dot.
 	RelayDomainNamePattern string
-	AllowNonTLSRelay       bool
+	// AllowProxyingToPrivateAddresses determines whether to allow forwarding
+	// client connections to private IP addresses.
+	// Useful when a Snowflake server (relay) is hosted on the same private network
+	// as this proxy.
+	AllowProxyingToPrivateAddresses bool
+	AllowNonTLSRelay                bool
 	// NATProbeURL is the URL of the probe service we use for NAT checks
 	NATProbeURL string
 	// NATTypeMeasurementInterval is time before NAT type is retested
@@ -601,7 +606,7 @@ func (sf *SnowflakeProxy) runSession(sid string) {
 	log.Printf("Received Offer From Broker: \n\t%s", strings.ReplaceAll(offer.SDP, "\n", "\n\t"))
 
 	if relayURL != "" {
-		if err := checkIsRelayURLAcceptable(sf.RelayDomainNamePattern, sf.AllowNonTLSRelay, relayURL); err != nil {
+		if err := checkIsRelayURLAcceptable(sf.RelayDomainNamePattern, sf.AllowProxyingToPrivateAddresses, sf.AllowNonTLSRelay, relayURL); err != nil {
 			log.Printf("bad offer from broker: %v", err)
 			tokens.ret()
 			return
@@ -644,6 +649,7 @@ func (sf *SnowflakeProxy) runSession(sid string) {
 // Returns nil if the relayURL is acceptable
 func checkIsRelayURLAcceptable(
 	allowedHostNamePattern string,
+	allowPrivateIPs bool,
 	allowNonTLSRelay bool,
 	relayURL string,
 ) error {
@@ -651,6 +657,16 @@ func checkIsRelayURLAcceptable(
 	if err != nil {
 		return fmt.Errorf("bad Relay URL %w", err)
 	}
+	if !allowPrivateIPs {
+		ip := net.ParseIP(parsedRelayURL.Hostname())
+		// Otherwise it's a domain name, or an invalid IP.
+		if ip != nil {
+			// We should probably use a ready library for this.
+			if !isRemoteAddress(ip) {
+				return fmt.Errorf("rejected Relay URL: private IPs are not allowed")
+			}
+		}
+	}
 	if !allowNonTLSRelay && parsedRelayURL.Scheme != "wss" {
 		return fmt.Errorf("rejected Relay URL protocol: non-TLS not allowed")
 	}

proxy/main.go

@@ -32,6 +32,7 @@ func main() {
 	probeURL := flag.String("nat-probe-server", sf.DefaultNATProbeURL, "NAT check probe server URL")
 	outboundAddress := flag.String("outbound-address", "", "prefer the given address as outbound address")
 	allowedRelayHostNamePattern := flag.String("allowed-relay-hostname-pattern", "snowflake.torproject.net$", "a pattern to specify allowed hostname pattern for relay URL.")
+	allowProxyingToPrivateAddresses := flag.Bool("allow-proxying-to-private-addresses", false, "allow forwarding client connections to private IP addresses.\nUseful when a Snowflake server (relay) is hosted on the same private network as this proxy.")
 	allowNonTLSRelay := flag.Bool("allow-non-tls-relay", false, "allow relay without tls encryption")
 	NATTypeMeasurementInterval := flag.Duration("nat-retest-interval", time.Hour*24,
 		"the time interval in second before NAT type is retested, 0s disables retest. Valid time units are \"s\", \"m\", \"h\". ")
@@ -105,8 +106,9 @@ func main() {
 		NATTypeMeasurementInterval: *NATTypeMeasurementInterval,
 		EventDispatcher:            eventLogger,
 
-		RelayDomainNamePattern: *allowedRelayHostNamePattern,
-		AllowNonTLSRelay:       *allowNonTLSRelay,
+		RelayDomainNamePattern:          *allowedRelayHostNamePattern,
+		AllowProxyingToPrivateAddresses: *allowProxyingToPrivateAddresses,
+		AllowNonTLSRelay:                *allowNonTLSRelay,
 
 		SummaryInterval: *summaryInterval,
 	}