From c18a1b7e69bbad971230b8a744593b5d46399d0b Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Sun, 13 Oct 2024 21:20:13 -0400 Subject: [PATCH 01/11] resolve host to IP to check if it's local before connecting --- proxy/lib/proxy-go_test.go | 9 +++++++++ proxy/lib/snowflake.go | 25 ++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/proxy/lib/proxy-go_test.go b/proxy/lib/proxy-go_test.go index 9021387..d1ef7e5 100644 --- a/proxy/lib/proxy-go_test.go +++ b/proxy/lib/proxy-go_test.go @@ -559,6 +559,15 @@ func TestUtilityFuncs(t *testing.T) { {pattern: "$", allowNonTLS: true, targetURL: "wss://😀", expects: nil}, {pattern: "$", allowNonTLS: true, targetURL: "wss://пример.рф", expects: nil}, + // Local URLs + {pattern: "localhost$", allowNonTLS: false, targetURL: "wss://localhost", expects: fmt.Errorf("")}, + {pattern: "test.internal$", allowNonTLS: false, targetURL: "wss://test.internal", expects: fmt.Errorf("")}, + {pattern: "test.invalid$", allowNonTLS: false, targetURL: "wss://test.invalid", expects: fmt.Errorf("")}, + {pattern: "test.localhost$", allowNonTLS: false, targetURL: "wss://test.localhost", expects: fmt.Errorf("")}, + {pattern: "test.local$", allowNonTLS: false, targetURL: "wss://test.local", expects: fmt.Errorf("")}, + {pattern: "test.onion$", allowNonTLS: false, targetURL: "wss://test.onion", expects: fmt.Errorf("")}, + {pattern: "test.test$", allowNonTLS: false, targetURL: "wss://test.test", expects: fmt.Errorf("")}, + // Non-websocket protocols {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "https://snowflake.torproject.net", expects: fmt.Errorf("")}, {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "ftp://snowflake.torproject.net", expects: fmt.Errorf("")}, diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index 7897200..d408c89 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -172,6 +172,25 @@ func isRemoteAddress(ip net.IP) bool { return !(util.IsLocal(ip) || ip.IsUnspecified() || ip.IsLoopback()) } +// Checks whether the hostname is local +func isHostnameLocal(hostname string) bool { + // Per https://en.wikipedia.org/wiki/Special-use_domain_name + tlds := []string{ + ".internal", + ".invalid", + ".local", + ".localhost", + ".onion", + ".test", + } + for _, tld := range tlds { + if strings.HasSuffix(hostname, tld) { + return true + } + } + return hostname == "localhost" +} + func genSessionID() string { buf := make([]byte, sessionIDLength) _, err := rand.Read(buf) @@ -670,7 +689,11 @@ func checkIsRelayURLAcceptable( return fmt.Errorf("bad Relay URL %w", err) } if !allowPrivateIPs { - ip := net.ParseIP(parsedRelayURL.Hostname()) + hostname := parsedRelayURL.Hostname() + if isHostnameLocal(hostname) { + return fmt.Errorf("rejected Relay URL: private hostnames are not allowed") + } + ip := net.ParseIP(hostname) // Otherwise it's a domain name, or an invalid IP. if ip != nil { // We should probably use a ready library for this. From 37a2570643abcf938201767540a9d057946e5c3c Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Mon, 14 Oct 2024 08:40:41 -0400 Subject: [PATCH 02/11] Block remote IPs and not just hostnames --- proxy/lib/snowflake.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index d408c89..6fcc46d 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -690,9 +690,15 @@ func checkIsRelayURLAcceptable( } if !allowPrivateIPs { hostname := parsedRelayURL.Hostname() + ipArray, _ := net.LookupIP(hostname) if isHostnameLocal(hostname) { return fmt.Errorf("rejected Relay URL: private hostnames are not allowed") } + for _, ip := range ipArray { + if !isRemoteAddress(ip) { + return fmt.Errorf("rejected Relay URL: private IPs are not allowed") + } + } ip := net.ParseIP(hostname) // Otherwise it's a domain name, or an invalid IP. if ip != nil { From e983f7425cd906d56bc00fee0efb4d72294bf0ab Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Mon, 14 Oct 2024 09:54:05 -0400 Subject: [PATCH 03/11] Add delay --- proxy/lib/snowflake.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index 6fcc46d..d6686cc 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -32,6 +32,7 @@ import ( "fmt" "io" "log" + mathRand "math/rand" "net" "net/http" "net/url" @@ -691,6 +692,7 @@ func checkIsRelayURLAcceptable( if !allowPrivateIPs { hostname := parsedRelayURL.Hostname() ipArray, _ := net.LookupIP(hostname) + time.Sleep(time.Second * time.Duration(mathRand.Float64())) if isHostnameLocal(hostname) { return fmt.Errorf("rejected Relay URL: private hostnames are not allowed") } From 79744636530e2d0ebb787762d7b82be0697be10d Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Mon, 14 Oct 2024 10:49:03 -0400 Subject: [PATCH 04/11] Revert "Add delay" This reverts commit 97c73bcab662de0033bbaeadb76dea0a3ca274da. --- proxy/lib/snowflake.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index d6686cc..6fcc46d 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -32,7 +32,6 @@ import ( "fmt" "io" "log" - mathRand "math/rand" "net" "net/http" "net/url" @@ -692,7 +691,6 @@ func checkIsRelayURLAcceptable( if !allowPrivateIPs { hostname := parsedRelayURL.Hostname() ipArray, _ := net.LookupIP(hostname) - time.Sleep(time.Second * time.Duration(mathRand.Float64())) if isHostnameLocal(hostname) { return fmt.Errorf("rejected Relay URL: private hostnames are not allowed") } From 5e4619bae1ce80aea652b50e4f132693bc7a2cae Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Thu, 17 Oct 2024 23:09:27 +0000 Subject: [PATCH 05/11] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Cecylia Bocovich --- proxy/lib/snowflake.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index 6fcc46d..bad617d 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -706,7 +706,9 @@ func checkIsRelayURLAcceptable( if !isRemoteAddress(ip) { return fmt.Errorf("rejected Relay URL: private IPs are not allowed") } - } + } else { + // move net.LookupIP(hostname) and isRemoteAddress checks here + } } if !allowNonTLSRelay && parsedRelayURL.Scheme != "wss" { return fmt.Errorf("rejected Relay URL protocol: non-TLS not allowed") From aaa59df3082984032a8d9f5c1348b40b2231f3c8 Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Thu, 17 Oct 2024 19:10:44 -0400 Subject: [PATCH 06/11] Reduce network load --- proxy/lib/snowflake.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index bad617d..d722438 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -690,10 +690,10 @@ func checkIsRelayURLAcceptable( } if !allowPrivateIPs { hostname := parsedRelayURL.Hostname() - ipArray, _ := net.LookupIP(hostname) if isHostnameLocal(hostname) { return fmt.Errorf("rejected Relay URL: private hostnames are not allowed") } + ipArray, _ := net.LookupIP(hostname) for _, ip := range ipArray { if !isRemoteAddress(ip) { return fmt.Errorf("rejected Relay URL: private IPs are not allowed") From 6fef2caaa82e9ecf0e6df818fefddca39acc4413 Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Thu, 17 Oct 2024 19:12:26 -0400 Subject: [PATCH 07/11] Check error --- proxy/lib/snowflake.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index d722438..ceefab9 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -693,7 +693,10 @@ func checkIsRelayURLAcceptable( if isHostnameLocal(hostname) { return fmt.Errorf("rejected Relay URL: private hostnames are not allowed") } - ipArray, _ := net.LookupIP(hostname) + ipArray, err := net.LookupIP(hostname) + if err != nil { + return fmt.Errorf("Could not look up IP") + } for _, ip := range ipArray { if !isRemoteAddress(ip) { return fmt.Errorf("rejected Relay URL: private IPs are not allowed") From 990d16593722c6ed8c8c4512d0fcb134a5255ea4 Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Thu, 17 Oct 2024 19:15:02 -0400 Subject: [PATCH 08/11] move IsHostnameLocal to common/util --- common/util/util.go | 20 ++++++++++++++++++++ proxy/lib/snowflake.go | 21 +-------------------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/common/util/util.go b/common/util/util.go index 844ef2f..aba2df5 100644 --- a/common/util/util.go +++ b/common/util/util.go @@ -8,6 +8,7 @@ import ( "net/http" "slices" "sort" + "strings" "github.com/pion/ice/v2" "github.com/pion/sdp/v3" @@ -165,3 +166,22 @@ func GetCandidateAddrs(sdpStr string) []net.IP { } return sortedIpAddr } + +// Checks whether the hostname is local +func IsHostnameLocal(hostname string) bool { + // Per https://en.wikipedia.org/wiki/Special-use_domain_name + tlds := []string{ + ".internal", + ".invalid", + ".local", + ".localhost", + ".onion", + ".test", + } + for _, tld := range tlds { + if strings.HasSuffix(hostname, tld) { + return true + } + } + return hostname == "localhost" +} diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index ceefab9..483a5e0 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -172,25 +172,6 @@ func isRemoteAddress(ip net.IP) bool { return !(util.IsLocal(ip) || ip.IsUnspecified() || ip.IsLoopback()) } -// Checks whether the hostname is local -func isHostnameLocal(hostname string) bool { - // Per https://en.wikipedia.org/wiki/Special-use_domain_name - tlds := []string{ - ".internal", - ".invalid", - ".local", - ".localhost", - ".onion", - ".test", - } - for _, tld := range tlds { - if strings.HasSuffix(hostname, tld) { - return true - } - } - return hostname == "localhost" -} - func genSessionID() string { buf := make([]byte, sessionIDLength) _, err := rand.Read(buf) @@ -690,7 +671,7 @@ func checkIsRelayURLAcceptable( } if !allowPrivateIPs { hostname := parsedRelayURL.Hostname() - if isHostnameLocal(hostname) { + if util.IsHostnameLocal(hostname) { return fmt.Errorf("rejected Relay URL: private hostnames are not allowed") } ipArray, err := net.LookupIP(hostname) From f1e9f58b47152a78e73138d88055551a05d70250 Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Thu, 17 Oct 2024 19:24:34 -0400 Subject: [PATCH 09/11] Move IP check --- proxy/lib/snowflake.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index 483a5e0..a90ea65 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -674,15 +674,6 @@ func checkIsRelayURLAcceptable( if util.IsHostnameLocal(hostname) { return fmt.Errorf("rejected Relay URL: private hostnames are not allowed") } - ipArray, err := net.LookupIP(hostname) - if err != nil { - return fmt.Errorf("Could not look up IP") - } - for _, ip := range ipArray { - if !isRemoteAddress(ip) { - return fmt.Errorf("rejected Relay URL: private IPs are not allowed") - } - } ip := net.ParseIP(hostname) // Otherwise it's a domain name, or an invalid IP. if ip != nil { @@ -691,8 +682,16 @@ func checkIsRelayURLAcceptable( return fmt.Errorf("rejected Relay URL: private IPs are not allowed") } } else { - // move net.LookupIP(hostname) and isRemoteAddress checks here - } + ipArray, err := net.LookupIP(hostname) + if err != nil { + return fmt.Errorf("Could not look up IP") + } + for _, ip := range ipArray { + 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") From e22f397d84076bcc3c610b8a6586f728569a08cf Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Thu, 17 Oct 2024 19:49:55 -0400 Subject: [PATCH 10/11] Remove false positives --- proxy/lib/snowflake.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index a90ea65..4413a90 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -682,10 +682,7 @@ func checkIsRelayURLAcceptable( return fmt.Errorf("rejected Relay URL: private IPs are not allowed") } } else { - ipArray, err := net.LookupIP(hostname) - if err != nil { - return fmt.Errorf("Could not look up IP") - } + ipArray, _ := net.LookupIP(hostname) for _, ip := range ipArray { if !isRemoteAddress(ip) { return fmt.Errorf("rejected Relay URL: private IPs are not allowed") From 2d128985bad98ae9a3e0cc9699426cdc86aa171d Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Thu, 17 Oct 2024 20:02:39 -0400 Subject: [PATCH 11/11] Enhance testing --- proxy/lib/proxy-go_test.go | 10 +++++----- proxy/lib/snowflake.go | 5 ++++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/proxy/lib/proxy-go_test.go b/proxy/lib/proxy-go_test.go index d1ef7e5..6428150 100644 --- a/proxy/lib/proxy-go_test.go +++ b/proxy/lib/proxy-go_test.go @@ -506,13 +506,13 @@ func TestUtilityFuncs(t *testing.T) { {pattern: "^snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://faketorproject.net", expects: fmt.Errorf("")}, {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://faketorproject.net", expects: fmt.Errorf("")}, {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://snowflake.torproject.net", expects: nil}, - {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://imaginary-01-snowflake.torproject.net", expects: nil}, - {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://imaginary-aaa-snowflake.torproject.net", expects: nil}, + {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://imaginary-01-snowflake.torproject.net", expects: fmt.Errorf("")}, + {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://imaginary-aaa-snowflake.torproject.net", expects: fmt.Errorf("")}, {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://imaginary-aaa-snowflake.faketorproject.net", expects: fmt.Errorf("")}, {pattern: "^torproject.net$", allowNonTLS: false, targetURL: "wss://faketorproject.net", expects: fmt.Errorf("")}, // Yes, this is how it works if there is no "^". - {pattern: "torproject.net$", allowNonTLS: false, targetURL: "wss://faketorproject.net", expects: nil}, + {pattern: "torproject.net$", allowNonTLS: false, targetURL: "wss://faketorproject.net", expects: fmt.Errorf("")}, // NonTLS {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "ws://snowflake.torproject.net", expects: fmt.Errorf("")}, @@ -556,8 +556,8 @@ func TestUtilityFuncs(t *testing.T) { {pattern: "$", allowNonTLS: true, targetURL: "//snowflake.torproject.net", expects: fmt.Errorf("")}, {pattern: "$", allowNonTLS: true, targetURL: "/path", expects: fmt.Errorf("")}, {pattern: "$", allowNonTLS: true, targetURL: "wss://snowflake.torproject .net", expects: fmt.Errorf("")}, - {pattern: "$", allowNonTLS: true, targetURL: "wss://😀", expects: nil}, - {pattern: "$", allowNonTLS: true, targetURL: "wss://пример.рф", expects: nil}, + {pattern: "$", allowNonTLS: true, targetURL: "wss://😀", expects: fmt.Errorf("")}, + {pattern: "$", allowNonTLS: true, targetURL: "wss://пример.рф", expects: fmt.Errorf("")}, // Local URLs {pattern: "localhost$", allowNonTLS: false, targetURL: "wss://localhost", expects: fmt.Errorf("")}, diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index 4413a90..9c6647a 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -682,7 +682,10 @@ func checkIsRelayURLAcceptable( return fmt.Errorf("rejected Relay URL: private IPs are not allowed") } } else { - ipArray, _ := net.LookupIP(hostname) + ipArray, err := net.LookupIP(hostname) + if err != nil { + return fmt.Errorf("Could not look up IP %s", hostname) + } for _, ip := range ipArray { if !isRemoteAddress(ip) { return fmt.Errorf("rejected Relay URL: private IPs are not allowed")