From 14f4c82ff78084534dd40ef703d1bd1dbc09d233 Mon Sep 17 00:00:00 2001 From: WofWca Date: Fri, 30 Aug 2024 16:42:52 +0400 Subject: [PATCH] test(proxy): add tests for relayURL check --- proxy/lib/proxy-go_test.go | 67 ++++++++++++++++++++++++++++++++++++++ proxy/lib/snowflake.go | 36 +++++++++++++------- 2 files changed, 91 insertions(+), 12 deletions(-) diff --git a/proxy/lib/proxy-go_test.go b/proxy/lib/proxy-go_test.go index 7c46480..91ed3e1 100644 --- a/proxy/lib/proxy-go_test.go +++ b/proxy/lib/proxy-go_test.go @@ -493,4 +493,71 @@ func TestUtilityFuncs(t *testing.T) { _, err = s2.Write(bytes) So(err, ShouldNotBeNil) }) + Convey("isRelayURLAcceptable", t, func() { + testingVector := []struct { + pattern string + allowNonTLS bool + targetURL string + expects error + }{ + // These are copied from `TestMatchMember`. + {pattern: "^snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://snowflake.torproject.net", expects: nil}, + {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-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}, + + // NonTLS + {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "ws://snowflake.torproject.net", expects: fmt.Errorf("")}, + {pattern: "snowflake.torproject.net$", allowNonTLS: true, targetURL: "ws://snowflake.torproject.net", expects: nil}, + + // Sneaky attempt to use path + {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://evil.com/snowflake.torproject.net", expects: fmt.Errorf("")}, + {pattern: "snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://evil.com/?test=snowflake.torproject.net", expects: fmt.Errorf("")}, + + // IP address + {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}, + + // Port + {pattern: "^snowflake.torproject.net$", allowNonTLS: false, targetURL: "wss://snowflake.torproject.net:8080/test?test=test#test", expects: nil}, + // This currently doesn't work as we only check hostname. + // {pattern: "^snowflake.torproject.net:443$", allowNonTLS: false, targetURL: "wss://snowflake.torproject.net:443", expects: nil}, + // {pattern: "^snowflake.torproject.net:443$", allowNonTLS: false, targetURL: "wss://snowflake.torproject.net:9999", expects: fmt.Errorf("")}, + + // Any URL + {pattern: "$", allowNonTLS: false, targetURL: "wss://any.com/test?test=test#test", expects: nil}, + {pattern: "$", allowNonTLS: false, targetURL: "wss://1.1.1.1/test?test=test#test", expects: nil}, + + // Weird / invalid / ambiguous URL + // {pattern: "$", allowNonTLS: true, targetURL: "snowflake.torproject.net", expects: fmt.Errorf("")}, + // {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}, + + // 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("")}, + // These are failing for now + // {pattern: "snowflake.torproject.net$", allowNonTLS: true, targetURL: "https://snowflake.torproject.net", expects: fmt.Errorf("")}, + // {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) + if v.expects != nil { + So(err, ShouldNotBeNil) + } else { + So(err, ShouldBeNil) + } + } + }) } diff --git a/proxy/lib/snowflake.go b/proxy/lib/snowflake.go index 65b4703..69ae9c2 100644 --- a/proxy/lib/snowflake.go +++ b/proxy/lib/snowflake.go @@ -600,18 +600,12 @@ func (sf *SnowflakeProxy) runSession(sid string) { } log.Printf("Received Offer From Broker: \n\t%s", strings.ReplaceAll(offer.SDP, "\n", "\n\t")) - matcher := namematcher.NewNameMatcher(sf.RelayDomainNamePattern) - parsedRelayURL, err := url.Parse(relayURL) - if err != nil { - log.Printf("bad offer from broker: bad Relay URL %v", err.Error()) - tokens.ret() - return - } - - if relayURL != "" && (!matcher.IsMember(parsedRelayURL.Hostname()) || (!sf.AllowNonTLSRelay && parsedRelayURL.Scheme != "wss")) { - log.Printf("bad offer from broker: rejected Relay URL") - tokens.ret() - return + if relayURL != "" { + if err := checkIsRelayURLAcceptable(sf.RelayDomainNamePattern, sf.AllowNonTLSRelay, relayURL); err != nil { + log.Printf("bad offer from broker: %v", err) + tokens.ret() + return + } } dataChan := make(chan struct{}) @@ -647,6 +641,24 @@ func (sf *SnowflakeProxy) runSession(sid string) { } } +// Returns nil if the relayURL is acceptable +func checkIsRelayURLAcceptable( + allowedHostNamePattern string, + allowNonTLSRelay bool, + relayURL string, +) error { + parsedRelayURL, err := url.Parse(relayURL) + if err != nil { + return fmt.Errorf("bad Relay URL %w", err) + } + matcher := namematcher.NewNameMatcher(allowedHostNamePattern) + if !matcher.IsMember(parsedRelayURL.Hostname()) || (!allowNonTLSRelay && parsedRelayURL.Scheme != "wss") { + return fmt.Errorf("rejected Relay URL") + } else { + return nil + } +} + // Start configures and starts a Snowflake, fully formed and special. Configuration // values that are unset will default to their corresponding default values. func (sf *SnowflakeProxy) Start() error {