diff --git a/app/controlplane/cmd/wire_gen.go b/app/controlplane/cmd/wire_gen.go index 4e2151e9e..b10e0e01c 100644 --- a/app/controlplane/cmd/wire_gen.go +++ b/app/controlplane/cmd/wire_gen.go @@ -179,8 +179,7 @@ func wireApp(contextContext context.Context, bootstrap *conf.Bootstrap, readerWr projectUseCase := biz.NewProjectsUseCase(logger, projectsRepo, membershipRepo, auditorUseCase, groupUseCase, membershipUseCase, orgInvitationUseCase, orgInvitationRepo, authzUseCase) v5 := serviceOpts(logger, authzUseCase, projectUseCase, groupUseCase) workflowService := service.NewWorkflowService(workflowUseCase, workflowContractUseCase, projectUseCase, organizationUseCase, userUseCase, v5...) - confServer := bootstrap.Server - authService, err := service.NewAuthService(userUseCase, organizationUseCase, membershipUseCase, orgInvitationUseCase, auth, confServer, auditorUseCase, v5...) + authService, err := service.NewAuthService(userUseCase, organizationUseCase, membershipUseCase, orgInvitationUseCase, auth, bootstrap, auditorUseCase, v5...) if err != nil { cleanup3() cleanup2() @@ -317,6 +316,7 @@ func wireApp(contextContext context.Context, bootstrap *conf.Bootstrap, readerWr prometheusService := service.NewPrometheusService(organizationUseCase, prometheusUseCase, v5...) groupService := service.NewGroupService(groupUseCase, v5...) projectService := service.NewProjectService(v5...) + confServer := bootstrap.Server federatedAuthentication := bootstrap.FederatedAuthentication operationAuthorizationProvider := bootstrap.OperationAuthorizationProvider validator, err := newProtoValidator() diff --git a/app/controlplane/internal/service/auth.go b/app/controlplane/internal/service/auth.go index 263258a2d..bf66ae7ce 100644 --- a/app/controlplane/internal/service/auth.go +++ b/app/controlplane/internal/service/auth.go @@ -21,9 +21,12 @@ import ( "encoding/base64" "errors" "fmt" + "net" "net/http" "net/mail" "net/url" + "slices" + "strings" "time" pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" @@ -115,16 +118,18 @@ type AuthService struct { AuthURLs *AuthURLs auditorUseCase *biz.AuditorUseCase devMode bool + // scheme://host destinations the post-login redirect may target + allowedCallbackOrigins []string } -func NewAuthService(userUC *biz.UserUseCase, orgUC *biz.OrganizationUseCase, mUC *biz.MembershipUseCase, inviteUC *biz.OrgInvitationUseCase, authConfig *conf.Auth, serverConfig *conf.Server, auc *biz.AuditorUseCase, opts ...NewOpt) (*AuthService, error) { +func NewAuthService(userUC *biz.UserUseCase, orgUC *biz.OrganizationUseCase, mUC *biz.MembershipUseCase, inviteUC *biz.OrgInvitationUseCase, authConfig *conf.Auth, bootstrapConfig *conf.Bootstrap, auc *biz.AuditorUseCase, opts ...NewOpt) (*AuthService, error) { oidcConfig := authConfig.GetOidc() if oidcConfig == nil { return nil, errors.New("oauth configuration missing") } // Craft Auth related endpoints - authURLs, err := getAuthURLs(serverConfig.GetHttp(), authConfig.GetOidc().GetLoginUrlOverride()) + authURLs, err := getAuthURLs(bootstrapConfig.GetServer().GetHttp(), authConfig.GetOidc().GetLoginUrlOverride()) if err != nil { return nil, fmt.Errorf("failed to get auth URLs: %w", err) } @@ -152,9 +157,72 @@ func NewAuthService(userUC *biz.UserUseCase, orgUC *biz.OrganizationUseCase, mUC membershipUseCase: mUC, orgInvitesUseCase: inviteUC, auditorUseCase: auc, + allowedCallbackOrigins: originsOf(authURLs.Login, bootstrapConfig.GetServer().GetHttp().GetExternalUrl(), + bootstrapConfig.GetUiDashboardUrl()), }, nil } +var errInvalidCallback = errors.New("invalid callback URL") + +// originsOf extracts the origin of the given URLs, skipping empty or malformed ones +func originsOf(urls ...string) []string { + origins := make([]string, 0, len(urls)) + for _, raw := range urls { + u, err := url.Parse(raw) + if err != nil || u.Scheme == "" || u.Host == "" { + continue + } + + origins = append(origins, originOf(u)) + } + + return origins +} + +// originOf normalizes the URL down to scheme://host. Hosts are case insensitive, so a +// config typo like "https://App.Example.com" still matches the browser-sent origin +func originOf(u *url.URL) string { + return strings.ToLower(u.Scheme + "://" + u.Host) +} + +// callbackAllowed rejects post-login redirect targets that would hand the user JWT over to +// a third party. Relative paths (CAS download redirect) and loopback (CLI login) are always +// allowed, anything else must match a known origin. +func callbackAllowed(callback string, allowedOrigins []string) error { + if callback == "" { + return nil + } + + // Browsers fold "\" into "/", so "/\evil.example" would escape a seemingly relative path + if strings.Contains(callback, `\`) { + return errInvalidCallback + } + + u, err := url.Parse(callback) + if err != nil { + return errInvalidCallback + } + + // Relative path. Both parts must be empty, otherwise "//evil.example" would pass as a path + if u.Scheme == "" && u.Host == "" { + return nil + } + + if u.Scheme != "http" && u.Scheme != "https" { + return errInvalidCallback + } + + if host := u.Hostname(); host == "localhost" || net.ParseIP(host).IsLoopback() { + return nil + } + + if slices.Contains(allowedOrigins, originOf(u)) { + return nil + } + + return fmt.Errorf("callback URL not allowed: %s", originOf(u)) +} + type AuthURLs struct { Login, callback string loginIsOverridden bool @@ -221,6 +289,13 @@ func (h oauthHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } func loginHandler(svc *AuthService, w http.ResponseWriter, r *http.Request) *oauthResp { + // The final destination where the auth token will be pushed to, i.e the CLI. + // Rejected upfront so a crafted callback never starts the OIDC dance + callback := r.URL.Query().Get(oauth.QueryParamCallback) + if err := callbackAllowed(callback, svc.allowedCallbackOrigins); err != nil { + return newOauthResp(http.StatusBadRequest, err, true) + } + b := make([]byte, 16) _, err := rand.Read(b) if err != nil { @@ -231,8 +306,7 @@ func loginHandler(svc *AuthService, w http.ResponseWriter, r *http.Request) *oau state := base64.URLEncoding.EncodeToString(b) svc.setOauthCookie(w, cookieOauthStateName, state) - // Store the final destination where the auth token will be pushed to, i.e the CLI - svc.setOauthCookie(w, cookieCallback, r.URL.Query().Get(oauth.QueryParamCallback)) + svc.setOauthCookie(w, cookieCallback, callback) // Wether the token should be short lived or not svc.setOauthCookie(w, cookieLongLived, r.URL.Query().Get(oauth.QueryParamLongLived)) @@ -362,12 +436,18 @@ func callbackHandler(svc *AuthService, w http.ResponseWriter, r *http.Request) * return newOauthResp(http.StatusOK, nil, false) } - // Redirect to the callback URL + // Redirect to the callback URL. The cookie was validated at login time, re-check it here + // so a stale or tampered cookie can't turn this into an open redirect either + if err := callbackAllowed(callbackValue, svc.allowedCallbackOrigins); err != nil { + return newOauthResp(http.StatusBadRequest, err, true) + } + callbackURL, err := crafCallbackURL(callbackValue, userToken) if err != nil { return newOauthResp(http.StatusInternalServerError, fmt.Errorf("failed to craft callback URL: %w", err), false) } + setTokenLeakHeaders(w) http.Redirect(w, r, callbackURL, http.StatusFound) return newOauthResp(http.StatusTemporaryRedirect, nil, false) } diff --git a/app/controlplane/internal/service/auth_test.go b/app/controlplane/internal/service/auth_test.go index e74d939ac..a35bc20bf 100644 --- a/app/controlplane/internal/service/auth_test.go +++ b/app/controlplane/internal/service/auth_test.go @@ -16,6 +16,8 @@ package service import ( + "net/http" + "net/http/httptest" "testing" conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1" @@ -124,3 +126,54 @@ func TestGetPreferredEmail(t *testing.T) { assert.Equal(t, tc.want, got) } } + +func TestCallbackAllowed(t *testing.T) { + // mixed case on purpose, origins are matched case insensitively + allowed := originsOf("https://app.chainloop.dev/login", "https://CP.Chainloop.dev", "https://app.chainloop.dev") + + testCases := []struct { + name string + callback string + wantErr bool + }{ + {name: "empty, token is rendered in a page", callback: ""}, + {name: "relative path, CAS download redirect", callback: "/download/sha256:deadbeef?foo=bar"}, + {name: "loopback with random port, CLI login", callback: "http://127.0.0.1:41337/auth/callback"}, + {name: "localhost with random port, CLI login", callback: "http://localhost:41337/auth/callback"}, + {name: "IPv6 loopback", callback: "http://[::1]:41337/auth/callback"}, + {name: "dashboard origin", callback: "https://app.chainloop.dev/login/callback?returnTo=%2Fprojects"}, + {name: "control plane origin, config declared it in a different case", callback: "https://cp.chainloop.dev/foo"}, + {name: "dashboard origin, browser sent a different case", callback: "https://APP.chainloop.dev/login/callback"}, + {name: "third party origin", callback: "https://evil.example/collect", wantErr: true}, + {name: "protocol relative", callback: "//evil.example/collect", wantErr: true}, + {name: "backslash escaping a relative path", callback: "/\\evil.example/collect", wantErr: true}, + {name: "non http scheme", callback: "javascript:alert(1)", wantErr: true}, + {name: "loopback lookalike host", callback: "http://localhost.evil.example/collect", wantErr: true}, + {name: "allowed host as a subdomain", callback: "https://app.chainloop.dev.evil.example/collect", wantErr: true}, + {name: "allowed host with a different scheme", callback: "http://app.chainloop.dev/collect", wantErr: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := callbackAllowed(tc.callback, allowed) + if tc.wantErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + }) + } +} + +// The callback cookie must not be set for a destination we would refuse to redirect to +func TestLoginHandlerRejectsForeignCallback(t *testing.T) { + svc := &AuthService{allowedCallbackOrigins: originsOf("https://app.chainloop.dev")} + + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/auth/login?callback=https%3A%2F%2Fevil.example%2Fcollect&long-lived=true", nil) + + resp := loginHandler(svc, w, r) + assert.Equal(t, http.StatusBadRequest, resp.code) + assert.Empty(t, w.Result().Cookies()) +} diff --git a/app/controlplane/internal/service/auth_token_page.go b/app/controlplane/internal/service/auth_token_page.go index 1e089c7d1..f25d9a52d 100644 --- a/app/controlplane/internal/service/auth_token_page.go +++ b/app/controlplane/internal/service/auth_token_page.go @@ -29,8 +29,7 @@ var tokenPageTemplate = template.Must(template.New("tokenPage").Parse(tokenPageH // leakage so the bearer token does not escape the page. func renderTokenPage(w http.ResponseWriter, token string) error { w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.Header().Set("Cache-Control", "no-store") - w.Header().Set("Referrer-Policy", "no-referrer") + setTokenLeakHeaders(w) if err := tokenPageTemplate.Execute(w, struct{ Token string }{Token: token}); err != nil { return fmt.Errorf("failed to render token page: %w", err) @@ -38,6 +37,12 @@ func renderTokenPage(w http.ResponseWriter, token string) error { return nil } +// setTokenLeakHeaders keeps a response carrying a user token out of caches and Referer headers +func setTokenLeakHeaders(w http.ResponseWriter) { + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Referrer-Policy", "no-referrer") +} + // #nosec G101 -- HTML template, not a credential const tokenPageHTML = `