From 540f8393612bec8066de0674f434e2198158e7e9 Mon Sep 17 00:00:00 2001 From: Zac Bergquist Date: Fri, 13 Dec 2024 13:58:44 -0700 Subject: [PATCH] Remove ineffective CSRF check for /webconfirm (#50102) The WithAuthCookieAndCSRF checks only apply CSRF checks for state-changing (ie non-GET) requests. Since /webconfirm is always a GET request, the previous code gave the impression that CSRF tokens were validated which is not the case. No behavior change here - just being more explicit about what is being checked. There is no exploit due to not checking CSRF here due to the strict session verification performed on the confirmation token. --- lib/web/apiserver.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 7bace84d3dbcc..6381653344a89 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -938,7 +938,7 @@ func (h *Handler) bindDefaultEndpoints() { // Device Trust. // Do not enforce bearer token for /webconfirm, it is called from outside the // Web UI. - h.GET("/webapi/devices/webconfirm", h.WithAuthCookieAndCSRF(h.deviceWebConfirm)) + h.GET("/webapi/devices/webconfirm", h.WithSession(h.deviceWebConfirm)) // trusted clusters h.POST("/webapi/trustedclusters/validate", h.WithUnauthenticatedLimiter(h.validateTrustedCluster)) @@ -4698,9 +4698,22 @@ func (h *Handler) WithMetaRedirect(fn redirectHandlerFunc) httprouter.Handle { } // WithAuth ensures that a request is authenticated. +// Authenticated requests require both a session cookie as well as a bearer token. func (h *Handler) WithAuth(fn ContextHandler) httprouter.Handle { return httplib.MakeHandler(func(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) { - sctx, err := h.AuthenticateRequest(w, r, true) + sctx, err := h.AuthenticateRequest(w, r, true /* check bearer token */) + if err != nil { + return nil, trace.Wrap(err) + } + return fn(w, r, p, sctx) + }) +} + +// WithSession ensures that the request provides a session cookie. +// It does not check for a bearer token. +func (h *Handler) WithSession(fn ContextHandler) httprouter.Handle { + return httplib.MakeHandler(func(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) { + sctx, err := h.AuthenticateRequest(w, r, false /* check bearer token */) if err != nil { return nil, trace.Wrap(err) }