diff options
author | Alessio Caiazza <acaiazza@gitlab.com> | 2023-01-10 19:34:05 +0300 |
---|---|---|
committer | Alessio Caiazza <acaiazza@gitlab.com> | 2023-01-10 19:34:05 +0300 |
commit | 5a68ccfa1089022cff571cce034c9aed9d580cc3 (patch) | |
tree | cad9c2fb9014a393013eba21df62ee08ac175c81 | |
parent | 5a5f8844c0cea73665d6aa3b2ab351f041b14927 (diff) | |
parent | 04fb8883461815038d83b9fa42986773f2047686 (diff) |
Merge branch 'sync-canonical-with-security-changes' into 'master'
Syncing master into gitlab-pages
See merge request https://gitlab.com/gitlab-org/gitlab-pages/-/merge_requests/845
Merged-by: Alessio Caiazza <acaiazza@gitlab.com>
Approved-by: Kassio Borges <kborges@gitlab.com>
Approved-by: Alessio Caiazza <acaiazza@gitlab.com>
Co-authored-by: Mayra Cabrera <mcabrera@gitlab.com>
Co-authored-by: GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>
Co-authored-by: ngala <ngala@gitlab.com>
-rw-r--r-- | CHANGELOG.md | 18 | ||||
-rw-r--r-- | internal/auth/auth.go | 19 | ||||
-rw-r--r-- | internal/auth/auth_test.go | 15 |
3 files changed, 48 insertions, 4 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 10402252..4d8eac5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 15.7.2 (2023-01-09) + +### Security (1 change) + +- [Restrict arbitrary protocol redirection to only https or http URLs](gitlab-org/security/gitlab-pages@349102115927947edd59ce0a03d1ffba3a74947f) ([merge request](gitlab-org/security/gitlab-pages!59)) + ## 15.7.1 (2023-01-05) No changes. @@ -6,6 +12,12 @@ No changes. No changes. Same content of 1.64.0. +## 15.6.4 (2023-01-09) + +### Security (1 change) + +- [Restrict arbitrary protocol redirection to only https or http URLs](gitlab-org/security/gitlab-pages@c0da7401a044a31d1ccf754716880e3e4721453f) ([merge request](gitlab-org/security/gitlab-pages!58)) + ## 15.6.3 (2022-12-21) No changes. @@ -14,6 +26,12 @@ No changes. No changes. Same content of 1.63.0. +## 15.5.7 (2023-01-09) + +### Security (1 change) + +- [Restrict arbitrary protocol redirection to only https or http URLs](gitlab-org/security/gitlab-pages@f14d39bbaacd76d8be26b6121e732c7327cc0d4d) ([merge request](gitlab-org/security/gitlab-pages!57)) + ## 15.5.6 (2022-12-15) No changes. Same content of 1.62.0. diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 05e6c23c..8df98178 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -38,10 +38,11 @@ const ( callbackPath = "/auth" authorizeProxyTemplate = "%s?domain=%s&state=%s" - failAuthErrMsg = "failed to authenticate request" - fetchAccessTokenErrMsg = "fetching access token failed" - queryParameterErrMsg = "failed to parse domain query parameter" - saveSessionErrMsg = "failed to save the session" + failAuthErrMsg = "failed to authenticate request" + fetchAccessTokenErrMsg = "fetching access token failed" + queryParameterErrMsg = "failed to parse domain query parameter" + saveSessionErrMsg = "failed to save the session" + domainQueryParameterErrMsg = "domain query parameter only supports http/https protocol" ) var ( @@ -197,6 +198,7 @@ func (a *Auth) domainAllowed(ctx context.Context, name string, domains source.So return (domain != nil && err == nil) } +// nolint: gocyclo // TODO refactor this function https://gitlab.com/gitlab-org/gitlab-pages/-/issues/813 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) { @@ -211,6 +213,15 @@ func (a *Auth) handleProxyingAuth(session *hostSession, w http.ResponseWriter, r httperrors.Serve500(w) return true } + + // domain query param can only contain https or http URLs. + if proxyurl.Scheme != "http" && proxyurl.Scheme != "https" { + logRequest(r).WithField("domain_query", domain).Warn(domainQueryParameterErrMsg) + + httperrors.Serve401(w) + return true + } + host, _, err := net.SplitHostPort(proxyurl.Host) if err != nil { host = proxyurl.Host diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index e16c8f0b..9226f847 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -194,6 +194,21 @@ func TestCheckAuthenticationWhenStateIsAlreadySet(t *testing.T) { require.Equal(t, "given_state", session.Values["state"], "did not reuse the pre-set state") } +func TestTryAuthenticateWithNonHttpDomainAndState(t *testing.T) { + auth := createTestAuth(t, "", "") + + result := httptest.NewRecorder() + + r, err := http.NewRequest("Get", "https://example.com/auth?domain=mailto://example.com?body=TESTBODY&state=state", nil) + require.NoError(t, err) + + mockCtrl := gomock.NewController(t) + + mockSource := mock.NewMockSource(mockCtrl) + require.True(t, auth.TryAuthenticate(result, r, mockSource)) + require.Equal(t, http.StatusUnauthorized, result.Code) +} + func testTryAuthenticateWithCodeAndState(t *testing.T, https bool) { t.Helper() |