Skip to content

Commit

Permalink
define boundary cases in documentation, refactor implementation of pa…
Browse files Browse the repository at this point in the history 
…ttern url matching, more tests.

Signed-off-by: vadym <[email protected]>
  • Loading branch information
@sukolenvo
sukolenvo committed Sep 3, 2020
1 parent 5051a21 commit 37fa1fe
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 101 deletions.
8 changes: 3 additions & 5 deletions Documentation/using-dex.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,16 @@ Matcher symbols will only function if all of the following conditions are met:

1. application is confidential (i.e. not public)

2. URL scheme is `http` or `https`

3. A TLD domain, and a first subdomain must be present in URL, and neither of these two domain name sections can include a matcher symbol.
2. A TLD domain, and a first subdomain (SLD) must be present in URL, and neither of these two domain name sections can include a matcher symbol.

* Invalid: `https://*hello.com`: matcher symbol is contained in the first subdomain below the TLD
* Valid: `https://*.engineer.learning.com`
* Valid: `https://*.learning.com`
* Valid: `https://**.learning.com`
* Valid: `https://abc**.learning.com` : will match abc.def.ghf.learning.com
* Valid: `https://abc**.learning.com` : will match abc.d.learning.com abcd.learning.com
* Valid: `https://abc.*.learning.com` : will match abc.def.learning.com (but not abc.def.ghf.learning.com)
* Valid: `https://*.learning.com`: will match abc.learning.com (but not abc.def.learning.com)
* Valid: `https://*.d*.learning.com`: will match abc.def.learning.com (but not abc.learning.com or abc.ef.learning.com)
* Valid: `https://*.d*.learning.com`: will match abc.def.learning.com (but not abc.learning.com or abc.ef.learning.com or abc.d.learning.com)

For best practice, matching symbols for subdomains in application callbacks should be carefully considered and used with caution, as it is possible it could make your application more vulnerable to certain types of attacks.

Expand Down
100 changes: 36 additions & 64 deletions server/oauth2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,8 @@ func validateRedirectURI(client storage.Client, wildcardCache *lru.ARCCache, red
if urlRequested, err := url.Parse(redirectURI); err == nil && urlRequested != nil {
// validly-formed url
clobberMatcher := getClobberHostMatcher(uri, wildcardCache)
if clobberMatcher.HostMatcher != nil && matchesClobber(clobberMatcher, urlRequested) {
validClobber := clobberMatcher.HostMatcher != nil && clobberMatcher.ClientRedirectURL != nil
if validClobber && matchesClobber(clobberMatcher, urlRequested) {
// clobber match success
return true
}
Expand Down Expand Up @@ -600,16 +601,16 @@ func validateRedirectURI(client storage.Client, wildcardCache *lru.ARCCache, red
}

func matchesClobber(clobberMatcher ClientRedirectClobberMatcher, urlRequested *url.URL) bool {
if !clobberMatcher.HostMatcher.MatchString(urlRequested.Host) {
// failed host match
if clobberMatcher.ClientRedirectURL.Path != urlRequested.Path {
// failed path match
return false
}
if clobberMatcher.ClientRedirectURLSections.Scheme != urlRequested.Scheme {
if clobberMatcher.ClientRedirectURL.Scheme != urlRequested.Scheme {
// failed scheme match
return false
}
if clobberMatcher.ClientRedirectURLSections.RawPath != urlRequested.Path {
// failed path match
if !clobberMatcher.HostMatcher.MatchString(urlRequested.Host) {
// failed host match
return false
}
return true
Expand All @@ -620,108 +621,79 @@ type HostSplit struct {
SubDomainPrefix string // e.g. dom.abc (from dom.abc.example.com)
}

type ClobberURLSectional struct {
Scheme string // scheme: `http` or `https`
HostPair *HostSplit // leading from top-level to subsequent subdomains
RawPath string // path: requires leading slash; and includes fragments and query params
}

type ClientRedirectClobberMatcher struct {
ClientRedirectURL string
ClientRedirectURLSections *ClobberURLSectional
HostMatcher *regexp.Regexp
ClientRedirectURL *url.URL
HostMatcher *regexp.Regexp
}

// Returns a clobber/wildcard subdomain matcher struct. The `ClientRedirectURLSections`
// and `ClientRedirectURLSections` pointers will be nil if clientRedirectURI does not meet
// the requirements to perform as a clobber/wildcard matcher. ClientRedirectURL is always set.
// Returns a clobber/wildcard subdomain matcher struct
func createClientRedirectClobberMatcher(clientRedirectURI string) ClientRedirectClobberMatcher {

splitResult := splitClobberHttpRedirectUrl(clientRedirectURI)
if splitResult == nil || splitResult.HostPair == nil {
// does not meet basic wildcard usage requirements regarding scheme or path
return ClientRedirectClobberMatcher{
ClientRedirectURL: clientRedirectURI,
}
redirectUrl, _ := url.Parse(clientRedirectURI)
if redirectUrl == nil {
return ClientRedirectClobberMatcher{}
}

clobberMatcher := ClientRedirectClobberMatcher{
ClientRedirectURL: clientRedirectURI,
ClientRedirectURLSections: splitResult,
splitResult := splitClobberHttpRedirectUrl(redirectUrl)
if splitResult == nil {
// not wildcard or invalid
return ClientRedirectClobberMatcher{}
}

// first replace double-asterisk globber with .+
var subdomain = strings.ReplaceAll(regexp.QuoteMeta(splitResult.HostPair.SubDomainPrefix), "\\*\\*", ".+")
var subdomain = strings.ReplaceAll(regexp.QuoteMeta(splitResult.SubDomainPrefix), "\\*\\*", ".+")
subdomain = strings.ReplaceAll(subdomain, "\\*", "[^.]+")

hostNameRegExStr := "^" + subdomain + "\\." + splitResult.HostPair.HostSuffix + "$"
hostNameRegExStr := "^" + subdomain + "\\." + regexp.QuoteMeta(splitResult.HostSuffix) + "$"

hostNameRegEx, err := regexp.Compile(hostNameRegExStr)
if err != nil {
// bad pattern - unexpected
return clobberMatcher
return ClientRedirectClobberMatcher{}
}

clobberMatcher.HostMatcher = hostNameRegEx
return clobberMatcher
return ClientRedirectClobberMatcher{
ClientRedirectURL: redirectUrl,
HostMatcher: hostNameRegEx,
}
}

// This method is similar to url.Parse stdlib, expect that it does not validate a url, it is
// used to extract the scheme, host and path for wildcard checks.
// extract the scheme, host and path for wildcard checks.
// If the scheme is not http or https, or does not specify an absolute path beginning with '/'
// the parser will return nil.
func splitClobberHttpRedirectUrl(clientRedirectUrlSpec string) *ClobberURLSectional {
schemaRe, err := regexp.Compile("^([hH][tT]{2}[pP][sS]?)://([^/]+?)(:\\d+)?(/.*)$")
if err != nil {
return nil // bad regex pattern
}
splitRes := schemaRe.FindStringSubmatch(clientRedirectUrlSpec)
if splitRes == nil {
// not using http or https
return nil
}

hostPortion := splitRes[2]
if !strings.Contains(hostPortion, "*") {
func splitClobberHttpRedirectUrl(clientRedirectUrlSpec *url.URL) *HostSplit {
host := clientRedirectUrlSpec.Host
if !strings.Contains(host, "*") {
// for efficiency, check wildcard before DomainComponents
return nil
}

if strings.Contains(hostPortion, "***") {
// more than wildcards chars are not permitted
if strings.Contains(host, "***") {
// a maximum of 2 '*' characters are permitted in sequence
return nil
}

// port := splitRes[3]

// sub-domain portion is first portion (greedy)
hostSplit, err := regexp.Compile("^(.+)\\.([^.]*[A-Za-z][^.]*\\.[^.]*[A-Za-z][^.]*[^.])$")
hostSplit, _ := regexp.Compile("^(.+)\\.([^.]*[A-Za-z][^.]*\\.[^.]*[A-Za-z][^.]+)$")
if hostSplit == nil {
// unexpected issue with regex
return nil
}
hostRes := hostSplit.FindStringSubmatch(hostPortion)
hostRes := hostSplit.FindStringSubmatch(host)
if hostRes == nil {
// does not conform to requirements of a subdomain spec followed by two higher order domains
return nil
}

return &ClobberURLSectional {
Scheme: splitRes[1],
HostPair: &HostSplit{
HostSuffix: hostRes[2],
SubDomainPrefix: hostRes[1],
},
RawPath: splitRes[4],
return &HostSplit{
HostSuffix: hostRes[2],
SubDomainPrefix: hostRes[1],
}
}

func getClobberHostMatcher(clientRedirectUri string, wildcardCache *lru.ARCCache) ClientRedirectClobberMatcher {
if wildcardCache == nil {
// unexpected error condition where cache is unavailable
return ClientRedirectClobberMatcher {
ClientRedirectURL: clientRedirectUri,
}
return ClientRedirectClobberMatcher{}
}

if redirectMatcherStruct, ok := wildcardCache.Get(clientRedirectUri); ok {
Expand Down
Loading

0 comments on commit 37fa1fe

Please sign in to comment.