git.iko.soy/typeable/oauth2-proxy
on master help sign in
Create redirect validator
commite7f304fc96f31d9879faf48aa549f38a1ac8ad53 authorJoel Speed <joel.speed@hotmail.co.uk> dateSun 30 May 2021, 09:34 (5 years ago) parent62436db
3 files changed+239
compare with master
unifiedsplit
pkg/app/redirect/pagewriter_suite_test.go+17 pkg/app/redirect/validator.go+120 pkg/app/redirect/validator_test.go+102
pkg/app/redirect/pagewriter_suite_test.gonew file+17
@@ -0,0 +1,17 @@
1+package redirect
2+
3+import (
4+ "testing"
5+
6+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
7+ . "github.com/onsi/ginkgo"
8+ . "github.com/onsi/gomega"
9+)
10+
11+func TestOptionsSuite(t *testing.T) {
12+ logger.SetOutput(GinkgoWriter)
13+ logger.SetErrOutput(GinkgoWriter)
14+
15+ RegisterFailHandler(Fail)
16+ RunSpecs(t, "Redirect Suite")
17+}
pkg/app/redirect/validator.gonew file+120
@@ -0,0 +1,120 @@
1+package redirect
2+
3+import (
4+ "net/url"
5+ "regexp"
6+ "strings"
7+
8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
9+)
10+
11+var (
12+ // Used to check final redirects are not susceptible to open redirects.
13+ // Matches //, /\ and both of these with whitespace in between (eg / / or / \).
14+ invalidRedirectRegex = regexp.MustCompile(`[/\\](?:[\s\v]*|\.{1,2})[/\\]`)
15+)
16+
17+// Validator is an interface to allow validation of application
18+// redirect URLs.
19+// As these values are determined from the request, they must go
20+// through thorough checks to ensure the safety of the end user.
21+type Validator interface {
22+ IsValidRedirect(redirect string) bool
23+}
24+
25+// NewValidator constructs a new redirect validator.
26+func NewValidator(allowedDomains []string) Validator {
27+ return &validator{
28+ allowedDomains: allowedDomains,
29+ }
30+}
31+
32+// validator implements the Validator interface to allow validation
33+// of redirect URLs.
34+type validator struct {
35+ allowedDomains []string
36+}
37+
38+// IsValidRedirect checks whether the redirect URL is safe and allowed.
39+func (v *validator) IsValidRedirect(redirect string) bool {
40+ switch {
41+ case redirect == "":
42+ // The user didn't specify a redirect.
43+ // In this case, we expect the proxt to fallback to `/`
44+ return false
45+ case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):
46+ return true
47+ case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
48+ redirectURL, err := url.Parse(redirect)
49+ if err != nil {
50+ logger.Printf("Rejecting invalid redirect %q: scheme unsupported or missing", redirect)
51+ return false
52+ }
53+ redirectHostname := redirectURL.Hostname()
54+
55+ for _, allowedDomain := range v.allowedDomains {
56+ allowedHost, allowedPort := splitHostPort(allowedDomain)
57+ if allowedHost == "" {
58+ continue
59+ }
60+
61+ if redirectHostname == strings.TrimPrefix(allowedHost, ".") ||
62+ (strings.HasPrefix(allowedHost, ".") &&
63+ strings.HasSuffix(redirectHostname, allowedHost)) {
64+ // the domain names match, now validate the ports
65+ // if the whitelisted domain's port is '*', allow all ports
66+ // if the whitelisted domain contains a specific port, only allow that port
67+ // if the whitelisted domain doesn't contain a port at all, only allow empty redirect ports ie http and https
68+ redirectPort := redirectURL.Port()
69+ if allowedPort == "*" ||
70+ allowedPort == redirectPort ||
71+ (allowedPort == "" && redirectPort == "") {
72+ return true
73+ }
74+ }
75+ }
76+
77+ logger.Printf("Rejecting invalid redirect %q: domain / port not in whitelist", redirect)
78+ return false
79+ default:
80+ logger.Printf("Rejecting invalid redirect %q: not an absolute or relative URL", redirect)
81+ return false
82+ }
83+}
84+
85+// splitHostPort separates host and port. If the port is not valid, it returns
86+// the entire input as host, and it doesn't check the validity of the host.
87+// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric.
88+// *** taken from net/url, modified validOptionalPort() to accept ":*"
89+func splitHostPort(hostport string) (host, port string) {
90+ host = hostport
91+
92+ colon := strings.LastIndexByte(host, ':')
93+ if colon != -1 && validOptionalPort(host[colon:]) {
94+ host, port = host[:colon], host[colon+1:]
95+ }
96+
97+ if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {
98+ host = host[1 : len(host)-1]
99+ }
100+
101+ return
102+}
103+
104+// validOptionalPort reports whether port is either an empty string
105+// or matches /^:\d*$/
106+// *** taken from net/url, modified to accept ":*"
107+func validOptionalPort(port string) bool {
108+ if port == "" || port == ":*" {
109+ return true
110+ }
111+ if port[0] != ':' {
112+ return false
113+ }
114+ for _, b := range port[1:] {
115+ if b < '0' || b > '9' {
116+ return false
117+ }
118+ }
119+ return true
120+}
pkg/app/redirect/validator_test.gonew file+102
@@ -0,0 +1,102 @@
1+package redirect
2+
3+import (
4+ "bufio"
5+ "net/url"
6+ "os"
7+
8+ . "github.com/onsi/ginkgo"
9+ . "github.com/onsi/ginkgo/extensions/table"
10+ . "github.com/onsi/gomega"
11+)
12+
13+var _ = Describe("Validator suite", func() {
14+ var testAllowedDomains []string
15+
16+ BeforeEach(func() {
17+ testAllowedDomains = []string{
18+ "foo.bar",
19+ ".bar.foo",
20+ "port.bar:8080",
21+ ".sub.port.bar:8080",
22+ "anyport.bar:*",
23+ ".sub.anyport.bar:*",
24+ "www.whitelisteddomain.tld",
25+ }
26+ })
27+
28+ Context("OpenRedirect List", func() {
29+ file, err := os.Open("../../../testdata/openredirects.txt")
30+ Expect(err).ToNot(HaveOccurred())
31+ defer func() {
32+ Expect(file.Close()).To(Succeed())
33+ }()
34+
35+ scanner := bufio.NewScanner(file)
36+ for scanner.Scan() {
37+ rd := scanner.Text()
38+ It(rd, func() {
39+ rdUnescaped, err := url.QueryUnescape(rd)
40+ Expect(err).ToNot(HaveOccurred())
41+
42+ validator := NewValidator(testAllowedDomains)
43+ Expect(validator.IsValidRedirect(rdUnescaped)).To(BeFalse(), "Expected redirect not to be valid")
44+ })
45+ }
46+
47+ Expect(scanner.Err()).ToNot(HaveOccurred())
48+ })
49+
50+ Context("Validator", func() {
51+ DescribeTable("IsValidRedirect",
52+ func(testRedirect string, expected bool) {
53+ validator := NewValidator(testAllowedDomains)
54+ Expect(validator.IsValidRedirect(testRedirect)).To(Equal(expected))
55+ },
56+ Entry("No Redirect", "", false),
57+ Entry("Single Slash", "/redirect", true),
58+ Entry("Double Slash", "//redirect", false),
59+ Entry("Valid HTTP", "http://foo.bar/redirect", true),
60+ Entry("Valid HTTPS", "https://foo.bar/redirect", true),
61+ Entry("Invalid HTTP subdomain", "http://baz.foo.bar/redirect", false),
62+ Entry("Invalid HTTPS subdomain", "https://baz.foo.bar/redirect", false),
63+ Entry("Valid HTTP subdomain", "http://baz.bar.foo/redirect", true),
64+ Entry("Valid HTTPS subdomain", "https://baz.bar.foo/redirect", true),
65+ Entry("Valid HTTP Domain", "http://bar.foo/redirect", true), // Is this correct, do we want to match the root domain?
66+ Entry("Invalid HTTP Similar Domain", "http://foo.bar.evil.corp/redirect", false),
67+ Entry("Invalid HTTPS Similar Domain", "https://foo.bar.evil.corp/redirect", false),
68+ Entry("Invalid HTTP RD Parameter", "http://evil.corp/redirect?rd=foo.bar", false),
69+ Entry("Invalid HTTPS RD Parameter", "https://evil.corp/redirect?rd=foo.bar", false),
70+ Entry("Invalid Port and Domain", "https://evil.corp:3838/redirect", false),
71+ Entry("Invalid Port on Allowed Domain", "http://foo.bar:3838/redirect", false),
72+ Entry("Invalid Port on Allowed Subdomain", "http://baz.bar.foo:3838/redirect", false),
73+ Entry("Valid Specified Port and Domain", "http://port.bar:8080/redirect", true),
74+ Entry("Invalid Specified Port and Domain", "http://port.bar:3838/redirect", false),
75+ Entry("Valid Specified Port and Subdomain", "http://foo.sub.port.bar:8080/redirect", true),
76+ Entry("Invalid Specified Port andSubdomain", "http://foo.subport.bar:3838/redirect", false),
77+ Entry("Valid Any Port, Specified Domain", "http://anyport.bar:8080/redirect", true),
78+ Entry("Valid Different Any Port, Specified Domain", "http://anyport.bar:8081/redirect", true),
79+ Entry("Valid Any Port, Specified Subdomain", "http://a.sub.anyport.bar:8080/redirect", true),
80+ Entry("Valid Different Any Port, Specified Subdomain", "http://a.sub.anyport.bar:8081/redirect", true),
81+ Entry("Escape Double Slash", "/\\evil.com", false),
82+ Entry("Space Single Slash", "/ /evil.com", false),
83+ Entry("Space Double Slash", "/ \\evil.com", false),
84+ Entry("Tab Single Slash", "/\t/evil.com", false),
85+ Entry("Tab Double Slash", "/\t\\evil.com", false),
86+ Entry("Vertical Tab Single Slash", "/\v/evil.com", false),
87+ Entry("Vertiacl Tab Double Slash", "/\v\\evil.com", false),
88+ Entry("New Line Single Slash", "/\n/evil.com", false),
89+ Entry("New Line Double Slash", "/\n\\evil.com", false),
90+ Entry("Carriage Return Single Slash", "/\r/evil.com", false),
91+ Entry("Carriage Return Double Slash", "/\r\\evil.com", false),
92+ Entry("Double Tab", "/\t/\t\\evil.com", false),
93+ Entry("Triple Tab 1", "/\t\t/\t/evil.com", false),
94+ Entry("Triple Tab 2", "/\t\t\\\t/evil.com", false),
95+ Entry("Quad Tab 1", "/\t\t/\t\t\\evil.com", false),
96+ Entry("Quad Tab 2", "/\t\t\\\t\t/evil.com", false),
97+ Entry("Relative Path", "/./\\evil.com", false),
98+ Entry("Relative Subpath", "/./../../\\evil.com", false),
99+ Entry("Partial Subdomain", "evilbar.foo", false),
100+ )
101+ })
102+})