Skip to content

Commit

Permalink
[breaking change] wildcard matching improved and additional matching …
Browse files Browse the repository at this point in the history
…types added

Original wildcard prefix match now allows multiple * characters.

For increased safety, a single '*' character now matches
within one path segment only. To match any number of path segments,
two consecutive charaters must be specified '**'.

In addition to original "prefix match", full URL match is possible:

1. Wildcard Prefix Match
   `/admin/*` matches /admin/overview, /admin/users, *not* /admin/users/1
   `/admin/**` matches what '/admin/*' matched + also '/admin/users/1'

2. Full URL Wildcard Match
   `*://a.com/admin` matches http://a.com/admin and https://a.com/admin
   `*://a.com/**` matches everything under http://a.com/ and https://a.com/
   `https://b.com/admin` matches https://b.com/admin only

3. Full URL Regular Expression Match (prefixed by ~ character!)
   `~^https?://[cd].com/.*` matches everything under
     http://c.com/, http://d.com/ and their https versions

Tests were extended for new functionality and updated for the fact that
single '*' now matches within the one path component only.

cherry-pick 36c3eee
  • Loading branch information
Mario Hros authored and jkoelker committed Feb 3, 2022
1 parent b89a5cd commit 39539aa
Show file tree
Hide file tree
Showing 15 changed files with 558 additions and 230 deletions.
13 changes: 10 additions & 3 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"os"
"time"
"fmt"

"github.com/gorilla/sessions"
logger "github.com/mesosphere/traefik-forward-auth/internal/log"
Expand All @@ -20,7 +21,11 @@ import (
// Main
func main() {
// Parse options
config := configuration.NewGlobalConfig(os.Args[1:])
config, err := configuration.NewGlobalConfig(os.Args[1:])
if err != nil {
fmt.Printf("%+v\n", err)
os.Exit(1)
}

// Setup logger
log := logger.NewDefaultLogger(config.LogLevel, config.LogFormat)
Expand All @@ -47,7 +52,9 @@ func main() {
var userInfoStore v1alpha1.UserInfoInterface
if !config.EnableInClusterStorage {
// Prepare cookie session store (first key is for auth, the second one for encryption)
cookieStore := sessions.NewCookieStore(config.Secret, []byte(config.SessionKey))
hashKey := []byte(config.SecretString)
blockKey := []byte(config.EncryptionKeyString)
cookieStore := sessions.NewCookieStore(hashKey, blockKey)
cookieStore.Options.MaxAge = int(config.Lifetime / time.Second)
cookieStore.Options.HttpOnly = true
cookieStore.Options.Secure = !config.InsecureCookie
Expand All @@ -61,7 +68,7 @@ func main() {
userInfoStore = cluster.NewClusterStore(
clientset,
config.ClusterStoreNamespace,
string(config.Secret),
config.SecretString,
config.Lifetime,
time.Duration(config.ClusterStoreCacheTTL)*time.Second,
authenticator)
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
github.com/googleapis/gnostic v0.3.1 // indirect
github.com/gorilla/context v1.1.1 // indirect
github.com/gorilla/sessions v1.2.0
github.com/gorilla/securecookie v1.1.1
github.com/gravitational/trace v0.0.0-20190409171327-f30095ced5ff // indirect
github.com/jonboulle/clockwork v0.1.0 // indirect
github.com/json-iterator/go v1.1.8 // indirect
Expand Down
84 changes: 33 additions & 51 deletions internal/authentication/auth.go
Original file line number Diff line number Diff line change
@@ -1,67 +1,51 @@
package authentication

import (
"crypto/hmac"
"crypto/rand"
"crypto/sha256"
"encoding/base64"
"errors"
"fmt"
"net/http"
"strconv"
"strings"
"time"

"github.com/gorilla/securecookie"

"github.com/mesosphere/traefik-forward-auth/internal/configuration"
)

type Authenticator struct {
config *configuration.Config
config *configuration.Config
secureCookie *securecookie.SecureCookie
}

func NewAuthenticator(config *configuration.Config) *Authenticator {
return &Authenticator{config}
cookieMaxAge := int(config.Lifetime / time.Second)
hashKey := []byte(config.SecretString)
blockKey := []byte(config.EncryptionKeyString)

return &Authenticator{
config: config,
secureCookie: securecookie.New(hashKey, blockKey).MaxAge(cookieMaxAge),
}
}

type ID struct {
Email string
Token string
}

// Request Validation

// ValidateCookie validates the ID cookie in the request
// IDCookie = hash(secret, cookie domain, email, expires)|expires|email|group
func (a *Authenticator) ValidateCookie(r *http.Request, c *http.Cookie) (string, error) {
parts := strings.Split(c.Value, "|")

if len(parts) != 3 {
return "", errors.New("invalid cookie format")
}

mac, err := base64.URLEncoding.DecodeString(parts[0])
if err != nil {
return "", errors.New("unable to decode cookie mac")
}

expectedSignature := a.cookieSignature(r, parts[2], parts[1])
expected, err := base64.URLEncoding.DecodeString(expectedSignature)
if err != nil {
return "", errors.New("unable to generate mac")
}

// Valid token?
if !hmac.Equal(mac, expected) {
return "", errors.New("invalid cookie mac")
}

expires, err := strconv.ParseInt(parts[1], 10, 64)
if err != nil {
return "", errors.New("unable to parse cookie expiry")
}
func (a *Authenticator) ValidateCookie(r *http.Request, c *http.Cookie) (*ID, error) {
var data ID

// Has it expired?
if time.Unix(expires, 0).Before(time.Now()) {
return "", errors.New("cookie has expired")
if err := a.secureCookie.Decode(a.config.CookieName, c.Value, &data); err != nil {
return nil, err
}

// Looks valid
return parts[2], nil
return &data, nil
}

// ValidateEmail validates that the provided email ends with one of the configured Domains or is part of the configured Whitelist.
Expand Down Expand Up @@ -117,14 +101,21 @@ func (a *Authenticator) useAuthDomain(r *http.Request) (bool, string) {
// Cookie methods

// MakeIDCookie creates an auth cookie
func (a *Authenticator) MakeIDCookie(r *http.Request, email string) *http.Cookie {
func (a *Authenticator) MakeIDCookie(r *http.Request, email string, token string) *http.Cookie {
expires := a.cookieExpiry()
mac := a.cookieSignature(r, email, fmt.Sprintf("%d", expires.Unix()))
value := fmt.Sprintf("%s|%d|%s", mac, expires.Unix(), email)
data := &ID{
Email: email,
Token: token,
}

encoded, err := a.secureCookie.Encode(a.config.CookieName, data)
if err != nil {
return nil
}

return &http.Cookie{
Name: a.config.CookieName,
Value: value,
Value: encoded,
Path: "/",
Domain: a.GetCookieDomain(r),
HttpOnly: true,
Expand Down Expand Up @@ -249,15 +240,6 @@ func (a *Authenticator) matchCookieDomains(domain string) (bool, string) {
return false, p[0]
}

// cookieSignature creates a cookie hmac
func (a *Authenticator) cookieSignature(r *http.Request, email, expires string) string {
hash := hmac.New(sha256.New, a.config.Secret)
hash.Write([]byte(a.GetCookieDomain(r)))
hash.Write([]byte(email))
hash.Write([]byte(expires))
return base64.URLEncoding.EncodeToString(hash.Sum(nil))
}

// Get cookie expirary
func (a *Authenticator) cookieExpiry() time.Time {
return time.Now().Local().Add(a.config.Lifetime)
Expand Down
57 changes: 30 additions & 27 deletions internal/authentication/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,67 +5,71 @@ import (
"github.com/mesosphere/traefik-forward-auth/internal/configuration"
"github.com/mesosphere/traefik-forward-auth/internal/util"
"net/http"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
)

var (
testAuthKey1 = "4Zhbg4n22r4I8Kdg1gHMzRWQpT7TOArD"
testEncKey1 = "8jAnK6NGuzEuH3y13V+5Bm2jgp5bv8ku"
)

func newTestConfig(authKey, encKey string) *configuration.Config {
c, _ := configuration.NewConfig([]string{})
c.SecretString = authKey
c.EncryptionKeyString = encKey

return c
}

/**
* Tests
*/

func TestAuthValidateCookie(t *testing.T) {
assert := assert.New(t)
config, _ := configuration.NewConfig([]string{})
config := newTestConfig(testAuthKey1, testEncKey1)
a := NewAuthenticator(config)
r, _ := http.NewRequest("GET", "http://example.com", nil)
c := &http.Cookie{}

// Should require 3 parts
// Should not accept an empty value
c.Value = ""
_, err := a.ValidateCookie(r, c)
if assert.Error(err) {
assert.Equal("invalid cookie format", err.Error())
}
c.Value = "1|2"
_, err = a.ValidateCookie(r, c)
if assert.Error(err) {
assert.Equal("invalid cookie format", err.Error())
}
c.Value = "1|2|3|4"
_, err = a.ValidateCookie(r, c)
if assert.Error(err) {
assert.Equal("invalid cookie format", err.Error())
assert.Equal("securecookie: the value is not valid", err.Error())
}

// Should catch invalid mac
c.Value = "MQ==|2|3"
c.Value = "MQ=="
_, err = a.ValidateCookie(r, c)
if assert.Error(err) {
assert.Equal("invalid cookie mac", err.Error())
assert.Equal("securecookie: the value is not valid", err.Error())
}

// Should catch expired
config.Lifetime = time.Second * time.Duration(-1)
c = a.MakeIDCookie(r, "[email protected]")
a = NewAuthenticator(config)
c = a.MakeIDCookie(r, "[email protected]", "")
_, err = a.ValidateCookie(r, c)
if assert.Error(err) {
assert.Equal("cookie has expired", err.Error())
assert.Equal("securecookie: expired timestamp", err.Error())
}

// Should accept valid cookie
config.Lifetime = time.Second * time.Duration(10)
c = a.MakeIDCookie(r, "[email protected]")
email, err := a.ValidateCookie(r, c)
a = NewAuthenticator(config)
c = a.MakeIDCookie(r, "[email protected]", "")
id, err := a.ValidateCookie(r, c)
assert.Nil(err, "valid request should not return an error")
assert.Equal("[email protected]", email, "valid request should return user email")
assert.Equal("[email protected]", id.Email, "valid request should return user email")
}

func TestAuthValidateEmail(t *testing.T) {
assert := assert.New(t)
config, _ := configuration.NewConfig([]string{})
config := newTestConfig(testAuthKey1, testEncKey1)

a := NewAuthenticator(config)
// Should allow any
Expand Down Expand Up @@ -106,7 +110,7 @@ func TestAuthValidateEmail(t *testing.T) {
// }

func getConfigWithLifetime() *configuration.Config {
config, _ := configuration.NewConfig([]string{})
config := newTestConfig(testAuthKey1, testEncKey1)
// Lifetime is set during validation, so we short circuit it here
config.Lifetime = time.Second * time.Duration(config.LifetimeString)
return config
Expand All @@ -120,10 +124,9 @@ func TestAuthMakeCookie(t *testing.T) {
r, _ := http.NewRequest("GET", "http://app.example.com", nil)
r.Header.Add("X-Forwarded-Host", "app.example.com")

c := a.MakeIDCookie(r, "[email protected]")
c := a.MakeIDCookie(r, "[email protected]", "")
assert.Equal("_forward_auth", c.Name)
parts := strings.Split(c.Value, "|")
assert.Len(parts, 3, "cookie should be 3 parts")
assert.Greater(len(c.Value), 18, "encoded securecookie should be longer")
_, err := a.ValidateCookie(r, c)
assert.Nil(err, "should generate valid cookie")
assert.Equal("/", c.Path)
Expand All @@ -135,7 +138,7 @@ func TestAuthMakeCookie(t *testing.T) {

config.CookieName = "testname"
config.InsecureCookie = true
c = a.MakeIDCookie(r, "[email protected]")
c = a.MakeIDCookie(r, "[email protected]", "")
assert.Equal("testname", c.Name)
assert.False(c.Secure)
}
Expand Down
4 changes: 3 additions & 1 deletion internal/authorization/authorizer.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package authorization

import "net/url"

// Authorizer is the interface for implementing user authorization (check to see if the user can perform the action)
type Authorizer interface {
Authorize(user User, requestVerb, requestResource string) (bool, error)
Authorize(user User, requestVerb string, resource *url.URL) (bool, error)
}
31 changes: 25 additions & 6 deletions internal/authorization/rbac/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package rbac

import (
"log"
"net/url"
"os"
"strings"
"time"
Expand Down Expand Up @@ -133,7 +134,7 @@ func (ra *Authorizer) GetRolesBoundToUser(user authorization.User) (*rbacv1.Clus
// Interface methods

// Authorize performs the authorization logic
func (ra *Authorizer) Authorize(user authorization.User, requestVerb, requestResource string) (bool, error) {
func (ra *Authorizer) Authorize(user authorization.User, requestVerb string, requestURL *url.URL) (bool, error) {
roles, err := ra.GetRolesBoundToUser(user)
if err != nil {
return false, err
Expand All @@ -147,7 +148,7 @@ func (ra *Authorizer) Authorize(user authorization.User, requestVerb, requestRes
// check all rules in the list of roles to see if any matches
for _, role := range roles.Items {
for _, rule := range role.Rules {
if verbMatches(&rule, requestVerb) && nonResourceURLMatches(&rule, requestResource) {
if verbMatches(&rule, requestVerb) && nonResourceURLMatches(&rule, requestURL) {
return true, nil
}
}
Expand Down Expand Up @@ -175,15 +176,33 @@ func verbMatches(rule *rbacv1.PolicyRule, requestedVerb string) bool {
}

// nonResourceURLMatches returns true if the requested URL matches a policy the rule
func nonResourceURLMatches(rule *rbacv1.PolicyRule, requestedURL string) bool {
func nonResourceURLMatches(rule *rbacv1.PolicyRule, requestedURL *url.URL) bool {
for _, ruleURL := range rule.NonResourceURLs {
if ruleURL == rbacv1.NonResourceAll {
// any (*) resource matches immediatelly
return true
}
if authorization.PathMatches(requestedURL, ruleURL) {
return true
} else if len(ruleURL) > 0 {
// determine match type depending on the first rune:

if ruleURL[0] == '~' { // regular expression match against the full url requested
fullURLWithoutQuery := requestedURL.Scheme + "://" + requestedURL.Host + requestedURL.Path
if authorization.URLMatchesRegexp(fullURLWithoutQuery, ruleURL[1:]) {
return true // return only if it matched
}
} else if ruleURL[0] == '/' { // path-only prefix match with optional wildcards (backward-compatible)
if authorization.URLMatchesWildcardPattern(requestedURL.Path, ruleURL) {
return true // return only if it matched
}
} else { // full url path-only prefix match with optional wildcards
fullURLWithoutQuery := requestedURL.Scheme + "://" + requestedURL.Host + requestedURL.Path
if authorization.URLMatchesWildcardPattern(fullURLWithoutQuery, ruleURL) {
return true // return only if it matched
}
}
}
}

// no rule matched
return false
}

Expand Down
Loading

0 comments on commit 39539aa

Please sign in to comment.