diff options
author | Nick Thomas <nick@gitlab.com> | 2018-02-19 17:41:21 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-02-19 17:41:21 +0300 |
commit | 32dcea08e8f320caac7585b15ab30681a339bda6 (patch) | |
tree | 1f01749f3792def7260a22536f1ed88022aacaf1 | |
parent | a57de7adc1288ceafb3e6dcd50a3f0be1cec0028 (diff) | |
parent | bcd69d777877aed61c7c77cd7920a3e662e359d9 (diff) |
Merge branch '97-insecure-redirects' into 'master'v0.6.1
Serve a secure redirect in case of accessing /foo
See merge request gitlab/gitlab-pages!3
-rw-r--r-- | CHANGELOG | 4 | ||||
-rw-r--r-- | VERSION | 2 | ||||
-rw-r--r-- | domain.go | 111 | ||||
-rw-r--r-- | domain_test.go | 25 |
4 files changed, 84 insertions, 58 deletions
@@ -1,5 +1,7 @@ -v 0.6.0 +v 0.6.1 +- Sanitize redirects by issuing a complete URI +v 0.6.0 - Use namsral/flag to support environment vars for config !40 - Cleanup the README file !41 - Add an artifacts proxy to GitLab Pages !44 !46 @@ -1 +1 @@ -0.6.0 +0.6.1 @@ -17,6 +17,15 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/httputil" ) +type locationDirectoryError struct { + FullPath string + RelativePath string +} + +func (l *locationDirectoryError) Error() string { + return "location error accessing directory where file expected" +} + type domain struct { Group string Project string @@ -115,20 +124,45 @@ func (d *domain) serveCustomFile(w http.ResponseWriter, r *http.Request, code in return nil } -func (d *domain) resolvePath(projectName string, subPath ...string) (fullPath string, err error) { +// Resolve the HTTP request to a path on disk, converting requests for +// directories to requests for index.html inside the directory if appropriate. +func (d *domain) resolvePath(projectName string, subPath ...string) (string, error) { publicPath := filepath.Join(d.Group, projectName, "public") - fullPath = filepath.Join(publicPath, filepath.Join(subPath...)) - fullPath, err = filepath.EvalSymlinks(fullPath) + // Don't use filepath.Join as cleans the path, + // where we want to traverse full path as supplied by user + // (including ..) + testPath := publicPath + "/" + strings.Join(subPath, "/") + fullPath, err := filepath.EvalSymlinks(testPath) if err != nil { - return + return "", err } + // The requested path resolved to somewhere outside of the public/ directory if !strings.HasPrefix(fullPath, publicPath+"/") && fullPath != publicPath { - err = fmt.Errorf("%q should be in %q", fullPath, publicPath) - return + return "", fmt.Errorf("%q should be in %q", fullPath, publicPath) + } + + fi, err := os.Lstat(fullPath) + if err != nil { + return "", err + } + + // The requested path is a directory, so try index.html via recursion + if fi.IsDir() { + return "", &locationDirectoryError{ + FullPath: fullPath, + RelativePath: strings.TrimPrefix(fullPath, publicPath), + } + } + + // The file exists, but is not a supported type to serve. Perhaps a block + // special device or something else that may be a security risk. + if !fi.Mode().IsRegular() { + return "", fmt.Errorf("%s: is not a regular file", fullPath) } - return + + return fullPath, nil } func (d *domain) tryNotFound(w http.ResponseWriter, r *http.Request, projectName string) error { @@ -137,12 +171,6 @@ func (d *domain) tryNotFound(w http.ResponseWriter, r *http.Request, projectName return err } - // Make sure that file is not symlink - fi, err := os.Lstat(page404) - if err != nil && !fi.Mode().IsRegular() { - return err - } - err = d.serveCustomFile(w, r, http.StatusNotFound, page404) if err != nil { return err @@ -150,45 +178,30 @@ func (d *domain) tryNotFound(w http.ResponseWriter, r *http.Request, projectName return nil } -func (d *domain) checkPath(w http.ResponseWriter, r *http.Request, path string) (fullPath string, err error) { - fullPath = path - fi, err := os.Lstat(fullPath) - if err != nil { - return - } - - switch { - // If the URL doesn't end with /, send location to client - case fi.IsDir() && !endsWithSlash(r.URL.Path): - newURL := *r.URL - newURL.Path += "/" - http.Redirect(w, r, newURL.String(), 302) - - // If this is directory, we try the index.html - case fi.IsDir(): - fullPath = filepath.Join(fullPath, "index.html") - fi, err = os.Lstat(fullPath) - if err != nil { - return +func (d *domain) tryFile(w http.ResponseWriter, r *http.Request, projectName, pathSuffix string, subPath ...string) error { + fullPath, err := d.resolvePath(projectName, subPath...) + + if locationError, _ := err.(*locationDirectoryError); locationError != nil { + if endsWithSlash(r.URL.Path) { + fullPath, err = d.resolvePath(projectName, filepath.Join(subPath...), "index.html") + } else { + redirectPath := "//" + r.Host + "/" + if pathSuffix != "" { + redirectPath += pathSuffix + "/" + } + if locationError.RelativePath != "" { + redirectPath += strings.TrimPrefix(locationError.RelativePath, "/") + "/" + } + http.Redirect(w, r, redirectPath, 302) + return nil } - - // We don't allow to open the regular file - case !fi.Mode().IsRegular(): - err = fmt.Errorf("%s: is not a regular file", fullPath) } - return -} -func (d *domain) tryFile(w http.ResponseWriter, r *http.Request, projectName string, subPath ...string) error { - path, err := d.resolvePath(projectName, subPath...) if err != nil { return err } - path, err = d.checkPath(w, r, path) - if err != nil { - return err - } - return d.serveFile(w, r, path) + + return d.serveFile(w, r, fullPath) } func (d *domain) serveFromGroup(w http.ResponseWriter, r *http.Request) { @@ -196,12 +209,12 @@ func (d *domain) serveFromGroup(w http.ResponseWriter, r *http.Request) { split := strings.SplitN(r.URL.Path, "/", 3) // Try to serve file for http://group.example.com/subpath/... => /group/subpath/... - if len(split) >= 2 && d.tryFile(w, r, split[1], split[2:]...) == nil { + if len(split) >= 2 && d.tryFile(w, r, split[1], split[1], split[2:]...) == nil { return } // Try to serve file for http://group.example.com/... => /group/group.example.com/... - if r.Host != "" && d.tryFile(w, r, strings.ToLower(r.Host), r.URL.Path) == nil { + if r.Host != "" && d.tryFile(w, r, strings.ToLower(r.Host), "", r.URL.Path) == nil { return } @@ -221,7 +234,7 @@ func (d *domain) serveFromGroup(w http.ResponseWriter, r *http.Request) { func (d *domain) serveFromConfig(w http.ResponseWriter, r *http.Request) { // Try to serve file for http://host/... => /group/project/... - if d.tryFile(w, r, d.Project, r.URL.Path) == nil { + if d.tryFile(w, r, d.Project, "", r.URL.Path) == nil { return } diff --git a/domain_test.go b/domain_test.go index e1d5154f..26be21eb 100644 --- a/domain_test.go +++ b/domain_test.go @@ -24,18 +24,24 @@ func TestGroupServeHTTP(t *testing.T) { assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/", nil, "main-dir") assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/index.html", nil, "main-dir") - assert.True(t, assert.HTTPRedirect(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project", nil)) + assert.HTTPRedirect(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project", nil) + assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project", nil, + `<a href="//group.test.io/project/">Found</a>`) assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project/", nil, "project-subdir") assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project/index.html", nil, "project-subdir") - assert.True(t, assert.HTTPRedirect(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project/subdir", nil)) + assert.HTTPRedirect(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project/subdir", nil) + assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project/subdir", nil, + `<a href="//group.test.io/project/subdir/">Found</a>`) assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project/subdir/", nil, "project-subsubdir") assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project2/", nil, "project2-main") assert.HTTPBodyContains(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project2/index.html", nil, "project2-main") - assert.True(t, assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/symlink", nil)) - assert.True(t, assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/symlink/index.html", nil)) - assert.True(t, assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/symlink/subdir/", nil)) - assert.True(t, assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project/fifo", nil)) - assert.True(t, assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/not-existing-file", nil)) + assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io//about.gitlab.com/%2e%2e", nil) + assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/symlink", nil) + assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/symlink/index.html", nil) + assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/symlink/subdir/", nil) + assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project/fifo", nil) + assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/not-existing-file", nil) + assert.HTTPError(t, testGroup.ServeHTTP, "GET", "http://group.test.io/project//about.gitlab.com/%2e%2e", nil) } func TestDomainServeHTTP(t *testing.T) { @@ -49,9 +55,14 @@ func TestDomainServeHTTP(t *testing.T) { }, } + assert.HTTPBodyContains(t, testDomain.ServeHTTP, "GET", "/", nil, "project2-main") assert.HTTPBodyContains(t, testDomain.ServeHTTP, "GET", "/index.html", nil, "project2-main") assert.HTTPRedirect(t, testDomain.ServeHTTP, "GET", "/subdir", nil) + assert.HTTPBodyContains(t, testDomain.ServeHTTP, "GET", "/subdir", nil, + `<a href="/subdir/">Found</a>`) assert.HTTPBodyContains(t, testDomain.ServeHTTP, "GET", "/subdir/", nil, "project2-subdir") + assert.HTTPBodyContains(t, testDomain.ServeHTTP, "GET", "/subdir/index.html", nil, "project2-subdir") + assert.HTTPError(t, testDomain.ServeHTTP, "GET", "//about.gitlab.com/%2e%2e", nil) assert.HTTPError(t, testDomain.ServeHTTP, "GET", "/not-existing-file", nil) } |