From 598ffd4d169262e15ba7cc5faf4c469c9e49706b Mon Sep 17 00:00:00 2001 From: Lisa Kim Date: Thu, 10 Aug 2023 13:29:32 -0700 Subject: [PATCH 01/11] Copy pasta revert back to previous app redirection logic Reverted parts of: https://github.com/gravitational/teleport/pull/17592 Kept json field renames --- integration/helpers/cookies.go | 6 ++ lib/httplib/httpheaders.go | 25 ----- lib/httplib/httplib_test.go | 26 ----- lib/web/apiserver.go | 11 +- lib/web/app/auth.go | 190 ++++++++++++++++++++++----------- lib/web/app/handler.go | 10 +- lib/web/app/handler_test.go | 187 +++++++++++--------------------- lib/web/app/middleware.go | 63 ++--------- lib/web/app/redirect.go | 56 ++++++++++ 9 files changed, 270 insertions(+), 304 deletions(-) diff --git a/integration/helpers/cookies.go b/integration/helpers/cookies.go index bff65ede43b46..81f262d1dbe10 100644 --- a/integration/helpers/cookies.go +++ b/integration/helpers/cookies.go @@ -29,6 +29,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. @@ -47,6 +48,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 } @@ -60,6 +64,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) } diff --git a/lib/httplib/httpheaders.go b/lib/httplib/httpheaders.go index 930f7c1bbe869..e32bd830e6a57 100644 --- a/lib/httplib/httpheaders.go +++ b/lib/httplib/httpheaders.go @@ -217,31 +217,6 @@ func SetIndexContentSecurityPolicy(h http.Header, cfg proto.Features, urlPath st h.Set("Content-Security-Policy", cspString) } -var appLaunchCSPStringCache *cspCache = newCSPCache() - -func getAppLaunchContentSecurityPolicyString(applicationURL string) string { - if cspString, ok := appLaunchCSPStringCache.get(applicationURL); ok { - return cspString - } - - cspString := getContentSecurityPolicyString( - defaultContentSecurityPolicy, - defaultFontSrc, - cspMap{ - "connect-src": {"'self'", applicationURL}, - }, - ) - appLaunchCSPStringCache.set(applicationURL, cspString) - - return cspString -} - -// SetAppLaunchContentSecurityPolicy sets the Content-Security-Policy header for /web/launch -func SetAppLaunchContentSecurityPolicy(h http.Header, applicationURL string) { - cspString := getAppLaunchContentSecurityPolicyString(applicationURL) - h.Set("Content-Security-Policy", cspString) -} - var redirectCSPStringCache *cspCache = newCSPCache() func getRedirectPageContentSecurityPolicyString(scriptSrc string) string { diff --git a/lib/httplib/httplib_test.go b/lib/httplib/httplib_test.go index 8a4e39a2e50ba..fe0ed1556a61e 100644 --- a/lib/httplib/httplib_test.go +++ b/lib/httplib/httplib_test.go @@ -376,32 +376,6 @@ func TestSetIndexContentSecurityPolicy(t *testing.T) { } } -func TestSetAppLaunchContentSecurityPolicy(t *testing.T) { - t.Parallel() - - applicationURL := "https://example.com" - - expectedCspVals := map[string]string{ - "default-src": "'self'", - "base-uri": "'self'", - "form-action": "'self'", - "frame-ancestors": "'none'", - "object-src": "'none'", - "style-src": "'self' 'unsafe-inline'", - "img-src": "'self' data: blob:", - "font-src": "'self' data:", - "connect-src": fmt.Sprintf("'self' %s", applicationURL), - } - - h := make(http.Header) - SetAppLaunchContentSecurityPolicy(h, applicationURL) - actualCsp := h.Get("Content-Security-Policy") - for k, v := range expectedCspVals { - expectedCspSubString := fmt.Sprintf("%s %s;", k, v) - require.Contains(t, actualCsp, expectedCspSubString) - } -} - func TestSetRedirectPageContentSecurityPolicy(t *testing.T) { t.Parallel() diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index b721fe966f32c..74a24b444d7b3 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -534,17 +534,8 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) { session.XCSRF = csrfToken httplib.SetNoCacheHeaders(w.Header()) + httplib.SetIndexContentSecurityPolicy(w.Header(), cfg.ClusterFeatures, r.URL.Path) - // app access needs to make a CORS fetch request, so we only set the default CSP on that page - if strings.HasPrefix(r.URL.Path, "/web/launch/") { - parts := strings.Split(r.URL.Path, "/") - // grab the FQDN from the URL to allow in the connect-src CSP - applicationURL := "https://" + parts[3] + ":*" - - httplib.SetAppLaunchContentSecurityPolicy(w.Header(), applicationURL) - } 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.") } diff --git a/lib/web/app/auth.go b/lib/web/app/auth.go index 0d9670b696e56..c2f2066af4699 100644 --- a/lib/web/app/auth.go +++ b/lib/web/app/auth.go @@ -20,86 +20,142 @@ package app import ( "crypto/subtle" + "encoding/json" + "fmt" "net/http" + "time" "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/auth" "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/httplib" + "github.com/gravitational/teleport/lib/utils" ) -// 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. -func (h *Handler) handleAuth(w http.ResponseWriter, r *http.Request, p httprouter.Params) error { - httplib.SetNoCacheHeaders(w.Header()) +type fragmentRequest struct { + StateValue string `json:"state_value"` + CookieValue string `json:"cookie_value"` + SubjectCookieValue string `json:"subject_cookie_value"` +} - cookieValue := r.Header.Get("X-Cookie-Value") - if cookieValue == "" { - return trace.AccessDenied("access denied") - } +// handleFragment handles fragment authentication. Returns a Javascript +// application that reads in the fragment which submits an POST request to +// the same handler which can validate and set the cookie. +func (h *Handler) handleFragment(w http.ResponseWriter, r *http.Request, p httprouter.Params) error { + switch r.Method { + case http.MethodGet: + q := r.URL.Query() + // If the state query parameter is not set, generate a new state token, + // store it in a cookie and redirect back to the app launcher. + if q.Get("state") == "" { + stateToken, err := utils.CryptoRandomHex(auth.TokenLenBytes) + if err != nil { + h.log.WithError(err).Debugf("Failed to generate and encode random numbers.") + return trace.AccessDenied("access denied") + } + h.setAuthStateCookie(w, stateToken) + urlParams := launcherURLParams{ + clusterName: q.Get("cluster"), + publicAddr: q.Get("addr"), + awsRole: q.Get("awsrole"), + path: q.Get("path"), + stateToken: stateToken, + } + return h.redirectToLauncher(w, r, urlParams) + } - subjectCookieValue := r.Header.Get("X-Subject-Cookie-Value") - if cookieValue == "" { - return trace.BadParameter("X-Subject-Cookie-Value header missing") - } + nonce, err := utils.CryptoRandomHex(auth.TokenLenBytes) + if err != nil { + h.log.WithError(err).Debugf("Failed to generate and encode random numbers.") + return trace.AccessDenied("access denied") + } + SetRedirectPageHeaders(w.Header(), nonce) + fmt.Fprintf(w, js, nonce) + return nil - // Validate that the caller is asking for a session that exists. - ws, err := h.c.AccessPoint.GetAppSession(r.Context(), types.GetAppSessionRequest{ - SessionID: cookieValue, - }) - if err != nil { - h.log.WithError(err).Warn("Request failed: unable to get app session") - return trace.AccessDenied("access denied") - } + case http.MethodPost: + httplib.SetNoCacheHeaders(w.Header()) + var req fragmentRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + return trace.Wrap(err) + } + + // Validate that the caller-provided state token matches the stored state token. + stateCookie, err := r.Cookie(AuthStateCookieName) + if err != nil || stateCookie.Value == "" { + h.log.Warn("Request failed: state cookie is not set.") + return trace.AccessDenied("access denied") + } + if subtle.ConstantTimeCompare([]byte(req.StateValue), []byte(stateCookie.Value)) != 1 { + h.log.Warn("Request failed: state token does not match.") + return trace.AccessDenied("access denied") + } - if err := checkSubjectToken(subjectCookieValue, ws); err != nil { - h.log.Warnf("Request failed: %v.", err) + // Prevent reuse of the same state token. + h.setAuthStateCookie(w, "") - 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(), - }, + // 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.Warn("Request failed: session does not exist.") + return trace.AccessDenied("access denied") + } + if err := checkSubjectToken(req.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(), + }, + }) + return trace.AccessDenied("access denied") + } - 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 + default: + return trace.BadParameter("unsupported method %q", r.Method) } - - http.SetCookie(w, &http.Cookie{ - Name: CookieName, - Value: cookieValue, - Path: "/", - HttpOnly: true, - Secure: true, - SameSite: http.SameSiteNoneMode, - }) - - http.SetCookie(w, &http.Cookie{ - Name: SubjectCookieName, - Value: subjectCookieValue, - Path: "/", - HttpOnly: true, - Secure: true, - SameSite: http.SameSiteNoneMode, - }) - - return nil } func checkSubjectToken(subjectCookieValue string, ws types.WebSession) error { @@ -111,3 +167,15 @@ func checkSubjectToken(subjectCookieValue string, ws types.WebSession) error { } return nil } + +func (h *Handler) setAuthStateCookie(w http.ResponseWriter, value string) { + http.SetCookie(w, &http.Cookie{ + Name: AuthStateCookieName, + Value: value, + Path: "/", + HttpOnly: true, + Secure: true, + SameSite: http.SameSiteNoneMode, + Expires: h.c.Clock.Now().UTC().Add(1 * time.Minute), + }) +} diff --git a/lib/web/app/handler.go b/lib/web/app/handler.go index 25a61378a6edb..26c9d12fbd4fa 100644 --- a/lib/web/app/handler.go +++ b/lib/web/app/handler.go @@ -130,8 +130,8 @@ func NewHandler(ctx context.Context, c *HandlerConfig) (*Handler, error) { // Create the application routes. h.router = httprouter.New() h.router.UseRawPath = true - h.router.POST("/x-teleport-auth", makeRouterHandler(h.withCustomCORS(h.handleAuth))) - h.router.OPTIONS("/x-teleport-auth", makeRouterHandler(h.withCustomCORS(nil))) + h.router.POST("/x-teleport-auth", makeRouterHandler(h.handleFragment)) + h.router.GET("/x-teleport-auth", makeRouterHandler(h.handleFragment)) h.router.GET("/teleport-logout", h.withRouterAuth(h.handleLogout)) h.router.NotFound = h.withAuth(h.handleHttp) @@ -293,7 +293,7 @@ func (h *Handler) handleForwardError(w http.ResponseWriter, req *http.Request, e // done to have a consistent UX to when launching an application. session, err := h.renewSession(req) if err != nil { - if redirectErr := h.redirectToLauncher(w, req); redirectErr == nil { + if redirectErr := h.redirectToLauncher(w, req, launcherURLParams{}); redirectErr == nil { return } @@ -561,6 +561,10 @@ const ( // SubjectCookieName is the name of the application session subject cookie. SubjectCookieName = "__Host-grv_app_session_subject" + + // AuthStateCookieName is the name of the state cookie used during the + // initial authentication flow. + AuthStateCookieName = "__Host-grv_app_auth_state" ) // makeAppRedirectURL constructs a URL that will redirect the user to the diff --git a/lib/web/app/handler_test.go b/lib/web/app/handler_test.go index 4fc6b0aa63194..12d67dd698969 100644 --- a/lib/web/app/handler_test.go +++ b/lib/web/app/handler_test.go @@ -23,6 +23,7 @@ import ( "context" "crypto/tls" "crypto/x509/pkix" + "encoding/json" "fmt" "io" "net" @@ -75,13 +76,13 @@ func hasAuditEventCount(want int) eventCheckFn { // TestAuthPOST tests the handler of POST /x-teleport-auth. func TestAuthPOST(t *testing.T) { const ( - cookieValue = "5588e2be54a2834b4f152c56bafcd789f53b15477129d2ab4044e9a3c1bf0f3b" // random value we set in the header and expect to get back as a cookie + stateValue = "012ac605867e5a7d693cd6f49c7ff0fb" + cookieValue = "5588e2be54a2834b4f152c56bafcd789f53b15477129d2ab4044e9a3c1bf0f3b" ) fakeClock := clockwork.NewFakeClockAt(time.Date(2017, 05, 10, 18, 53, 0, 0, time.UTC)) clusterName := "test-cluster" - publicAddr := "proxy.goteleport.com:443" - + publicAddr := "app.example.com" // Generate CA TLS key and cert with the cluster and application DNS. key, cert, err := tlsca.GenerateSelfSignedCA( pkix.Name{CommonName: clusterName}, @@ -89,56 +90,39 @@ func TestAuthPOST(t *testing.T) { defaults.CATTL, ) require.NoError(t, err) - appSession := createAppSession(t, fakeClock, key, cert, clusterName, publicAddr) tests := []struct { - desc string - headers map[string]string - sessionError error - outStatusCode int - eventChecks []eventCheckFn - proxyAddrs []utils.NetAddr - cookieValue string - subjectCookieValue string + desc string + stateInRequest string + stateInCookie string + subjectInRequest string + sessionError error + outStatusCode int + eventChecks []eventCheckFn }{ { - desc: "success", - headers: map[string]string{ - "Origin": "https://proxy.goteleport.com", - "X-Cookie-Value": cookieValue, - "X-Subject-Cookie-Value": appSession.GetBearerToken(), - }, - outStatusCode: http.StatusOK, - eventChecks: []eventCheckFn{hasAuditEventCount(0)}, - proxyAddrs: []utils.NetAddr{ - *utils.MustParseAddr(publicAddr), - }, - cookieValue: cookieValue, - subjectCookieValue: appSession.GetBearerToken(), + desc: "success", + stateInRequest: stateValue, + stateInCookie: stateValue, + subjectInRequest: appSession.GetBearerToken(), + outStatusCode: http.StatusOK, + eventChecks: []eventCheckFn{hasAuditEventCount(0)}, }, { - desc: "success - proxy addr with custom port", - headers: map[string]string{ - "Origin": "https://proxy.goteleport.com:3080", - "X-Cookie-Value": cookieValue, - "X-Subject-Cookie-Value": appSession.GetBearerToken(), - }, - outStatusCode: http.StatusOK, - eventChecks: []eventCheckFn{hasAuditEventCount(0)}, - proxyAddrs: []utils.NetAddr{ - *utils.MustParseAddr("proxy.goteleport.com:3080"), - }, - cookieValue: cookieValue, - subjectCookieValue: appSession.GetBearerToken(), + desc: "missing state token in request", + stateInRequest: "", + stateInCookie: stateValue, + subjectInRequest: appSession.GetBearerToken(), + outStatusCode: http.StatusForbidden, + eventChecks: []eventCheckFn{hasAuditEventCount(0)}, }, { - desc: "missing subject session token in request", - headers: map[string]string{ - "Origin": "https://proxy.goteleport.com", - "X-Cookie-Value": cookieValue, - }, - outStatusCode: http.StatusForbidden, + desc: "missing subject session token in request", + stateInRequest: stateValue, + stateInCookie: stateValue, + subjectInRequest: "", + outStatusCode: http.StatusForbidden, eventChecks: []eventCheckFn{ hasAuditEventCount(1), hasAuditEvent(0, &apievents.AuthAttempt{ @@ -156,18 +140,13 @@ func TestAuthPOST(t *testing.T) { }, }), }, - proxyAddrs: []utils.NetAddr{ - *utils.MustParseAddr(publicAddr), - }, }, { - desc: "subject session token in request does not match", - headers: map[string]string{ - "Origin": "https://proxy.goteleport.com", - "X-Cookie-Value": cookieValue, - "X-Subject-Cookie-Value": "foobar", - }, - outStatusCode: http.StatusForbidden, + desc: "subject session token in request does not match", + stateInRequest: stateValue, + stateInCookie: stateValue, + subjectInRequest: "foobar", + outStatusCode: http.StatusForbidden, eventChecks: []eventCheckFn{ hasAuditEventCount(1), hasAuditEvent(0, &apievents.AuthAttempt{ @@ -185,84 +164,39 @@ func TestAuthPOST(t *testing.T) { }, }), }, - proxyAddrs: []utils.NetAddr{ - *utils.MustParseAddr(publicAddr), - }, - }, - { - desc: "invalid session", - headers: map[string]string{ - "Origin": "https://proxy.goteleport.com", - "X-Cookie-Value": "foobar", - "X-Subject-Cookie-Value": appSession.GetBearerToken(), - }, - sessionError: trace.NotFound("invalid session"), - outStatusCode: http.StatusForbidden, - eventChecks: []eventCheckFn{hasAuditEventCount(0)}, - proxyAddrs: []utils.NetAddr{ - *utils.MustParseAddr(publicAddr), - }, }, { - desc: "incorrect origin", - headers: map[string]string{ - "Origin": "https://incorrect.origin.com", - "X-Cookie-Value": "foobar", - "X-Subject-Cookie-Value": appSession.GetBearerToken(), - }, - outStatusCode: http.StatusForbidden, - eventChecks: []eventCheckFn{hasAuditEventCount(0)}, - proxyAddrs: []utils.NetAddr{ - *utils.MustParseAddr(publicAddr), - }, - }, - { - desc: "incorrect origin port", - headers: map[string]string{ - "Origin": "https://proxy.goteleport.com:3080", - "X-Cookie-Value": "foobar", - "X-Subject-Cookie-Value": appSession.GetBearerToken(), - }, - outStatusCode: http.StatusForbidden, - eventChecks: []eventCheckFn{hasAuditEventCount(0)}, - proxyAddrs: []utils.NetAddr{ - *utils.MustParseAddr(publicAddr), - }, + desc: "invalid session", + stateInRequest: stateValue, + stateInCookie: stateValue, + subjectInRequest: appSession.GetBearerToken(), + sessionError: trace.NotFound("invalid session"), + outStatusCode: http.StatusForbidden, + eventChecks: []eventCheckFn{hasAuditEventCount(0)}, }, } - for _, test := range tests { test := test t.Run(test.desc, func(t *testing.T) { t.Parallel() - authClient := &mockAuthClient{ sessionError: test.sessionError, appSession: appSession, } + p := setup(t, fakeClock, authClient, nil) - p := setup(t, fakeClock, authClient, nil, test.proxyAddrs) - - res := p.makeRequestWithHeaders(t, "/x-teleport-auth", test.headers) - - require.NoError(t, res.Body.Close()) - require.Equal(t, test.outStatusCode, res.StatusCode) - - var cookieValue string - var subjectCookieValue string - for _, cookie := range res.Cookies() { - if cookie.Name == CookieName { - cookieValue = cookie.Value - } - - if cookie.Name == SubjectCookieName { - subjectCookieValue = cookie.Value - } - } - - require.Equal(t, subjectCookieValue, test.subjectCookieValue) - require.Equal(t, cookieValue, test.cookieValue) + req, err := json.Marshal(fragmentRequest{ + StateValue: test.stateInRequest, + CookieValue: cookieValue, + SubjectCookieValue: test.subjectInRequest, + }) + require.NoError(t, err) + status, _ := p.makeRequest(t, "POST", "/x-teleport-auth", req, []http.Cookie{{ + Name: AuthStateCookieName, + Value: test.stateInCookie, + }}) + require.Equal(t, test.outStatusCode, status) for _, check := range test.eventChecks { check(t, authClient.emittedEvents) } @@ -380,7 +314,7 @@ func TestMatchApplicationServers(t *testing.T) { server.Close() }) - p := setup(t, fakeClock, authClient, tunnel, nil) + p := setup(t, fakeClock, authClient, tunnel) status, content := p.makeRequest(t, "GET", "/", []byte{}, []http.Cookie{ { Name: CookieName, @@ -502,14 +436,13 @@ type testServer struct { serverURL *url.URL } -func setup(t *testing.T, clock clockwork.FakeClock, authClient auth.ClientI, proxyClient reversetunnelclient.Tunnel, proxyPublicAddrs []utils.NetAddr) *testServer { +func setup(t *testing.T, clock clockwork.FakeClock, authClient auth.ClientI, proxyClient reversetunnelclient.Tunnel) *testServer { appHandler, err := NewHandler(context.Background(), &HandlerConfig{ - Clock: clock, - AuthClient: authClient, - AccessPoint: authClient, - ProxyClient: proxyClient, - CipherSuites: utils.DefaultCipherSuites(), - ProxyPublicAddrs: proxyPublicAddrs, + Clock: clock, + AuthClient: authClient, + AccessPoint: authClient, + ProxyClient: proxyClient, + CipherSuites: utils.DefaultCipherSuites(), }) require.NoError(t, err) @@ -796,7 +729,7 @@ func TestMakeAppRedirectURL(t *testing.T) { req, err := http.NewRequest(http.MethodGet, test.reqURL, nil) require.NoError(t, err) - urlStr := makeAppRedirectURL(req, "proxy.com", "grafana.localhost") + urlStr := makeAppRedirectURL(req, "proxy.com", "grafana.localhost", launcherURLParams{}) require.Equal(t, test.expectedURL, urlStr) }) } diff --git a/lib/web/app/middleware.go b/lib/web/app/middleware.go index e5249ea3fa728..df586ef87ea18 100644 --- a/lib/web/app/middleware.go +++ b/lib/web/app/middleware.go @@ -20,8 +20,6 @@ package app import ( "net/http" - "net/url" - "strconv" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" @@ -51,7 +49,7 @@ func (h *Handler) withAuth(handler handlerAuthFunc) http.HandlerFunc { // If the caller fails to authenticate, redirect the caller to Teleport. session, err := h.authenticate(r.Context(), r) if err != nil { - if redirectErr := h.redirectToLauncher(w, r); redirectErr == nil { + if redirectErr := h.redirectToLauncher(w, r, launcherURLParams{}); redirectErr == nil { return nil } return trace.Wrap(err) @@ -65,7 +63,7 @@ func (h *Handler) withAuth(handler handlerAuthFunc) http.HandlerFunc { // redirectToLauncher redirects to the proxy web's app launcher if the public // address of the proxy is set. -func (h *Handler) redirectToLauncher(w http.ResponseWriter, r *http.Request) error { +func (h *Handler) redirectToLauncher(w http.ResponseWriter, r *http.Request, p launcherURLParams) error { // The application launcher can only generate browser sessions (based on // Cookies). Given this, we should only redirect to it when this format is // already in use. @@ -87,58 +85,11 @@ func (h *Handler) redirectToLauncher(w http.ResponseWriter, r *http.Request) err return trace.Wrap(err) } - urlString := makeAppRedirectURL(r, h.c.WebPublicAddr, addr.Host()) + urlString := makeAppRedirectURL(r, h.c.WebPublicAddr, addr.Host(), p) http.Redirect(w, r, urlString, http.StatusFound) return nil } -func (h *Handler) withCustomCORS(handle routerFunc) routerFunc { - return func(w http.ResponseWriter, r *http.Request, p httprouter.Params) error { - // Allow minimal CORS from only the proxy origin - // This allows for requests from the proxy to `POST` to `/x-teleport-auth` and only - // permits the headers `X-Cookie-Value` and `X-Subject-Cookie-Value`. - // This is for the web UI to post a request to the application to get the proper app session - // cookie set on the right application subdomain. - w.Header().Set("Access-Control-Allow-Methods", "POST") - w.Header().Set("Access-Control-Allow-Credentials", "true") - w.Header().Set("Access-Control-Allow-Headers", "X-Cookie-Value, X-Subject-Cookie-Value") - - // Validate that the origin for the request matches any of the public proxy addresses. - // This is instead of protecting via CORS headers, as that only supports a single domain. - originValue := r.Header.Get("Origin") - origin, err := url.Parse(originValue) - if err != nil { - return trace.BadParameter("malformed Origin header: %v", err) - } - - var match bool - originPort := origin.Port() - if originPort == "" { - originPort = "443" - } - - for _, addr := range h.c.ProxyPublicAddrs { - if strconv.Itoa(addr.Port(0)) == originPort && addr.Host() == origin.Hostname() { - match = true - break - } - } - - if !match { - return trace.AccessDenied("port or hostname did not match") - } - - // As we've already checked the origin matches a public proxy address, we can allow requests from that origin - // We do this dynamically as this header can only contain one value - w.Header().Set("Access-Control-Allow-Origin", originValue) - if handle != nil { - return handle(w, r, p) - } - - return nil - } -} - // makeRouterHandler creates a httprouter.Handle. func makeRouterHandler(handler routerFunc) httprouter.Handle { return func(w http.ResponseWriter, r *http.Request, p httprouter.Params) { @@ -171,3 +122,11 @@ type routerAuthFunc func(http.ResponseWriter, *http.Request, httprouter.Params, type handlerAuthFunc func(http.ResponseWriter, *http.Request, *session) error type handlerFunc func(http.ResponseWriter, *http.Request) error + +type launcherURLParams struct { + clusterName string + publicAddr string + stateToken string + awsRole string + path string +} diff --git a/lib/web/app/redirect.go b/lib/web/app/redirect.go index 9983405e69b03..baaec4cea763b 100644 --- a/lib/web/app/redirect.go +++ b/lib/web/app/redirect.go @@ -60,3 +60,59 @@ func MetaRedirect(w http.ResponseWriter, redirectURL string) error { SetRedirectPageHeaders(w.Header(), "") return trace.Wrap(metaRedirectTemplate.Execute(w, redirectURL)) } + +const js = ` + + + + Teleport Redirection Service + + + + +` From 93fefb96ed56ac30f8aecdbb3b3384ccb330cdea Mon Sep 17 00:00:00 2001 From: Lisa Kim Date: Fri, 11 Aug 2023 08:13:38 -0700 Subject: [PATCH 02/11] Remove unused fields (were renamed while back) --- web/packages/teleport/src/services/apps/apps.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/packages/teleport/src/services/apps/apps.ts b/web/packages/teleport/src/services/apps/apps.ts index 81a61a3a7c0e6..2c6f32cdab736 100644 --- a/web/packages/teleport/src/services/apps/apps.ts +++ b/web/packages/teleport/src/services/apps/apps.ts @@ -50,8 +50,6 @@ const service = { }) .then(json => ({ fqdn: json.fqdn as string, - value: json.value as string, - subject: json.subject as string, cookieValue: json.cookie_value as string, subjectCookieValue: json.subject_cookie_value as string, })); From 4f73d527aa020c037b1aed57d2d0e9b148e2fa82 Mon Sep 17 00:00:00 2001 From: Lisa Kim Date: Fri, 11 Aug 2023 13:30:34 -0700 Subject: [PATCH 03/11] Refactor previous implementation - Create unique cookie for each state token created - Preserve path and queries --- lib/web/app/auth.go | 89 +++++++++--- lib/web/app/handler.go | 135 ++++++++++++------ lib/web/app/handler_test.go | 68 ++++++--- lib/web/app/middleware.go | 4 +- lib/web/app/redirect.go | 6 +- .../teleport/src/AppLauncher/AppLauncher.tsx | 114 +++++++++++---- 6 files changed, 306 insertions(+), 110 deletions(-) diff --git a/lib/web/app/auth.go b/lib/web/app/auth.go index c2f2066af4699..0fa4d6fb36857 100644 --- a/lib/web/app/auth.go +++ b/lib/web/app/auth.go @@ -23,7 +23,7 @@ import ( "encoding/json" "fmt" "net/http" - "time" + "strings" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" @@ -49,23 +49,51 @@ func (h *Handler) handleFragment(w http.ResponseWriter, r *http.Request, p httpr switch r.Method { case http.MethodGet: q := r.URL.Query() - // If the state query parameter is not set, generate a new state token, - // store it in a cookie and redirect back to the app launcher. + // The "state" query param is empty on the initial launch of an application. + // + // We use the "double submit cookie" technique to prevent CSRF where 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 it unaccessible and hard to tamper with. + // + // For subsequent requests, the server will expect both the "state" query param + // and the cookie (which the browser will automatically send). if q.Get("state") == "" { - stateToken, err := utils.CryptoRandomHex(auth.TokenLenBytes) + + // secretToken is the token we will look for in both the cookie + // and in the request "state" query param. + secretToken, err := utils.CryptoRandomHex(auth.TokenLenBytes) + if err != nil { + h.log.WithError(err).Debugf("Failed to generate and encode random numbers.") + 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(auth.TokenLenBytes) if err != nil { h.log.WithError(err).Debugf("Failed to generate and encode random numbers.") return trace.AccessDenied("access denied") } - h.setAuthStateCookie(w, stateToken) - urlParams := launcherURLParams{ + + h.setAuthStateCookie(w, secretToken, cookieIdentifier) + + webLauncherURLParams := launcherURLParams{ clusterName: q.Get("cluster"), publicAddr: q.Get("addr"), - awsRole: q.Get("awsrole"), + arn: q.Get("arn"), path: q.Get("path"), - stateToken: stateToken, + // 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, urlParams) + return h.redirectToLauncher(w, r, webLauncherURLParams) } nonce, err := utils.CryptoRandomHex(auth.TokenLenBytes) @@ -74,7 +102,10 @@ func (h *Handler) handleFragment(w http.ResponseWriter, r *http.Request, p httpr return trace.AccessDenied("access denied") } SetRedirectPageHeaders(w.Header(), nonce) - fmt.Fprintf(w, js, nonce) + + // Serve an empty HTML page with an inline JS. + fmt.Fprintf(w, appRedirectionJs, nonce) + return nil case http.MethodPost: @@ -84,19 +115,27 @@ func (h *Handler) handleFragment(w http.ResponseWriter, r *http.Request, p httpr return trace.Wrap(err) } + tokens := strings.Split(req.StateValue, "_") + if len(tokens) != 2 { + h.log.Warn("Request failed: request state token is not in the expected format") + return trace.AccessDenied("access denied") + } + secretToken := tokens[0] + cookieID := tokens[1] + // Validate that the caller-provided state token matches the stored state token. - stateCookie, err := r.Cookie(AuthStateCookieName) + stateCookie, err := r.Cookie(getAuthStateCookieName(cookieID)) if err != nil || stateCookie.Value == "" { h.log.Warn("Request failed: state cookie is not set.") return trace.AccessDenied("access denied") } - if subtle.ConstantTimeCompare([]byte(req.StateValue), []byte(stateCookie.Value)) != 1 { + if subtle.ConstantTimeCompare([]byte(secretToken), []byte(stateCookie.Value)) != 1 { h.log.Warn("Request failed: state token does not match.") return trace.AccessDenied("access denied") } // Prevent reuse of the same state token. - h.setAuthStateCookie(w, "") + clearAuthStateCookie(w, cookieID) // Validate that the caller is asking for a session that exists and that they have the secret // session token for. @@ -168,14 +207,30 @@ func checkSubjectToken(subjectCookieValue string, ws types.WebSession) error { return nil } -func (h *Handler) setAuthStateCookie(w http.ResponseWriter, value string) { +func (h *Handler) setAuthStateCookie(w http.ResponseWriter, cookieValue string, cookieID string) { http.SetCookie(w, &http.Cookie{ - Name: AuthStateCookieName, - Value: value, + Name: getAuthStateCookieName(cookieID), + Value: cookieValue, Path: "/", HttpOnly: true, Secure: true, SameSite: http.SameSiteNoneMode, - Expires: h.c.Clock.Now().UTC().Add(1 * time.Minute), + 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) +} diff --git a/lib/web/app/handler.go b/lib/web/app/handler.go index 26c9d12fbd4fa..6e3e2b6a43eb5 100644 --- a/lib/web/app/handler.go +++ b/lib/web/app/handler.go @@ -28,6 +28,7 @@ import ( "net" "net/http" "net/url" + "path" "github.com/gravitational/trace" "github.com/jonboulle/clockwork" @@ -130,8 +131,8 @@ func NewHandler(ctx context.Context, c *HandlerConfig) (*Handler, error) { // Create the application routes. h.router = httprouter.New() h.router.UseRawPath = true - h.router.POST("/x-teleport-auth", makeRouterHandler(h.handleFragment)) - h.router.GET("/x-teleport-auth", makeRouterHandler(h.handleFragment)) + h.router.GET("/x-teleport-auth", makeRouterHandler(h.handleFragment)) // beginAuthExchange + h.router.POST("/x-teleport-auth", makeRouterHandler(h.handleFragment)) // finishAuthExchange h.router.GET("/teleport-logout", h.withRouterAuth(h.handleLogout)) h.router.NotFound = h.withAuth(h.handleHttp) @@ -551,7 +552,7 @@ func HasName(r *http.Request, proxyPublicAddrs []utils.NetAddr) (string, bool) { // At this point, it is assumed the caller is requesting an application and // not the proxy, redirect the caller to the application launcher. - urlString := makeAppRedirectURL(r, proxyPublicAddrs[0].String(), raddr.Host()) + urlString := makeAppRedirectURL(r, proxyPublicAddrs[0].String(), raddr.Host(), launcherURLParams{}) return urlString, true } @@ -570,56 +571,98 @@ const ( // makeAppRedirectURL constructs a URL that will redirect the user to the // application launcher route in the web UI. // -// Given app URL example: some-domain.com/arbitrary/path?foo=bar&baz=qux -// The original requested URL will be separated into three parts: -// - hostname (or fqdn): some-domain.com -// - path (the URL parts after the app's hostname): arbitrary/path -// - query: foo=bar&baz=qux +// Depending on how user initially accesses the app, the URL construction +// can take on two formats: // -// which will be constructed into a redirect URL using this form: -// - /web/launch/?path=&query= +// 1: When a user uses the web UI to launch the app, the webapp can +// determine the app's clusterId, publicAddr, and its AWS role name +// (this allows a direct lookup of the app when it's time to create an +// app session) and the launcher route is created with format: +// - /web/launch//:clusterID?/:publicAddr?/:arn? +// We will need to reconstruct this exact redirect URL when initiating +// an auth exchange (with a stateToken query param). // -// where the final result for the example URL will be: -// - /web/launch/some-domain.com?path=%2Farbitrary%2Fpath&query=foo%3Dbar%26baz%3Dqux +// 2: When a user requests an app outside of web UI (eg. clicking on bookmark) +// aside from knowing the `fqdn`, the other params of the web launcher +// cannot be determined so the launcher route will be constructed as: +// - /web/launch/?path=&query= +// Often bookmarked links will have additional path and queries we will +// need to preserve. // -// The URL is formed this way to help isolate the `fqdn` param -// from the rest of the URL. +// Example Flow: // -// The original path and query cannot be formed as `web/launch/` -// because `web/launch` route can differ depending on how the user hits the app -// endpoint: -// 1. /web/launch/:fqdn/:clusterID/:publicAddr?/:arn? -// This route is formed when user clicks on the web UI's app launcher -// button from the application listing screen. The app can be directly -// resolved since we are able to determine the app's cluster name, -// public address, and AWS role name (if defined). -// 2. /web/launch/?path=&query= -// This route is formed when a user hits the app endpoint outside of -// the web UI (clicking from a link or copy/pasta link), and the app will -// have to be resolved by the fqdn. +// 1. When a user requests the app endpoint directly, we will need to redirect +// the user to the web launcher first to start the auth exchange. +// Example app endpoint: https://some-domain.com/arbitrary/path?foo=bar&baz=qux // -// Isolating the `fqdn` prevents confusing the rest of the param reserved for -// clusterId, publicAddr, and arn (where the non-query param values are used to -// create app session). The `web/launcher` will reconstruct the original -// app URL when ready to redirect the user to the requested endpoint. -func makeAppRedirectURL(r *http.Request, proxyPublicAddr, hostname string) string { - // Note that r.URL.Path field is stored in decoded form where: - // - `/%47%6f%2f` becomes `/Go/` - // - `siema%20elo` becomes `siema elo` - // And QueryEscape() will encode spaces as `+` - // - // QueryEscape is used on the `r.URL.Path` since it is being placed - // into the query part of the URL. - query := fmt.Sprintf("path=%s", url.QueryEscape(r.URL.Path)) - if len(r.URL.RawQuery) > 0 { - query = fmt.Sprintf("%s&query=%s", query, url.QueryEscape(r.URL.RawQuery)) +// The original requested URL will be separated into three parts: +// - hostname (or fqdn): some-domain.com +// - path (the URL parts after the app's hostname): arbitrary/path +// - query: foo=bar&baz=qux +// +// which will be constructed into a redirect URL using this form: +// - /web/launch/?path=&query= +// +// where the final result for the example URL will be: +// - /web/launch/some-domain.com?path=%2Farbitrary%2Fpath&query=foo%3Dbar%26baz%3Dqux +// +// 2. Building off from previous step, the web app launcher can now redirect the user +// to the apps "x-teleport-auth" endpoint to start the auth exchange: +// https://some-domain.com/x-teleport-auth?path=%2Farbitrary%2Fpath&query=foo%3Dbar%26baz%3Dqux +// +// We will need to reconstruct the same URL ^ along with the stateToken created +// by server: +// - /web/launch/some-domain.com?path=%2Farbitrary%2Fpath&query=foo%3Dbar%26baz%3Dqux&state=ABCD +// +// The URL's are formed this way to help isolate the path params reserved for the app +// launchers route, where order and existence of previous params matter for this route. +func makeAppRedirectURL(r *http.Request, proxyPublicAddr, hostname string, req launcherURLParams) string { + u := url.URL{ + Scheme: "https", + Host: proxyPublicAddr, + Path: fmt.Sprintf("/web/launch/%s", hostname), } - u := url.URL{ - Scheme: "https", - Host: proxyPublicAddr, - Path: fmt.Sprintf("/web/launch/%s", hostname), - RawQuery: query, + // Presence of a stateToken means we are beginning an app auth exchange. + if req.stateToken != "" { + u.RawQuery = fmt.Sprintf("state=%s&path=%s", url.QueryEscape(req.stateToken), url.QueryEscape(req.path)) + + urlPath := []string{"web", "launch", hostname} + + // The order and existence of previous params matter. + // + // If the user requested app through our web UI (click launch button), + // the webapp populate these fields and will be defined. + // + // If the user requested the app endpoint outside of web UI (click from link), + // these fields can't be determined and will be empty. + if req.clusterName != "" && req.publicAddr != "" { + urlPath = append(urlPath, req.clusterName, req.publicAddr) + + if req.arn != "" { + urlPath = append(urlPath, req.arn) + } + } + + u.Path = path.Join(urlPath...) + + } else { + // Hitting this case means the user has hit an endpoint directly + // and will need to be redirected to the web launcher to + // start the auth exchange. + + // Note that r.URL.Path field is stored in decoded form where: + // - `/%47%6f%2f` becomes `/Go/` + // - `siema%20elo` becomes `siema elo` + // And QueryEscape() will encode spaces as `+` + // + // QueryEscape is used on the `r.URL.Path` since it is being placed + // into the query part of the URL. + query := fmt.Sprintf("path=%s", url.QueryEscape(r.URL.Path)) + if len(r.URL.RawQuery) > 0 { + query = fmt.Sprintf("%s&query=%s", query, url.QueryEscape(r.URL.RawQuery)) + } + u.RawQuery = query } return u.String() diff --git a/lib/web/app/handler_test.go b/lib/web/app/handler_test.go index 12d67dd698969..6c172ffb232ed 100644 --- a/lib/web/app/handler_test.go +++ b/lib/web/app/handler_test.go @@ -37,7 +37,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/google/uuid" - "github.com/gravitational/trace" "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" @@ -53,6 +52,7 @@ import ( "github.com/gravitational/teleport/lib/sshutils" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/trace" ) type eventCheckFn func(t *testing.T, events []apievents.AuditEvent) @@ -75,10 +75,10 @@ func hasAuditEventCount(want int) eventCheckFn { // TestAuthPOST tests the handler of POST /x-teleport-auth. func TestAuthPOST(t *testing.T) { - const ( - stateValue = "012ac605867e5a7d693cd6f49c7ff0fb" - cookieValue = "5588e2be54a2834b4f152c56bafcd789f53b15477129d2ab4044e9a3c1bf0f3b" - ) + secretToken := "012ac605867e5a7d693cd6f49c7ff0fb" + cookieID := "cookie-name" + stateValue := fmt.Sprintf("%s_%s", secretToken, cookieID) + appCookieValue := "5588e2be54a2834b4f152c56bafcd789f53b15477129d2ab4044e9a3c1bf0f3b" fakeClock := clockwork.NewFakeClockAt(time.Date(2017, 05, 10, 18, 53, 0, 0, time.UTC)) clusterName := "test-cluster" @@ -104,7 +104,7 @@ func TestAuthPOST(t *testing.T) { { desc: "success", stateInRequest: stateValue, - stateInCookie: stateValue, + stateInCookie: secretToken, subjectInRequest: appSession.GetBearerToken(), outStatusCode: http.StatusOK, eventChecks: []eventCheckFn{hasAuditEventCount(0)}, @@ -112,7 +112,7 @@ func TestAuthPOST(t *testing.T) { { desc: "missing state token in request", stateInRequest: "", - stateInCookie: stateValue, + stateInCookie: secretToken, subjectInRequest: appSession.GetBearerToken(), outStatusCode: http.StatusForbidden, eventChecks: []eventCheckFn{hasAuditEventCount(0)}, @@ -120,7 +120,7 @@ func TestAuthPOST(t *testing.T) { { desc: "missing subject session token in request", stateInRequest: stateValue, - stateInCookie: stateValue, + stateInCookie: secretToken, subjectInRequest: "", outStatusCode: http.StatusForbidden, eventChecks: []eventCheckFn{ @@ -144,7 +144,7 @@ func TestAuthPOST(t *testing.T) { { desc: "subject session token in request does not match", stateInRequest: stateValue, - stateInCookie: stateValue, + stateInCookie: secretToken, subjectInRequest: "foobar", outStatusCode: http.StatusForbidden, eventChecks: []eventCheckFn{ @@ -168,7 +168,7 @@ func TestAuthPOST(t *testing.T) { { desc: "invalid session", stateInRequest: stateValue, - stateInCookie: stateValue, + stateInCookie: secretToken, subjectInRequest: appSession.GetBearerToken(), sessionError: trace.NotFound("invalid session"), outStatusCode: http.StatusForbidden, @@ -187,16 +187,16 @@ func TestAuthPOST(t *testing.T) { req, err := json.Marshal(fragmentRequest{ StateValue: test.stateInRequest, - CookieValue: cookieValue, + CookieValue: appCookieValue, SubjectCookieValue: test.subjectInRequest, }) require.NoError(t, err) status, _ := p.makeRequest(t, "POST", "/x-teleport-auth", req, []http.Cookie{{ - Name: AuthStateCookieName, + Name: fmt.Sprintf("%s_%s", AuthStateCookieName, cookieID), Value: test.stateInCookie, }}) - require.Equal(t, test.outStatusCode, status) + require.Equal(t, status, test.outStatusCode) for _, check := range test.eventChecks { check(t, authClient.emittedEvents) } @@ -675,10 +675,12 @@ func createAppServer(t *testing.T, publicAddr string) types.AppServer { func TestMakeAppRedirectURL(t *testing.T) { for _, test := range []struct { - name string - reqURL string - expectedURL string + name string + reqURL string + expectedURL string + launderURLParams launcherURLParams }{ + // with launcherURLParams empty (will be empty if user did not launch app from our web UI) { name: "OK - no path", reqURL: "https://grafana.localhost", @@ -724,12 +726,44 @@ func TestMakeAppRedirectURL(t *testing.T) { reqURL: "https://grafana.localhost/alerting /list?search=state:inactive type:alerting health:nodata", expectedURL: "https://proxy.com/web/launch/grafana.localhost?path=%2Falerting+%2Flist&query=search%3Dstate%3Ainactive+type%3Aalerting+health%3Anodata", }, + + // with launcherURLParams (defined if user used the "launcher" button from our web UI) + { + name: "OK - with clusterId and publicAddr", + launderURLParams: launcherURLParams{ + stateToken: "abc123", + clusterName: "im-a-cluster-name", + publicAddr: "grafana.localhost", + }, + expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost?state=abc123&path=", + }, + { + name: "OK - with clusterId, publicAddr, and arn", + launderURLParams: launcherURLParams{ + stateToken: "abc123", + clusterName: "im-a-cluster-name", + publicAddr: "grafana.localhost", + arn: "arn:aws:iam::123456789012:role%2Frole-name", + }, + expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%252Frole-name?state=abc123&path=", + }, + { + name: "OK - with clusterId, publicAddr, arn and path", + launderURLParams: launcherURLParams{ + stateToken: "abc123", + clusterName: "im-a-cluster-name", + publicAddr: "grafana.localhost", + arn: "arn:aws:iam::123456789012:role%2Frole-name", + path: "/foo/bar?qux=qex", + }, + expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%252Frole-name?state=abc123&path=%2Ffoo%2Fbar%3Fqux%3Dqex", + }, } { t.Run(test.name, func(t *testing.T) { req, err := http.NewRequest(http.MethodGet, test.reqURL, nil) require.NoError(t, err) - urlStr := makeAppRedirectURL(req, "proxy.com", "grafana.localhost", launcherURLParams{}) + urlStr := makeAppRedirectURL(req, "proxy.com", "grafana.localhost", test.launderURLParams) require.Equal(t, test.expectedURL, urlStr) }) } diff --git a/lib/web/app/middleware.go b/lib/web/app/middleware.go index df586ef87ea18..080e19e1fb1b2 100644 --- a/lib/web/app/middleware.go +++ b/lib/web/app/middleware.go @@ -67,7 +67,7 @@ func (h *Handler) redirectToLauncher(w http.ResponseWriter, r *http.Request, p l // The application launcher can only generate browser sessions (based on // Cookies). Given this, we should only redirect to it when this format is // already in use. - if !HasSession(r) { + if p.stateToken == "" && !HasSession(r) { return trace.BadParameter("redirecting to launcher when using client certificate is not valid") } @@ -127,6 +127,6 @@ type launcherURLParams struct { clusterName string publicAddr string stateToken string - awsRole string + arn string path string } diff --git a/lib/web/app/redirect.go b/lib/web/app/redirect.go index baaec4cea763b..9c0960b22e28e 100644 --- a/lib/web/app/redirect.go +++ b/lib/web/app/redirect.go @@ -49,7 +49,9 @@ func SetRedirectPageHeaders(h http.Header, nonce string) { // Set content security policy flags scriptSrc := "none" if nonce != "" { - // Should match the