Skip to content

G
gitlab-ce
Commit 506ecf64 authored by Alessio Caiazza's avatar Alessio Caiazza Committed by Nick Thomas
Browse files
Options

Sign requests that goes trough direct_upload 

This commit is part of a security fix, details on the issues are
available at https://gitlab.com/gitlab-org/security/gitlab/issues/21
parent 482bee85
Hide whitespace changes
Inline Side-by-side
Showing with 82 additions and 52 deletions
CHANGELOG
View file @ 506ecf64
......@@ -2,6 +2,10 @@
Formerly known as 'gitlab-git-http-server'.
v 8.20.0
- Sign file upload requests modified by workhorse
v 8.19.0
- Use multipart uploads for nuget packages !451
......
internal/upstream/routes.go
View file @ 506ecf64
......@@ -187,8 +187,8 @@ func (u *upstream) configureRoutes() {
route("PUT", gitProjectPattern+`gitlab-lfs/objects/([0-9a-f]{64})/([0-9]+)\z`, lfs.PutStore(api, signingProxy), withMatcher(isContentType("application/octet-stream"))),
// CI Artifacts
route("POST", apiPattern+`v4/jobs/[0-9]+/artifacts\z`, contentEncodingHandler(artifacts.UploadArtifacts(api, proxy))),
route("POST", ciAPIPattern+`v1/builds/[0-9]+/artifacts\z`, contentEncodingHandler(artifacts.UploadArtifacts(api, proxy))),
route("POST", apiPattern+`v4/jobs/[0-9]+/artifacts\z`, contentEncodingHandler(artifacts.UploadArtifacts(api, signingProxy))),
route("POST", ciAPIPattern+`v1/builds/[0-9]+/artifacts\z`, contentEncodingHandler(artifacts.UploadArtifacts(api, signingProxy))),
// Terminal websocket
wsRoute(projectPattern+`-/environments/[0-9]+/terminal.ws\z`, channel.Handler(api)),
......@@ -202,13 +202,13 @@ func (u *upstream) configureRoutes() {
route("", ciAPIPattern+`v1/builds/register.json\z`, ciAPILongPolling),
// Maven Artifact Repository
route("PUT", apiPattern+`v4/projects/[0-9]+/packages/maven/`, filestore.BodyUploader(api, proxy, nil)),
route("PUT", apiPattern+`v4/projects/[0-9]+/packages/maven/`, filestore.BodyUploader(api, signingProxy, nil)),
// Conan Artifact Repository
route("PUT", apiPattern+`v4/packages/conan/`, filestore.BodyUploader(api, proxy, nil)),
route("PUT", apiPattern+`v4/packages/conan/`, filestore.BodyUploader(api, signingProxy, nil)),
// NuGet Artifact Repository
route("PUT", apiPattern+`v4/projects/[0-9]+/packages/nuget/`, upload.Accelerate(api, proxy)),
route("PUT", apiPattern+`v4/projects/[0-9]+/packages/nuget/`, upload.Accelerate(api, signingProxy)),
// We are porting API to disk acceleration
// we need to declare each routes until we have fixed all the routes on the rails codebase.
......@@ -232,9 +232,9 @@ func (u *upstream) configureRoutes() {
),
// Uploads
route("POST", projectPattern+`uploads\z`, upload.Accelerate(api, proxy)),
route("POST", snippetUploadPattern, upload.Accelerate(api, proxy)),
route("POST", userUploadPattern, upload.Accelerate(api, proxy)),
route("POST", projectPattern+`uploads\z`, upload.Accelerate(api, signingProxy)),
route("POST", snippetUploadPattern, upload.Accelerate(api, signingProxy)),
route("POST", userUploadPattern, upload.Accelerate(api, signingProxy)),
// For legacy reasons, user uploads are stored under the document root.
// To prevent anybody who knows/guesses the URL of a user-uploaded file
......
upload_test.go
View file @ 506ecf64
......@@ -42,7 +42,7 @@ func testArtifactsUpload(t *testing.T, uploadArtifacts uploadArtifactsFunction)
reqBody, contentType, err := multipartBodyWithFile()
require.NoError(t, err)
ts := uploadTestServer(t, nil)
ts := signedUploadTestServer(t, nil)
defer ts.Close()
ws := startWorkhorseServer(ts.URL)
......@@ -60,15 +60,25 @@ func TestArtifactsUpload(t *testing.T) {
testArtifactsUpload(t, uploadArtifactsV4)
}
func expectSignedRequest(t *testing.T, r *http.Request) {
t.Helper()
_, err := jwt.Parse(r.Header.Get(secret.RequestHeader), parseJWT)
require.NoError(t, err)
}
func uploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest.Server {
return testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) {
if strings.HasSuffix(r.URL.Path, "/authorize") {
expectSignedRequest(t, r)
w.Header().Set("Content-Type", api.ResponseContentType)
if _, err := fmt.Fprintf(w, `{"TempPath":"%s"}`, scratchDir); err != nil {
t.Fatal(err)
}
return
}
err := r.ParseMultipartForm(100000)
if err != nil {
t.Fatal(err)
......@@ -87,6 +97,18 @@ func uploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest.
})
}
func signedUploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest.Server {
t.Helper()
return uploadTestServer(t, func(r *http.Request) {
expectSignedRequest(t, r)
if extraTests != nil {
extraTests(r)
}
})
}
func parseJWT(token *jwt.Token) (interface{}, error) {
// Don't forget to validate the alg is what you expect:
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
......@@ -103,45 +125,53 @@ func parseJWT(token *jwt.Token) (interface{}, error) {
}
func TestAcceleratedUpload(t *testing.T) {
ts := uploadTestServer(t, func(r *http.Request) {
jwtToken, err := jwt.Parse(r.Header.Get(upload.RewrittenFieldsHeader), parseJWT)
require.NoError(t, err)
rewrittenFields := jwtToken.Claims.(jwt.MapClaims)["rewritten_fields"].(map[string]interface{})
if len(rewrittenFields) != 1 || len(rewrittenFields["file"].(string)) == 0 {
t.Fatalf("Unexpected rewritten_fields value: %v", rewrittenFields)
}
})
defer ts.Close()
ws := startWorkhorseServer(ts.URL)
defer ws.Close()
tests := []struct {
method string
resource string
method string
resource string
signedFinalization bool
}{
{"POST", `/example`},
{"POST", `/uploads/personal_snippet`},
{"POST", `/uploads/user`},
{"POST", `/api/v4/projects/1/wikis/attachments`},
{"POST", `/api/graphql`},
{"PUT", "/api/v4/projects/9001/packages/nuget/v1/files"},
{"POST", `/example`, false},
{"POST", `/uploads/personal_snippet`, true},
{"POST", `/uploads/user`, true},
{"POST", `/api/v4/projects/1/wikis/attachments`, false},
{"POST", `/api/graphql`, false},
{"PUT", "/api/v4/projects/9001/packages/nuget/v1/files", true},
}
for _, tt := range tests {
reqBody, contentType, err := multipartBodyWithFile()
require.NoError(t, err)
t.Run(tt.resource, func(t *testing.T) {
ts := uploadTestServer(t,
func(r *http.Request) {
if tt.signedFinalization {
expectSignedRequest(t, r)
}
jwtToken, err := jwt.Parse(r.Header.Get(upload.RewrittenFieldsHeader), parseJWT)
require.NoError(t, err)
rewrittenFields := jwtToken.Claims.(jwt.MapClaims)["rewritten_fields"].(map[string]interface{})
if len(rewrittenFields) != 1 || len(rewrittenFields["file"].(string)) == 0 {
t.Fatalf("Unexpected rewritten_fields value: %v", rewrittenFields)
}
})
defer ts.Close()
ws := startWorkhorseServer(ts.URL)
defer ws.Close()
reqBody, contentType, err := multipartBodyWithFile()
require.NoError(t, err)
req, err := http.NewRequest(tt.method, ws.URL+tt.resource, reqBody)
require.NoError(t, err)
req, err := http.NewRequest(tt.method, ws.URL+tt.resource, reqBody)
require.NoError(t, err)
req.Header.Set("Content-Type", contentType)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
require.Equal(t, 200, resp.StatusCode)
req.Header.Set("Content-Type", contentType)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
require.Equal(t, 200, resp.StatusCode)
resp.Body.Close()
resp.Body.Close()
})
}
}
......@@ -223,19 +253,15 @@ func TestLfsUpload(t *testing.T) {
require.Equal(t, r.Method, "PUT")
switch r.RequestURI {
case resource + "/authorize":
// Expect the authorization call to be signed
_, err := jwt.Parse(r.Header.Get(secret.RequestHeader), parseJWT)
require.NoError(t, err)
expectSignedRequest(t, r)
// Instruct workhorse to accept the upload
w.Header().Set("Content-Type", api.ResponseContentType)
_, err = fmt.Fprint(w, lfsApiResponse)
_, err := fmt.Fprint(w, lfsApiResponse)
require.NoError(t, err)
case resource:
// Expect the finalization call to be signed
_, err := jwt.Parse(r.Header.Get(secret.RequestHeader), parseJWT)
require.NoError(t, err)
expectSignedRequest(t, r)
// Expect the request to point to a file on disk containing the data
require.NoError(t, r.ParseForm())
......@@ -282,16 +308,16 @@ func packageUploadTestServer(t *testing.T, resource string, reqBody string, rspB
)
switch r.RequestURI {
case resource + "/authorize":
// Expect the authorization call to be signed
_, err := jwt.Parse(r.Header.Get(secret.RequestHeader), parseJWT)
require.NoError(t, err)
expectSignedRequest(t, r)
// Instruct workhorse to accept the upload
w.Header().Set("Content-Type", api.ResponseContentType)
_, err = fmt.Fprint(w, apiResponse)
_, err := fmt.Fprint(w, apiResponse)
require.NoError(t, err)
case resource:
expectSignedRequest(t, r)
// Expect the request to point to a file on disk containing the data
require.NoError(t, r.ParseForm())
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7