From a12c5fc2c6d42c5b0df1a74263774cf4e2cb6ad3 Mon Sep 17 00:00:00 2001 From: cppla Date: Sat, 3 Oct 2026 05:10:46 +0800 Subject: [PATCH] fix(cover): confine static files and hide private paths --- .github/workflows/ci.yml | 18 ++ docs/DEPLOYMENT.md | 7 + docs/WEB_COVER.md | 22 ++ internal/cover/handler.go | 35 --- internal/cover/static.go | 157 ++++++++++ internal/cover/static_filesystem_test.go | 276 +++++++++++++++++ internal/cover/static_security_test.go | 293 ++++++++++++++++++ .../tunnel/web_cover_static_security_test.go | 245 +++++++++++++++ 8 files changed, 1018 insertions(+), 35 deletions(-) create mode 100644 internal/cover/static.go create mode 100644 internal/cover/static_filesystem_test.go create mode 100644 internal/cover/static_security_test.go create mode 100644 internal/tunnel/web_cover_static_security_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 492a42f..416b5aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -97,6 +97,24 @@ jobs: } Write-Output "Verified ${testName}: $passes passes, $skips skips" } + - name: Exercise static cover path policy on Windows + shell: pwsh + run: | + $staticTests = @('TestStaticHandlerServesGETAndHEAD', 'TestStaticHandlerUsesStandardErrors', 'TestStaticHandlerValidatesDirectory', 'TestStaticFileSystemPathPolicy') + $selection = '^(' + ($staticTests -join '|') + ')$' + $testOutput = & go test ./internal/cover -run $selection -count=1 -timeout=30s -json + $testExit = $LASTEXITCODE + $testOutput | Write-Output + if ($testExit -ne 0) { exit $testExit } + $testEvents = @($testOutput | ForEach-Object { $_ | ConvertFrom-Json -ErrorAction Stop }) + foreach ($testName in $staticTests) { + $passes = @($testEvents | Where-Object { $_.Test -eq $testName -and $_.Action -eq 'pass' }).Count + $skips = @($testEvents | Where-Object { $_.Test -eq $testName -and $_.Action -eq 'skip' }).Count + if ($passes -ne 1 -or $skips -ne 0) { + throw "$testName requires exactly 1 pass and 0 skips; got $passes passes and $skips skips" + } + Write-Output "Verified ${testName}: $passes passes, $skips skips" + } race: name: Race detector diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 88f5ee2..7740984 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -140,6 +140,13 @@ sudo -u autocar /usr/local/bin/autocar server \ --token-file /etc/autocar/relay-token ``` +Use a dedicated public-only site tree. Source builds after v1.0.1 reject static +links that escape `--cover-root`, absolute symbolic links, and dot-prefixed +paths; root-level `/.well-known/` remains available. Replace absolute asset +links before upgrading. These checks do not isolate bind mounts, hard links, +or public aliases to private content, so never include credentials in the +site tree. See [static-directory boundaries](WEB_COVER.md#static-directory). + For a fixed authorized origin, replace `--cover-root` with, for example, `--cover-upstream https://origin.example.net`. The two flags are mutually exclusive and exactly one is required. The reverse proxy fixes the upstream diff --git a/docs/WEB_COVER.md b/docs/WEB_COVER.md index 60705c2..7114d02 100644 --- a/docs/WEB_COVER.md +++ b/docs/WEB_COVER.md @@ -152,6 +152,28 @@ The static handler serves `GET` and `HEAD`. Other methods receive the same ordinary `405 Method Not Allowed` behavior whether they came from a random web client or from an invalid tunnel probe. +Source builds after v1.0.1 confine each static request to `--cover-root` using +Go's [traversal-resistant file API](https://go.dev/blog/osroot). Relative symbolic +links that stay inside the root continue to work; links outside the root and +absolute symbolic links (even those pointing back inside it) are not served. +Dot-prefixed path components such as `.env` and `.git` return ordinary `404` +responses and are omitted from directory listings. This policy applies to +normalized, decoded URL paths; backslash paths are rejected, and Windows +also rejects colon paths to prevent alternate-data-stream access. The root-level +`/.well-known/` directory remains public, including ACME challenge files, but +dot-prefixed entries beneath it are still hidden. Normal index pages, +directory redirects/listings, `HEAD`, and byte ranges retain standard HTTP +file-server behavior. + +The root handle is opened and closed per request, so deploying a replacement +site at the configured directory takes effect on subsequent requests without +leaving a long-lived handler-owned descriptor. This is not a filesystem sandbox: +mount points, hard links and public aliases to hidden content are not separated +by this policy. Keep the site tree public-only and the configured directory and +its parent under trusted control; +do not mount credentials or device files inside it. On upgrade, replace any +absolute site links with confined relative links or ordinary copied files. + Source builds after v1.0.1 send `OPTIONS *` through the configured cover handler on HTTP/1.1, HTTP/2, and HTTP/3, instead of letting the TCP server return a separate automatic response. Static cover therefore returns its ordinary `405` diff --git a/internal/cover/handler.go b/internal/cover/handler.go index 38d5379..cfb5b6f 100644 --- a/internal/cover/handler.go +++ b/internal/cover/handler.go @@ -12,8 +12,6 @@ import ( "net/http/httputil" "net/textproto" "net/url" - "os" - "path/filepath" "strings" "sync" ) @@ -30,39 +28,6 @@ var hopByHopHeaders = [...]string{ "Upgrade", } -// NewStaticHandler returns a handler rooted at directory. Only GET and HEAD -// are accepted; all other methods receive a normal HTTP 405 response. -func NewStaticHandler(directory string) (http.Handler, error) { - if strings.TrimSpace(directory) == "" { - return nil, errors.New("static directory is required") - } - root, err := filepath.Abs(directory) - if err != nil { - return nil, errors.New("resolve static directory") - } - info, err := os.Stat(root) - if err != nil { - return nil, errors.New("open static directory") - } - if !info.IsDir() { - return nil, errors.New("static path is not a directory") - } - return &staticHandler{files: http.FileServer(http.Dir(root))}, nil -} - -type staticHandler struct { - files http.Handler -} - -func (h *staticHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet && r.Method != http.MethodHead { - w.Header().Set("Allow", "GET, HEAD") - http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) - return - } - h.files.ServeHTTP(w, r) -} - // NewReverseProxyHandler returns a reverse proxy that can dial only origin. // The requester controls the path, query, and ordinary end-to-end headers, but // never the upstream scheme, authority, or Host header. diff --git a/internal/cover/static.go b/internal/cover/static.go new file mode 100644 index 0000000..4061607 --- /dev/null +++ b/internal/cover/static.go @@ -0,0 +1,157 @@ +package cover + +import ( + "errors" + "io" + "io/fs" + "net/http" + "os" + "path" + "path/filepath" + "runtime" + "strings" +) + +// NewStaticHandler returns a handler rooted at directory. Only GET and HEAD +// are accepted; all other methods receive a normal HTTP 405 response. +// Each request opens its own root, so the handler owns no persistent handle. +// Root confinement does not exclude mounts, hard links, or public aliases to +// hidden files inside the root. The configured directory remains operator-owned. +func NewStaticHandler(directory string) (http.Handler, error) { + if strings.TrimSpace(directory) == "" { + return nil, errors.New("static directory is required") + } + root, err := filepath.Abs(directory) + if err != nil { + return nil, errors.New("resolve static directory") + } + info, err := os.Stat(root) + if err != nil { + return nil, errors.New("open static directory") + } + if !info.IsDir() { + return nil, errors.New("static path is not a directory") + } + // Validate actual access without retaining a handle through configuration + // checks, listener failures, or the shared H2/H3 cover lifecycle. + validationRoot, err := os.OpenRoot(root) + if err != nil { + return nil, errors.New("open static directory") + } + if err := validationRoot.Close(); err != nil { + return nil, errors.New("close static directory") + } + return &staticHandler{directory: root}, nil +} + +type staticHandler struct { + directory string +} + +func (h *staticHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet && r.Method != http.MethodHead { + w.Header().Set("Allow", "GET, HEAD") + http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) + return + } + if !staticPathAllowed(r.URL.Path) { + http.NotFound(w, r) + return + } + root, err := os.OpenRoot(h.directory) + if err != nil { + http.NotFound(w, r) + return + } + defer root.Close() + // FileServerFS keeps ordinary redirects, index lookup, HEAD, and Range. + // All files opened for this request use the same root and are closed by + // FileServer before this request's root handle is released. + http.FileServerFS(staticFileSystem{base: root.FS()}).ServeHTTP(w, r) +} + +// staticPathAllowed uses the decoded, normalized URL path, just as FileServer +// does. The well-known exception belongs only to the root directory. Backslash +// is never a public path separator; Windows alternate streams are also denied. +func staticPathAllowed(name string) bool { + if strings.ContainsRune(name, '\\') || (runtime.GOOS == "windows" && strings.ContainsRune(name, ':')) { + return false + } + cleaned := strings.TrimPrefix(path.Clean("/"+name), "/") + if cleaned == "" { + return true + } + for index, component := range strings.Split(cleaned, "/") { + if !staticComponentAllowed(component, index == 0) { + return false + } + } + return true +} + +func staticComponentAllowed(name string, root bool) bool { + if strings.ContainsRune(name, '\\') || (runtime.GOOS == "windows" && strings.ContainsRune(name, ':')) { + return false + } + return !strings.HasPrefix(name, ".") || (root && name == ".well-known") +} + +type staticFileSystem struct { + base fs.FS +} + +func (s staticFileSystem) Open(name string) (fs.File, error) { + if !fs.ValidPath(name) || !staticPathAllowed(name) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + file, err := s.base.Open(name) + if err != nil { + // Keep denied links, unavailable files, and platform-specific unsafe + // names indistinguishable, without exposing local paths in errors. + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + return &staticFile{File: file, directory: name}, nil +} + +type staticFile struct { + fs.File + directory string +} + +func (f *staticFile) Seek(offset int64, whence int) (int64, error) { + seeker, ok := f.File.(io.Seeker) + if !ok { + return 0, fs.ErrInvalid + } + return seeker.Seek(offset, whence) +} + +func (f *staticFile) ReadDir(n int) ([]fs.DirEntry, error) { + directory, ok := f.File.(fs.ReadDirFile) + if !ok { + return nil, fs.ErrInvalid + } + entries := make([]fs.DirEntry, 0) + for { + count := n + if n > 0 { + count -= len(entries) + } + batch, err := directory.ReadDir(count) + for _, entry := range batch { + if staticComponentAllowed(entry.Name(), f.directory == ".") { + entries = append(entries, entry) + } + } + if err != nil || n <= 0 || len(entries) >= n { + return entries, err + } + if len(batch) == 0 { + // A positive-count ReadDir must either advance or return an + // error. Never spin if an implementation violates that contract. + return entries, io.ErrNoProgress + } + // A batch containing only hidden entries is not the end of the + // directory: continue until the public count or a real error. + } +} diff --git a/internal/cover/static_filesystem_test.go b/internal/cover/static_filesystem_test.go new file mode 100644 index 0000000..7e46237 --- /dev/null +++ b/internal/cover/static_filesystem_test.go @@ -0,0 +1,276 @@ +package cover + +import ( + "errors" + "io" + "io/fs" + "reflect" + "runtime" + "testing" + "time" +) + +// These controls exercise the candidate-only filesystem wrapper. They are +// deliberately separate from the public API's old-production negative tests. +// No path in this file is opened on a real filesystem or through a symlink. +func TestStaticFileSystemPathPolicy(t *testing.T) { + for _, test := range []struct { + name string + pathAllowed, open bool + }{ + {".", true, true}, + {"", true, false}, + {"/", true, false}, + {"public.txt", true, true}, + {"assets/public.txt", true, true}, + {".well-known/acme-challenge/token", true, true}, + {"/.well-known/acme-challenge/token", true, false}, + {".env", false, false}, + {"assets/.env", false, false}, + {".well-known/.env", false, false}, + {"assets/.well-known/token", false, false}, + {".Well-known/token", false, false}, + {".well-known-extra/token", false, false}, + {"assets\\public.txt", false, false}, + {"assets/\\public.txt", false, false}, + {".well-known\\token", false, false}, + {"./public.txt", true, false}, + {"../public.txt", true, false}, + {"assets/../public.txt", true, false}, + {"assets//public.txt", true, false}, + {"assets/public.txt/", true, false}, + // Colon is an ordinary Unix filename byte, but on Windows these + // spellings can address alternate data streams or drive-relative paths. + {"public.txt:private", runtime.GOOS != "windows", runtime.GOOS != "windows"}, + {"assets/public.txt:private", runtime.GOOS != "windows", runtime.GOOS != "windows"}, + {"C:private", runtime.GOOS != "windows", runtime.GOOS != "windows"}, + } { + t.Run(test.name, func(t *testing.T) { + if got := staticPathAllowed(test.name); got != test.pathAllowed { + t.Errorf("decoded path policy on %s = %v, want %v", runtime.GOOS, got, test.pathAllowed) + } + file := &staticFileSystemTestFile{} + backend := &staticFileSystemTestFS{file: file} + result, err := (staticFileSystem{base: backend}).Open(test.name) + if test.open { + if err != nil || result == nil || backend.opens != 1 || backend.name != test.name { + t.Fatalf("ordinary FS open = %v/%v, calls=%d name=%q", result, err, backend.opens, backend.name) + } + if err := result.Close(); err != nil || file.closes != 1 { + t.Errorf("ordinary wrapper Close = %v / calls %d", err, file.closes) + } + return + } + var pathError *fs.PathError + if result != nil || !errors.As(err, &pathError) || pathError.Op != "open" || pathError.Path != test.name || pathError.Err != fs.ErrNotExist { + t.Errorf("denied FS open must return ordinary requested-path not-exist error: %v/%v", result, err) + } + if backend.opens != 0 { + t.Errorf("unsafe path reached backend Open %d times", backend.opens) + } + }) + } +} + +func TestStaticFileSystemReadDirContract(t *testing.T) { + t.Run("positive_count_and_eof", func(t *testing.T) { + base := staticFileSystemTestDirectory{".hidden", "first", ".env", "second", "third", ".private"} + file := &staticFileSystemTestDirFile{entries: base} + wrapper := &staticFile{File: file, directory: "assets"} + for index, test := range []struct { + names []string + err error + }{ + {[]string{"first", "second"}, nil}, + {[]string{"third"}, io.EOF}, + {[]string{}, io.EOF}, + } { + entries, err := wrapper.ReadDir(2) + if got := staticFileSystemTestNames(entries); !reflect.DeepEqual(got, test.names) || err != test.err { + t.Errorf("positive ReadDir call %d = %v/%v, want %v/%v", index, got, err, test.names, test.err) + } + } + // The wrapper asks only for the still-needed visible count and does + // not read ahead after it has filled the caller's requested batch. + if want := []int{2, 1, 1, 2, 1, 2}; !reflect.DeepEqual(file.counts, want) { + t.Errorf("underlying positive ReadDir counts = %v, want %v", file.counts, want) + } + if file.reads != 0 { + t.Errorf("directory filtering unexpectedly read file body %d times", file.reads) + } + }) + for _, count := range []int{0, -1} { + for _, directory := range []string{".", "assets"} { + t.Run("all_"+directory+"_"+map[int]string{0: "zero", -1: "negative"}[count], func(t *testing.T) { + file := &staticFileSystemTestDirFile{entries: staticFileSystemTestDirectory{".hidden", "first", ".well-known", "last"}} + wrapper := &staticFile{File: file, directory: directory} + entries, err := wrapper.ReadDir(count) + want := []string{"first", "last"} + if directory == "." { + want = []string{"first", ".well-known", "last"} + } + if got := staticFileSystemTestNames(entries); !reflect.DeepEqual(got, want) || err != nil || !reflect.DeepEqual(file.counts, []int{count}) { + t.Errorf("all ReadDir = %v/%v counts=%v, want %v/nil/one call", got, err, file.counts, want) + } + entries, err = wrapper.ReadDir(count) + if len(entries) != 0 || err != nil { + t.Errorf("all ReadDir after end = %v/%v, want empty/nil", entries, err) + } + }) + } + } + t.Run("original_error_with_partial_batch", func(t *testing.T) { + failure := errors.New("owned original ReadDir failure") + file := &staticFileSystemTestDirFile{entries: staticFileSystemTestDirectory{".hidden", "visible"}, finalErr: failure} + entries, err := (&staticFile{File: file, directory: "."}).ReadDir(3) + if got := staticFileSystemTestNames(entries); !reflect.DeepEqual(got, []string{"visible"}) || err != failure || !reflect.DeepEqual(file.counts, []int{3}) { + t.Errorf("partial original ReadDir = %v/%v counts=%v", got, err, file.counts) + } + }) + t.Run("empty_nil_does_not_spin", func(t *testing.T) { + file := &staticFileSystemTestDirFile{emptyNil: true} + entries, err := (&staticFile{File: file, directory: "."}).ReadDir(1) + if len(entries) != 0 || err != io.ErrNoProgress || !reflect.DeepEqual(file.counts, []int{1}) { + t.Errorf("invalid non-advancing backend = %v/%v counts=%v", entries, err, file.counts) + } + }) +} + +func TestStaticFileSystemFileDelegationAndErrors(t *testing.T) { + t.Run("read_close_stat_seek", func(t *testing.T) { + readFailure, closeFailure := errors.New("original read failure"), errors.New("original close failure") + statFailure, seekFailure := errors.New("original stat failure"), errors.New("original seek failure") + base := &staticFileSystemTestSeekFile{staticFileSystemTestFile: staticFileSystemTestFile{ + readErr: readFailure, closeErr: closeFailure, statErr: statFailure, + }, seekErr: seekFailure} + file := &staticFile{File: base, directory: "public.txt"} + buffer := make([]byte, 4) + if n, err := file.Read(buffer); n != 3 || err != readFailure || string(buffer[:n]) != "abc" || base.reads != 1 { + t.Errorf("Read changed original n/payload/error/call count = %d/%q/%v/%d", n, buffer, err, base.reads) + } + if err := file.Close(); err != closeFailure || base.closes != 1 { + t.Errorf("Close changed original error/call count = %v/%d", err, base.closes) + } + if info, err := file.Stat(); info == nil || info.Name() != "public.txt" || err != statFailure || base.stats != 1 { + t.Errorf("Stat changed original info/error/call count = %v/%v/%d", info, err, base.stats) + } + if offset, err := file.Seek(7, io.SeekCurrent); offset != 19 || err != seekFailure || base.seeks != 1 || base.offset != 7 || base.whence != io.SeekCurrent { + t.Errorf("Seek changed original input/output = %d/%v calls=%d offset=%d whence=%d", offset, err, base.seeks, base.offset, base.whence) + } + }) + t.Run("unsupported_optional_operations", func(t *testing.T) { + file := &staticFile{File: &staticFileSystemTestFile{}, directory: "public.txt"} + if entries, err := file.ReadDir(1); entries != nil || err != fs.ErrInvalid { + t.Errorf("unsupported ReadDir = %v/%v", entries, err) + } + if offset, err := file.Seek(0, io.SeekStart); offset != 0 || err != fs.ErrInvalid { + t.Errorf("unsupported Seek = %d/%v", offset, err) + } + }) + t.Run("backend_open_error_is_generic", func(t *testing.T) { + failure := errors.New("owned backend private path failure") + base := &staticFileSystemTestFS{err: failure} + file, err := (staticFileSystem{base: base}).Open("public.txt") + var pathError *fs.PathError + if file != nil || !errors.As(err, &pathError) || pathError.Op != "open" || pathError.Path != "public.txt" || pathError.Err != fs.ErrNotExist || errors.Is(err, failure) || base.opens != 1 { + t.Errorf("backend Open failure exposed identity or changed call count = %v/%v calls=%d", file, err, base.opens) + } + }) +} + +type staticFileSystemTestFS struct { + file fs.File + err error + opens int + name string +} + +func (f *staticFileSystemTestFS) Open(name string) (fs.File, error) { + f.opens++ + f.name = name + return f.file, f.err +} + +type staticFileSystemTestFile struct { + readErr, closeErr, statErr error + reads, closes, stats int +} + +func (f *staticFileSystemTestFile) Read(p []byte) (int, error) { + f.reads++ + return copy(p, "abc"), f.readErr +} + +func (f *staticFileSystemTestFile) Close() error { f.closes++; return f.closeErr } +func (f *staticFileSystemTestFile) Stat() (fs.FileInfo, error) { + f.stats++ + return staticFileSystemTestInfo("public.txt"), f.statErr +} + +type staticFileSystemTestDirectory []string + +type staticFileSystemTestDirFile struct { + staticFileSystemTestFile + entries staticFileSystemTestDirectory + position int + counts []int + finalErr error + emptyNil bool +} + +func (f *staticFileSystemTestDirFile) ReadDir(n int) ([]fs.DirEntry, error) { + f.counts = append(f.counts, n) + if f.emptyNil { + return nil, nil + } + if f.position == len(f.entries) { + if n > 0 { + return nil, io.EOF + } + return []fs.DirEntry{}, nil + } + end := len(f.entries) + if n > 0 && n < end-f.position { + end = f.position + n + } + entries := make([]fs.DirEntry, 0, end-f.position) + for _, name := range f.entries[f.position:end] { + entries = append(entries, fs.FileInfoToDirEntry(staticFileSystemTestInfo(name))) + } + f.position = end + if f.position == len(f.entries) && f.finalErr != nil { + return entries, f.finalErr + } + return entries, nil +} + +type staticFileSystemTestSeekFile struct { + staticFileSystemTestFile + seekErr error + seeks int + offset int64 + whence int +} + +func (f *staticFileSystemTestSeekFile) Seek(offset int64, whence int) (int64, error) { + f.seeks++ + f.offset, f.whence = offset, whence + return 19, f.seekErr +} + +type staticFileSystemTestInfo string + +func (f staticFileSystemTestInfo) Name() string { return string(f) } +func (staticFileSystemTestInfo) Size() int64 { return 3 } +func (staticFileSystemTestInfo) Mode() fs.FileMode { return 0o600 } +func (staticFileSystemTestInfo) ModTime() time.Time { return time.Time{} } +func (staticFileSystemTestInfo) IsDir() bool { return false } +func (staticFileSystemTestInfo) Sys() any { return nil } + +func staticFileSystemTestNames(entries []fs.DirEntry) []string { + names := make([]string, 0, len(entries)) + for _, entry := range entries { + names = append(names, entry.Name()) + } + return names +} diff --git a/internal/cover/static_security_test.go b/internal/cover/static_security_test.go new file mode 100644 index 0000000..83e516c --- /dev/null +++ b/internal/cover/static_security_test.go @@ -0,0 +1,293 @@ +package cover + +import ( + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" +) + +const ( + staticSecurityOrdinary = "ordinary-static-cover-payload\n" + staticSecurityIndex = "ordinary static cover index\n" + staticSecurityHidden = "DUMMY-STATIC-HIDDEN-CONTENT" + staticSecurityOutside = "DUMMY-STATIC-OUTSIDE-CONTENT" + staticSecurityWellKnown = "ordinary-well-known-fixture" +) + +func TestStaticSecurityOrdinaryCompatibility(t *testing.T) { + fixture := newStaticSecurityFixture(t, true) + for _, test := range []struct { + name, method, path, rangeValue, body string + status int + }{ + {"index_get", http.MethodGet, "/", "", staticSecurityIndex, http.StatusOK}, + {"ordinary_get", http.MethodGet, "/assets/public.txt", "", staticSecurityOrdinary, http.StatusOK}, + {"ordinary_head", http.MethodHead, "/assets/public.txt", "", "", http.StatusOK}, + {"ordinary_range", http.MethodGet, "/assets/public.txt", "bytes=0-7", staticSecurityOrdinary[:8], http.StatusPartialContent}, + {"ordinary_head_range", http.MethodHead, "/assets/public.txt", "bytes=0-7", "", http.StatusPartialContent}, + {"missing", http.MethodGet, "/missing-fixture.txt", "", "404 page not found\n", http.StatusNotFound}, + {"method", http.MethodPost, "/assets/public.txt", "", "Method Not Allowed\n", http.StatusMethodNotAllowed}, + } { + t.Run(test.name, func(t *testing.T) { + response := staticSecurityRequest(t, fixture.handler, test.method, test.path, test.rangeValue) + if response.Code != test.status || response.Body.String() != test.body { + t.Fatalf("ordinary response = %d/%q, want %d/%q", response.Code, response.Body, test.status, test.body) + } + if test.method == http.MethodPost && response.Header().Get("Allow") != "GET, HEAD" { + t.Errorf("ordinary method Allow = %q", response.Header().Get("Allow")) + } + if test.rangeValue != "" { + want := fmt.Sprintf("bytes 0-7/%d", len(staticSecurityOrdinary)) + if got := response.Header().Get("Content-Range"); got != want { + t.Errorf("ordinary Content-Range = %q, want %q", got, want) + } + } + assertNoProductMarker(t, response.Result()) + }) + } +} + +func TestStaticSecurityWellKnownAndHiddenPaths(t *testing.T) { + fixture := newStaticSecurityFixture(t, true) + for _, path := range []string{ + "/.well-known/acme-challenge/public-token", + "/%2ewell-known/acme-challenge/public-token", + "/.well-known/acme-challenge/public%2dtoken", + } { + t.Run("allowed_"+path, func(t *testing.T) { + response := staticSecurityRequest(t, fixture.handler, http.MethodGet, path, "") + if response.Code != http.StatusOK || response.Body.String() != staticSecurityWellKnown { + t.Fatalf("root .well-known response = %d/%q", response.Code, response.Body) + } + }) + } + for _, path := range []string{ + "/.env", + "/%2Eenv", + "/.private/dummy.txt", + "/%2eprivate/dummy.txt", + "/assets/.hidden.txt", + "/assets/%2Ehidden.txt", + "/assets%2f.hidden.txt", + "/assets/.well-known/dummy.txt", + "/.well-known/.private.txt", + "/.well-known/%2Eprivate.txt", + "/.well-known%2f.private.txt", + } { + t.Run("blocked_"+path, func(t *testing.T) { + staticSecurityAssertDenied(t, staticSecurityRequest(t, fixture.handler, http.MethodGet, path, "")) + }) + } +} + +func TestStaticSecurityConfinesSymlinks(t *testing.T) { + fixture := newStaticSecurityFixture(t, true) + for _, path := range []string{"/inside-relative.txt", "/inside-relative-dir/public.txt"} { + t.Run("internal_"+path, func(t *testing.T) { + response := staticSecurityRequest(t, fixture.handler, http.MethodGet, path, "") + if response.Code != http.StatusOK || response.Body.String() != staticSecurityOrdinary { + t.Fatalf("internal relative symlink response = %d/%q", response.Code, response.Body) + } + }) + } + for _, path := range []string{ + "/outside-absolute.txt", + "/outside-relative.txt", + "/outside-dir/", + "/outside-dir/index.html", + "/outside-relative-dir/", + "/outside-relative-dir/index.html", + } { + t.Run("external_"+path, func(t *testing.T) { + // FileServer may redirect index.html to ./ before opening a file. + // Follow only bounded local frontend redirects, then inspect the + // actual terminal response rather than calling a 301 a content leak. + staticSecurityAssertDenied(t, staticSecurityRequest(t, fixture.handler, http.MethodGet, path, "")) + }) + } +} + +func TestStaticSecurityAbsoluteInternalSymlinkBlocked(t *testing.T) { + fixture := newStaticSecurityFixture(t, true) + response := staticSecurityRequest(t, fixture.handler, http.MethodGet, "/inside-absolute.txt", "") + if response.Code != http.StatusNotFound { + t.Errorf("absolute internal symlink status/body = %d/%q, want ordinary 404", response.Code, response.Body) + } + if strings.Contains(response.Body.String(), staticSecurityOrdinary) { + t.Error("blocked absolute internal symlink returned actual owned dummy file content") + } + staticSecurityAssertNoDummySecrets(t, response) + assertNoProductMarker(t, response.Result()) +} + +func TestStaticSecurityAdditionalOrdinaryCompatibility(t *testing.T) { + fixture := newStaticSecurityFixture(t, true) + t.Run("invalid_range", func(t *testing.T) { + response := staticSecurityRequest(t, fixture.handler, http.MethodGet, "/assets/public.txt", "bytes=1000-2000") + if response.Code != http.StatusRequestedRangeNotSatisfiable || response.Body.String() != "invalid range: failed to overlap\n" { + t.Fatalf("ordinary invalid Range response = %d/%q, want standard 416", response.Code, response.Body) + } + if want, got := fmt.Sprintf("bytes */%d", len(staticSecurityOrdinary)), response.Header().Get("Content-Range"); got != want { + t.Errorf("invalid Range Content-Range = %q, want %q", got, want) + } + assertNoProductMarker(t, response.Result()) + }) + t.Run("directory_redirect", func(t *testing.T) { + // Do not use the redirect-following helper: the first 301 and its + // exact relative Location (including the query) are the control. + request := httptest.NewRequest(http.MethodGet, "https://static.example/assets?q=fixture", nil) + response := httptest.NewRecorder() + fixture.handler.ServeHTTP(response, request) + if response.Code != http.StatusMovedPermanently || response.Header().Get("Location") != "assets/?q=fixture" || response.Body.Len() != 0 { + t.Fatalf("ordinary directory redirect = %d/%q/%q, want 301/assets/?q=fixture/empty", response.Code, response.Header().Get("Location"), response.Body) + } + assertNoProductMarker(t, response.Result()) + }) +} + +func TestStaticSecurityDirectoryListings(t *testing.T) { + fixture := newStaticSecurityFixture(t, false) + for _, test := range []struct { + name, path string + present, absent []string + }{ + {"root", "/", []string{"assets/", ".well-known/"}, []string{".env", ".private/"}}, + {"ordinary_directory", "/listing/", []string{"visible.txt"}, []string{".listing-secret", ".listing-private/"}}, + {"root_well_known", "/.well-known/", []string{"acme-challenge/"}, []string{".private.txt"}}, + } { + t.Run(test.name, func(t *testing.T) { + response := staticSecurityRequest(t, fixture.handler, http.MethodGet, test.path, "") + if response.Code != http.StatusOK { + t.Fatalf("directory listing status = %d, want 200", response.Code) + } + for _, name := range test.present { + if !strings.Contains(response.Body.String(), name) { + t.Errorf("ordinary directory entry %q missing from listing", name) + } + } + for _, name := range test.absent { + if strings.Contains(response.Body.String(), name) { + t.Errorf("hidden directory entry %q exposed in listing", name) + } + } + staticSecurityAssertNoDummySecrets(t, response) + }) + } +} + +type staticSecurityFixture struct { + handler http.Handler +} + +func newStaticSecurityFixture(t *testing.T, withIndex bool) staticSecurityFixture { + t.Helper() + base := t.TempDir() + root := filepath.Join(base, "public") + for _, directory := range []string{"public", "outside", "public/assets/.well-known", "public/.private", "public/.well-known/acme-challenge", "public/listing/.listing-private"} { + if err := os.MkdirAll(filepath.Join(base, directory), 0o700); err != nil { + t.Fatal(err) + } + } + files := map[string]string{ + "public/assets/public.txt": staticSecurityOrdinary, + "public/.env": staticSecurityHidden, + "public/.private/dummy.txt": staticSecurityHidden, + "public/assets/.hidden.txt": staticSecurityHidden, + "public/assets/.well-known/dummy.txt": staticSecurityHidden, + "public/.well-known/acme-challenge/public-token": staticSecurityWellKnown, + "public/.well-known/.private.txt": staticSecurityHidden, + "public/listing/visible.txt": staticSecurityOrdinary, + "public/listing/.listing-secret": staticSecurityHidden, + "public/listing/.listing-private/dummy.txt": staticSecurityHidden, + "outside/dummy.txt": staticSecurityOutside, + "outside/index.html": staticSecurityOutside, + } + if withIndex { + files["public/index.html"] = staticSecurityIndex + } + for path, contents := range files { + if err := os.WriteFile(filepath.Join(base, path), []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + } + for name, target := range map[string]string{ + "inside-relative.txt": filepath.Join("assets", "public.txt"), + "inside-relative-dir": "assets", + "inside-absolute.txt": filepath.Join(root, "assets", "public.txt"), + "outside-absolute.txt": filepath.Join(base, "outside", "dummy.txt"), + "outside-relative.txt": filepath.Join("..", "outside", "dummy.txt"), + "outside-dir": filepath.Join(base, "outside"), + "outside-relative-dir": filepath.Join("..", "outside"), + } { + if err := os.Symlink(target, filepath.Join(root, name)); err != nil { + t.Fatalf("create owned dummy symlink %q: %v", name, err) + } + } + handler, err := NewStaticHandler(root) + if err != nil { + t.Fatal(err) + } + return staticSecurityFixture{handler: handler} +} + +func staticSecurityRequest(t *testing.T, handler http.Handler, method, path, rangeValue string) *httptest.ResponseRecorder { + t.Helper() + target, err := url.Parse("https://static.example" + path) + if err != nil { + t.Fatal(err) + } + for redirects := 0; redirects <= 4; redirects++ { + request := httptest.NewRequest(method, target.String(), nil) + if rangeValue != "" { + request.Header.Set("Range", rangeValue) + } + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + staticSecurityAssertNoDummySecretsOnRedirect(t, response) + switch response.Code { + case http.StatusMovedPermanently, http.StatusFound, http.StatusTemporaryRedirect, http.StatusPermanentRedirect: + reference, err := url.Parse(response.Header().Get("Location")) + if err != nil || response.Header().Get("Location") == "" { + t.Fatalf("invalid ordinary static redirect Location %q", response.Header().Get("Location")) + } + target = target.ResolveReference(reference) + if target.Scheme != "https" || target.Host != "static.example" || target.User != nil { + t.Fatalf("static redirect left owned dummy frontend: %q", target) + } + default: + return response + } + } + t.Fatal("static response exceeded four owned local redirects") + return nil +} + +func staticSecurityAssertNoDummySecretsOnRedirect(t *testing.T, response *httptest.ResponseRecorder) { + t.Helper() + if response.Code >= 300 && response.Code < 400 { + staticSecurityAssertNoDummySecrets(t, response) + } +} + +func staticSecurityAssertDenied(t *testing.T, response *httptest.ResponseRecorder) { + t.Helper() + if response.Code != http.StatusNotFound { + t.Errorf("unsafe static terminal status = %d, want ordinary 404", response.Code) + } + staticSecurityAssertNoDummySecrets(t, response) + assertNoProductMarker(t, response.Result()) +} + +func staticSecurityAssertNoDummySecrets(t *testing.T, response *httptest.ResponseRecorder) { + t.Helper() + for _, marker := range []string{staticSecurityHidden, staticSecurityOutside} { + if strings.Contains(response.Body.String(), marker) { + t.Errorf("owned dummy content outside public policy exposed: %q", marker) + } + } +} diff --git a/internal/tunnel/web_cover_static_security_test.go b/internal/tunnel/web_cover_static_security_test.go new file mode 100644 index 0000000..b03f8b5 --- /dev/null +++ b/internal/tunnel/web_cover_static_security_test.go @@ -0,0 +1,245 @@ +package tunnel + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/netip" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/cppla/autocar/internal/cover" + "github.com/cppla/autocar/internal/transport" +) + +const ( + webStaticSecurityPublic = "ordinary static content\n" + webStaticSecurityToken = "owned-acme-challenge-token" + webStaticSecurityOutside = "fictional-outside-marker-never-public" + webStaticSecurityHidden = "fictional-hidden-marker-never-public" +) + +// All files are synthetic, owned temporary fixtures. The public handler is the +// actual constructor, not a test scrubber or a copy of the production policy. +func TestWebStaticCoverFilesystemSecurityOnWire(t *testing.T) { + root := webStaticSecurityFiles(t) + site, err := cover.NewStaticHandler(root) + if err != nil { + t.Fatal(err) + } + var handlerMu sync.Mutex + var handlerWorkers sync.WaitGroup + closing := false + handlerResults := make(chan string, 64) + website := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handlerMu.Lock() + if closing { + handlerMu.Unlock() + http.Error(w, "closing", http.StatusServiceUnavailable) + return + } + handlerWorkers.Add(1) + handlerMu.Unlock() + defer handlerWorkers.Done() + defer func() { handlerResults <- r.Header.Get("X-Static-Fixture") }() + site.ServeHTTP(w, r) + }) + serverTLS, clientTLS := testTLSConfigs(t) + var targetDials, targetResolutions atomic.Int64 + server, err := ListenWeb(WebServerConfig{ + TCPAddress: "127.0.0.1:0", UDPAddress: "127.0.0.1:0", + Token: webTestToken, TLSConfig: serverTLS, Cover: website, + Dialer: transport.DialFunc(func(context.Context, string, string) (net.Conn, error) { + targetDials.Add(1) + return nil, errors.New("static cover fixture forbids every tunnel destination") + }), + UDPResolver: UDPResolverFunc(func(context.Context, string) ([]netip.AddrPort, error) { + targetResolutions.Add(1) + return nil, errors.New("static cover fixture forbids UDP target resolution") + }), + }) + if err != nil { + t.Fatal(err) + } + serveCtx, cancelServe := context.WithCancel(context.Background()) + serveResult, serveJoined := make(chan error, 1), make(chan struct{}) + go func() { defer close(serveJoined); serveResult <- server.Serve(serveCtx) }() + t.Cleanup(func() { + // Freeze worker admission before Wait, including on a Fatal path. + handlerMu.Lock() + closing = true + handlerMu.Unlock() + cancelServe() + _ = server.Close() + joined := make(chan struct{}) + go func() { defer close(joined); handlerWorkers.Wait() }() + webCoverMetadataJoin(t, "static cover handlers", joined) + webCoverMetadataJoin(t, "static combined Serve", serveJoined) + select { + case err := <-serveResult: + if err != nil { + t.Errorf("static combined Serve: %v", err) + } + default: + } + }) + cases := []struct { + name, method, path, rangeHeader string + status int + body, contentRange, allow string + listing bool + }{ + {name: "outside_file", method: "GET", path: "/external-file", status: 404}, + {name: "outside_directory", method: "GET", path: "/external-dir/outside.txt", status: 404}, + {name: "outside_file_head", method: "HEAD", path: "/external-file", status: 404}, + {name: "hidden_file", method: "GET", path: "/.env", status: 404}, + {name: "encoded_hidden_file", method: "GET", path: "/%2eenv", status: 404}, + {name: "hidden_directory", method: "GET", path: "/.git/config", status: 404}, + {name: "encoded_hidden_directory", method: "GET", path: "/%2Egit/config", status: 404}, + {name: "encoded_nested_hidden", method: "GET", path: "/listing/%2eenv", status: 404}, + {name: "well_known_nested_hidden", method: "GET", path: "/.well-known/%2eenv", status: 404}, + {name: "listing_filters_hidden", method: "GET", path: "/listing/", status: 200, listing: true}, + {name: "ordinary_get", method: "GET", path: "/public.txt", status: 200, body: webStaticSecurityPublic}, + {name: "ordinary_head", method: "HEAD", path: "/public.txt", status: 200}, + {name: "ordinary_range", method: "GET", path: "/public.txt", rangeHeader: "bytes=0-7", status: 206, body: webStaticSecurityPublic[:8], contentRange: fmt.Sprintf("bytes 0-7/%d", len(webStaticSecurityPublic))}, + {name: "inside_symlink", method: "GET", path: "/inside-link.txt", status: 200, body: webStaticSecurityPublic}, + {name: "well_known_get", method: "GET", path: "/.well-known/acme-challenge/token", status: 200, body: webStaticSecurityToken}, + {name: "well_known_head", method: "HEAD", path: "/.well-known/acme-challenge/token", status: 200}, + {name: "ordinary_missing", method: "GET", path: "/missing.txt", status: 404}, + {name: "ordinary_post", method: "POST", path: "/public.txt", status: 405, allow: "GET, HEAD"}, + {name: "decoded_traversal", method: "GET", path: "/../outside/outside.txt", status: 404}, + {name: "encoded_traversal", method: "GET", path: "/%2e%2e/outside/outside.txt", status: 404}, + } + for _, proto := range []int{1, 2, 3} { + t.Run(fmt.Sprintf("h%d", proto), func(t *testing.T) { + rt := webCoverMetadataPublicTransport(t, clientTLS, proto) + address := server.TCPAddr().String() + if proto == 3 { + address = server.UDPAddr().String() + } + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + request, err := http.NewRequestWithContext(ctx, test.method, "https://"+address+test.path, nil) + if err != nil { + t.Fatal(err) + } + request.Header.Set("X-Static-Fixture", t.Name()) + request.Header.Set("Proxy-Authorization", "Bearer fictional-invalid-ticket") + if test.rangeHeader != "" { + request.Header.Set("Range", test.rangeHeader) + } + response, err := rt.RoundTrip(request) + if err != nil { + t.Fatal(err) + } + body, readErr := io.ReadAll(response.Body) + _ = response.Body.Close() + if readErr != nil { + t.Fatal(readErr) + } + if response.ProtoMajor != proto || response.TLS == nil || len(response.TLS.VerifiedChains) == 0 { + t.Fatalf("actual static response = %s, verified TLS=%t, want H%d", response.Proto, response.TLS != nil && len(response.TLS.VerifiedChains) != 0, proto) + } + if response.StatusCode != test.status { + t.Errorf("actual %s %s status=%d body=%q, want %d", test.method, test.path, response.StatusCode, body, test.status) + } + if strings.Contains(string(body), webStaticSecurityOutside) || strings.Contains(string(body), webStaticSecurityHidden) { + t.Errorf("dummy private marker escaped on actual H%d %s: %q", proto, test.path, body) + } + if test.body != "" && string(body) != test.body { + t.Errorf("ordinary body=%q, want %q", body, test.body) + } + if test.method == http.MethodHead && len(body) != 0 { + t.Errorf("HEAD returned a body: %q", body) + } + if test.method == http.MethodHead && test.status == 200 { + length := len(webStaticSecurityPublic) + if test.path == "/.well-known/acme-challenge/token" { + length = len(webStaticSecurityToken) + } + if response.Header.Get("Content-Length") != strconv.Itoa(length) { + t.Errorf("ordinary HEAD Content-Length=%q, want %d", response.Header.Get("Content-Length"), length) + } + } + if test.contentRange != "" && response.Header.Get("Content-Range") != test.contentRange { + t.Errorf("ordinary Content-Range=%q, want %q", response.Header.Get("Content-Range"), test.contentRange) + } + if test.allow != "" && response.Header.Get("Allow") != test.allow { + t.Errorf("ordinary Allow=%q, want %q", response.Header.Get("Allow"), test.allow) + } + if test.listing { + if !strings.Contains(string(body), "visible.txt") { + t.Errorf("ordinary listing entry lost: %q", body) + } + for _, hidden := range []string{".env", ".git"} { + if strings.Contains(string(body), hidden) { + t.Errorf("hidden listing entry %q escaped: %q", hidden, body) + } + } + } + if response.Header.Get(webAuthResponseHeader) != "" || response.Header.Get("Proxy-Authenticate") != "" { + t.Errorf("ordinary static response exposed relay authentication metadata: %v", response.Header) + } + select { + case name := <-handlerResults: + if name != t.Name() { + t.Errorf("static handler completion=%q, want %q", name, t.Name()) + } + case <-time.After(2 * time.Second): + t.Fatal("actual static handler did not finish within its independent budget") + } + }) + } + }) + } + if targetDials.Load() != 0 || targetResolutions.Load() != 0 { + t.Errorf("static public target dial/resolution=%d/%d, want 0/0", targetDials.Load(), targetResolutions.Load()) + } + t.Log("actual verified H1/H2/H3 static filesystem matrix; target dial/resolution=0/0") +} + +func webStaticSecurityFiles(t *testing.T) string { + t.Helper() + fixture := t.TempDir() + root, outside := filepath.Join(fixture, "site"), filepath.Join(fixture, "outside") + files := map[string]string{ + filepath.Join(root, "public.txt"): webStaticSecurityPublic, + filepath.Join(root, ".env"): webStaticSecurityHidden, + filepath.Join(root, ".git", "config"): webStaticSecurityHidden, + filepath.Join(root, "listing", "visible.txt"): "ordinary visible listing entry", + filepath.Join(root, "listing", ".env"): webStaticSecurityHidden, + filepath.Join(root, "listing", ".git", "config"): webStaticSecurityHidden, + filepath.Join(root, ".well-known", "acme-challenge", "token"): webStaticSecurityToken, + filepath.Join(root, ".well-known", ".env"): webStaticSecurityHidden, + filepath.Join(outside, "outside.txt"): webStaticSecurityOutside, + } + for name, body := range files { + if err := os.MkdirAll(filepath.Dir(name), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(name, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + } + for name, target := range map[string]string{ + "external-file": filepath.Join(outside, "outside.txt"), + "external-dir": outside, + "inside-link.txt": "public.txt", + } { + if err := os.Symlink(target, filepath.Join(root, name)); err != nil { + t.Fatal(err) + } + } + return root +}