Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v14] Fix app redirection loop on browser's incognito mode and 3rd party cookie block #37692

Merged
merged 4 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions integration/helpers/cookies.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
type AppCookies struct {
SessionCookie *http.Cookie
SubjectSessionCookie *http.Cookie
AuthStateCookie *http.Cookie
}

// WithSubjectCookie returns a copy of AppCookies with the specified subject session cookie.
Expand All @@ -43,6 +44,9 @@ func (ac *AppCookies) ToSlice() []*http.Cookie {
if ac.SubjectSessionCookie != nil {
out = append(out, ac.SubjectSessionCookie)
}
if ac.AuthStateCookie != nil {
out = append(out, ac.AuthStateCookie)
}
return out
}

Expand All @@ -56,6 +60,8 @@ func ParseCookies(t *testing.T, cookies []*http.Cookie) *AppCookies {
out.SessionCookie = c
case app.SubjectCookieName:
out.SubjectSessionCookie = c
case app.AuthStateCookieName:
out.AuthStateCookie = c
default:
t.Fatalf("unrecognized cookie name: %q", c.Name)
}
Expand Down
4 changes: 4 additions & 0 deletions lib/httplib/httpheaders.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,10 @@ func SetIndexContentSecurityPolicy(h http.Header, cfg proto.Features, urlPath st
h.Set("Content-Security-Policy", cspString)
}

// DELETE IN 17.0: Kept for legacy app access.
var appLaunchCSPStringCache *cspCache = newCSPCache()

// DELETE IN 17.0: Kept for legacy app access.
func getAppLaunchContentSecurityPolicyString(applicationURL string) string {
if cspString, ok := appLaunchCSPStringCache.get(applicationURL); ok {
return cspString
Expand All @@ -229,6 +231,8 @@ func getAppLaunchContentSecurityPolicyString(applicationURL string) string {
return cspString
}

// DELETE IN 17.0: Kept for legacy app access.
//
// SetAppLaunchContentSecurityPolicy sets the Content-Security-Policy header for /web/launch
func SetAppLaunchContentSecurityPolicy(h http.Header, applicationURL string) {
cspString := getAppLaunchContentSecurityPolicyString(applicationURL)
Expand Down
12 changes: 10 additions & 2 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ func (h *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// request has a session cookie or a client cert, forward to
// application handlers. If the request is requesting a
// FQDN that is not of the proxy, redirect to application launcher.
if h.appHandler != nil && (app.HasFragment(r) || app.HasSession(r) || app.HasClientCert(r)) {
if h.appHandler != nil && (app.HasFragment(r) || app.HasSessionCookie(r) || app.HasClientCert(r)) {
h.appHandler.ServeHTTP(w, r)
return
}
Expand Down Expand Up @@ -520,8 +520,15 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {

httplib.SetNoCacheHeaders(w.Header())

// app access needs to make a CORS fetch request, so we only set the default CSP on that page
// DELETE IN 17.0: Delete the first if block. Keep the else case.
// Kept for backwards compatibility.
//
// This case only adds an additional CSP `content-src` value of the
// app URL which allows requesting to the app domain required by
// the legacy app access.
if strings.HasPrefix(r.URL.Path, "/web/launch/") {
// legacy app access needs to make a CORS fetch request,
// so we only set the default CSP on that page
parts := strings.Split(r.URL.Path, "/")
// grab the FQDN from the URL to allow in the connect-src CSP
applicationURL := "https://" + parts[3] + ":*"
Expand All @@ -530,6 +537,7 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {
} else {
httplib.SetIndexContentSecurityPolicy(w.Header(), cfg.ClusterFeatures, r.URL.Path)
}

if err := indexPage.Execute(w, session); err != nil {
h.log.WithError(err).Error("Failed to execute index page template.")
}
Expand Down
269 changes: 250 additions & 19 deletions lib/web/app/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,186 @@ package app

import (
"crypto/subtle"
"fmt"
"net/http"
"strings"

"github.com/gravitational/trace"
"github.com/julienschmidt/httprouter"

"github.com/gravitational/teleport/api/types"
apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/httplib"
"github.com/gravitational/teleport/lib/utils"
)

type fragmentRequest struct {
StateValue string `json:"state_value"`
CookieValue string `json:"cookie_value"`
SubjectCookieValue string `json:"subject_cookie_value"`
}

// startAppAuthExchange will do two actions depending on the following:
//
// 1): On initiating auth exchange (indicated by an empty "state" query param)
// we create a crypto safe random token and send it back as part of a "state"
// query param in the redirection URL, as well as in a cookie with attributes
// that makes the cookie unaccesible and hard to tamper with. We use this
// "double submit cookie" method to protect the entire auth exchange flow
// from CSRF.
//
// 2): If the "state" query param is present, we will serve a blank HTML page
// that has inline JS that contains logic to complete the auth exchange.
func (h *Handler) startAppAuthExchange(w http.ResponseWriter, r *http.Request, p httprouter.Params) error {
q := r.URL.Query()

// Initiate auth exchange.
if q.Get("state") == "" {
// secretToken is the token we will look for in both the cookie
// and in the request "state" query param.
secretToken, err := utils.CryptoRandomHex(defaults.TokenLenBytes)
if err != nil {
h.log.WithError(err).Debug("Failed to generate token required for app auth exchange")
return trace.AccessDenied("access denied")
}

// cookieIdentifier is used to uniquely identify this cookie
// that will be used to store this secret token.
//
// This prevents a race condition (state token mismatch error)
// where we can overwrite existing cookie (with the same name) with a
// different token value eg: launch app in multiple tabs in quick succession
cookieIdentifier, err := utils.CryptoRandomHex(defaults.TokenLenBytes)
if err != nil {
h.log.WithError(err).Debug("Failed to generate an UID for the app auth state cookie")
return trace.AccessDenied("access denied")
}

h.setAuthStateCookie(w, secretToken, cookieIdentifier)

webLauncherURLParams := launcherURLParams{
clusterName: q.Get("cluster"),
publicAddr: q.Get("addr"),
arn: q.Get("arn"),
path: q.Get("path"),
// The state token concats both the secret token and the cookie ID.
// The server will break this token to its individual parts:
// - secretToken to compare against the one stored in cookie
// - cookieIdentifier to look up cookie sent by browser.
stateToken: fmt.Sprintf("%s_%s", secretToken, cookieIdentifier),
}
return h.redirectToLauncher(w, r, webLauncherURLParams)
}

// Continue the auth exchange.

nonce, err := utils.CryptoRandomHex(defaults.TokenLenBytes)
if err != nil {
h.log.WithError(err).Debug("Failed to create a random nonce for the app redirection HTML inline script")
return trace.AccessDenied("access denied")
}
SetRedirectPageHeaders(w.Header(), nonce)

// Serving the HTML page.
if err := appRedirectTemplate.Execute(w, nonce); err != nil {
h.log.WithError(err).Debug("Failed executing appRedirect template")
return trace.AccessDenied("access denied")
}
return nil
}

// completeAppAuthExchange completes the auth exchange flow started by "startAppAuthExchange" handler
// by validating the values passed in the request body, and upon success sets cookies required
// for the current app session.
func (h *Handler) completeAppAuthExchange(w http.ResponseWriter, r *http.Request, p httprouter.Params) error {
httplib.SetNoCacheHeaders(w.Header())
var req fragmentRequest
if err := httplib.ReadJSON(r, &req); err != nil {
h.log.WithError(err).Debug("Failed to decode JSON from request body")
return trace.AccessDenied("access denied")
}

secretToken, cookieID, ok := strings.Cut(req.StateValue, "_")
if !ok {
h.log.Warn("Request failed: request state token is not in the expected format")
h.emitErrorEventAndDeleteAppSession(r, emitErrorEventFields{
sessionID: req.CookieValue,
err: "state token was not in the expected format",
})
return trace.AccessDenied("access denied")
}

// Validate that the caller-provided state token matches the stored state token (CSRF check)
stateCookie, err := r.Cookie(getAuthStateCookieName(cookieID))
if err != nil || stateCookie.Value == "" {
h.log.Warn("Request failed: state cookie is not set.")
h.emitErrorEventAndDeleteAppSession(r, emitErrorEventFields{
sessionID: req.CookieValue,
err: "auth state cookie was not set",
})
return trace.AccessDenied("access denied")
}
if subtle.ConstantTimeCompare([]byte(secretToken), []byte(stateCookie.Value)) != 1 {
h.log.Warn("Request failed: state token does not match.")
h.emitErrorEventAndDeleteAppSession(r, emitErrorEventFields{
sessionID: req.CookieValue,
err: "state token did not match",
})
return trace.AccessDenied("access denied")
}

// Prevent reuse of the same state token.
clearAuthStateCookie(w, cookieID)

// Validate that the caller is asking for a session that exists and that they have the secret
// session token for.
ws, err := h.c.AccessPoint.GetAppSession(r.Context(), types.GetAppSessionRequest{
SessionID: req.CookieValue,
})
if err != nil {
h.log.WithError(err).Warn("Request failed: session does not exist.")
return trace.AccessDenied("access denied")
}
if err := checkSubjectToken(req.SubjectCookieValue, ws); err != nil {
h.log.WithError(err).Warn("Request failed")
h.emitErrorEventAndDeleteAppSession(r, emitErrorEventFields{
sessionID: req.CookieValue,
err: err.Error(),
loginName: ws.GetUser(),
})
return trace.AccessDenied("access denied")
}

// Set the "Set-Cookie" header on the response.
// Set Same-Site policy for the session cookies to None in order to
// support redirects that identity providers do during SSO auth.
// Otherwise the session cookie won't be sent and the user will
// get redirected to the application launcher.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
http.SetCookie(w, &http.Cookie{
Name: CookieName,
Value: req.CookieValue,
Path: "/",
HttpOnly: true,
Secure: true,
SameSite: http.SameSiteNoneMode,
})
http.SetCookie(w, &http.Cookie{
Name: SubjectCookieName,
Value: ws.GetBearerToken(),
Path: "/",
HttpOnly: true,
Secure: true,
SameSite: http.SameSiteNoneMode,
})

return nil
}

// DELETE IN 17.0: kept for backwards compat.
//
// handleAuth handles authentication for an app
// When a `POST` request comes in from a trusted proxy address, it'll set the value from the
// `X-Cookie-Value` header to the `__Host-grv_app_session` cookie.
Expand All @@ -37,12 +206,21 @@ func (h *Handler) handleAuth(w http.ResponseWriter, r *http.Request, p httproute

cookieValue := r.Header.Get("X-Cookie-Value")
if cookieValue == "" {
h.log.Warn("Request failed: missing X-Cookie-Value header")
h.emitErrorEventAndDeleteAppSession(r, emitErrorEventFields{
err: "missing X-Cookie-Value header",
})
return trace.AccessDenied("access denied")
}

subjectCookieValue := r.Header.Get("X-Subject-Cookie-Value")
if cookieValue == "" {
return trace.BadParameter("X-Subject-Cookie-Value header missing")
if subjectCookieValue == "" {
h.log.Warn("Request failed: X-Subject-Cookie-Value")
h.emitErrorEventAndDeleteAppSession(r, emitErrorEventFields{
err: "missing X-Subject-Cookie-Value header",
sessionID: cookieValue,
})
return trace.AccessDenied("access denied")
}

// Validate that the caller is asking for a session that exists.
Expand All @@ -57,23 +235,10 @@ func (h *Handler) handleAuth(w http.ResponseWriter, r *http.Request, p httproute
if err := checkSubjectToken(subjectCookieValue, ws); err != nil {
h.log.Warnf("Request failed: %v.", err)

h.c.AuthClient.EmitAuditEvent(h.closeContext, &apievents.AuthAttempt{
Metadata: apievents.Metadata{
Type: events.AuthAttemptEvent,
Code: events.AuthAttemptFailureCode,
},
UserMetadata: apievents.UserMetadata{
Login: ws.GetUser(),
User: "unknown",
},
ConnectionMetadata: apievents.ConnectionMetadata{
LocalAddr: r.Host,
RemoteAddr: r.RemoteAddr,
},
Status: apievents.Status{
Success: false,
Error: err.Error(),
},
h.emitErrorEventAndDeleteAppSession(r, emitErrorEventFields{
sessionID: cookieValue,
err: err.Error(),
loginName: ws.GetUser(),
})

return trace.AccessDenied("access denied")
Expand Down Expand Up @@ -109,3 +274,69 @@ func checkSubjectToken(subjectCookieValue string, ws types.WebSession) error {
}
return nil
}

func (h *Handler) setAuthStateCookie(w http.ResponseWriter, cookieValue string, cookieID string) {
http.SetCookie(w, &http.Cookie{
Name: getAuthStateCookieName(cookieID),
Value: cookieValue,
Path: "/",
HttpOnly: true,
Secure: true,
SameSite: http.SameSiteNoneMode,
MaxAge: 60, // Expire in 1 minute.
})
}

func clearAuthStateCookie(w http.ResponseWriter, cookieID string) {
http.SetCookie(w, &http.Cookie{
Name: getAuthStateCookieName(cookieID),
Value: "",
Path: "/",
HttpOnly: true,
Secure: true,
SameSite: http.SameSiteNoneMode,
MaxAge: -1,
})
}

func getAuthStateCookieName(cookieID string) string {
return fmt.Sprintf("%s_%s", AuthStateCookieName, cookieID)
}

type emitErrorEventFields struct {
loginName string
err string
sessionID string
}

func (h *Handler) emitErrorEventAndDeleteAppSession(r *http.Request, f emitErrorEventFields) {
// Attempt to delete app session.
if f.sessionID != "" {
if err := h.c.AuthClient.DeleteAppSession(r.Context(), types.DeleteAppSessionRequest{
SessionID: f.sessionID,
}); err != nil {
h.log.WithError(err).Warn("Failed to delete app session after failing authentication")
}
}

event := &apievents.AuthAttempt{
Metadata: apievents.Metadata{
Type: events.AuthAttemptEvent,
Code: events.AuthAttemptFailureCode,
},
UserMetadata: apievents.UserMetadata{
User: "unknown",
Login: f.loginName,
},
ConnectionMetadata: apievents.ConnectionMetadata{
LocalAddr: r.Host,
RemoteAddr: r.RemoteAddr,
},
Status: apievents.Status{
Success: false,
Error: fmt.Sprintf("Failed app access authentication: %s", f.err),
},
}

h.c.AuthClient.EmitAuditEvent(h.closeContext, event)
}
Loading
Loading