183cb12 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Support HTTP method based allowlists

183cb12Nick Meves6 years ago

CHANGELOG.md+4

@@ -7,6 +7,9 @@

7- [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Sessions from v5.1.1 or earlier will no longer validate since they were not signed with SHA1.

8 - Sessions from v6.0.0 or later had a graceful conversion to SHA256 that resulted in no reauthentication

9 - Upgrading from v5.1.1 or earlier will result in a reauthentication

10+- [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) `--skip-auth-route` is (almost) backwards compatible with `--skip-auth-regex`

11+ - We are marking `--skip-auth-regex` as DEPRECATED and will remove it in the next major version.

12+ - If your regex contains an `=` and you want it for all methods, you will need to add a leading `=` (this is the area where `--skip-auth-regex` doesn't port perfectly)

10- [#616](https://github.com/oauth2-proxy/oauth2-proxy/pull/616) Ensure you have configured oauth2-proxy to use the `groups` scope. The user may be logged out initially as they may not currently have the `groups` claim however after going back through login process wil be authenticated.

13- [#616](https://github.com/oauth2-proxy/oauth2-proxy/pull/616) Ensure you have configured oauth2-proxy to use the `groups` scope. The user may be logged out initially as they may not currently have the `groups` claim however after going back through login process wil be authenticated.

12## Breaking Changes

15## Breaking Changes

@@ -23,6 +26,7 @@

23## Changes since v6.1.1

26## Changes since v6.1.1

25- [#753](https://github.com/oauth2-proxy/oauth2-proxy/pull/753) Pass resource parameter in login url (@codablock)

28- [#753](https://github.com/oauth2-proxy/oauth2-proxy/pull/753) Pass resource parameter in login url (@codablock)

29+- [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) Add `--skip-auth-route` configuration option for `METHOD=pathRegex` based allowlists (@NickMeves)

26- [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Stop accepting legacy SHA1 signed cookies (@NickMeves)

30- [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Stop accepting legacy SHA1 signed cookies (@NickMeves)

27- [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) Validate Redis configuration options at startup (@NickMeves)

31- [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) Validate Redis configuration options at startup (@NickMeves)

28- [#791](https://github.com/oauth2-proxy/oauth2-proxy/pull/791) Remove GetPreferredUsername method from provider interface (@NickMeves)

32- [#791](https://github.com/oauth2-proxy/oauth2-proxy/pull/791) Remove GetPreferredUsername method from provider interface (@NickMeves)

docs/configuration/configuration.md+3−2

@@ -119,8 +119,9 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example

124+| `--skip-auth-strip-headers` | bool | strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy for allowlisted requests (`--skip-auth-route`, `--skip-auth-regex`, `--skip-auth-preflight`, `--trusted-ip`) | false |

oauthproxy.go+68−18

@@ -48,6 +48,12 @@ var (

48 invalidRedirectRegex = regexp.MustCompile(`[/\\](?:[\s\v]*|\.{1,2})[/\\]`)

49)

51+// allowedRoute manages method + path based allowlists

52+type allowedRoute struct {

53+ method string

54+ pathRegex *regexp.Regexp

55+}

56+

51// OAuthProxy is the main authentication proxy

57// OAuthProxy is the main authentication proxy

52type OAuthProxy struct {

58type OAuthProxy struct {

53 CookieSeed string

59 CookieSeed string

@@ -70,6 +76,7 @@ type OAuthProxy struct {

70 AuthOnlyPath string

76 AuthOnlyPath string

71 UserInfoPath string

77 UserInfoPath string

79+ allowedRoutes []*allowedRoute

73 redirectURL *url.URL // the url to receive requests at

80 redirectURL *url.URL // the url to receive requests at

74 whitelistDomains []string

81 whitelistDomains []string

75 provider providers.Provider

82 provider providers.Provider

@@ -90,13 +97,11 @@ type OAuthProxy struct {

90 SetAuthorization bool

97 SetAuthorization bool

91 PassAuthorization bool

98 PassAuthorization bool

92 PreferEmailToUser bool

99 PreferEmailToUser bool

93− skipAuthRegex []string

94 skipAuthPreflight bool

100 skipAuthPreflight bool

95 skipAuthStripHeaders bool

101 skipAuthStripHeaders bool

96 skipJwtBearerTokens bool

102 skipJwtBearerTokens bool

97 mainJwtBearerVerifier *oidc.IDTokenVerifier

103 mainJwtBearerVerifier *oidc.IDTokenVerifier

98 extraJwtBearerVerifiers []*oidc.IDTokenVerifier

104 extraJwtBearerVerifiers []*oidc.IDTokenVerifier

99− compiledRegex []*regexp.Regexp

100 templates *template.Template

105 templates *template.Template

101 realClientIPParser ipapi.RealClientIPParser

106 realClientIPParser ipapi.RealClientIPParser

102 trustedIPs *ip.NetSet

107 trustedIPs *ip.NetSet

@@ -121,10 +126,6 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

121 return nil, fmt.Errorf("error initialising upstream proxy: %v", err)

126 return nil, fmt.Errorf("error initialising upstream proxy: %v", err)

122 }

127 }

123

128

124− for _, u := range opts.GetCompiledRegex() {

125− logger.Printf("compiled skip-auth-regex => %q", u)

126− }

127−

128 if opts.SkipJwtBearerTokens {

129 if opts.SkipJwtBearerTokens {

129 logger.Printf("Skipping JWT tokens from configured OIDC issuer: %q", opts.OIDCIssuerURL)

130 logger.Printf("Skipping JWT tokens from configured OIDC issuer: %q", opts.OIDCIssuerURL)

130 for _, issuer := range opts.ExtraJwtIssuers {

131 for _, issuer := range opts.ExtraJwtIssuers {

@@ -163,6 +164,11 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

163 }

164 }

165 }

165

166

167+ allowedRoutes, err := buildRoutesAllowlist(opts)

168+ if err != nil {

169+ return nil, err

170+ }

171+

166 sessionChain := buildSessionChain(opts, sessionStore, basicAuthValidator)

172 sessionChain := buildSessionChain(opts, sessionStore, basicAuthValidator)

167

173

168 return &OAuthProxy{

174 return &OAuthProxy{

@@ -192,14 +198,13 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

192 sessionStore: sessionStore,

198 sessionStore: sessionStore,

193 serveMux: upstreamProxy,

199 serveMux: upstreamProxy,

194 redirectURL: redirectURL,

200 redirectURL: redirectURL,

201+ allowedRoutes: allowedRoutes,

195 whitelistDomains: opts.WhitelistDomains,

202 whitelistDomains: opts.WhitelistDomains,

196− skipAuthRegex: opts.SkipAuthRegex,

197 skipAuthPreflight: opts.SkipAuthPreflight,

203 skipAuthPreflight: opts.SkipAuthPreflight,

198 skipAuthStripHeaders: opts.SkipAuthStripHeaders,

204 skipAuthStripHeaders: opts.SkipAuthStripHeaders,

199 skipJwtBearerTokens: opts.SkipJwtBearerTokens,

205 skipJwtBearerTokens: opts.SkipJwtBearerTokens,

200 mainJwtBearerVerifier: opts.GetOIDCVerifier(),

206 mainJwtBearerVerifier: opts.GetOIDCVerifier(),

201 extraJwtBearerVerifiers: opts.GetJWTBearerVerifiers(),

207 extraJwtBearerVerifiers: opts.GetJWTBearerVerifiers(),

202− compiledRegex: opts.GetCompiledRegex(),

203 realClientIPParser: opts.GetRealClientIPParser(),

208 realClientIPParser: opts.GetRealClientIPParser(),

204 SetXAuthRequest: opts.SetXAuthRequest,

209 SetXAuthRequest: opts.SetXAuthRequest,

205 PassBasicAuth: opts.PassBasicAuth,

210 PassBasicAuth: opts.PassBasicAuth,

@@ -277,6 +282,51 @@ func buildSignInMessage(opts *options.Options) string {

277 return msg

282 return msg

278}

283}

279

284

285+// buildRoutesAllowlist builds an []allowedRoute list from either the legacy

286+// SkipAuthRegex option (paths only support) or newer SkipAuthRoutes option

287+// (method=path support)

288+func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {

289+ var routes []*allowedRoute

290+

291+ for _, path := range opts.SkipAuthRegex {

292+ compiledRegex, err := regexp.Compile(path)

293+ if err != nil {

294+ return nil, err

295+ }

296+ routes = append(routes, &allowedRoute{

297+ method: "",

298+ pathRegex: compiledRegex,

299+ })

300+ }

301+

302+ for _, methodPath := range opts.SkipAuthRoutes {

303+ var (

304+ method string

305+ path string

306+ )

307+

308+ parts := strings.Split(methodPath, "=")

309+ if len(parts) == 1 {

310+ method = ""

311+ path = parts[0]

312+ } else {

313+ method = strings.ToUpper(parts[0])

314+ path = strings.Join(parts[1:], "=")

315+ }

316+

317+ compiledRegex, err := regexp.Compile(path)

318+ if err != nil {

319+ return nil, err

320+ }

321+ routes = append(routes, &allowedRoute{

322+ method: method,

323+ pathRegex: compiledRegex,

324+ })

325+ }

326+

327+ return routes, nil

328+}

329+

280// GetRedirectURI returns the redirectURL that the upstream OAuth Provider will

330// GetRedirectURI returns the redirectURL that the upstream OAuth Provider will

281// redirect clients to once authenticated

331// redirect clients to once authenticated

282func (p *OAuthProxy) GetRedirectURI(host string) string {

332func (p *OAuthProxy) GetRedirectURI(host string) string {

@@ -584,16 +634,16 @@ func (p *OAuthProxy) IsValidRedirect(redirect string) bool {

584 }

634 }

585}

635}

586

636

587−// IsWhitelistedRequest is used to check if auth should be skipped for this request

637+// IsAllowedRequest is used to check if auth should be skipped for this request

588−func (p *OAuthProxy) IsWhitelistedRequest(req *http.Request) bool {

638+func (p *OAuthProxy) IsAllowedRequest(req *http.Request) bool {

589 isPreflightRequestAllowed := p.skipAuthPreflight && req.Method == "OPTIONS"

639 isPreflightRequestAllowed := p.skipAuthPreflight && req.Method == "OPTIONS"

590− return isPreflightRequestAllowed || p.IsWhitelistedPath(req.URL.Path) || p.IsTrustedIP(req)

640+ return isPreflightRequestAllowed || p.isAllowedRoute(req) || p.IsTrustedIP(req)

591}

641}

592

642

593−// IsWhitelistedPath is used to check if the request path is allowed without auth

643+// IsAllowedRoute is used to check if the request method & path is allowed without auth

594−func (p *OAuthProxy) IsWhitelistedPath(path string) bool {

644+func (p *OAuthProxy) isAllowedRoute(req *http.Request) bool {

595− for _, u := range p.compiledRegex {

645+ for _, route := range p.allowedRoutes {

596− if u.MatchString(path) {

646+ if (route.method == "" || req.Method == route.method) && route.pathRegex.MatchString(req.URL.Path) {

597 return true

647 return true

598 }

648 }

599 }

649 }

@@ -643,7 +693,7 @@ func (p *OAuthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {

643 switch path := req.URL.Path; {

693 switch path := req.URL.Path; {

644 case path == p.RobotsPath:

694 case path == p.RobotsPath:

645 p.RobotsTxt(rw)

695 p.RobotsTxt(rw)

646− case p.IsWhitelistedRequest(req):

696+ case p.IsAllowedRequest(req):

647 p.SkipAuthProxy(rw, req)

697 p.SkipAuthProxy(rw, req)

648 case path == p.SignInPath:

698 case path == p.SignInPath:

649 p.SignIn(rw, req)

699 p.SignIn(rw, req)

@@ -831,7 +881,7 @@ func (p *OAuthProxy) AuthenticateOnly(rw http.ResponseWriter, req *http.Request)

831 rw.WriteHeader(http.StatusAccepted)

881 rw.WriteHeader(http.StatusAccepted)

832}

882}

833

883

834−// SkipAuthProxy proxies whitelisted requests and skips authentication

884+// SkipAuthProxy proxies allowlisted requests and skips authentication

835func (p *OAuthProxy) SkipAuthProxy(rw http.ResponseWriter, req *http.Request) {

885func (p *OAuthProxy) SkipAuthProxy(rw http.ResponseWriter, req *http.Request) {

836 if p.skipAuthStripHeaders {

886 if p.skipAuthStripHeaders {

837 p.stripAuthHeaders(req)

887 p.stripAuthHeaders(req)

@@ -1026,7 +1076,7 @@ func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Req

1026 }

1076 }

1027}

1077}

1028

1078

1029−// stripAuthHeaders removes Auth headers for whitelisted routes from skipAuthRegex

1079+// stripAuthHeaders removes Auth headers for allowlisted routes from skipAuthRegex

1030func (p *OAuthProxy) stripAuthHeaders(req *http.Request) {

1080func (p *OAuthProxy) stripAuthHeaders(req *http.Request) {

1031 if p.PassBasicAuth {

1081 if p.PassBasicAuth {

1032 req.Header.Del("X-Forwarded-User")

1082 req.Header.Del("X-Forwarded-User")

pkg/apis/options/options.go+4−6

@@ -3,7 +3,6 @@ package options

3import (

4 "crypto"

5 "net/url"

6− "regexp"

8 oidc "github.com/coreos/go-oidc"

7 oidc "github.com/coreos/go-oidc"

9 ipapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/ip"

8 ipapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/ip"

@@ -67,6 +66,7 @@ type Options struct {

67 UpstreamServers Upstreams `cfg:",internal"`

66 UpstreamServers Upstreams `cfg:",internal"`

69 SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"`

68 SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"`

69+ SkipAuthRoutes []string `flag:"skip-auth-route" cfg:"skip_auth_routes"`

70 SkipAuthStripHeaders bool `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"`

71 SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"`

72 ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"`

@@ -114,7 +114,6 @@ type Options struct {

114

115 // internal values that are set after config validation

116 redirectURL *url.URL

117− compiledRegex []*regexp.Regexp

118 provider providers.Provider

117 provider providers.Provider

119 signatureData *SignatureData

118 signatureData *SignatureData

120 oidcVerifier *oidc.IDTokenVerifier

119 oidcVerifier *oidc.IDTokenVerifier

@@ -124,7 +123,6 @@ type Options struct {

124

123

125// Options for Getting internal values

124// Options for Getting internal values

126func (o *Options) GetRedirectURL() *url.URL { return o.redirectURL }

125func (o *Options) GetRedirectURL() *url.URL { return o.redirectURL }

127−func (o *Options) GetCompiledRegex() []*regexp.Regexp { return o.compiledRegex }

128func (o *Options) GetProvider() providers.Provider { return o.provider }

126func (o *Options) GetProvider() providers.Provider { return o.provider }

129func (o *Options) GetSignatureData() *SignatureData { return o.signatureData }

127func (o *Options) GetSignatureData() *SignatureData { return o.signatureData }

130func (o *Options) GetOIDCVerifier() *oidc.IDTokenVerifier { return o.oidcVerifier }

128func (o *Options) GetOIDCVerifier() *oidc.IDTokenVerifier { return o.oidcVerifier }

@@ -133,7 +131,6 @@ func (o *Options) GetRealClientIPParser() ipapi.RealClientIPParser { return o.re

133

131

134// Options for Setting internal values

132// Options for Setting internal values

135func (o *Options) SetRedirectURL(s *url.URL) { o.redirectURL = s }

133func (o *Options) SetRedirectURL(s *url.URL) { o.redirectURL = s }

136−func (o *Options) SetCompiledRegex(s []*regexp.Regexp) { o.compiledRegex = s }

137func (o *Options) SetProvider(s providers.Provider) { o.provider = s }

134func (o *Options) SetProvider(s providers.Provider) { o.provider = s }

138func (o *Options) SetSignatureData(s *SignatureData) { o.signatureData = s }

135func (o *Options) SetSignatureData(s *SignatureData) { o.signatureData = s }

139func (o *Options) SetOIDCVerifier(s *oidc.IDTokenVerifier) { o.oidcVerifier = s }

136func (o *Options) SetOIDCVerifier(s *oidc.IDTokenVerifier) { o.oidcVerifier = s }

@@ -195,8 +192,9 @@ func NewFlagSet() *pflag.FlagSet {

195 flagSet.Bool("pass-access-token", false, "pass OAuth access_token to upstream via X-Forwarded-Access-Token header")

192 flagSet.Bool("pass-access-token", false, "pass OAuth access_token to upstream via X-Forwarded-Access-Token header")

196 flagSet.Bool("pass-authorization-header", false, "pass the Authorization Header to upstream")

193 flagSet.Bool("pass-authorization-header", false, "pass the Authorization Header to upstream")

197 flagSet.Bool("set-authorization-header", false, "set Authorization response headers (useful in Nginx auth_request mode)")

194 flagSet.Bool("set-authorization-header", false, "set Authorization response headers (useful in Nginx auth_request mode)")

198− flagSet.StringSlice("skip-auth-regex", []string{}, "bypass authentication for requests path's that match (may be given multiple times)")

195+ flagSet.StringSlice("skip-auth-regex", []string{}, "(DEPRECATED for --skip-auth-route) bypass authentication for requests path's that match (may be given multiple times)")

196+ flagSet.StringSlice("skip-auth-route", []string{}, "bypass authentication for requests that match the method & path. Format: method=path_regex OR path_regex alone for all methods")

199− flagSet.Bool("skip-auth-strip-headers", false, "strips X-Forwarded-* style authentication headers & Authorization header if they would be set by oauth2-proxy for request paths in --skip-auth-regex")

197+ flagSet.Bool("skip-auth-strip-headers", false, "strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy for allowlisted requests (`--skip-auth-route`, `--skip-auth-regex`, `--skip-auth-preflight`, `--trusted-ip`)")

200 flagSet.Bool("skip-provider-button", false, "will skip sign-in-page to directly reach the next step: oauth/start")

198 flagSet.Bool("skip-provider-button", false, "will skip sign-in-page to directly reach the next step: oauth/start")

201 flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests")

199 flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests")

202 flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS providers")

200 flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS providers")

pkg/validation/allowlist.gonew file+70

@@ -0,0 +1,70 @@

1+package validation

3+import (

4+ "fmt"

5+ "os"

6+ "regexp"

7+ "strings"

9+ "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"

10+ "github.com/oauth2-proxy/oauth2-proxy/pkg/ip"

11+)

12+

13+func validateAllowlists(o *options.Options) []string {

14+ msgs := []string{}

15+

16+ msgs = append(msgs, validateRoutes(o)...)

17+ msgs = append(msgs, validateRegexes(o)...)

18+ msgs = append(msgs, validateTrustedIPs(o)...)

19+

20+ if len(o.TrustedIPs) > 0 && o.ReverseProxy {

21+ _, err := fmt.Fprintln(os.Stderr, "WARNING: mixing --trusted-ip with --reverse-proxy is a potential security vulnerability. An attacker can inject a trusted IP into an X-Real-IP or X-Forwarded-For header if they aren't properly protected outside of oauth2-proxy")

22+ if err != nil {

23+ panic(err)

24+ }

25+ }

26+

27+ return msgs

28+}

29+

30+// validateRoutes validates method=path routes passed with options.SkipAuthRoutes

31+func validateRoutes(o *options.Options) []string {

32+ msgs := []string{}

33+ for _, route := range o.SkipAuthRoutes {

34+ var regex string

35+ parts := strings.Split(route, "=")

36+ if len(parts) == 1 {

37+ regex = parts[0]

38+ } else {

39+ regex = strings.Join(parts[1:], "=")

40+ }

41+ _, err := regexp.Compile(regex)

42+ if err != nil {

43+ msgs = append(msgs, fmt.Sprintf("error compiling regex /%s/: %v", regex, err))

44+ }

45+ }

46+ return msgs

47+}

48+

49+// validateRegex validates regex paths passed with options.SkipAuthRegex

50+func validateRegexes(o *options.Options) []string {

51+ msgs := []string{}

52+ for _, regex := range o.SkipAuthRegex {

53+ _, err := regexp.Compile(regex)

54+ if err != nil {

55+ msgs = append(msgs, fmt.Sprintf("error compiling regex /%s/: %v", regex, err))

56+ }

57+ }

58+ return msgs

59+}

60+

61+// validateTrustedIPs validates IP/CIDRs for IP based allowlists

62+func validateTrustedIPs(o *options.Options) []string {

63+ msgs := []string{}

64+ for i, ipStr := range o.TrustedIPs {

65+ if nil == ip.ParseIPNet(ipStr) {

66+ msgs = append(msgs, fmt.Sprintf("trusted_ips[%d] (%s) could not be recognized", i, ipStr))

67+ }

68+ }

69+ return msgs

70+}

pkg/validation/allowlist_test.gonew file+149

@@ -0,0 +1,149 @@

1+package validation

3+import (

4+ "testing"

6+ "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"

7+ "github.com/stretchr/testify/assert"

8+)

10+func Test_validateAllowlists(t *testing.T) {

11+ opts := &options.Options{

12+ SkipAuthRoutes: []string{

13+ "POST=/foo/bar",

14+ "PUT=^/foo/bar$",

15+ },

16+ SkipAuthRegex: []string{"/foo/baz"},

17+ TrustedIPs: []string{

18+ "10.32.0.1/32",

19+ "43.36.201.0/24",

20+ },

21+ }

22+ assert.Equal(t, []string{}, validateAllowlists(opts))

23+}

24+

25+func Test_validateRoutes(t *testing.T) {

26+ testCases := map[string]struct {

27+ Regexes []string

28+ Expected []string

29+ }{

30+ "Valid regex routes": {

31+ Regexes: []string{

32+ "/foo",

33+ "POST=/foo/bar",

34+ "PUT=^/foo/bar$",

35+ "DELETE=/crazy/(?:regex)?/[^/]+/stuff$",

36+ },

37+ Expected: []string{},

38+ },

39+ "Bad regexes do not compile": {

40+ Regexes: []string{

41+ "POST=/(foo",

42+ "OPTIONS=/foo/bar)",

43+ "GET=^]/foo/bar[$",

44+ "GET=^]/foo/bar[$",

45+ },

46+ Expected: []string{

47+ "error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",

48+ "error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",

49+ "error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",

50+ "error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",

51+ },

52+ },

53+ }

54+

55+ for testName, tc := range testCases {

56+ t.Run(testName, func(t *testing.T) {

57+ opts := &options.Options{

58+ SkipAuthRoutes: tc.Regexes,

59+ }

60+ msgs := validateRoutes(opts)

61+ assert.Equal(t, tc.Expected, msgs)

62+ })

63+ }

64+}

65+

66+func Test_validateRegexes(t *testing.T) {

67+ testCases := map[string]struct {

68+ Regexes []string

69+ Expected []string

70+ }{

71+ "Valid regex routes": {

72+ Regexes: []string{

73+ "/foo",

74+ "/foo/bar",

75+ "^/foo/bar$",

76+ "/crazy/(?:regex)?/[^/]+/stuff$",

77+ },

78+ Expected: []string{},

79+ },

80+ "Bad regexes do not compile": {

81+ Regexes: []string{

82+ "/(foo",

83+ "/foo/bar)",

84+ "^]/foo/bar[$",

85+ "^]/foo/bar[$",

86+ },

87+ Expected: []string{

88+ "error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",

89+ "error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",

90+ "error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",

91+ "error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",

92+ },

93+ },

94+ }

95+

96+ for testName, tc := range testCases {

97+ t.Run(testName, func(t *testing.T) {

98+ opts := &options.Options{

99+ SkipAuthRegex: tc.Regexes,

100+ }

101+ msgs := validateRegexes(opts)

102+ assert.Equal(t, tc.Expected, msgs)

103+ })

104+ }

105+}

106+

107+func Test_validateTrustedIPs(t *testing.T) {

108+ testCases := map[string]struct {

109+ TrustedIPs []string

110+ Expected []string

111+ }{

112+ "Non-overlapping valid IPs": {

113+ TrustedIPs: []string{

114+ "127.0.0.1",

115+ "10.32.0.1/32",

116+ "43.36.201.0/24",

117+ "::1",

118+ "2a12:105:ee7:9234:0:0:0:0/64",

119+ },

120+ Expected: []string{},

121+ },

122+ "Overlapping valid IPs": {

123+ TrustedIPs: []string{

124+ "135.180.78.199",

125+ "135.180.78.199/32",

126+ "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4",

127+ "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128",

128+ },

129+ Expected: []string{},

130+ },

131+ "Invalid IPs": {

132+ TrustedIPs: []string{"[::1]", "alkwlkbn/32"},

133+ Expected: []string{

134+ "trusted_ips[0] ([::1]) could not be recognized",

135+ "trusted_ips[1] (alkwlkbn/32) could not be recognized",

136+ },

137+ },

138+ }

139+

140+ for testName, tc := range testCases {

141+ t.Run(testName, func(t *testing.T) {

142+ opts := &options.Options{

143+ TrustedIPs: tc.TrustedIPs,

144+ }

145+ msgs := validateTrustedIPs(opts)

146+ assert.Equal(t, tc.Expected, msgs)

147+ })

148+ }

149+}

pkg/validation/options.go+2−22

@@ -9,7 +9,6 @@ import (

9 "net/http"

10 "net/url"

11 "os"

12− "regexp"

13 "strings"

12 "strings"

15 "github.com/coreos/go-oidc"

14 "github.com/coreos/go-oidc"

@@ -184,15 +183,6 @@ func Validate(o *options.Options) error {

184 o.SetRedirectURL(redirectURL)

183 o.SetRedirectURL(redirectURL)

185

184

186 msgs = append(msgs, validateUpstreams(o.UpstreamServers)...)

185 msgs = append(msgs, validateUpstreams(o.UpstreamServers)...)

187−

188− for _, u := range o.SkipAuthRegex {

189− compiledRegex, err := regexp.Compile(u)

190− if err != nil {

191− msgs = append(msgs, fmt.Sprintf("error compiling regex=%q %s", u, err))

192− continue

193− }

194− o.SetCompiledRegex(append(o.GetCompiledRegex(), compiledRegex))

195− }

196 msgs = parseProviderInfo(o, msgs)

186 msgs = parseProviderInfo(o, msgs)

197

187

198 if len(o.GoogleGroups) > 0 || o.GoogleAdminEmail != "" || o.GoogleServiceAccountJSON != "" {

188 if len(o.GoogleGroups) > 0 || o.GoogleAdminEmail != "" || o.GoogleServiceAccountJSON != "" {

@@ -223,18 +213,8 @@ func Validate(o *options.Options) error {

223 })

213 })

224 }

214 }

225

215

226− if len(o.TrustedIPs) > 0 && o.ReverseProxy {

216+ // Do this after ReverseProxy validation for TrustedIP coordinated checks

227− _, err := fmt.Fprintln(os.Stderr, "WARNING: trusting of IPs with --reverse-proxy poses risks if a header spoofing attack is possible.")

217+ msgs = append(msgs, validateAllowlists(o)...)

228− if err != nil {

229− panic(err)

230− }

231− }

232−

233− for i, ipStr := range o.TrustedIPs {

234− if nil == ip.ParseIPNet(ipStr) {

235− msgs = append(msgs, fmt.Sprintf("trusted_ips[%d] (%s) could not be recognized", i, ipStr))

236− }

237− }

238

218

239 if len(msgs) != 0 {

219 if len(msgs) != 0 {

240 return fmt.Errorf("invalid configuration:\n %s",

220 return fmt.Errorf("invalid configuration:\n %s",

pkg/validation/options_test.go+9−77

@@ -2,7 +2,6 @@ package validation

3import (

4 "crypto"

5− "errors"

6 "io/ioutil"

5 "io/ioutil"

7 "net/url"

6 "net/url"

8 "os"

7 "os"

@@ -78,12 +77,19 @@ func TestClientSecretFileOption(t *testing.T) {

78 if err != nil {

77 if err != nil {

79 t.Fatalf("failed to create temp file: %v", err)

78 t.Fatalf("failed to create temp file: %v", err)

80 }

79 }

81− f.WriteString("testcase")

80+ _, err = f.WriteString("testcase")

81+ if err != nil {

82+ t.Fatalf("failed to write to temp file: %v", err)

83+ }

82 if err := f.Close(); err != nil {

84 if err := f.Close(); err != nil {

83 t.Fatalf("failed to close temp file: %v", err)

85 t.Fatalf("failed to close temp file: %v", err)

84 }

86 }

85 clientSecretFileName := f.Name()

87 clientSecretFileName := f.Name()

88+ defer func(t *testing.T) {

86− defer os.Remove(clientSecretFileName)

89+ if err := os.Remove(clientSecretFileName); err != nil {

90+ t.Fatalf("failed to delete temp file: %v", err)

91+ }

92+ }(t)

88 o := options.NewOptions()

94 o := options.NewOptions()

89 o.Cookie.Secret = cookieSecret

95 o.Cookie.Secret = cookieSecret

@@ -144,41 +150,6 @@ func TestRedirectURL(t *testing.T) {

144 assert.Equal(t, expected, o.GetRedirectURL())

150 assert.Equal(t, expected, o.GetRedirectURL())

145}

151}

146

152

147−func TestCompiledRegex(t *testing.T) {

148− o := testOptions()

149− regexps := []string{"/foo/.*", "/ba[rz]/quux"}

150− o.SkipAuthRegex = regexps

151− assert.Equal(t, nil, Validate(o))

152− actual := make([]string, 0)

153− for _, regex := range o.GetCompiledRegex() {

154− actual = append(actual, regex.String())

155− }

156− assert.Equal(t, regexps, actual)

157−}

158−

159−func TestCompiledRegexError(t *testing.T) {

160− o := testOptions()

161− o.SkipAuthRegex = []string{"(foobaz", "barquux)"}

162− err := Validate(o)

163− assert.NotEqual(t, nil, err)

164−

165− expected := errorMsg([]string{

166− "error compiling regex=\"(foobaz\" error parsing regexp: " +

167− "missing closing ): `(foobaz`",

168− "error compiling regex=\"barquux)\" error parsing regexp: " +

169− "unexpected ): `barquux)`"})

170− assert.Equal(t, expected, err.Error())

171−

172− o.SkipAuthRegex = []string{"foobaz", "barquux)"}

173− err = Validate(o)

174− assert.NotEqual(t, nil, err)

175−

176− expected = errorMsg([]string{

177− "error compiling regex=\"barquux)\" error parsing regexp: " +

178− "unexpected ): `barquux)`"})

179− assert.Equal(t, expected, err.Error())

180−}

181−

182func TestDefaultProviderApiSettings(t *testing.T) {

153func TestDefaultProviderApiSettings(t *testing.T) {

183 o := testOptions()

154 o := testOptions()

184 assert.Equal(t, nil, Validate(o))

155 assert.Equal(t, nil, Validate(o))

@@ -337,45 +308,6 @@ func TestRealClientIPHeader(t *testing.T) {

337 assert.Nil(t, o.GetRealClientIPParser())

308 assert.Nil(t, o.GetRealClientIPParser())

338}

309}

339

310

340−func TestIPCIDRSetOption(t *testing.T) {

341− tests := []struct {

342− name string

343− trustedIPs []string

344− err error

345− }{

346− {

347− "TestSomeIPs",

348− []string{"127.0.0.1", "10.32.0.1/32", "43.36.201.0/24", "::1", "2a12:105:ee7:9234:0:0:0:0/64"},

349− nil,

350− }, {

351− "TestOverlappingIPs",

352− []string{"135.180.78.199", "135.180.78.199/32", "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4", "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128"},

353− nil,

354− }, {

355− "TestInvalidIPs",

356− []string{"[::1]", "alkwlkbn/32"},

357− errors.New(

358− "invalid configuration:\n" +

359− " trusted_ips[0] ([::1]) could not be recognized\n" +

360− " trusted_ips[1] (alkwlkbn/32) could not be recognized",

361− ),

362− },

363− }

364−

365− for _, tt := range tests {

366− t.Run(tt.name, func(t *testing.T) {

367− o := testOptions()

368− o.TrustedIPs = tt.trustedIPs

369− err := Validate(o)

370− if tt.err == nil {

371− assert.Nil(t, err)

372− } else {

373− assert.Equal(t, tt.err.Error(), err.Error())

374− }

375− })

376− }

377−}

378−

379func TestProviderCAFilesError(t *testing.T) {

311func TestProviderCAFilesError(t *testing.T) {

380 file, err := ioutil.TempFile("", "absent.*.crt")

312 file, err := ioutil.TempFile("", "absent.*.crt")

381 assert.NoError(t, err)

313 assert.NoError(t, err)