diff --git a/.gitignore b/.gitignore index 909575be7233e93aa6d00a963b8bf4d56705c9eb..81041ca19fbdeb7de8f9ee2fb2ebb132429ba555 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ gitlab-workhorse testdata/data testdata/scratch +testdata/public diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7d5115a0859ed5eff23ca7f8f406c84d7318c33c..f7e1a434eb395e81a08db7380114d8892ae4ca35 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,10 +1,8 @@ before_script: - - rm -rf /usr/local/go - - apt-get update -qq - - apt-get install -y curl unzip bzip2 + - test -f /.dockerinit && apt-get update -qq && apt-get install -y curl unzip bzip2 - curl -O https://storage.googleapis.com/golang/go1.5.2.linux-amd64.tar.gz - echo 'cae87ed095e8d94a81871281d35da7829bd1234e go1.5.2.linux-amd64.tar.gz' | shasum -c - - - tar -C /usr/local -xzf go1.5.2.linux-amd64.tar.gz + - test -f /.dockerinit && rm -rf /usr/local/go && tar -C /usr/local -xzf go1.5.2.linux-amd64.tar.gz - export PATH=/usr/local/go/bin:$PATH test: diff --git a/CHANGELOG b/CHANGELOG index fb05607204f972aef59c5e33870d7b0f1f3eef6d..ccd7dcf73ae4ae1b77dee8247d7a6fa6f2b8642a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,17 @@ Formerly known as 'gitlab-git-http-server'. +0.5.3 + +Fixes merge error in 0.5.2. + +0.5.2 (broken!) + +- Always check with upstream if files in /uploads/ may be served +- Fix project%2Fnamespace API project ID's +- Prevent archive zombies when using gzip or bzip2 +- Don't show pretty error pages in development mode + 0.5.1 Deprecate -relativeURLRoot option, use -authBackend instead. @@ -50,4 +61,4 @@ This makes the REPO_ROOT command line argument obsolete. 0.2.14 -This is the last version that works with GitLab 8.0. \ No newline at end of file +This is the last version that works with GitLab 8.0. diff --git a/VERSION b/VERSION index 4b9fcbec101a6ff8ec68e0f95131ccda4861407f..be14282b7fffb9ba95d51c6546ed9816dc8f3ff8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.5.1 +0.5.3 diff --git a/internal/staticpages/servefile.go b/internal/staticpages/servefile.go index 5437b499d77ba42dc86c8b5bcf46a5eb234e1c52..53499356e91a6ce5fd569d20d1eb8b131f8f2424 100644 --- a/internal/staticpages/servefile.go +++ b/internal/staticpages/servefile.go @@ -18,6 +18,9 @@ const ( CacheExpireMax ) +// BUG/QUIRK: If a client requests 'foo%2Fbar' and 'foo/bar' exists, +// handleServeFile will serve foo/bar instead of passing the request +// upstream. func (s *Static) ServeExisting(prefix urlprefix.Prefix, cache CacheMode, notFoundHandler http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { file := filepath.Join(s.DocumentRoot, prefix.Strip(r.URL.Path)) diff --git a/internal/upstream/routes.go b/internal/upstream/routes.go index 2c0681574490fd6d50c71fb0e99bd6556463a706..1e8a9c2dabc3864ada19f9cbe5f7da86644c58b1 100644 --- a/internal/upstream/routes.go +++ b/internal/upstream/routes.go @@ -21,8 +21,9 @@ const projectPattern = `^/[^/]+/[^/]+/` const gitProjectPattern = `^/[^/]+/[^/]+\.git/` const apiPattern = `^/api/` -const projectsAPIPattern = `^/api/v3/projects/[^/]+/` +// A project ID in an API request is either a number or two strings 'namespace/project' +const projectsAPIPattern = `^/api/v3/projects/(\d+)|([^/]+/[^/]+)/` const ciAPIPattern = `^/ci/api/` // Routing table @@ -84,6 +85,12 @@ func (u *Upstream) configureRoutes() { ), }, + // For legacy reasons, user uploads are stored under the document root. + // To prevent anybody who knows/guesses the URL of a user-uploaded file + // from downloading it we make sure requests to /uploads/ do _not_ pass + // through static.ServeExisting. + route{"", regexp.MustCompile(`^/uploads/`), static.ErrorPages(u.DevelopmentMode, proxy)}, + // Serve static files or forward the requests route{"", nil, static.ServeExisting(u.URLPrefix(), staticpages.CacheDisabled, diff --git a/internal/upstream/upstream.go b/internal/upstream/upstream.go index 385efa622db0b0ce73295233204414d017b89d8f..778bedb669a10327055057c1830bd01ca2fc8037 100644 --- a/internal/upstream/upstream.go +++ b/internal/upstream/upstream.go @@ -81,7 +81,7 @@ func (u *Upstream) ServeHTTP(ow http.ResponseWriter, r *http.Request) { } // Check URL Root - URIPath := urlprefix.CleanURIPath(r.URL.EscapedPath()) + URIPath := urlprefix.CleanURIPath(r.URL.Path) prefix := u.URLPrefix() if !prefix.Match(URIPath) { httpError(&w, r, fmt.Sprintf("Not found %q", URIPath), http.StatusNotFound) diff --git a/main_test.go b/main_test.go index 6f402a31dd7f9c55d6bd89dd25f9a1ebb7588cb3..a50a3d9b23a13ec6569ac6fff4e938a6a08e02cb 100644 --- a/main_test.go +++ b/main_test.go @@ -7,6 +7,7 @@ import ( "bytes" "encoding/json" "fmt" + "io" "io/ioutil" "log" "net/http" @@ -22,6 +23,7 @@ import ( const scratchDir = "testdata/scratch" const testRepoRoot = "testdata/data" +const testDocumentRoot = "testdata/public" const testRepo = "group/test.git" const testProject = "group/test" @@ -288,6 +290,143 @@ func TestDeniedXSendfileDownload(t *testing.T) { deniedXSendfileDownload(t, contentFilename, "foo/uploads/bar") } +func TestAllowedStaticFile(t *testing.T) { + content := "PUBLIC" + if err := setupStaticFile("static file.txt", content); err != nil { + t.Fatalf("create public/static file.txt: %v", err) + } + + proxied := false + ts := helper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) { + proxied = true + w.WriteHeader(404) + }) + defer ts.Close() + ws := startWorkhorseServer(ts.URL) + defer ws.Close() + + for _, resource := range []string{ + "/static%20file.txt", + "/static file.txt", + } { + resp, err := http.Get(ws.URL + resource) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + buf := &bytes.Buffer{} + if _, err := io.Copy(buf, resp.Body); err != nil { + t.Fatal(err) + } + if buf.String() != content { + t.Fatalf("GET %q: Expected %q, got %q", resource, content, buf.String()) + } + if resp.StatusCode != 200 { + t.Fatalf("GET %q: expected 200, got %d", resource, resp.StatusCode) + } + if proxied { + t.Fatalf("GET %q: should not have made it to backend", resource) + } + } +} + +func TestAllowedPublicUploadsFile(t *testing.T) { + content := "PRIVATE but allowed" + if err := setupStaticFile("uploads/static file.txt", content); err != nil { + t.Fatalf("create public/uploads/static file.txt: %v", err) + } + + proxied := false + ts := helper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) { + proxied = true + w.Header().Add("X-Sendfile", *documentRoot+r.URL.Path) + w.WriteHeader(200) + }) + defer ts.Close() + ws := startWorkhorseServer(ts.URL) + defer ws.Close() + + for _, resource := range []string{ + "/uploads/static%20file.txt", + "/uploads/static file.txt", + } { + resp, err := http.Get(ws.URL + resource) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + buf := &bytes.Buffer{} + if _, err := io.Copy(buf, resp.Body); err != nil { + t.Fatal(err) + } + if buf.String() != content { + t.Fatalf("GET %q: Expected %q, got %q", resource, content, buf.String()) + } + if resp.StatusCode != 200 { + t.Fatalf("GET %q: expected 200, got %d", resource, resp.StatusCode) + } + if !proxied { + t.Fatalf("GET %q: never made it to backend", resource) + } + } +} + +func TestDeniedPublicUploadsFile(t *testing.T) { + content := "PRIVATE" + if err := setupStaticFile("uploads/static.txt", content); err != nil { + t.Fatalf("create public/uploads/static.txt: %v", err) + } + + proxied := false + ts := helper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, _ *http.Request) { + proxied = true + w.WriteHeader(404) + }) + defer ts.Close() + ws := startWorkhorseServer(ts.URL) + defer ws.Close() + + for _, resource := range []string{ + "/uploads/static.txt", + "/uploads%2Fstatic.txt", + } { + resp, err := http.Get(ws.URL + resource) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + buf := &bytes.Buffer{} + if _, err := io.Copy(buf, resp.Body); err != nil { + t.Fatal(err) + } + if buf.String() == content { + t.Fatalf("GET %q: Got private file contents which should have been blocked by upstream", resource) + } + if resp.StatusCode != 404 { + t.Fatalf("GET %q: expected 404, got %d", resource, resp.StatusCode) + } + if !proxied { + t.Fatalf("GET %q: never made it to backend", resource) + } + } +} + +func setupStaticFile(fpath, content string) error { + cwd, err := os.Getwd() + if err != nil { + return err + } + *documentRoot = path.Join(cwd, testDocumentRoot) + if err := os.MkdirAll(path.Join(*documentRoot, path.Dir(fpath)), 0755); err != nil { + return err + } + static_file := path.Join(*documentRoot, fpath) + if err := ioutil.WriteFile(static_file, []byte(content), 0666); err != nil { + return err + } + return nil +} + func prepareDownloadDir(t *testing.T) { if err := os.RemoveAll(scratchDir); err != nil { t.Fatal(err) @@ -335,7 +474,11 @@ func testAuthServer(url *regexp.Regexp, code int, body interface{}) *httptest.Se } func startWorkhorseServer(authBackend string) *httptest.Server { - u := &upstream.Upstream{Backend: helper.URLMustParse(authBackend), Version: "123"} + u := &upstream.Upstream{ + Backend: helper.URLMustParse(authBackend), + Version: "123", + DocumentRoot: testDocumentRoot, + } return httptest.NewServer(u) }