diff --git a/README.md b/README.md index 3ca3006..1edea7f 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,7 @@ Following environment variables are used by the software. | `VERIFY_AUTH_URL` | `AUTHSERVICE_URL_PREFIX/verify` | Path to the `/verify` endpoint. This endpoint examines a subrequest and returns `204` if the user is authenticated and authorized to perform such a request, otherwise it will return `401` if the user cannot be authenticated or `403` if the user is authenticated but they are not authorized to perform this request. | | `AUTH_HEADER` | `Authorization` | When the AuthService logs in a user, it creates a session for them and saves it in its database. The session secret value is saved in a cookie in the user's browser. However, for programmatic access to endpoints, it is better to use headers to authenticate. The AuthService also accepts credentials in a header configured by the `AUTH_HEADER` setting. | | `ID_TOKEN_HEADER` | `Authorization` | When id token is carried in this header, OIDC Authservice verifies the id token and uses the `USERID_CLAIM` inside the id token. If the `USERID_CLAIM` doesn't exist, the authentication would fail.| +| `DYNAMIC_CSRF_COOKIE_NAME` | `false` | Make the name of the oidc csrf cookie dynamic by adding a random suffix. If there are multiple browser tabs attempting an auth flow (eg: when the main session cookie times out) this can lead to a frustrating loss of context where the original tab ends up in a non-sensical auth url. This setting ensures each tab has a unique csrf cookie, which means they should all be able to complete an auth flow if required. | | `LOG_LEVEL` | "INFO" | Set the log level to one of "FATAL", "ERROR", "WARN", "INFO", or "DEBUG" to specify the verbosity of the OIDC-Authservice logs. | The AuthService provides a web server with some defaults pages for a `homepage` diff --git a/common/settings.go b/common/settings.go index 818eec9..68e8fd1 100644 --- a/common/settings.go +++ b/common/settings.go @@ -23,18 +23,20 @@ type Config struct { OIDCStateStorePath string `split_words:"true" default:"/var/lib/authservice/data.db"` // General - AuthserviceURLPrefix *url.URL `required:"true" split_words:"true"` - SkipAuthURLs []string `split_words:"true" envconfig:"SKIP_AUTH_URLS"` - AuthHeader string `split_words:"true" default:"Authorization"` - AuthMethodHeader string `split_words:"true" default:"Auth-Method"` - SchemeDefault string `split_words:"true" default:"https"` - SchemeHeader string `split_words:"true" default:"X-Forwarded-Proto"` - Audiences []string `default:"istio-ingressgateway.istio-system.svc.cluster.local"` - HomepageURL *url.URL `split_words:"true"` - AfterLoginURL *url.URL `split_words:"true"` - AfterLogoutURL *url.URL `split_words:"true"` - VerifyAuthURL *url.URL `split_words:"true"` - LogLevel string `split_words:"true" default:"INFO"` + AuthserviceURLPrefix *url.URL `required:"true" split_words:"true"` + SkipAuthURLs []string `split_words:"true" envconfig:"SKIP_AUTH_URLS"` + AuthHeader string `split_words:"true" default:"Authorization"` + AuthMethodHeader string `split_words:"true" default:"Auth-Method"` + SchemeDefault string `split_words:"true" default:"https"` + SchemeHeader string `split_words:"true" default:"X-Forwarded-Proto"` + Audiences []string `default:"istio-ingressgateway.istio-system.svc.cluster.local"` + HomepageURL *url.URL `split_words:"true"` + AfterLoginURL *url.URL `split_words:"true"` + AfterLogoutURL *url.URL `split_words:"true"` + VerifyAuthURL *url.URL `split_words:"true"` + LogLevel string `split_words:"true" default:"INFO"` + DynamicCsrfCookieName bool `split_words:"true"` + // Identity Headers UserIDHeader string `split_words:"true" envconfig:"USERID_HEADER"` diff --git a/main.go b/main.go index 965700c..ae3feff 100644 --- a/main.go +++ b/main.go @@ -232,6 +232,7 @@ func main() { cacheEnabled: c.CacheEnabled, cacheExpirationMinutes: c.CacheExpirationMinutes, jwtCookie: c.JWTCookie, + dynamicCsrfCookieName: c.DynamicCsrfCookieName, IDTokenAuthnEnabled: c.IDTokenAuthnEnabled, KubernetesAuthnEnabled: c.KubernetesAuthnEnabled, diff --git a/server.go b/server.go index 8ddbe95..74b79c1 100644 --- a/server.go +++ b/server.go @@ -47,6 +47,7 @@ type server struct { verifyAuthURL string sessionMaxAgeSeconds int jwtCookie string + dynamicCsrfCookieName bool // Cache Configurations cacheEnabled bool @@ -365,7 +366,8 @@ func (s *server) authCodeFlowAuthenticationRequest(w http.ResponseWriter, r *htt logger := common.RequestLogger(r, logModuleInfo) // Initiate OIDC Flow with Authorization Request. - state, err := sessions.CreateState(r, w, s.oidcStateStore, s.sessionDomain, s.newState) + state, err := sessions.CreateState(r, w, s.oidcStateStore, s.sessionDomain, + s.newState, s.dynamicCsrfCookieName) if err != nil { logger.Errorf("Failed to save state in store: %v", err) common.ReturnMessage(w, http.StatusInternalServerError, "Failed to save state in store.") @@ -403,7 +405,7 @@ func (s *server) callback(w http.ResponseWriter, r *http.Request) { } // If state is loaded, then it's correct, as it is saved by its id. - state, err := sessions.VerifyState(r, w, s.oidcStateStore) + state, err := sessions.VerifyState(r, w, s.oidcStateStore, s.dynamicCsrfCookieName) if err != nil { logger.Errorf("Failed to verify state parameter: %v", err) common.ReturnMessage(w, http.StatusBadRequest, "CSRF check failed."+ diff --git a/sessions/state.go b/sessions/state.go index 6585163..670e51a 100644 --- a/sessions/state.go +++ b/sessions/state.go @@ -4,6 +4,7 @@ package sessions import ( "encoding/gob" + "math/rand" "net/http" "net/url" "strings" @@ -17,8 +18,12 @@ import ( const ( oidcStateCookie = "oidc_state_csrf" sessionValueState = "state" + charset = "abcdefghijklmnopqrstuvwxyz" ) +var seededRand *rand.Rand = rand.New( + rand.NewSource(time.Now().UnixNano())) + func init() { gob.Register(State{}) } @@ -79,14 +84,33 @@ func newSchemeAndHost(config *Config) StateFunc { } } +// stringWithCharset creates a random string of length from charset +func stringWithCharset(length int, charset string) string { + b := make([]byte, length) + for i := range b { + b[i] = charset[seededRand.Intn(len(charset))] + } + return string(b) + } + +// randString returns a random string of given length +func randString(length int) string { + return stringWithCharset(length, charset) +} + // CreateState creates the state parameter from the incoming request, stores // it in the session store and sets a cookie with the session key. // It returns the session key, which can be used as the state value to start // an OIDC authentication request. -func CreateState(r *http.Request, w http.ResponseWriter, - store sessions.Store, sessionDomain string, fn StateFunc) (string, error) { +func CreateState(r *http.Request, w http.ResponseWriter, store sessions.Store, + sessionDomain string, fn StateFunc, dynamicOidcStateCookieName bool) (string, error) { + nonce := randString(8) + oidcStateCookieName := oidcStateCookie + if (dynamicOidcStateCookieName) { + oidcStateCookieName += "_" + nonce + } s := fn(r) - session := sessions.NewSession(store, oidcStateCookie) + session := sessions.NewSession(store, oidcStateCookieName) session.Options.MaxAge = int((20 * time.Minute).Seconds()) session.Options.Path = "/" session.Options.Domain = sessionDomain @@ -100,11 +124,15 @@ func CreateState(r *http.Request, w http.ResponseWriter, // Cookie is persisted in ResponseWriter, make a request to parse it. tempReq := &http.Request{Header: make(http.Header)} tempReq.Header.Set("Cookie", w.Header().Get("Set-Cookie")) - c, err := tempReq.Cookie(oidcStateCookie) + c, err := tempReq.Cookie(oidcStateCookieName) if err != nil { return "", errors.Wrap(err, "error trying to save session") } - return c.Value, nil + stateValue := c.Value + if (dynamicOidcStateCookieName) { + stateValue += "." + nonce + } + return stateValue, nil } // VerifyState gets the state from the cookie 'initState' saved. It also gets @@ -117,7 +145,7 @@ func CreateState(r *http.Request, w http.ResponseWriter, // Finally, it returns a State struct, which contains information associated // with the particular OIDC flow. func VerifyState(r *http.Request, w http.ResponseWriter, - store sessions.Store) (*State, error) { + store sessions.Store, dynamicOidcStateCookieName bool) (*State, error) { // Get the state from the HTTP param. var stateParam = r.FormValue("state") @@ -125,14 +153,24 @@ func VerifyState(r *http.Request, w http.ResponseWriter, return nil, errors.New("Missing url parameter: state") } + oidcStateCookieName := oidcStateCookie + stateValue := stateParam + nonce := "" + if (dynamicOidcStateCookieName) { + stateParamParts := strings.Split(stateParam, ".") + stateValue = stateParamParts[0] + nonce = stateParamParts[1] + oidcStateCookieName += "_" + nonce + } + // Get the state from the cookie the user-agent sent. - stateCookie, err := r.Cookie(oidcStateCookie) + stateCookie, err := r.Cookie(oidcStateCookieName) if err != nil { - return nil, errors.Errorf("Missing cookie: '%s'", oidcStateCookie) + return nil, errors.Errorf("Missing cookie: '%s'", oidcStateCookieName) } // Confirm the two values match. - if stateParam != stateCookie.Value { + if stateValue != stateCookie.Value { return nil, errors.New("State value from http params doesn't match " + "value in cookie. Possible reasons for this error include " + "opening the login form in more than 1 browser tabs OR a CSRF " + @@ -140,7 +178,7 @@ func VerifyState(r *http.Request, w http.ResponseWriter, } // Retrieve session from store. If it doesn't exist, it may have expired. - session, err := store.Get(r, oidcStateCookie) + session, err := store.Get(r, oidcStateCookieName) if err != nil { return nil, errors.WithStack(err) }