diff options
Diffstat (limited to 'internal')
-rw-r--r-- | internal/artifact/artifact.go | 10 | ||||
-rw-r--r-- | internal/auth/auth.go | 106 | ||||
-rw-r--r-- | internal/auth/auth_test.go | 161 | ||||
-rw-r--r-- | internal/auth/session.go | 62 | ||||
-rw-r--r-- | internal/boring/boring.go | 18 | ||||
-rw-r--r-- | internal/boring/notboring.go | 7 | ||||
-rw-r--r-- | internal/config/config.go | 39 | ||||
-rw-r--r-- | internal/config/flags.go | 35 | ||||
-rw-r--r-- | internal/domain/domain.go | 9 | ||||
-rw-r--r-- | internal/errortracking/errortracking.go | 27 | ||||
-rw-r--r-- | internal/httperrors/httperrors.go | 5 | ||||
-rw-r--r-- | internal/serving/disk/reader.go | 5 | ||||
-rw-r--r-- | internal/vfs/serving/serving.go | 6 | ||||
-rw-r--r-- | internal/vfs/zip/vfs.go | 4 |
14 files changed, 295 insertions, 199 deletions
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 053ea940..0a4f0ab4 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -12,8 +12,7 @@ import ( "strings" "time" - "gitlab.com/gitlab-org/labkit/errortracking" - + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/httptransport" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" @@ -80,7 +79,7 @@ func (a *Artifact) makeRequest(w http.ResponseWriter, r *http.Request, reqURL *u req, err := http.NewRequestWithContext(r.Context(), "GET", reqURL.String(), nil) if err != nil { logging.LogRequest(r).WithError(err).Error(createArtifactRequestErrMsg) - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return } @@ -89,10 +88,9 @@ func (a *Artifact) makeRequest(w http.ResponseWriter, r *http.Request, reqURL *u req.Header.Add("Authorization", "Bearer "+token) } resp, err := a.client.Do(req) - if err != nil { logging.LogRequest(r).WithError(err).Error(artifactRequestErrMsg) - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve502(w) return } @@ -110,7 +108,7 @@ func (a *Artifact) makeRequest(w http.ResponseWriter, r *http.Request, reqURL *u if resp.StatusCode == http.StatusInternalServerError { logging.LogRequest(r).Error(errArtifactResponse) - errortracking.Capture(errArtifactResponse, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(errArtifactResponse, r) httperrors.Serve500(w) return } diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 98bbf49d..21398e56 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -17,10 +17,10 @@ import ( "github.com/gorilla/securecookie" "github.com/gorilla/sessions" "github.com/sirupsen/logrus" - "gitlab.com/gitlab-org/labkit/errortracking" "gitlab.com/gitlab-org/labkit/log" "golang.org/x/crypto/hkdf" + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/httptransport" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" @@ -85,41 +85,6 @@ type domain interface { ServeNotFoundAuthFailed(w http.ResponseWriter, r *http.Request) } -func (a *Auth) getSessionFromStore(r *http.Request) (*sessions.Session, error) { - session, err := a.store.Get(r, "gitlab-pages") - - if session != nil { - // Cookie just for this domain - session.Options.Path = "/" - session.Options.HttpOnly = true - session.Options.Secure = request.IsHTTPS(r) - session.Options.MaxAge = authSessionMaxAge - } - - return session, err -} - -func (a *Auth) checkSession(w http.ResponseWriter, r *http.Request) (*sessions.Session, error) { - // Create or get session - session, errsession := a.getSessionFromStore(r) - - if errsession != nil { - // Save cookie again - errsave := session.Save(r, w) - if errsave != nil { - logRequest(r).WithError(errsave).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(errsave, r) - httperrors.Serve500(w) - return nil, errsave - } - - http.Redirect(w, r, getRequestAddress(r), http.StatusFound) - return nil, errsession - } - - return session, nil -} - // TryAuthenticate tries to authenticate user and fetch access token if request is a callback to /auth? func (a *Auth) TryAuthenticate(w http.ResponseWriter, r *http.Request, domains source.Source) bool { if a == nil { @@ -159,7 +124,7 @@ func (a *Auth) TryAuthenticate(w http.ResponseWriter, r *http.Request, domains s return false } -func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.ResponseWriter, r *http.Request) { +func (a *Auth) checkAuthenticationResponse(session *hostSession, w http.ResponseWriter, r *http.Request) { if !validateState(r, session) { // State is NOT ok logRequest(r).Warn("Authentication state did not match expected") @@ -178,7 +143,7 @@ func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.Res decryptedCode, err := a.DecryptCode(r.URL.Query().Get("code"), getRequestDomain(r)) if err != nil { logRequest(r).WithError(err).Error("failed to decrypt secure code") - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return } @@ -190,12 +155,7 @@ func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.Res logRequest(r).WithError(err).WithField( "redirect_uri", redirectURI, ).Error(fetchAccessTokenErrMsg) - errortracking.Capture( - err, - errortracking.WithRequest(r), - errortracking.WithField("redirect_uri", redirectURI), - errortracking.WithStackTrace(), - ) + errortracking.CaptureErrWithReqAndStackTrace(err, r, errortracking.WithField("redirect_uri", redirectURI)) httperrors.Serve503(w) return @@ -206,7 +166,7 @@ func (a *Auth) checkAuthenticationResponse(session *sessions.Session, w http.Res err = session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return @@ -233,7 +193,7 @@ func (a *Auth) domainAllowed(ctx context.Context, name string, domains source.So return (domain != nil && err == nil) } -func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWriter, r *http.Request, domains source.Source) bool { +func (a *Auth) handleProxyingAuth(session *hostSession, w http.ResponseWriter, r *http.Request, domains source.Source) bool { // handle auth callback e.g. https://gitlab.io/auth?domain=domain&state=state if shouldProxyAuthToGitlab(r) { domain := r.URL.Query().Get("domain") @@ -242,11 +202,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit proxyurl, err := url.Parse(domain) if err != nil { logRequest(r).WithField("domain", domain).Error(queryParameterErrMsg) - errortracking.Capture(err, - errortracking.WithRequest(r), - errortracking.WithField("domain", domain), - errortracking.WithStackTrace(), - ) + errortracking.CaptureErrWithReqAndStackTrace(err, r, errortracking.WithField("domain", domain)) httperrors.Serve500(w) return true @@ -269,7 +225,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit err = session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return true @@ -300,7 +256,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit err := session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return true @@ -314,7 +270,7 @@ func (a *Auth) handleProxyingAuth(session *sessions.Session, w http.ResponseWrit signedCode, err := a.EncryptAndSignCode(proxyDomain, query.Get("code")) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve503(w) return true @@ -354,11 +310,11 @@ func shouldProxyAuthToGitlab(r *http.Request) bool { return r.URL.Query().Get("domain") != "" && r.URL.Query().Get("state") != "" } -func shouldProxyCallbackToCustomDomain(session *sessions.Session) bool { +func shouldProxyCallbackToCustomDomain(session *hostSession) bool { return session.Values["proxy_auth_domain"] != nil } -func validateState(r *http.Request, session *sessions.Session) bool { +func validateState(r *http.Request, session *hostSession) bool { state := r.URL.Query().Get("state") if state == "" { // No state param @@ -402,7 +358,6 @@ func (a *Auth) fetchAccessToken(ctx context.Context, code string) (tokenResponse // Request token resp, err := a.apiClient.Do(req) - if err != nil { return token, err } @@ -411,7 +366,7 @@ func (a *Auth) fetchAccessToken(ctx context.Context, code string) (tokenResponse if resp.StatusCode != 200 { err = errResponseNotOk - captureErrWithReqAndStackTrace(err, req) + errortracking.CaptureErrWithReqAndStackTrace(err, req) return token, err } @@ -424,7 +379,7 @@ func (a *Auth) fetchAccessToken(ctx context.Context, code string) (tokenResponse return token, nil } -func (a *Auth) checkSessionIsValid(w http.ResponseWriter, r *http.Request) *sessions.Session { +func (a *Auth) checkSessionIsValid(w http.ResponseWriter, r *http.Request) *hostSession { session, err := a.checkSession(w, r) if err != nil { return nil @@ -438,7 +393,7 @@ func (a *Auth) checkSessionIsValid(w http.ResponseWriter, r *http.Request) *sess return session } -func (a *Auth) checkTokenExists(session *sessions.Session, w http.ResponseWriter, r *http.Request) bool { +func (a *Auth) checkTokenExists(session *hostSession, w http.ResponseWriter, r *http.Request) bool { // If no access token redirect to OAuth login page if session.Values["access_token"] == nil { logRequest(r).Debug("No access token exists, redirecting user to OAuth2 login") @@ -454,7 +409,7 @@ func (a *Auth) checkTokenExists(session *sessions.Session, w http.ResponseWriter err := session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return true @@ -473,7 +428,7 @@ func (a *Auth) getProxyAddress(r *http.Request, state string) string { return fmt.Sprintf(authorizeProxyTemplate, a.redirectURI, getRequestDomain(r), state) } -func destroySession(session *sessions.Session, w http.ResponseWriter, r *http.Request) { +func destroySession(session *hostSession, w http.ResponseWriter, r *http.Request) { logRequest(r).Debug("Destroying session") // Invalidate access token and redirect back for refreshing and re-authenticating @@ -481,7 +436,7 @@ func destroySession(session *sessions.Session, w http.ResponseWriter, r *http.Re err := session.Save(r, w) if err != nil { logRequest(r).WithError(err).Error(saveSessionErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return @@ -510,10 +465,9 @@ func (a *Auth) checkAuthentication(w http.ResponseWriter, r *http.Request, domai url = fmt.Sprintf(apiURLUserTemplate, a.internalGitlabServer) } req, err := http.NewRequestWithContext(r.Context(), "GET", url, nil) - if err != nil { logRequest(r).WithError(err).Error(failAuthErrMsg) - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve500(w) return true @@ -521,10 +475,9 @@ func (a *Auth) checkAuthentication(w http.ResponseWriter, r *http.Request, domai req.Header.Add("Authorization", "Bearer "+session.Values["access_token"].(string)) resp, err := a.apiClient.Do(req) - if err != nil { logRequest(r).WithError(err).Error("Failed to retrieve info with token") - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) // call serve404 handler when auth fails domain.ServeNotFoundAuthFailed(w, r) return true @@ -543,7 +496,7 @@ func (a *Auth) checkAuthentication(w http.ResponseWriter, r *http.Request, domai "status": resp.StatusCode, "status_text": resp.Status, }).Error("Unexpected response fetching access token") - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) domain.ServeNotFoundAuthFailed(w, r) return true } @@ -591,7 +544,7 @@ func (a *Auth) CheckAuthentication(w http.ResponseWriter, r *http.Request, domai if a == nil { logRequest(r).Error(errAuthNotConfigured) - captureErrWithReqAndStackTrace(errAuthNotConfigured, r) + errortracking.CaptureErrWithReqAndStackTrace(errAuthNotConfigured, r) httperrors.Serve500(w) return true @@ -602,7 +555,8 @@ func (a *Auth) CheckAuthentication(w http.ResponseWriter, r *http.Request, domai // CheckResponseForInvalidToken checks response for invalid token and destroys session if it was invalid func (a *Auth) CheckResponseForInvalidToken(w http.ResponseWriter, r *http.Request, - resp *http.Response) bool { + resp *http.Response, +) bool { if a == nil { // No auth supported return false @@ -620,7 +574,7 @@ func (a *Auth) CheckResponseForInvalidToken(w http.ResponseWriter, r *http.Reque return false } -func checkResponseForInvalidToken(resp *http.Response, session *sessions.Session, w http.ResponseWriter, r *http.Request) bool { +func checkResponseForInvalidToken(resp *http.Response, session *hostSession, w http.ResponseWriter, r *http.Request) bool { if resp.StatusCode == http.StatusUnauthorized { errResp := errorResponse{} @@ -628,7 +582,7 @@ func checkResponseForInvalidToken(resp *http.Response, session *sessions.Session defer resp.Body.Close() err := json.NewDecoder(resp.Body).Decode(&errResp) if err != nil { - captureErrWithReqAndStackTrace(err, r) + errortracking.CaptureErrWithReqAndStackTrace(err, r) return false } @@ -671,7 +625,7 @@ func generateKeys(secret string, count int) ([][]byte, error) { } // New when authentication supported this will be used to create authentication handler -func New(pagesDomain, storeSecret, clientID, clientSecret, redirectURI, internalGitlabServer, publicGitlabServer, authScope string) (*Auth, error) { +func New(pagesDomain, storeSecret, clientID, clientSecret, redirectURI, internalGitlabServer, publicGitlabServer, authScope string, authTimeout time.Duration) (*Auth, error) { // generate 3 keys, 2 for the cookie store and 1 for JWT signing keys, err := generateKeys(storeSecret, 3) if err != nil { @@ -686,7 +640,7 @@ func New(pagesDomain, storeSecret, clientID, clientSecret, redirectURI, internal internalGitlabServer: strings.TrimRight(internalGitlabServer, "/"), publicGitlabServer: strings.TrimRight(publicGitlabServer, "/"), apiClient: &http.Client{ - Timeout: 5 * time.Second, + Timeout: authTimeout, Transport: httptransport.DefaultTransport, }, store: sessions.NewCookieStore(keys[0], keys[1]), @@ -697,7 +651,3 @@ func New(pagesDomain, storeSecret, clientID, clientSecret, redirectURI, internal now: time.Now, }, nil } - -func captureErrWithReqAndStackTrace(err error, r *http.Request) { - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) -} diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 715cb22e..586c3839 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -8,11 +8,10 @@ import ( "net/http" "net/http/httptest" "net/url" - "strings" "testing" + "time" "github.com/golang/mock/gomock" - "github.com/gorilla/sessions" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-pages/internal/request" @@ -30,7 +29,7 @@ func createTestAuth(t *testing.T, internalServer string, publicServer string) *A "http://pages.gitlab-example.com/auth", internalServer, publicServer, - "scope") + "scope", 5*time.Second) require.NoError(t, err) @@ -56,17 +55,18 @@ func (dm *domainMock) ServeNotFoundAuthFailed(w http.ResponseWriter, r *http.Req // Which leads to negative side effects: we can't test encryption, and cookie params // like max-age and secure are not being properly set // To avoid that we use fake request, and set only session cookie without copying context -func setSessionValues(t *testing.T, r *http.Request, store sessions.Store, values map[interface{}]interface{}) { +func setSessionValues(t *testing.T, r *http.Request, auth *Auth, values map[interface{}]interface{}) { t.Helper() - tmpRequest, err := http.NewRequest("GET", "/", nil) + tmpRequest, err := http.NewRequest("GET", "http://"+r.Host, nil) require.NoError(t, err) result := httptest.NewRecorder() - session, _ := store.Get(tmpRequest, "gitlab-pages") + session, _ := auth.getSessionFromStore(tmpRequest) session.Values = values - session.Save(tmpRequest, result) + err = session.Save(tmpRequest, result) + require.NoError(t, err) res := result.Result() testhelpers.Close(t, res.Body) @@ -112,16 +112,13 @@ func TestTryAuthenticateWithCodeButInvalidState(t *testing.T) { auth := createTestAuth(t, "", "") result := httptest.NewRecorder() - reqURL, err := url.Parse("/auth?code=1&state=invalid") - require.NoError(t, err) - reqURL.Scheme = request.SchemeHTTPS - r := &http.Request{URL: reqURL} - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com/auth?code=1&state=invalid", nil) require.NoError(t, err) - session.Values["state"] = "state" - session.Save(r, result) + setSessionValues(t, r, auth, map[interface{}]interface{}{ + "state": "state", + }) mockCtrl := gomock.NewController(t) @@ -134,19 +131,15 @@ func TestTryAuthenticateRemoveTokenFromRedirect(t *testing.T) { auth := createTestAuth(t, "", "") result := httptest.NewRecorder() - reqURL, err := url.Parse("/auth?code=1&state=state&token=secret") - require.NoError(t, err) - require.Equal(t, reqURL.Query().Get("token"), "secret", "token is present before redirecting") - reqURL.Scheme = request.SchemeHTTPS - r := &http.Request{URL: reqURL} - - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com/auth?code=1&state=state&token=secret", nil) + require.Equal(t, r.URL.Query().Get("token"), "secret", "token is present before redirecting") require.NoError(t, err) - session.Values["state"] = "state" - session.Values["proxy_auth_domain"] = "https://domain.com" - session.Save(r, result) + setSessionValues(t, r, auth, map[interface{}]interface{}{ + "state": "state", + "proxy_auth_domain": "https://domain.com", + }) mockCtrl := gomock.NewController(t) @@ -202,15 +195,15 @@ func testTryAuthenticateWithCodeAndState(t *testing.T, https bool) { auth := createTestAuth(t, apiServer.URL, "") - domain := apiServer.URL + host := "http://example.com" if https { - domain = strings.Replace(apiServer.URL, "http://", "https://", -1) + host = "https://example.com" } - code, err := auth.EncryptAndSignCode(domain, "1") + code, err := auth.EncryptAndSignCode(host, "1") require.NoError(t, err) - r, err := http.NewRequest("GET", "/auth?code="+code+"&state=state", nil) + r, err := http.NewRequest("GET", host+"/auth?code="+code+"&state=state", nil) require.NoError(t, err) if https { r.URL.Scheme = request.SchemeHTTPS @@ -218,9 +211,7 @@ func testTryAuthenticateWithCodeAndState(t *testing.T, https bool) { r.URL.Scheme = request.SchemeHTTP } - r.Host = strings.TrimPrefix(apiServer.URL, "http://") - - setSessionValues(t, r, auth.store, map[interface{}]interface{}{ + setSessionValues(t, r, auth, map[interface{}]interface{}{ "uri": "https://pages.gitlab-example.com/project/", "state": "state", }) @@ -268,16 +259,10 @@ func TestCheckAuthenticationWhenAccess(t *testing.T) { auth := createTestAuth(t, apiServer.URL, "") result := httptest.NewRecorder() - reqURL, err := url.Parse("/auth?code=1&state=state") - require.NoError(t, err) - reqURL.Scheme = request.SchemeHTTPS - r := &http.Request{URL: reqURL} - - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com/", nil) require.NoError(t, err) - session.Values["access_token"] = "abc" - session.Save(r, result) + setSessionValues(t, r, auth, map[interface{}]interface{}{"access_token": "abc"}) contentServed := auth.CheckAuthentication(result, r, &domainMock{projectID: 1000}) require.False(t, contentServed) @@ -305,16 +290,12 @@ func TestCheckAuthenticationWhenNoAccess(t *testing.T) { w := httptest.NewRecorder() - reqURL, err := url.Parse("/auth?code=1&state=state") - require.NoError(t, err) - reqURL.Scheme = request.SchemeHTTPS - r := &http.Request{URL: reqURL} - - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com/auth?code=1&state=state", nil) require.NoError(t, err) - session.Values["access_token"] = "abc" - session.Save(r, w) + setSessionValues(t, r, auth, map[interface{}]interface{}{ + "access_token": "abc", + }) contentServed := auth.CheckAuthentication(w, r, &domainMock{projectID: 1000, notFoundContent: "Generic 404"}) require.True(t, contentServed) @@ -328,6 +309,43 @@ func TestCheckAuthenticationWhenNoAccess(t *testing.T) { require.Equal(t, string(body), "Generic 404") } +func TestCheckAuthenticationWithSessionFromDifferentHost(t *testing.T) { + apiServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v4/projects/1000/pages_access": + require.Equal(t, "Bearer abc", r.Header.Get("Authorization")) + w.WriteHeader(http.StatusOK) + default: + t.Logf("Unexpected r.URL.RawPath: %q", r.URL.Path) + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusNotFound) + } + })) + + apiServer.Start() + defer apiServer.Close() + + auth := createTestAuth(t, apiServer.URL, "") + + result := httptest.NewRecorder() + r, err := http.NewRequest("Get", "https://different.com/", nil) + require.NoError(t, err) + setSessionValues(t, r, auth, map[interface{}]interface{}{"access_token": "abc"}) + + r, err = http.NewRequest("Get", "https://example.com/", nil) + require.NoError(t, err) + contentServed := auth.CheckAuthentication(result, r, &domainMock{projectID: 1000}) + require.True(t, contentServed) + + // should redirect to auth + require.Equal(t, http.StatusFound, result.Code) + redirectURL, err := url.Parse(result.Header().Get("Location")) + require.NoError(t, err) + require.Equal(t, "pages.gitlab-example.com", redirectURL.Host) + require.Equal(t, "/auth", redirectURL.Path) + require.Equal(t, "https://example.com", redirectURL.Query().Get("domain")) +} + func TestCheckAuthenticationWhenInvalidToken(t *testing.T) { apiServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -348,16 +366,13 @@ func TestCheckAuthenticationWhenInvalidToken(t *testing.T) { auth := createTestAuth(t, apiServer.URL, "") result := httptest.NewRecorder() - reqURL, err := url.Parse("/auth?code=1&state=state") - require.NoError(t, err) - r := &http.Request{URL: reqURL} - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com", nil) require.NoError(t, err) - session.Values["access_token"] = "abc" - err = session.Save(r, result) - require.NoError(t, err) + setSessionValues(t, r, auth, map[interface{}]interface{}{ + "access_token": "abc", + }) contentServed := auth.CheckAuthentication(result, r, &domainMock{projectID: 1000}) require.True(t, contentServed) @@ -383,16 +398,13 @@ func TestCheckAuthenticationWithoutProject(t *testing.T) { auth := createTestAuth(t, apiServer.URL, "") result := httptest.NewRecorder() - reqURL, err := url.Parse("/auth?code=1&state=state") - require.NoError(t, err) - reqURL.Scheme = request.SchemeHTTPS - r := &http.Request{URL: reqURL} - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com/auth?code=1&state=state", nil) require.NoError(t, err) - session.Values["access_token"] = "abc" - session.Save(r, result) + setSessionValues(t, r, auth, map[interface{}]interface{}{ + "access_token": "abc", + }) contentServed := auth.CheckAuthenticationWithoutProject(result, r, &domainMock{projectID: 0}) require.False(t, contentServed) @@ -419,15 +431,13 @@ func TestCheckAuthenticationWithoutProjectWhenInvalidToken(t *testing.T) { auth := createTestAuth(t, apiServer.URL, "") result := httptest.NewRecorder() - reqURL, err := url.Parse("/auth?code=1&state=state") - require.NoError(t, err) - r := &http.Request{URL: reqURL} - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com/", nil) require.NoError(t, err) - session.Values["access_token"] = "abc" - session.Save(r, result) + setSessionValues(t, r, auth, map[interface{}]interface{}{ + "access_token": "abc", + }) contentServed := auth.CheckAuthenticationWithoutProject(result, r, &domainMock{projectID: 0}) require.True(t, contentServed) @@ -452,15 +462,13 @@ func TestGetTokenIfExistsWhenTokenExists(t *testing.T) { auth := createTestAuth(t, "", "") result := httptest.NewRecorder() - reqURL, err := url.Parse("/") - require.NoError(t, err) - r := &http.Request{URL: reqURL} - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com", nil) require.NoError(t, err) - session.Values["access_token"] = "abc" - session.Save(r, result) + setSessionValues(t, r, auth, map[interface{}]interface{}{ + "access_token": "abc", + }) token, err := auth.GetTokenIfExists(result, r) require.NoError(t, err) @@ -471,14 +479,11 @@ func TestGetTokenIfExistsWhenTokenDoesNotExist(t *testing.T) { auth := createTestAuth(t, "", "") result := httptest.NewRecorder() - reqURL, err := url.Parse("http://pages.gitlab-example.com/test") - require.NoError(t, err) - r := &http.Request{URL: reqURL, Host: "pages.gitlab-example.com", RequestURI: "/test"} - session, err := auth.store.Get(r, "gitlab-pages") + r, err := http.NewRequest("Get", "https://example.com", nil) require.NoError(t, err) - session.Save(r, result) + setSessionValues(t, r, auth, map[interface{}]interface{}{}) token, err := auth.GetTokenIfExists(result, r) require.Equal(t, "", token) diff --git a/internal/auth/session.go b/internal/auth/session.go new file mode 100644 index 00000000..d6402bf9 --- /dev/null +++ b/internal/auth/session.go @@ -0,0 +1,62 @@ +package auth + +import ( + "net/http" + + "github.com/gorilla/sessions" + + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" + "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" + "gitlab.com/gitlab-org/gitlab-pages/internal/request" +) + +type hostSession struct { + *sessions.Session +} + +const sessionHostKey = "_session_host" + +func (s *hostSession) Save(r *http.Request, w http.ResponseWriter) error { + s.Session.Values[sessionHostKey] = r.Host + + return s.Session.Save(r, w) +} + +func (a *Auth) getSessionFromStore(r *http.Request) (*hostSession, error) { + session, err := a.store.Get(r, "gitlab-pages") + + if session != nil { + // Cookie just for this domain + session.Options.Path = "/" + session.Options.HttpOnly = true + session.Options.Secure = request.IsHTTPS(r) + session.Options.MaxAge = authSessionMaxAge + + if session.Values[sessionHostKey] == nil || session.Values[sessionHostKey] != r.Host { + session.Values = make(map[interface{}]interface{}) + } + } + + return &hostSession{session}, err +} + +func (a *Auth) checkSession(w http.ResponseWriter, r *http.Request) (*hostSession, error) { + // Create or get session + session, errsession := a.getSessionFromStore(r) + + if errsession != nil { + // Save cookie again + errsave := session.Save(r, w) + if errsave != nil { + logRequest(r).WithError(errsave).Error(saveSessionErrMsg) + errortracking.CaptureErrWithReqAndStackTrace(errsave, r) + httperrors.Serve500(w) + return nil, errsave + } + + http.Redirect(w, r, getRequestAddress(r), http.StatusFound) + return nil, errsession + } + + return session, nil +} diff --git a/internal/boring/boring.go b/internal/boring/boring.go new file mode 100644 index 00000000..0a59ec4a --- /dev/null +++ b/internal/boring/boring.go @@ -0,0 +1,18 @@ +//go:build boringcrypto +// +build boringcrypto + +package boring + +import ( + "crypto/boring" + + "gitlab.com/gitlab-org/labkit/log" +) + +func CheckBoring() { + if boring.Enabled() { + log.Info("FIPS mode is enabled. Using BoringSSL.") + return + } + log.Info("GitLab Pages was compiled with FIPS mode but BoringSSL is not enabled.") +} diff --git a/internal/boring/notboring.go b/internal/boring/notboring.go new file mode 100644 index 00000000..6dbf3c39 --- /dev/null +++ b/internal/boring/notboring.go @@ -0,0 +1,7 @@ +//go:build !boringcrypto +// +build !boringcrypto + +package boring + +func CheckBoring() { +} diff --git a/internal/config/config.go b/internal/config/config.go index 24a811ec..dbb88806 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -20,6 +20,7 @@ type Config struct { GitLab GitLab Log Log Sentry Sentry + Server Server TLS TLS Zip ZipServing @@ -35,15 +36,16 @@ type Config struct { // General groups settings that are general to GitLab Pages and can not // be categorized under other head. type General struct { - Domain string - MaxConns int - MaxURILength int - MetricsAddress string - RedirectHTTP bool - RootCertificate []byte - RootDir string - RootKey []byte - StatusPath string + Domain string + MaxConns int + MaxURILength int + MetricsAddress string + RedirectHTTP bool + RootCertificate []byte + RootDir string + RootKey []byte + ServerShutdownTimeout time.Duration + StatusPath string DisableCrossOriginRequests bool InsecureCiphers bool @@ -84,6 +86,7 @@ type Auth struct { ClientSecret string RedirectURI string Scope string + Timeout time.Duration } // Cache configuration for GitLab API @@ -133,6 +136,14 @@ type ZipServing struct { RefreshInterval time.Duration OpenTimeout time.Duration AllowedPaths []string + HTTPClientTimeout time.Duration +} + +type Server struct { + ReadTimeout time.Duration + ReadHeaderTimeout time.Duration + WriteTimeout time.Duration + ListenKeepAlive time.Duration } func internalGitlabServerFromFlags() string { @@ -177,6 +188,7 @@ func loadConfig() (*Config, error) { RedirectHTTP: *redirectHTTP, RootDir: *pagesRoot, StatusPath: *pagesStatus, + ServerShutdownTimeout: *serverShutdownTimeout, DisableCrossOriginRequests: *disableCrossOriginRequests, InsecureCiphers: *insecureCiphers, PropagateCorrelationID: *propagateCorrelationID, @@ -217,6 +229,7 @@ func loadConfig() (*Config, error) { ClientSecret: *clientSecret, RedirectURI: *redirectURI, Scope: *authScope, + Timeout: *authTimeout, }, Log: Log{ Format: *logFormat, @@ -236,6 +249,13 @@ func loadConfig() (*Config, error) { RefreshInterval: *zipCacheRefresh, OpenTimeout: *zipOpenTimeout, AllowedPaths: []string{*pagesRoot}, + HTTPClientTimeout: *zipHTTPClientTimeout, + }, + Server: Server{ + ReadTimeout: *serverReadTimeout, + ReadHeaderTimeout: *serverReadHeaderTimeout, + WriteTimeout: *serverWriteTimeout, + ListenKeepAlive: *serverKeepAlive, }, // Actual listener pointers will be populated in appMain. We populate the @@ -311,6 +331,7 @@ func LogConfig(config *Config) { "zip-cache-cleanup": config.Zip.CleanupInterval, "zip-cache-refresh": config.Zip.RefreshInterval, "zip-open-timeout": config.Zip.OpenTimeout, + "zip-http-client-timeout": config.Zip.HTTPClientTimeout, "rate-limit-source-ip": config.RateLimit.SourceIPLimitPerSecond, "rate-limit-source-ip-burst": config.RateLimit.SourceIPBurst, "rate-limit-domain": config.RateLimit.DomainLimitPerSecond, diff --git a/internal/config/flags.go b/internal/config/flags.go index 409ecdc7..a5d9c221 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -47,6 +47,7 @@ var ( _ = flag.Bool("daemon-enable-jail", false, "DEPRECATED and ignored, will be removed in 15.0") _ = flag.Bool("daemon-inplace-chroot", false, "DEPRECATED and ignored, will be removed in 15.0") // TODO: https://gitlab.com/gitlab-org/gitlab-pages/-/issues/599 propagateCorrelationID = flag.Bool("propagate-correlation-id", false, "Reuse existing Correlation-ID from the incoming request header `X-Request-ID` if present") + serverShutdownTimeout = flag.Duration("server-shutdown-timeout", 30*time.Second, "GitLab Pages server shutdown timeout (default: 30s)") logFormat = flag.String("log-format", "json", "The log output format: 'text' or 'json'") logVerbose = flag.Bool("log-verbose", false, "Verbose logging") secret = flag.String("auth-secret", "", "Cookie store hash key, should be at least 32 bytes long") @@ -65,19 +66,27 @@ var ( _ = flag.String("domain-config-source", "gitlab", "DEPRECATED and has not affect, see https://gitlab.com/gitlab-org/gitlab-pages/-/merge_requests/541") enableDisk = flag.Bool("enable-disk", true, "Enable disk access, shall be disabled in environments where shared disk storage isn't available") - clientID = flag.String("auth-client-id", "", "GitLab application Client ID") - clientSecret = flag.String("auth-client-secret", "", "GitLab application Client Secret") - redirectURI = flag.String("auth-redirect-uri", "", "GitLab application redirect URI") - authScope = flag.String("auth-scope", "api", "Scope to be used for authentication (must match GitLab Pages OAuth application settings)") - maxConns = flag.Int("max-conns", 0, "Limit on the number of concurrent connections to the HTTP, HTTPS or proxy listeners, 0 for no limit") - maxURILength = flag.Int("max-uri-length", 1024, "Limit the length of URI, 0 for unlimited.") - insecureCiphers = flag.Bool("insecure-ciphers", false, "Use default list of cipher suites, may contain insecure ones like 3DES and RC4") - tlsMinVersion = flag.String("tls-min-version", "tls1.2", tlsVersionFlagUsage("min")) - tlsMaxVersion = flag.String("tls-max-version", "", tlsVersionFlagUsage("max")) - zipCacheExpiration = flag.Duration("zip-cache-expiration", 60*time.Second, "Zip serving archive cache expiration interval") - zipCacheCleanup = flag.Duration("zip-cache-cleanup", 30*time.Second, "Zip serving archive cache cleanup interval") - zipCacheRefresh = flag.Duration("zip-cache-refresh", 30*time.Second, "Zip serving archive cache refresh interval") - zipOpenTimeout = flag.Duration("zip-open-timeout", 30*time.Second, "Zip archive open timeout") + clientID = flag.String("auth-client-id", "", "GitLab application Client ID") + clientSecret = flag.String("auth-client-secret", "", "GitLab application Client Secret") + redirectURI = flag.String("auth-redirect-uri", "", "GitLab application redirect URI") + authScope = flag.String("auth-scope", "api", "Scope to be used for authentication (must match GitLab Pages OAuth application settings)") + authTimeout = flag.Duration("auth-timeout", 5*time.Second, "GitLab application client timeout for authentication") + maxConns = flag.Int("max-conns", 0, "Limit on the number of concurrent connections to the HTTP, HTTPS or proxy listeners, 0 for no limit") + maxURILength = flag.Int("max-uri-length", 1024, "Limit the length of URI, 0 for unlimited.") + insecureCiphers = flag.Bool("insecure-ciphers", false, "Use default list of cipher suites, may contain insecure ones like 3DES and RC4") + tlsMinVersion = flag.String("tls-min-version", "tls1.2", tlsVersionFlagUsage("min")) + tlsMaxVersion = flag.String("tls-max-version", "", tlsVersionFlagUsage("max")) + zipCacheExpiration = flag.Duration("zip-cache-expiration", 60*time.Second, "Zip serving archive cache expiration interval") + zipCacheCleanup = flag.Duration("zip-cache-cleanup", 30*time.Second, "Zip serving archive cache cleanup interval") + zipCacheRefresh = flag.Duration("zip-cache-refresh", 30*time.Second, "Zip serving archive cache refresh interval") + zipOpenTimeout = flag.Duration("zip-open-timeout", 30*time.Second, "Zip archive open timeout") + zipHTTPClientTimeout = flag.Duration("zip-http-client-timeout", 30*time.Minute, "Zip HTTP client timeout") + + // HTTP server timeouts + serverReadTimeout = flag.Duration("server-read-timeout", 5*time.Second, "ReadTimeout is the maximum duration for reading the entire request, including the body. A zero or negative value means there will be no timeout.") + serverReadHeaderTimeout = flag.Duration("server-read-header-timeout", time.Second, "ReadHeaderTimeout is the amount of time allowed to read request headers. A zero or negative value means there will be no timeout.") + serverWriteTimeout = flag.Duration("server-write-timeout", 30*time.Second, "WriteTimeout is the maximum duration before timing out writes of the response. A zero or negative value means there will be no timeout.") + serverKeepAlive = flag.Duration("server-keep-alive", 15*time.Second, "KeepAlive specifies the keep-alive period for network connections accepted by this listener. If zero, keep-alives are enabled if supported by the protocol and operating system. If negative, keep-alives are disabled.") disableCrossOriginRequests = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests") diff --git a/internal/domain/domain.go b/internal/domain/domain.go index bf202cd3..76a1cf25 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -7,8 +7,7 @@ import ( "net/http" "sync" - "gitlab.com/gitlab-org/labkit/errortracking" - + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" ) @@ -131,7 +130,7 @@ func (d *Domain) ServeFileHTTP(w http.ResponseWriter, r *http.Request) bool { return true } - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve503(w) return true } @@ -149,7 +148,7 @@ func (d *Domain) ServeNotFoundHTTP(w http.ResponseWriter, r *http.Request) { return } - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve503(w) return } @@ -173,7 +172,7 @@ func (d *Domain) ServeNamespaceNotFound(w http.ResponseWriter, r *http.Request) return } - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) httperrors.Serve503(w) return } diff --git a/internal/errortracking/errortracking.go b/internal/errortracking/errortracking.go new file mode 100644 index 00000000..d82674cc --- /dev/null +++ b/internal/errortracking/errortracking.go @@ -0,0 +1,27 @@ +package errortracking + +import ( + "net/http" + + "gitlab.com/gitlab-org/labkit/errortracking" +) + +// CaptureOption alias to avoid importing labkit/errortracking in internal packages +type CaptureOption = errortracking.CaptureOption + +// WithField alias to avoid importing labkit/errortracking in internal packages +func WithField(key, value string) CaptureOption { + return errortracking.WithField(key, value) +} + +// CaptureErrWithReqAndStackTrace calls labkit's errortracking function and attaches the request, stack trace and any additional fields +func CaptureErrWithReqAndStackTrace(err error, r *http.Request, fields ...errortracking.CaptureOption) { + opts := append( + fields, + errortracking.WithContext(r.Context()), + errortracking.WithRequest(r), + errortracking.WithStackTrace(), + ) + + errortracking.Capture(err, opts...) +} diff --git a/internal/httperrors/httperrors.go b/internal/httperrors/httperrors.go index 7de3b6bc..d6a9a860 100644 --- a/internal/httperrors/httperrors.go +++ b/internal/httperrors/httperrors.go @@ -5,8 +5,9 @@ import ( "net/http" "gitlab.com/gitlab-org/labkit/correlation" - "gitlab.com/gitlab-org/labkit/errortracking" "gitlab.com/gitlab-org/labkit/log" + + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" ) type content struct { @@ -214,7 +215,7 @@ func Serve500WithRequest(w http.ResponseWriter, r *http.Request, reason string, "host": r.Host, "path": r.URL.Path, }).WithError(err).Error(reason) - errortracking.Capture(err, errortracking.WithRequest(r), errortracking.WithStackTrace()) + errortracking.CaptureErrWithReqAndStackTrace(err, r) serveErrorPage(w, content500) } diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index 4078d5ef..f250a0d6 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -12,8 +12,8 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" - "gitlab.com/gitlab-org/labkit/errortracking" + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/redirects" @@ -53,7 +53,8 @@ func (reader *Reader) tryRedirects(h serving.Handler) bool { if !errors.Is(err, redirects.ErrNoRedirect) { // We assume that rewrite failure is not fatal // and we only capture the error - errortracking.Capture(err, errortracking.WithRequest(h.Request), errortracking.WithStackTrace()) + + errortracking.CaptureErrWithReqAndStackTrace(err, h.Request) } return false } diff --git a/internal/vfs/serving/serving.go b/internal/vfs/serving/serving.go index 6f45e1d5..4d8b7d90 100644 --- a/internal/vfs/serving/serving.go +++ b/internal/vfs/serving/serving.go @@ -12,8 +12,7 @@ import ( "strings" "time" - "gitlab.com/gitlab-org/labkit/errortracking" - + "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" @@ -40,7 +39,8 @@ func serveContent(w http.ResponseWriter, r *http.Request, modtime time.Time, con _, haveType := w.Header()["Content-Type"] if !haveType { // this shouldn't happen - errortracking.Capture(errUnknownContentType, errortracking.WithRequest(r), errortracking.WithStackTrace()) + + errortracking.CaptureErrWithReqAndStackTrace(errUnknownContentType, r) logging.LogRequest(r).WithError(errUnknownContentType).Error("could not serve content") httperrors.Serve500(w) diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index d0608c81..3fcef556 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -67,9 +67,7 @@ func New(cfg *config.ZipServing) vfs.VFS { cacheCleanupInterval: cfg.CleanupInterval, openTimeout: cfg.OpenTimeout, httpClient: &http.Client{ - // TODO: make this timeout configurable - // https://gitlab.com/gitlab-org/gitlab-pages/-/issues/457 - Timeout: 30 * time.Minute, + Timeout: cfg.HTTPClientTimeout, Transport: httptransport.NewMeteredRoundTripper( httptransport.NewTransport(), "zip_vfs", |