From 36c2ee4bffc8688533b2b7cf9aee7feaeaab4672 Mon Sep 17 00:00:00 2001 From: Elise Ng Date: Mon, 17 Jun 2024 21:43:40 -0400 Subject: [PATCH 1/7] Implement csrf debug cookie middleware Ref DEV-1337 --- pkg/auth/routes.go | 3 ++ pkg/auth/webapp/csrf.go | 49 +++++++++++++++++++++++- pkg/auth/webapp/csrf_debug_middleware.go | 21 ++++++++++ pkg/auth/webapp/deps.go | 1 + pkg/auth/wire_gen.go | 17 ++++++++ pkg/auth/wire_middleware.go | 7 ++++ 6 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 pkg/auth/webapp/csrf_debug_middleware.go diff --git a/pkg/auth/routes.go b/pkg/auth/routes.go index cdecb63de9..861eef5ab3 100644 --- a/pkg/auth/routes.go +++ b/pkg/auth/routes.go @@ -167,6 +167,7 @@ func NewRouter(p *deps.RootProvider, configSource *configsource.ConfigSource) *h newWebappPageChain := func(idpSessionOnly bool) httproute.Middleware { return httproute.Chain( newWebappChain(idpSessionOnly), + p.Middleware(newCSRFDebugMiddleware), p.Middleware(newCSRFMiddleware), // Turbo no longer requires us to tell the redirected location. // It can now determine redirection from the response. @@ -178,6 +179,7 @@ func NewRouter(p *deps.RootProvider, configSource *configsource.ConfigSource) *h webappPageChain := newWebappPageChain(false) webappSIWEChain := httproute.Chain( webappChain, + p.Middleware(newCSRFDebugMiddleware), p.Middleware(newCSRFMiddleware), p.Middleware(newUnsafeDynamicCSPMiddleware), ) @@ -207,6 +209,7 @@ func NewRouter(p *deps.RootProvider, configSource *configsource.ConfigSource) *h // consent page only accepts idp session webappConsentPageChain := httproute.Chain( newWebappChain(true), + p.Middleware(newCSRFDebugMiddleware), p.Middleware(newCSRFMiddleware), p.Middleware(newConsentPageDynamicCSPMiddleware), ) diff --git a/pkg/auth/webapp/csrf.go b/pkg/auth/webapp/csrf.go index 70702800cd..55dfdf6495 100644 --- a/pkg/auth/webapp/csrf.go +++ b/pkg/auth/webapp/csrf.go @@ -1,6 +1,12 @@ package webapp -import "github.com/authgear/authgear-server/pkg/lib/config" +import ( + "net/http" + + "github.com/authgear/authgear-server/pkg/lib/config" + "github.com/authgear/authgear-server/pkg/util/duration" + "github.com/authgear/authgear-server/pkg/util/httputil" +) // CSRFFieldName is the same as the default, but public. const CSRFFieldName = "gorilla.csrf.Token" @@ -21,3 +27,44 @@ func NewCSRFCookieDef(cfg *config.HTTPConfig) CSRFCookieDef { return def } + +type CSRFDebugMiddleware struct { + Cookies CookieManager +} + +var CSRFDebugCookieMaxAge = int(duration.UserInteraction.Seconds()) + +// NOTE: SameSiteDefaultMode means do not emit attribute, +// ref: https://github.com/golang/go/blob/3e10c1ff8141fae6b4d35a42e2631e7830c79830/src/net/http/cookie.go#L279 + +var CSRFDebugCookieSameSiteOmitDef = &httputil.CookieDef{ + NameSuffix: "debug_csrf_same_site_omit", + Path: "/", + AllowScriptAccess: false, + SameSite: http.SameSiteDefaultMode, + MaxAge: &CSRFDebugCookieMaxAge, +} + +var CSRFDebugCookieSameSiteNoneDef = &httputil.CookieDef{ + NameSuffix: "debug_csrf_same_site_none", + Path: "/", + AllowScriptAccess: false, + SameSite: http.SameSiteNoneMode, + MaxAge: &CSRFDebugCookieMaxAge, +} + +var CSRFDebugCookieSameSiteLaxDef = &httputil.CookieDef{ + NameSuffix: "debug_csrf_same_site_lax", + Path: "/", + AllowScriptAccess: false, + SameSite: http.SameSiteLaxMode, + MaxAge: &CSRFDebugCookieMaxAge, +} + +var CSRFDebugCookieSameSiteStrictDef = &httputil.CookieDef{ + NameSuffix: "debug_csrf_same_site_strict", + Path: "/", + AllowScriptAccess: false, + SameSite: http.SameSiteStrictMode, + MaxAge: &CSRFDebugCookieMaxAge, +} diff --git a/pkg/auth/webapp/csrf_debug_middleware.go b/pkg/auth/webapp/csrf_debug_middleware.go new file mode 100644 index 0000000000..47b970fd3d --- /dev/null +++ b/pkg/auth/webapp/csrf_debug_middleware.go @@ -0,0 +1,21 @@ +package webapp + +import ( + "net/http" + + "github.com/authgear/authgear-server/pkg/util/httputil" +) + +func (m *CSRFDebugMiddleware) Handle(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + omitCookie := m.Cookies.ValueCookie(CSRFDebugCookieSameSiteOmitDef, "exists") + httputil.UpdateCookie(w, omitCookie) + noneCookie := m.Cookies.ValueCookie(CSRFDebugCookieSameSiteNoneDef, "exists") + httputil.UpdateCookie(w, noneCookie) + laxCookie := m.Cookies.ValueCookie(CSRFDebugCookieSameSiteLaxDef, "exists") + httputil.UpdateCookie(w, laxCookie) + strictCookie := m.Cookies.ValueCookie(CSRFDebugCookieSameSiteStrictDef, "exists") + httputil.UpdateCookie(w, strictCookie) + next.ServeHTTP(w, r) + }) +} diff --git a/pkg/auth/webapp/deps.go b/pkg/auth/webapp/deps.go index fa0c658170..ee59178309 100644 --- a/pkg/auth/webapp/deps.go +++ b/pkg/auth/webapp/deps.go @@ -14,6 +14,7 @@ var DependencySet = wire.NewSet( wire.Struct(new(ErrorCookie), "*"), wire.Struct(new(CSRFMiddleware), "*"), + wire.Struct(new(CSRFDebugMiddleware), "*"), wire.Struct(new(SessionMiddleware), "*"), wire.Bind(new(SessionMiddlewareStore), new(*SessionStoreRedis)), wire.Bind(new(SessionMiddlewareSessionService), new(*Service2)), diff --git a/pkg/auth/wire_gen.go b/pkg/auth/wire_gen.go index d59655de20..a91afc8441 100644 --- a/pkg/auth/wire_gen.go +++ b/pkg/auth/wire_gen.go @@ -120507,6 +120507,23 @@ func newCSRFMiddleware(p *deps.RequestProvider) httproute.Middleware { return csrfMiddleware } +func newCSRFDebugMiddleware(p *deps.RequestProvider) httproute.Middleware { + request := p.Request + appProvider := p.AppProvider + rootProvider := appProvider.RootProvider + environmentConfig := rootProvider.EnvironmentConfig + trustProxy := environmentConfig.TrustProxy + appContext := appProvider.AppContext + config := appContext.Config + appConfig := config.AppConfig + httpConfig := appConfig.HTTP + cookieManager := deps.NewCookieManager(request, trustProxy, httpConfig) + csrfDebugMiddleware := &webapp2.CSRFDebugMiddleware{ + Cookies: cookieManager, + } + return csrfDebugMiddleware +} + func newAuthEntryPointMiddleware(p *deps.RequestProvider) httproute.Middleware { appProvider := p.AppProvider rootProvider := appProvider.RootProvider diff --git a/pkg/auth/wire_middleware.go b/pkg/auth/wire_middleware.go index 0e53a7a271..c9f217d5c4 100644 --- a/pkg/auth/wire_middleware.go +++ b/pkg/auth/wire_middleware.go @@ -92,6 +92,13 @@ func newCSRFMiddleware(p *deps.RequestProvider) httproute.Middleware { )) } +func newCSRFDebugMiddleware(p *deps.RequestProvider) httproute.Middleware { + panic(wire.Build( + DependencySet, + wire.Bind(new(httproute.Middleware), new(*webapp.CSRFDebugMiddleware)), + )) +} + func newAuthEntryPointMiddleware(p *deps.RequestProvider) httproute.Middleware { panic(wire.Build( DependencySet, From 99fa8e95080e3da5be4719a4c8363e70352d1949 Mon Sep 17 00:00:00 2001 From: Elise Ng Date: Mon, 17 Jun 2024 23:01:00 -0400 Subject: [PATCH 2/7] Log csrf errors with debug info Ref DEV-1337 --- pkg/auth/webapp/csrf_middleware.go | 40 ++++++++++++++++++++++++++++++ pkg/auth/webapp/deps.go | 1 + pkg/auth/wire_gen.go | 6 +++++ 3 files changed, 47 insertions(+) diff --git a/pkg/auth/webapp/csrf_middleware.go b/pkg/auth/webapp/csrf_middleware.go index 2986af600e..fa6078c5f4 100644 --- a/pkg/auth/webapp/csrf_middleware.go +++ b/pkg/auth/webapp/csrf_middleware.go @@ -1,19 +1,30 @@ package webapp import ( + "fmt" "net/http" "github.com/gorilla/csrf" + "github.com/sirupsen/logrus" "github.com/authgear/authgear-server/pkg/lib/config" "github.com/authgear/authgear-server/pkg/util/httputil" "github.com/authgear/authgear-server/pkg/util/jwkutil" + "github.com/authgear/authgear-server/pkg/util/log" ) +type CSRFMiddlewareLogger struct{ *log.Logger } + +func NewCSRFMiddlewareLogger(lf *log.Factory) CSRFMiddlewareLogger { + return CSRFMiddlewareLogger{lf.New("webapp-csrf-middleware")} +} + type CSRFMiddleware struct { Secret *config.CSRFKeyMaterials CookieDef CSRFCookieDef TrustProxy config.TrustProxy + Cookies CookieManager + Logger CSRFMiddlewareLogger } func (m *CSRFMiddleware) Handle(next http.Handler) http.Handler { @@ -43,6 +54,8 @@ func (m *CSRFMiddleware) Handle(next http.Handler) http.Handler { options = append(options, csrf.SameSite(0)) } + options = append(options, csrf.ErrorHandler(http.HandlerFunc(m.unauthorizedHandler))) + key, err := jwkutil.ExtractOctetKey(m.Secret.Set, "") if err != nil { panic("webapp: CSRF key not found") @@ -52,3 +65,30 @@ func (m *CSRFMiddleware) Handle(next http.Handler) http.Handler { h.ServeHTTP(w, r) }) } + +func (m *CSRFMiddleware) unauthorizedHandler(w http.ResponseWriter, r *http.Request) { + // Check debug cookies and inject info for reporting + omitCookie, err := m.Cookies.GetCookie(r, CSRFDebugCookieSameSiteOmitDef) + hasOmitCookie := (err == nil && omitCookie.Value == "exists") + + noneCookie, err := m.Cookies.GetCookie(r, CSRFDebugCookieSameSiteNoneDef) + hasNoneCookie := (err == nil && noneCookie.Value == "exists") + + laxCookie, err := m.Cookies.GetCookie(r, CSRFDebugCookieSameSiteLaxDef) + hasLaxCookie := (err == nil && laxCookie.Value == "exists") + + strictCookie, err := m.Cookies.GetCookie(r, CSRFDebugCookieSameSiteStrictDef) + hasStrictCookie := (err == nil && strictCookie.Value == "exists") + + m.Logger.WithFields(logrus.Fields{ + "hasOmitCookie": hasOmitCookie, + "hasNoneCookie": hasNoneCookie, + "hasLaxCookie": hasLaxCookie, + "hasStrictCookie": hasStrictCookie, + }).Errorf("CSRF Forbidden: %s", csrf.FailureReason(r)) + + // TODO: beautify error page ui + http.Error(w, fmt.Sprintf("%s - %s", + http.StatusText(http.StatusForbidden), csrf.FailureReason(r)), + http.StatusForbidden) +} diff --git a/pkg/auth/webapp/deps.go b/pkg/auth/webapp/deps.go index ee59178309..8a23eca69d 100644 --- a/pkg/auth/webapp/deps.go +++ b/pkg/auth/webapp/deps.go @@ -13,6 +13,7 @@ var DependencySet = wire.NewSet( NewSignedUpCookieDef, wire.Struct(new(ErrorCookie), "*"), + NewCSRFMiddlewareLogger, wire.Struct(new(CSRFMiddleware), "*"), wire.Struct(new(CSRFDebugMiddleware), "*"), wire.Struct(new(SessionMiddleware), "*"), diff --git a/pkg/auth/wire_gen.go b/pkg/auth/wire_gen.go index a91afc8441..9f32da0618 100644 --- a/pkg/auth/wire_gen.go +++ b/pkg/auth/wire_gen.go @@ -120499,10 +120499,16 @@ func newCSRFMiddleware(p *deps.RequestProvider) httproute.Middleware { rootProvider := appProvider.RootProvider environmentConfig := rootProvider.EnvironmentConfig trustProxy := environmentConfig.TrustProxy + request := p.Request + cookieManager := deps.NewCookieManager(request, trustProxy, httpConfig) + factory := appProvider.LoggerFactory + csrfMiddlewareLogger := webapp2.NewCSRFMiddlewareLogger(factory) csrfMiddleware := &webapp2.CSRFMiddleware{ Secret: csrfKeyMaterials, CookieDef: csrfCookieDef, TrustProxy: trustProxy, + Cookies: cookieManager, + Logger: csrfMiddlewareLogger, } return csrfMiddleware } From f3bde5aea87c8f66f5daab54b76784b93148d560 Mon Sep 17 00:00:00 2001 From: Elise Ng Date: Tue, 18 Jun 2024 18:47:47 -0400 Subject: [PATCH 3/7] Add request origin header for sentry report Ref DEV-1337 --- pkg/util/sentry/request.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/util/sentry/request.go b/pkg/util/sentry/request.go index 858323d772..d82dae13cd 100644 --- a/pkg/util/sentry/request.go +++ b/pkg/util/sentry/request.go @@ -7,6 +7,7 @@ import ( ) var HeaderWhiteList = []string{ + "Origin", "Referer", "User-Agent", "X-Original-For", From 72815602bb39418836223d8f4f49c459b6fe20be Mon Sep 17 00:00:00 2001 From: Elise Ng Date: Tue, 18 Jun 2024 18:49:59 -0400 Subject: [PATCH 4/7] Set age of csrf cookie to 20 mins Ref DEV-1337 --- pkg/auth/webapp/csrf_middleware.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/auth/webapp/csrf_middleware.go b/pkg/auth/webapp/csrf_middleware.go index fa6078c5f4..e982931713 100644 --- a/pkg/auth/webapp/csrf_middleware.go +++ b/pkg/auth/webapp/csrf_middleware.go @@ -8,6 +8,7 @@ import ( "github.com/sirupsen/logrus" "github.com/authgear/authgear-server/pkg/lib/config" + "github.com/authgear/authgear-server/pkg/util/duration" "github.com/authgear/authgear-server/pkg/util/httputil" "github.com/authgear/authgear-server/pkg/util/jwkutil" "github.com/authgear/authgear-server/pkg/util/log" @@ -31,6 +32,7 @@ func (m *CSRFMiddleware) Handle(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { secure := httputil.GetProto(r, bool(m.TrustProxy)) == "https" options := []csrf.Option{ + csrf.MaxAge(int(duration.UserInteraction.Seconds())), csrf.FieldName(CSRFFieldName), csrf.CookieName(m.CookieDef.Name), csrf.Path("/"), From 6514f3aad165a4479e72692928a5b925da3974f9 Mon Sep 17 00:00:00 2001 From: Elise Ng Date: Tue, 18 Jun 2024 19:12:07 -0400 Subject: [PATCH 5/7] Add csrf cookie size to sentry report Ref DEV-1337 --- pkg/auth/webapp/csrf_middleware.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/auth/webapp/csrf_middleware.go b/pkg/auth/webapp/csrf_middleware.go index e982931713..a5a92e6d44 100644 --- a/pkg/auth/webapp/csrf_middleware.go +++ b/pkg/auth/webapp/csrf_middleware.go @@ -82,11 +82,19 @@ func (m *CSRFMiddleware) unauthorizedHandler(w http.ResponseWriter, r *http.Requ strictCookie, err := m.Cookies.GetCookie(r, CSRFDebugCookieSameSiteStrictDef) hasStrictCookie := (err == nil && strictCookie.Value == "exists") + csrfCookie, _ := r.Cookie(m.CookieDef.Name) + csrfCookieSizeInBytes := 0 + if csrfCookie != nil { + // do not return value but length only for debug. + csrfCookieSizeInBytes = len([]byte(csrfCookie.Value)) + } + m.Logger.WithFields(logrus.Fields{ - "hasOmitCookie": hasOmitCookie, - "hasNoneCookie": hasNoneCookie, - "hasLaxCookie": hasLaxCookie, - "hasStrictCookie": hasStrictCookie, + "hasOmitCookie": hasOmitCookie, + "hasNoneCookie": hasNoneCookie, + "hasLaxCookie": hasLaxCookie, + "hasStrictCookie": hasStrictCookie, + "csrfCookieSizeInBytes": csrfCookieSizeInBytes, }).Errorf("CSRF Forbidden: %s", csrf.FailureReason(r)) // TODO: beautify error page ui From 9162fd7793960cb58453140d24438e96e0031220 Mon Sep 17 00:00:00 2001 From: Elise Ng Date: Wed, 19 Jun 2024 21:47:28 -0400 Subject: [PATCH 6/7] Further decode and mask csrf cookie for debug Ref DEV-1337 --- pkg/auth/webapp/csrf_middleware.go | 31 +++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/pkg/auth/webapp/csrf_middleware.go b/pkg/auth/webapp/csrf_middleware.go index a5a92e6d44..94fde1fb95 100644 --- a/pkg/auth/webapp/csrf_middleware.go +++ b/pkg/auth/webapp/csrf_middleware.go @@ -1,8 +1,10 @@ package webapp import ( + "encoding/base64" "fmt" "net/http" + "strings" "github.com/gorilla/csrf" "github.com/sirupsen/logrus" @@ -84,17 +86,36 @@ func (m *CSRFMiddleware) unauthorizedHandler(w http.ResponseWriter, r *http.Requ csrfCookie, _ := r.Cookie(m.CookieDef.Name) csrfCookieSizeInBytes := 0 + maskedCsrfCookieContent := "" if csrfCookie != nil { // do not return value but length only for debug. csrfCookieSizeInBytes = len([]byte(csrfCookie.Value)) + if data, err := base64.StdEncoding.DecodeString(csrfCookie.Value); err != nil { + csrfToken := string(data) + maskedTokenParts := make([]string, 0, 4) + for i, part := range strings.Split(csrfToken, "|") { + // token format is date|value|mac + // ref: https://github.com/gorilla/securecookie/blob/eae3c1840ec4adda88a4af683ad0f60bb690e7c2/securecookie.go#L320C30-L320C44 + // we will mask value and sig + if i == 0 { + maskedTokenParts = append(maskedTokenParts, part) + continue + } + maskedTokenParts = append(maskedTokenParts, strings.Repeat("*", len(part))) + } + maskedCsrfCookieContent = strings.Join(maskedTokenParts, "|") + } else { + maskedCsrfCookieContent = fmt.Sprintf("failed to decode: %s", err.Error()) + } } m.Logger.WithFields(logrus.Fields{ - "hasOmitCookie": hasOmitCookie, - "hasNoneCookie": hasNoneCookie, - "hasLaxCookie": hasLaxCookie, - "hasStrictCookie": hasStrictCookie, - "csrfCookieSizeInBytes": csrfCookieSizeInBytes, + "hasOmitCookie": hasOmitCookie, + "hasNoneCookie": hasNoneCookie, + "hasLaxCookie": hasLaxCookie, + "hasStrictCookie": hasStrictCookie, + "csrfCookieSizeInBytes": csrfCookieSizeInBytes, + "maskedCsrfCookieContent": maskedCsrfCookieContent, }).Errorf("CSRF Forbidden: %s", csrf.FailureReason(r)) // TODO: beautify error page ui From 36f2f94e347c528facaa9a1b6397c8b5bd76b2ee Mon Sep 17 00:00:00 2001 From: Louis Chan Date: Thu, 20 Jun 2024 12:03:01 +0800 Subject: [PATCH 7/7] Fix a lint error of using err.Error() --- pkg/auth/webapp/csrf_middleware.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/auth/webapp/csrf_middleware.go b/pkg/auth/webapp/csrf_middleware.go index 94fde1fb95..a0b93bd3ec 100644 --- a/pkg/auth/webapp/csrf_middleware.go +++ b/pkg/auth/webapp/csrf_middleware.go @@ -104,8 +104,6 @@ func (m *CSRFMiddleware) unauthorizedHandler(w http.ResponseWriter, r *http.Requ maskedTokenParts = append(maskedTokenParts, strings.Repeat("*", len(part))) } maskedCsrfCookieContent = strings.Join(maskedTokenParts, "|") - } else { - maskedCsrfCookieContent = fmt.Sprintf("failed to decode: %s", err.Error()) } }