mirror of
https://github.com/oauth2-proxy/oauth2-proxy.git
synced 2025-03-21 21:47:11 +02:00
Create redirect validator
This commit is contained in:
Joel Speed
parent
62436dbc02
commit
e7f304fc96
17
pkg/app/redirect/pagewriter_suite_test.go
Normal file
17
pkg/app/redirect/pagewriter_suite_test.go
Normal file
@ -0,0 +1,17 @@
|
||||
package redirect
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
|
||||
. "github.com/onsi/ginkgo"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
func TestOptionsSuite(t *testing.T) {
|
||||
logger.SetOutput(GinkgoWriter)
|
||||
logger.SetErrOutput(GinkgoWriter)
|
||||
|
||||
RegisterFailHandler(Fail)
|
||||
RunSpecs(t, "Redirect Suite")
|
||||
}
|
||||
120
pkg/app/redirect/validator.go
Normal file
120
pkg/app/redirect/validator.go
Normal file
@ -0,0 +1,120 @@
|
||||
package redirect
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
|
||||
)
|
||||
|
||||
var (
|
||||
// Used to check final redirects are not susceptible to open redirects.
|
||||
// Matches //, /\ and both of these with whitespace in between (eg / / or / \).
|
||||
invalidRedirectRegex = regexp.MustCompile(`[/\\](?:[\s\v]*|\.{1,2})[/\\]`)
|
||||
)
|
||||
|
||||
// Validator is an interface to allow validation of application
|
||||
// redirect URLs.
|
||||
// As these values are determined from the request, they must go
|
||||
// through thorough checks to ensure the safety of the end user.
|
||||
type Validator interface {
|
||||
IsValidRedirect(redirect string) bool
|
||||
}
|
||||
|
||||
// NewValidator constructs a new redirect validator.
|
||||
func NewValidator(allowedDomains []string) Validator {
|
||||
return &validator{
|
||||
allowedDomains: allowedDomains,
|
||||
}
|
||||
}
|
||||
|
||||
// validator implements the Validator interface to allow validation
|
||||
// of redirect URLs.
|
||||
type validator struct {
|
||||
allowedDomains []string
|
||||
}
|
||||
|
||||
// IsValidRedirect checks whether the redirect URL is safe and allowed.
|
||||
func (v *validator) IsValidRedirect(redirect string) bool {
|
||||
switch {
|
||||
case redirect == "":
|
||||
// The user didn't specify a redirect.
|
||||
// In this case, we expect the proxt to fallback to `/`
|
||||
return false
|
||||
case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):
|
||||
return true
|
||||
case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
|
||||
redirectURL, err := url.Parse(redirect)
|
||||
if err != nil {
|
||||
logger.Printf("Rejecting invalid redirect %q: scheme unsupported or missing", redirect)
|
||||
return false
|
||||
}
|
||||
redirectHostname := redirectURL.Hostname()
|
||||
|
||||
for _, allowedDomain := range v.allowedDomains {
|
||||
allowedHost, allowedPort := splitHostPort(allowedDomain)
|
||||
if allowedHost == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
if redirectHostname == strings.TrimPrefix(allowedHost, ".") ||
|
||||
(strings.HasPrefix(allowedHost, ".") &&
|
||||
strings.HasSuffix(redirectHostname, allowedHost)) {
|
||||
// the domain names match, now validate the ports
|
||||
// if the whitelisted domain's port is '*', allow all ports
|
||||
// if the whitelisted domain contains a specific port, only allow that port
|
||||
// if the whitelisted domain doesn't contain a port at all, only allow empty redirect ports ie http and https
|
||||
redirectPort := redirectURL.Port()
|
||||
if allowedPort == "*" ||
|
||||
allowedPort == redirectPort ||
|
||||
(allowedPort == "" && redirectPort == "") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
logger.Printf("Rejecting invalid redirect %q: domain / port not in whitelist", redirect)
|
||||
return false
|
||||
default:
|
||||
logger.Printf("Rejecting invalid redirect %q: not an absolute or relative URL", redirect)
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// splitHostPort separates host and port. If the port is not valid, it returns
|
||||
// the entire input as host, and it doesn't check the validity of the host.
|
||||
// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric.
|
||||
// *** taken from net/url, modified validOptionalPort() to accept ":*"
|
||||
func splitHostPort(hostport string) (host, port string) {
|
||||
host = hostport
|
||||
|
||||
colon := strings.LastIndexByte(host, ':')
|
||||
if colon != -1 && validOptionalPort(host[colon:]) {
|
||||
host, port = host[:colon], host[colon+1:]
|
||||
}
|
||||
|
||||
if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {
|
||||
host = host[1 : len(host)-1]
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// validOptionalPort reports whether port is either an empty string
|
||||
// or matches /^:\d*$/
|
||||
// *** taken from net/url, modified to accept ":*"
|
||||
func validOptionalPort(port string) bool {
|
||||
if port == "" || port == ":*" {
|
||||
return true
|
||||
}
|
||||
if port[0] != ':' {
|
||||
return false
|
||||
}
|
||||
for _, b := range port[1:] {
|
||||
if b < '0' || b > '9' {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
102
pkg/app/redirect/validator_test.go
Normal file
102
pkg/app/redirect/validator_test.go
Normal file
@ -0,0 +1,102 @@
|
||||
package redirect
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"net/url"
|
||||
"os"
|
||||
|
||||
. "github.com/onsi/ginkgo"
|
||||
. "github.com/onsi/ginkgo/extensions/table"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("Validator suite", func() {
|
||||
var testAllowedDomains []string
|
||||
|
||||
BeforeEach(func() {
|
||||
testAllowedDomains = []string{
|
||||
"foo.bar",
|
||||
".bar.foo",
|
||||
"port.bar:8080",
|
||||
".sub.port.bar:8080",
|
||||
"anyport.bar:*",
|
||||
".sub.anyport.bar:*",
|
||||
"www.whitelisteddomain.tld",
|
||||
}
|
||||
})
|
||||
|
||||
Context("OpenRedirect List", func() {
|
||||
file, err := os.Open("../../../testdata/openredirects.txt")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
defer func() {
|
||||
Expect(file.Close()).To(Succeed())
|
||||
}()
|
||||
|
||||
scanner := bufio.NewScanner(file)
|
||||
for scanner.Scan() {
|
||||
rd := scanner.Text()
|
||||
It(rd, func() {
|
||||
rdUnescaped, err := url.QueryUnescape(rd)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
validator := NewValidator(testAllowedDomains)
|
||||
Expect(validator.IsValidRedirect(rdUnescaped)).To(BeFalse(), "Expected redirect not to be valid")
|
||||
})
|
||||
}
|
||||
|
||||
Expect(scanner.Err()).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
Context("Validator", func() {
|
||||
DescribeTable("IsValidRedirect",
|
||||
func(testRedirect string, expected bool) {
|
||||
validator := NewValidator(testAllowedDomains)
|
||||
Expect(validator.IsValidRedirect(testRedirect)).To(Equal(expected))
|
||||
},
|
||||
Entry("No Redirect", "", false),
|
||||
Entry("Single Slash", "/redirect", true),
|
||||
Entry("Double Slash", "//redirect", false),
|
||||
Entry("Valid HTTP", "http://foo.bar/redirect", true),
|
||||
Entry("Valid HTTPS", "https://foo.bar/redirect", true),
|
||||
Entry("Invalid HTTP subdomain", "http://baz.foo.bar/redirect", false),
|
||||
Entry("Invalid HTTPS subdomain", "https://baz.foo.bar/redirect", false),
|
||||
Entry("Valid HTTP subdomain", "http://baz.bar.foo/redirect", true),
|
||||
Entry("Valid HTTPS subdomain", "https://baz.bar.foo/redirect", true),
|
||||
Entry("Valid HTTP Domain", "http://bar.foo/redirect", true), // Is this correct, do we want to match the root domain?
|
||||
Entry("Invalid HTTP Similar Domain", "http://foo.bar.evil.corp/redirect", false),
|
||||
Entry("Invalid HTTPS Similar Domain", "https://foo.bar.evil.corp/redirect", false),
|
||||
Entry("Invalid HTTP RD Parameter", "http://evil.corp/redirect?rd=foo.bar", false),
|
||||
Entry("Invalid HTTPS RD Parameter", "https://evil.corp/redirect?rd=foo.bar", false),
|
||||
Entry("Invalid Port and Domain", "https://evil.corp:3838/redirect", false),
|
||||
Entry("Invalid Port on Allowed Domain", "http://foo.bar:3838/redirect", false),
|
||||
Entry("Invalid Port on Allowed Subdomain", "http://baz.bar.foo:3838/redirect", false),
|
||||
Entry("Valid Specified Port and Domain", "http://port.bar:8080/redirect", true),
|
||||
Entry("Invalid Specified Port and Domain", "http://port.bar:3838/redirect", false),
|
||||
Entry("Valid Specified Port and Subdomain", "http://foo.sub.port.bar:8080/redirect", true),
|
||||
Entry("Invalid Specified Port andSubdomain", "http://foo.subport.bar:3838/redirect", false),
|
||||
Entry("Valid Any Port, Specified Domain", "http://anyport.bar:8080/redirect", true),
|
||||
Entry("Valid Different Any Port, Specified Domain", "http://anyport.bar:8081/redirect", true),
|
||||
Entry("Valid Any Port, Specified Subdomain", "http://a.sub.anyport.bar:8080/redirect", true),
|
||||
Entry("Valid Different Any Port, Specified Subdomain", "http://a.sub.anyport.bar:8081/redirect", true),
|
||||
Entry("Escape Double Slash", "/\\evil.com", false),
|
||||
Entry("Space Single Slash", "/ /evil.com", false),
|
||||
Entry("Space Double Slash", "/ \\evil.com", false),
|
||||
Entry("Tab Single Slash", "/\t/evil.com", false),
|
||||
Entry("Tab Double Slash", "/\t\\evil.com", false),
|
||||
Entry("Vertical Tab Single Slash", "/\v/evil.com", false),
|
||||
Entry("Vertiacl Tab Double Slash", "/\v\\evil.com", false),
|
||||
Entry("New Line Single Slash", "/\n/evil.com", false),
|
||||
Entry("New Line Double Slash", "/\n\\evil.com", false),
|
||||
Entry("Carriage Return Single Slash", "/\r/evil.com", false),
|
||||
Entry("Carriage Return Double Slash", "/\r\\evil.com", false),
|
||||
Entry("Double Tab", "/\t/\t\\evil.com", false),
|
||||
Entry("Triple Tab 1", "/\t\t/\t/evil.com", false),
|
||||
Entry("Triple Tab 2", "/\t\t\\\t/evil.com", false),
|
||||
Entry("Quad Tab 1", "/\t\t/\t\t\\evil.com", false),
|
||||
Entry("Quad Tab 2", "/\t\t\\\t\t/evil.com", false),
|
||||
Entry("Relative Path", "/./\\evil.com", false),
|
||||
Entry("Relative Subpath", "/./../../\\evil.com", false),
|
||||
Entry("Partial Subdomain", "evilbar.foo", false),
|
||||
)
|
||||
})
|
||||
})
|
||||
Loading…
x
Reference in New Issue
Block a user