Skip to content

Commit

Permalink
Uses separate secret reader for grant filters (#1660)
Browse files Browse the repository at this point in the history
Using shared secret reader is not safe in multitenant setup where users
can define their own routes and thus can access grant secret via `bearerinjector` filter.

Signed-off-by: Alexander Yastrebov <[email protected]>
  • Loading branch information
AlexanderYastrebov authored Jan 11, 2021
1 parent 5f5c202 commit cbc94b1
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 23 deletions.
3 changes: 1 addition & 2 deletions docs/reference/filters.md
Original file line number Diff line number Diff line change
Expand Up @@ -1195,8 +1195,7 @@ Skipper arguments:
| `-oauth2-client-secret-file` | conditional | path to the file containing the OAuth2 client secret. Required if you have not set `-oauth2-client-secret`. Example: `-oauth2-client-secret-file=/path/to/client_secret` |
| `-oauth2-client-id` | conditional | OAuth2 client ID for authenticating with your OAuth2 provider. Required if you have not set `-oauth2-client-id-file`. Example: `-oauth2-client-id=myclientid` |
| `-oauth2-client-secret` | conditional | OAuth2 client secret for authenticating with your OAuth2 provider. Required if you have not set `-oauth2-client-secret-file`. Example: `-oauth2-client-secret=myclientsecret` |
| `-credentials-paths` | conditional | path to the directories containing the cookie encryption secret client ID, and client secret files. Required if you want Skipper to automatically update the secrets periodically. Example: `-credentials-paths=/path/to/secrets/,/path/to/othersecrets/` |
| `-credentials-update-interval` | no | the time interval for updating credentials from files. Example: `-credentials-update-interval=30s` |
| `-credentials-update-interval` | no | the time interval for updating client id and client secret from files. Example: `-credentials-update-interval=30s` |
| `-oauth2-access-token-header-name` | no | the name of the request header where the user's bearer token should be set. Default: `Authorization`. Example: `-oauth2-access-token-header-name=X-Grant-Authorization` |
| `-oauth2-auth-url-parameters` | no | any additional URL query parameters to set for the OAuth2 provider's authorize and token endpoint calls. Example: `-oauth2-auth-url-parameters=key1=foo,key2=bar` |
| `-oauth2-callback-path` | no | path of the Skipper route containing the `grantCallback()` filter for accepting an authorization code and using it to get an access token. Example: `-oauth2-callback-path=/oauth/callback` |
Expand Down
8 changes: 5 additions & 3 deletions docs/tutorials/auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,14 @@ secret as follows:
skipper -oauth2-client-id-file=/path/to/client_id \
-oauth2-client-secret-file=/path/to/client_secret \
-oauth2-secret-file=/path/to/cookie_encryption_secret \
-credentials-paths=/path/to/ \
-credentials-update-interval=30s
```

You must set the `-credentials-paths` argument to the directory containing the secrets. You
can modify the secret update interval using the `-credentials-update-interval` argument. In
Care must be taken when used in conjunction with `-credentials-paths` option because files
from `-credentials-paths` are available to `bearerinjector` filter.
That is `-credentials-paths=/path/to` in above example will expose grant files to `bearerinjector` filter.

You can modify the secret update interval using the `-credentials-update-interval` argument. In
example above, the interval is configured to reload the secrets from the files every 30
seconds.

Expand Down
30 changes: 12 additions & 18 deletions skipper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1017,22 +1017,6 @@ func listenAndServe(proxy http.Handler, o *Options) error {
return listenAndServeQuit(proxy, o, nil, nil, nil)
}

func initGrant(c *auth.OAuthConfig, o *Options) error {
if err := c.Init(); err != nil {
return err
}

o.CustomFilters = append(
o.CustomFilters,
c.NewGrant(),
c.NewGrantCallback(),
c.NewGrantClaimsQuery(),
c.NewGrantLogout(),
)

return nil
}

func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {
// init log
err := initLog(o)
Expand Down Expand Up @@ -1284,6 +1268,9 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {

oauthConfig := &auth.OAuthConfig{}
if o.EnableOAuth2GrantFlow /* explicitly enable grant flow */ {
grantSecrets := secrets.NewSecretPaths(o.CredentialsUpdateInterval)
defer grantSecrets.Close()

oauthConfig.AuthURL = o.OAuth2AuthURL
oauthConfig.TokenURL = o.OAuth2TokenURL
oauthConfig.RevokeTokenURL = o.OAuth2RevokeTokenURL
Expand All @@ -1295,7 +1282,7 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {
oauthConfig.ClientSecretFile = o.OAuth2ClientSecretFile
oauthConfig.CallbackPath = o.OAuth2CallbackPath
oauthConfig.AuthURLParameters = o.OAuth2AuthURLParameters
oauthConfig.SecretsProvider = sp
oauthConfig.SecretsProvider = grantSecrets
oauthConfig.Secrets = o.SecretsRegistry
oauthConfig.AccessTokenHeaderName = o.OAuth2AccessTokenHeaderName
oauthConfig.TokeninfoSubjectKey = o.OAuth2TokeninfoSubjectKey
Expand All @@ -1304,10 +1291,17 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error {
oauthConfig.MaxIdleConnectionsPerHost = o.IdleConnectionsPerHost
oauthConfig.Tracer = tracer

if err := initGrant(oauthConfig, &o); err != nil {
if err := oauthConfig.Init(); err != nil {
log.Errorf("Failed to initialize oauth grant filter: %v.", err)
return err
}

o.CustomFilters = append(o.CustomFilters,
oauthConfig.NewGrant(),
oauthConfig.NewGrantCallback(),
oauthConfig.NewGrantClaimsQuery(),
oauthConfig.NewGrantLogout(),
)
}

// create a filter registry with the available filter specs registered,
Expand Down

0 comments on commit cbc94b1

Please sign in to comment.