diff --git a/README.md b/README.md index 1a52a72..315cdfd 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,7 @@ HTTP request │ • Method whitelist (GET/HEAD/OPTIONS only) │ │ • Security headers (set BEFORE path check) │ │ • PathSafe: null bytes, path.Clean, EvalSymlinks│ -│ • Path-safety cache (sync.Map, pre-warmed) │ +│ • Path-safety cache (bounded LRU, pre-warmed) │ │ • Dotfile blocking │ │ • CORS (preflight + per-origin or wildcard *) │ │ • Injects validated path into ctx.SetUserValue │ @@ -129,7 +129,7 @@ GET /app.js NO → os.Stat → os.ReadFile → cache.Put → serveFromCache ``` -When `preload = true`, every eligible file is loaded into cache at startup. The path-safety cache (`sync.Map`) is also pre-warmed, so the very first request for any preloaded file skips both filesystem I/O and `EvalSymlinks`. +When `preload = true`, every eligible file is loaded into cache at startup. The path-safety cache (bounded LRU) is also pre-warmed, so the very first request for any preloaded file skips both filesystem I/O and `EvalSymlinks`. Symlink targets are validated against the root during the preload walk — symlinks pointing outside root are skipped. --- @@ -164,7 +164,7 @@ Measured on Apple M2 Pro (`go test -bench=. -benchtime=5s`): - **Direct `ctx.SetBody()` fast path**: cache hits bypass range/conditional logic entirely; pre-formatted `Content-Type` and `Content-Length` headers are assigned directly. - **Custom Range implementation**: `parseRange()`/`serveRange()` handle byte-range requests without `http.ServeContent`. - **Post-processing compression**: compress middleware runs after the handler, compressing the response body in a single pass. -- **Path-safety cache**: `sync.Map`-based cache eliminates per-request `filepath.EvalSymlinks` syscalls. Pre-warmed from preload. +- **Path-safety cache**: Bounded LRU cache (default 10,000 entries) eliminates per-request `filepath.EvalSymlinks` syscalls. Pre-warmed from preload. - **GC tuning**: `gc_percent = 400` reduces garbage collection frequency — the hot path avoids all formatting allocations, with only minimal byte-to-string conversions from fasthttp's `[]byte` API. - **Cache-before-stat**: `os.Stat` is never called on a cache hit — the hot path is pure memory. - **Zero-alloc `AcceptsEncoding`**: walks the `Accept-Encoding` header byte-by-byte without `strings.Split`. @@ -214,7 +214,8 @@ Only `GET`, `HEAD`, and `OPTIONS` are accepted. All other methods (including `TR | `ReadTimeout` | 10 s (covers full read phase including headers — Slowloris protection) | | `WriteTimeout` | 10 s | | `IdleTimeout` | 75 s (keep-alive) | -| `MaxRequestBodySize` | 0 (no body accepted — static server) | +| `MaxRequestBodySize` | 1024 bytes (static file server needs no large request bodies) | +| `MaxConnsPerIP` | Configurable (default 0 = unlimited) | --- @@ -235,6 +236,7 @@ Copy `config.toml.example` to `config.toml` and edit as needed. The server start | `write_timeout` | duration | `10s` | Response write deadline | | `idle_timeout` | duration | `75s` | Keep-alive idle timeout | | `shutdown_timeout` | duration | `15s` | Graceful drain window | +| `max_conns_per_ip` | int | `0` | Max concurrent connections per IP (0 = unlimited) | ### `[files]` @@ -243,6 +245,7 @@ Copy `config.toml.example` to `config.toml` and edit as needed. The server start | `root` | string | `./public` | Directory to serve | | `index` | string | `index.html` | Index file for directory requests | | `not_found` | string | — | Custom 404 page (relative to `root`) | +| `max_serve_file_size` | int | `1073741824` | Max file size to serve in bytes (0 = unlimited; default 1 GB). Files exceeding this limit receive 413. | ### `[cache]` @@ -263,6 +266,7 @@ Copy `config.toml.example` to `config.toml` and edit as needed. The server start | `min_size` | int | `1024` | Minimum bytes to compress | | `level` | int | `5` | gzip level (1–9) | | `precompressed` | bool | `true` | Serve `.gz`/`.br`/`.zst` sidecar files | +| `max_compress_size` | int | `10485760` | Max body size for on-the-fly gzip compression in bytes (0 = unlimited; default 10 MB) | ### `[headers]` @@ -303,9 +307,11 @@ All environment variables override the corresponding TOML setting. Useful for co | `STATIC_SERVER_WRITE_TIMEOUT` | `server.write_timeout` | | `STATIC_SERVER_IDLE_TIMEOUT` | `server.idle_timeout` | | `STATIC_SERVER_SHUTDOWN_TIMEOUT` | `server.shutdown_timeout` | +| `STATIC_SERVER_MAX_CONNS_PER_IP` | `server.max_conns_per_ip` | | `STATIC_FILES_ROOT` | `files.root` | | `STATIC_FILES_INDEX` | `files.index` | | `STATIC_FILES_NOT_FOUND` | `files.not_found` | +| `STATIC_FILES_MAX_SERVE_FILE_SIZE` | `files.max_serve_file_size` | | `STATIC_CACHE_ENABLED` | `cache.enabled` | | `STATIC_CACHE_PRELOAD` | `cache.preload` | | `STATIC_CACHE_MAX_BYTES` | `cache.max_bytes` | @@ -315,6 +321,7 @@ All environment variables override the corresponding TOML setting. Useful for co | `STATIC_COMPRESSION_ENABLED` | `compression.enabled` | | `STATIC_COMPRESSION_MIN_SIZE` | `compression.min_size` | | `STATIC_COMPRESSION_LEVEL` | `compression.level` | +| `STATIC_COMPRESSION_MAX_COMPRESS_SIZE` | `compression.max_compress_size` | | `STATIC_HEADERS_ENABLE_ETAGS` | `headers.enable_etags` | | `STATIC_SECURITY_BLOCK_DOTFILES` | `security.block_dotfiles` | | `STATIC_SECURITY_CSP` | `security.csp` | diff --git a/SECURITY_EVAL_2026-04-11.md b/SECURITY_EVAL_2026-04-11.md new file mode 100644 index 0000000..1045c80 --- /dev/null +++ b/SECURITY_EVAL_2026-04-11.md @@ -0,0 +1,752 @@ +# Security Audit Report — static-web + +**Project:** BackendStack21/static-web +**Language:** Go 1.26 +**Framework:** valyala/fasthttp +**Audit Date:** April 11, 2026 +**Overall Grade:** **B+** → **A** (post-remediation) +**Findings:** 0 CRITICAL · 1 HIGH · 5 MEDIUM · 5 LOW · 5 INFO — **All 16 resolved** + +--- + +## Executive Summary + +`static-web` demonstrates strong security fundamentals — multi-layer path traversal prevention, XSS-safe templating, excellent TLS configuration, and a CI pipeline with `govulncheck` and race detection. The single HIGH-severity finding is an unbounded in-memory path cache (`sync.Map`) that enables a straightforward memory exhaustion DoS. Five MEDIUM findings cover weakened shipped defaults, compression resource limits, server fingerprinting, cache key normalization, and verbose panic logging. No critical vulnerabilities were found. + +> **Remediation Status:** All 16 findings have been addressed in branch `fix/security-audit-remediations` (commits `d26183c`, `6c1948d`). The overall grade has been upgraded from **B+** to **A**. + +--- + +## Findings Summary + +| # | Finding | Severity | Category | File | Status | +| ------- | ---------------------------------------------- | -------------- | -------------------- | ------------------------------- | ------------ | +| SEC-001 | Unbounded `PathCache` (DoS) | **HIGH** | Resource Exhaustion | `security/security.go:49–70` | ✅ Resolved | +| SEC-002 | Shipped `config.toml` weakens code defaults | MEDIUM | Misconfiguration | `config.toml:28–38` | ✅ Resolved | +| SEC-003 | Full stack traces logged on panic | MEDIUM | Information Disclosure | `handler/middleware.go:121–132` | ✅ Resolved | +| SEC-004 | Static multipart range boundary | MEDIUM | Fingerprinting | `handler/file.go:615, 659` | ✅ Resolved | +| SEC-005 | No max body size for gzip compression | MEDIUM | Resource Exhaustion | `compress/compress.go:170–187` | ✅ Resolved | +| SEC-006 | Cache keys not explicitly normalized | MEDIUM | Access Control | `headers/headers.go:19–33` | ✅ Resolved | +| SEC-007 | Server name disclosed in headers | LOW | Fingerprinting | `server/server.go:70, 112` | ✅ Resolved | +| SEC-008 | Unsanitized paths in log output | LOW | Log Injection | `handler/middleware.go:113–115` | ✅ Resolved | +| SEC-009 | Deprecated `PreferServerCipherSuites` | LOW | Cryptography | `server/server.go:93` | ✅ Resolved | +| SEC-010 | Template execution error silently discarded | LOW | Error Handling | `handler/dirlist.go:191` | ✅ Resolved | +| SEC-011 | Large files read entirely into memory | LOW | Resource Exhaustion | `handler/file.go:338–377` | ✅ Resolved | +| SEC-012 | CORS wildcard `Vary` header note | INFO | CORS | `security/security.go:313` | ✅ Resolved | +| SEC-013 | ETag truncated to 64 bits | INFO | Cryptography | `handler/file.go:480–483` | ✅ Resolved | +| SEC-014 | `MaxRequestBodySize: 0` uses fasthttp default | INFO | Misconfiguration | `server/server.go:74` | ✅ Resolved | +| SEC-015 | No built-in rate limiting | INFO | Resource Exhaustion | Architectural | ✅ Resolved | +| SEC-016 | Preload walker doesn't validate symlink targets | INFO | Access Control | `cache/preload.go:74–158` | ✅ Resolved | + +--- + +## Detailed Findings + +### SEC-001: Unbounded Path Validation Cache (Denial of Service) + +| Attribute | Value | +| ----------- | ------------------------------------------------------ | +| **Severity**| **HIGH** | +| **Status** | ✅ **Resolved** — Replaced `sync.Map` with `hashicorp/golang-lru/v2` bounded LRU cache (10,000 entries). `NewPathCache(maxEntries int)` constructor added. | +| **CWE** | CWE-400 (Uncontrolled Resource Consumption) | +| **OWASP** | A05:2021 — Security Misconfiguration | +| **File** | `internal/security/security.go:49–70` | + +#### Description + +The `PathCache` struct wraps a bare `sync.Map` with no upper bound on the number of entries. Every unique URL path that passes `PathSafe` validation is unconditionally cached (line 304 of `security.go`). Because `PathSafe` successfully validates *non-existent* file paths (they pass the prefix check and return the unresolved candidate at line 165), an attacker doesn't even need to target real files — any fabricated path like `/aaa`, `/aab`, `/aac`, … will be validated, cached, and never evicted. + +#### Evidence + +```go +// security.go:49-51 — no size limit declared +type PathCache struct { + m sync.Map // urlPath (string) -> safePath (string) +} + +// security.go:68-70 — unconditional store, no eviction +func (pc *PathCache) Store(urlPath, safePath string) { + pc.m.Store(urlPath, safePath) +} + +// security.go:302-305 — stored on every cache miss +if pathCache != nil { + pathCache.Store(urlPath, safePath) +} +``` + +#### Attack Scenario + +1. Attacker scripts HTTP requests to unique, non-existent paths: `GET /rand_000001`, `GET /rand_000002`, …, `GET /rand_99999999`. +2. Each path passes `PathSafe` (it's a valid path that simply doesn't exist on disk). +3. Each path is stored in `sync.Map` — two strings (URL path + resolved filesystem path) per entry. +4. With ~100-byte average key+value per entry, 100 million requests consume ~10 GB of RAM. +5. The `sync.Map` has no eviction, no TTL, no maximum size. Memory grows monotonically until OOM kill. +6. The `Flush()` method (line 74) is only called on SIGHUP — not automatically. + +#### Recommendation + +Replace `sync.Map` with a bounded LRU cache (the project already depends on `hashicorp/golang-lru/v2`), or only cache paths for files that actually exist on disk: + +```go +// Option A: Bounded LRU +import lru "github.com/hashicorp/golang-lru/v2" + +type PathCache struct { + m *lru.Cache[string, string] +} + +func NewPathCache(maxEntries int) *PathCache { + c, _ := lru.New[string, string](maxEntries) // e.g., 65536 + return &PathCache{m: c} +} + +// Option B: Only cache existing files (in Middleware, after PathSafe) +if pathCache != nil { + if _, err := os.Stat(safePath); err == nil { + pathCache.Store(urlPath, safePath) + } +} +``` + +--- + +### SEC-002: Shipped `config.toml` Weakens Code Defaults + +| Attribute | Value | +| ----------- | ------------------------------------------------------ | +| **Severity**| MEDIUM | +| **Status** | ✅ **Resolved** — `config.toml` (gitignored, local only) updated with secure defaults. Tracked `config.toml.example` already had correct values. | +| **CWE** | CWE-1188 (Insecure Default Initialization of Resource) | +| **OWASP** | A05:2021 — Security Misconfiguration | +| **File** | `config.toml:28–38` | + +#### Description + +The code in `config.go:147–178` sets strong security defaults (`EnableETags = true`, `CSP = "default-src 'self'"`, `ReferrerPolicy = "strict-origin-when-cross-origin"`, `PermissionsPolicy = "geolocation=(), microphone=(), camera=()"`, `HSTSMaxAge = 31536000`). However, the shipped `config.toml` overrides several of these with weaker values. + +#### Evidence + +```toml +# config.toml:33 — disables ETag generation +enable_etags = false + +# config.toml:38 — empties CSP entirely +csp = "" + +# config.toml — MISSING these keys entirely (reset to zero-values): +# referrer_policy = "" <- code default: "strict-origin-when-cross-origin" +# permissions_policy = "" <- code default: "geolocation=(), microphone=(), camera=()" +# hsts_max_age = 0 <- code default: 31536000 +``` + +Because `toml.DecodeFile` merges into the struct *after* `applyDefaults` runs (config.go:131–138), any key present in the TOML file replaces the secure code default. Keys absent from the TOML get reset to Go zero values. + +#### Attack Scenario + +1. Operator deploys with the shipped `config.toml` without reviewing every security field. +2. CSP is empty — no Content-Security-Policy header — XSS payloads injected via user-uploaded HTML files execute freely. +3. ETags disabled — clients can't use `If-None-Match` for conditional requests. +4. No Referrer-Policy — browser uses default (leaks full URL to third parties). +5. No Permissions-Policy — embedded iframes can request geolocation, camera, microphone. +6. No HSTS — first-visit connections on HTTP are not upgraded, enabling SSL-stripping attacks. + +#### Recommendation + +Update `config.toml` to include all security defaults matching the code: + +```toml +[headers] +enable_etags = true + +[security] +block_dotfiles = true +directory_listing = false +cors_origins = [] +csp = "default-src 'self'" +referrer_policy = "strict-origin-when-cross-origin" +permissions_policy = "geolocation=(), microphone=(), camera=()" +hsts_max_age = 31536000 +hsts_include_subdomains = false +``` + +--- + +### SEC-003: Full Stack Trace Logged on Panic in Production + +| Attribute | Value | +| ----------- | ------------------------------------------------------------ | +| **Severity**| MEDIUM | +| **Status** | ✅ **Resolved** — Stack traces now only logged when `STATIC_DEBUG=1` env var is set. Default: panic value only. | +| **CWE** | CWE-209 (Error Message Containing Sensitive Information) | +| **OWASP** | A04:2021 — Insecure Design | +| **File** | `internal/handler/middleware.go:121–132` | + +#### Description + +The `recoveryMiddleware` calls `debug.Stack()` on every panic and logs the full Go stack trace. This trace includes absolute file paths, function names, goroutine IDs, and line numbers — information that aids an attacker in understanding the server's internals. While the stack trace is NOT sent to the client (only "Internal Server Error" is returned), it is an information disclosure risk in logging pipelines. + +#### Evidence + +```go +// middleware.go:121-132 +func recoveryMiddleware(next fasthttp.RequestHandler) fasthttp.RequestHandler { + return func(ctx *fasthttp.RequestCtx) { + defer func() { + if rec := recover(); rec != nil { + stack := debug.Stack() + log.Printf("PANIC recovered: %v\n%s", rec, stack) + ctx.Error("Internal Server Error", fasthttp.StatusInternalServerError) + } + }() + next(ctx) + } +} +``` + +#### Attack Scenario + +1. Attacker finds a way to trigger a panic (e.g., a malformed Range header causing a slice-bounds-out-of-range). +2. Full stack trace is written to stdout/stderr, which may be forwarded to a centralized logging system. +3. If logs are accessible to a broader team or leak through a log aggregation UI, the stack trace reveals internal file structure, function names, and Go version/module paths. + +#### Recommendation + +Make stack trace logging configurable, defaulting to a truncated version in production: + +```go +func recoveryMiddleware(next fasthttp.RequestHandler, verbose bool) fasthttp.RequestHandler { + return func(ctx *fasthttp.RequestCtx) { + defer func() { + if rec := recover(); rec != nil { + if verbose { + log.Printf("PANIC recovered: %v\n%s", rec, debug.Stack()) + } else { + log.Printf("PANIC recovered: %v", rec) + } + ctx.Error("Internal Server Error", fasthttp.StatusInternalServerError) + } + }() + next(ctx) + } +} +``` + +--- + +### SEC-004: Static Multipart Range Boundary Enables Server Fingerprinting + +| Attribute | Value | +| ----------- | -------------------------------------------------- | +| **Severity**| MEDIUM | +| **Status** | ✅ **Resolved** — Boundary now generated per-response using `crypto/rand` (16 random bytes → hex). | +| **CWE** | CWE-200 (Exposure of Sensitive Information) | +| **OWASP** | A05:2021 — Security Misconfiguration | +| **File** | `internal/handler/file.go:615` and `file.go:659` | + +#### Description + +Multi-range responses use a hardcoded boundary string `"static_web_range_boundary"`. This same string appears in two separate functions (`serveRange` and `serveLargeFileRange`). This boundary is globally constant across all responses and all server instances, uniquely identifying the server software. + +#### Evidence + +```go +// file.go:615 +boundary := "static_web_range_boundary" + +// file.go:659 +boundary := "static_web_range_boundary" +``` + +#### Attack Scenario + +1. Attacker sends a multi-range request: `Range: bytes=0-0,1-1`. +2. Response contains `Content-Type: multipart/byteranges; boundary=static_web_range_boundary`. +3. This uniquely identifies the server software as "static-web" even if the `Server` header is stripped by a reverse proxy. +4. The static boundary also has a theoretical MIME confusion risk if an attacker can control file content containing the exact boundary string. + +#### Recommendation + +Generate a random boundary per response: + +```go +import "crypto/rand" + +func randomBoundary() string { + var buf [16]byte + _, _ = rand.Read(buf[:]) + return hex.EncodeToString(buf[:]) +} + +// Usage: +boundary := randomBoundary() +``` + +--- + +### SEC-005: No Upper Bound on On-The-Fly Gzip Compression Body Size + +| Attribute | Value | +| ----------- | ----------------------------------------------- | +| **Severity**| MEDIUM | +| **Status** | ✅ **Resolved** — Added `MaxCompressSize` config field (default 10 MB). Bodies exceeding limit skip on-the-fly compression. Env: `STATIC_COMPRESSION_MAX_COMPRESS_SIZE`. | +| **CWE** | CWE-400 (Uncontrolled Resource Consumption) | +| **OWASP** | A05:2021 — Security Misconfiguration | +| **File** | `internal/compress/compress.go:170–187` | + +#### Description + +The compression middleware checks `len(body) < cfg.MinSize` (minimum threshold) but has no *maximum* threshold. If a large compressible response bypasses the file cache (e.g., `serveLargeFile` serving a 500 MB HTML file), the entire body is gzip-compressed in memory. + +#### Evidence + +```go +// compress.go:170-187 +body := ctx.Response.Body() +if len(body) < cfg.MinSize { + return +} +// No upper bound check here! + +buf := gzipBufPool.Get().(*bytes.Buffer) +buf.Reset() +buf.Grow(len(body) / 2) // Allocates len(body)/2 upfront + +gz := gzipWriterPool.Get().(*gzip.Writer) +gz.Reset(buf) +gz.Write(body) // Compresses entire body in memory +gz.Close() +``` + +#### Attack Scenario + +1. Operator configures `max_file_size = 1073741824` (1 GB). +2. Attacker requests a 500 MB `.html` file with `Accept-Encoding: gzip`. +3. The file handler reads 500 MB into memory. +4. Compression middleware allocates an additional ~250 MB buffer and compresses. +5. Peak memory usage for this single request: ~750 MB. A handful of concurrent requests exhaust available memory. + +#### Recommendation + +Add a maximum compression threshold: + +```go +const maxCompressSize = 10 * 1024 * 1024 // 10 MB + +body := ctx.Response.Body() +if len(body) < cfg.MinSize || len(body) > maxCompressSize { + return +} +``` + +--- + +### SEC-006: Cache Keys Not Explicitly Normalized Before Lookup + +| Attribute | Value | +| ----------- | ------------------------------------------------------------- | +| **Severity**| MEDIUM | +| **Status** | ✅ **Resolved** — Added `path.Clean("/" + urlPath)` in `CacheKeyForPath`. Trailing-slash semantics preserved via `isDir` check before cleaning. | +| **CWE** | CWE-706 (Use of Incorrectly-Resolved Name or Reference) | +| **OWASP** | A01:2021 — Broken Access Control | +| **File** | `internal/headers/headers.go:19–33` and `cache/cache.go:209` | + +#### Description + +Cache keys are derived from `ctx.Path()` (which fasthttp normalizes) and passed through `CacheKeyForPath()`, but this function does NOT call `path.Clean()`. While fasthttp does normalize most paths, edge cases with percent-encoding or unusual Unicode normalization could theoretically produce distinct cache keys that resolve to the same filesystem file. + +#### Evidence + +```go +// headers.go:19-33 +func CacheKeyForPath(urlPath, indexFile string) string { + // No path.Clean() call + if urlPath == "" || urlPath == "/" { + if indexFile == "index.html" { + return "/index.html" + } + return "/" + indexFile + } + if strings.HasSuffix(urlPath, "/") { + return urlPath + indexFile + } + return urlPath // passed through verbatim +} +``` + +#### Attack Scenario + +1. If fasthttp's URI normalization has a bypass, two different URL strings could map to the same file but produce different cache keys. +2. Request A (`/styles/app.css`) is served and cached. +3. Request B (`/styles/./app.css` — if somehow not normalized) would get a cache MISS and be re-read from disk, bypassing cache-based controls. +4. Low probability because fasthttp *does* normalize paths, but defense-in-depth argues for explicit normalization. + +#### Recommendation + +Apply `path.Clean` in the cache key function: + +```go +func CacheKeyForPath(urlPath, indexFile string) string { + urlPath = path.Clean("/" + urlPath) // explicit normalization + if indexFile == "" { + indexFile = "index.html" + } + // ... rest of function +} +``` + +--- + +### SEC-007: Server Name Disclosed in HTTP Response Headers + +| Attribute | Value | +| ----------- | ------------------------------------------------------------------ | +| **Severity**| LOW | +| **Status** | ✅ **Resolved** — Set `Name: ""` on both HTTP and HTTPS `fasthttp.Server` instances. `Server` header no longer emitted. | +| **CWE** | CWE-200 (Exposure of Sensitive Information to Unauthorized Actor) | +| **OWASP** | A05:2021 — Security Misconfiguration | +| **File** | `internal/server/server.go:70` and `server.go:112` | + +#### Description + +Both the HTTP and HTTPS `fasthttp.Server` instances set `Name: "static-web"`. Fasthttp uses this value to populate the `Server` response header on every response, identifying the software. + +#### Evidence + +```go +s.http = &fasthttp.Server{ + Handler: httpHandler, + Name: "static-web", // disclosed +} +s.https = &fasthttp.Server{ + Handler: httpsHandler, + Name: "static-web", // disclosed +} +``` + +#### Recommendation + +Set `Name` to an empty string to suppress the `Server` header, or make it configurable: + +```go +s.http = &fasthttp.Server{ + Name: "", // suppress Server header +} +``` + +--- + +### SEC-008: Unsanitized File Paths in Log Output + +| Attribute | Value | +| ----------- | --------------------------------------------------------- | +| **Severity**| LOW | +| **Status** | ✅ **Resolved** — Added `sanitizeForLog()` that replaces ASCII control chars (0x00–0x1F, 0x7F) with `\xNN` hex escapes. Applied to URI in access logging. | +| **CWE** | CWE-117 (Improper Output Neutralization for Logs) | +| **OWASP** | A09:2021 — Security Logging and Monitoring Failures | +| **File** | `internal/handler/middleware.go:113–115` and `file.go:257` | + +#### Description + +Access logs include the raw request URI without sanitizing control characters (newlines, carriage returns, ANSI escape sequences). An attacker can inject fake log lines via specially crafted URLs. + +#### Attack Scenario + +1. Attacker sends: `GET /legit%0a2026/04/11%2012:00:00%20GET%20/admin%20200%200%201us HTTP/1.1` +2. When decoded, the URI contains a newline, creating a fake log line that appears to show a successful request to `/admin`. +3. Log analysis tools or human reviewers may be misled. + +#### Recommendation + +Sanitize URIs before logging by replacing control characters: + +```go +func sanitizeForLog(s string) string { + return strings.Map(func(r rune) rune { + if r < 0x20 || r == 0x7f { + return '?' + } + return r + }, s) +} + +uri := sanitizeForLog(string(ctx.RequestURI())) +``` + +--- + +### SEC-009: Deprecated `PreferServerCipherSuites` TLS Field + +| Attribute | Value | +| ----------- | ------------------------------------------------------ | +| **Severity**| LOW | +| **Status** | ✅ **Resolved** — Removed `PreferServerCipherSuites: true`. Added explanatory comment noting Go ≥1.21 manages cipher order automatically. | +| **CWE** | CWE-327 (Use of a Broken or Risky Cryptographic Algorithm) | +| **OWASP** | A02:2021 — Cryptographic Failures | +| **File** | `internal/server/server.go:93` | + +#### Description + +The TLS configuration sets `PreferServerCipherSuites: true`, which has been deprecated since Go 1.17 and is a no-op since Go 1.21. The cipher suite selection itself is excellent (all AEAD ciphers, no CBC, no RSA key exchange). This is purely a code hygiene issue. + +#### Recommendation + +Remove the deprecated field: + +```go +tlsCfg := &tls.Config{ + MinVersion: tls.VersionTLS12, + CurvePreferences: []tls.CurveID{ + tls.X25519, + tls.CurveP256, + }, + CipherSuites: []uint16{ + // ... same excellent suite list + }, + // PreferServerCipherSuites removed -- Go >=1.21 always prefers server order +} +``` + +--- + +### SEC-010: Template Execution Error Silently Discarded + +| Attribute | Value | +| ----------- | -------------------------------------------------- | +| **Severity**| LOW | +| **Status** | ✅ **Resolved** — Template error now checked; returns 500 Internal Server Error with log message on failure. | +| **CWE** | CWE-755 (Improper Handling of Exceptional Conditions) | +| **OWASP** | A04:2021 — Insecure Design | +| **File** | `internal/handler/dirlist.go:191` | + +#### Description + +The directory listing template execution assigns the error to the blank identifier `_`. If the template fails to render, the client receives a 200 OK with an empty or partial HTML body and no indication of failure. + +#### Evidence + +```go +var buf bytes.Buffer +_ = dirListTemplate.Execute(&buf, data) +ctx.SetBody(buf.Bytes()) +``` + +#### Recommendation + +Handle the error and return 500: + +```go +var buf bytes.Buffer +if err := dirListTemplate.Execute(&buf, data); err != nil { + log.Printf("handler: directory listing template error: %v", err) + ctx.Error("Internal Server Error", fasthttp.StatusInternalServerError) + return +} +ctx.SetBody(buf.Bytes()) +``` + +--- + +### SEC-011: Large Files Read Entirely Into Memory + +| Attribute | Value | +| ----------- | ----------------------------------------------- | +| **Severity**| LOW | +| **Status** | ✅ **Resolved** — Added `MaxServeFileSize` config field (default 1 GB). Files exceeding limit receive 413 Payload Too Large. Env: `STATIC_FILES_MAX_SERVE_FILE_SIZE`. | +| **CWE** | CWE-770 (Allocation of Resources Without Limits or Throttling) | +| **OWASP** | A05:2021 — Security Misconfiguration | +| **File** | `internal/handler/file.go:338–377` | + +#### Description + +The `serveLargeFile` function reads the entire file into memory via `io.ReadAll`. For very large files, this can consume significant RAM per concurrent request. This is a known limitation of fasthttp's buffered response model. + +#### Recommendation + +1. Document the constraint — warn operators that `MaxFileSize` also implicitly limits the maximum servable file size before memory pressure. +2. Add a hard maximum: + +```go +const absoluteMaxFileSize = 512 * 1024 * 1024 // 512 MB + +if info.Size() > absoluteMaxFileSize { + ctx.Error("File too large", fasthttp.StatusRequestEntityTooLarge) + return +} +``` + +3. Consider `net/http` for a future streaming path. + +--- + +### SEC-012: CORS Wildcard Does Not Set `Vary: Origin` + +| Attribute | Value | +| ----------- | ------------------------- | +| **Severity**| INFO | +| **Status** | ✅ **Resolved** — Expanded inline comment in `security.go` explaining why `Vary: Origin` is NOT set with wildcard (per Fetch spec). | +| **CWE** | N/A (informational) | +| **File** | `internal/security/security.go:313–316` | + +This is actually **correct** per the Fetch specification. A literal `*` response is not origin-dependent, so `Vary: Origin` would needlessly fragment proxy caches. No code change needed. + +--- + +### SEC-013: ETag Truncation to 16 Hex Characters + +| Attribute | Value | +| ----------- | ------------------------- | +| **Severity**| INFO | +| **Status** | ✅ **Resolved** — Expanded `computeETag` doc comment with collision analysis and rationale for 64-bit truncation. | +| **CWE** | CWE-328 (Use of Weak Hash) | +| **File** | `internal/handler/file.go:480–483` | + +ETags are computed as `sha256(data)[:8]` (64 bits). For cache validation purposes, the ~10^19 possible values yield negligible collision probability at realistic file counts. ETags are not used for authentication. No change needed — appropriate trade-off for header size. + +--- + +### SEC-014: `MaxRequestBodySize: 0` Relies on Fasthttp Default + +| Attribute | Value | +| ----------- | ------------------------- | +| **Severity**| INFO | +| **Status** | ✅ **Resolved** — Set `MaxRequestBodySize: 1024` (1 KB) on both HTTP and HTTPS servers. Static file server needs no request body. | +| **CWE** | CWE-770 (Allocation of Resources Without Limits or Throttling) | +| **File** | `internal/server/server.go:74` | + +In fasthttp, `0` means "use the default" (4 MB). For a static file server that should never receive request bodies, consider setting an explicit small value: + +```go +MaxRequestBodySize: 1024, // 1 KB -- static server needs no request body +``` + +--- + +### SEC-015: No Rate Limiting or Request Throttling + +| Attribute | Value | +| ----------- | ------------------------- | +| **Severity**| INFO | +| **Status** | ✅ **Resolved** — Added `MaxConnsPerIP` config field (default 0 = unlimited) wired to `fasthttp.Server.MaxConnsPerIP`. Env: `STATIC_SERVER_MAX_CONNS_PER_IP`. | +| **CWE** | CWE-770 (Allocation of Resources Without Limits or Throttling) | +| **File** | N/A (architectural) | + +No built-in rate limiting. This is typical for a static file server (usually handled by a reverse proxy or CDN). Consider adding `MaxConnsPerIP` via fasthttp's built-in support for direct-exposure deployments. + +--- + +### SEC-016: Preload Walker Does Not Validate Symlink Targets + +| Attribute | Value | +| ----------- | ------------------------- | +| **Severity**| INFO | +| **Status** | ✅ **Resolved** — Added symlink detection (`d.Type()&os.ModeSymlink`), target resolution via `filepath.EvalSymlinks`, and validation that target stays within `absRoot`. | +| **CWE** | CWE-59 (Improper Link Resolution Before File Access) | +| **File** | `internal/cache/preload.go:74–158` | + +#### Description + +The `Preload` function uses `filepath.WalkDir` to traverse the root directory and load files into cache. Files that are symlinks are read via `os.ReadFile(fpath)`, which follows the symlink without verifying that the target is still within the root directory. The request-time path via `PathSafe` **does** perform symlink resolution and blocks this — the vulnerability is only during preload at startup. + +#### Recommendation + +Add symlink target validation in the preload walker: + +```go +realPath, err := filepath.EvalSymlinks(fpath) +if err != nil { + stats.Skipped++ + return nil +} +if !strings.HasPrefix(realPath, absRoot+string(filepath.Separator)) && realPath != absRoot { + stats.Skipped++ + return nil +} +``` + +--- + +## Positive Security Observations + +The following practices demonstrate strong security awareness and are worth preserving: + +### 1. Multi-Layer Path Traversal Prevention +**File:** `internal/security/security.go:120–187` + +The `PathSafe` function implements defense-in-depth with 5 sequential checks: null byte rejection, `path.Clean` normalization, `filepath.Join` with prefix verification, `filepath.EvalSymlinks` with re-verification, and dotfile component blocking. Textbook path traversal prevention. + +### 2. HTTP Method Whitelist (GET/HEAD/OPTIONS Only) +**File:** `internal/security/security.go:272–275` + +Prevents TRACE (XST attacks), PUT/POST/DELETE, and any other method. Correct for a static file server. + +### 3. XSS-Safe Directory Listing via `html/template` +**File:** `internal/handler/dirlist.go:40` + +Using `html/template` (not `text/template`) ensures all interpolated values are automatically HTML-escaped. + +### 4. CORS Wildcard Does Not Reflect Origin +**File:** `internal/security/security.go:313–316` + +Emits a literal `*` rather than reflecting the `Origin` header. Prevents credential-based cross-origin attacks. + +### 5. CI/CD Actions Pinned to Commit SHAs +**Files:** `.github/workflows/ci.yml`, `.github/workflows/release.yml` + +GitHub Actions are pinned to specific commit SHAs rather than mutable tags, preventing supply-chain attacks. + +### 6. `govulncheck` in CI Pipeline +Proactive vulnerability scanning against the Go vulnerability database on every CI run. + +### 7. Race Detector in Tests +Tests run with `-race`, detecting data races in concurrent code (`sync.Map`, `sync.Pool`, atomics, goroutines). + +### 8. Zero Hardcoded Secrets +No API keys, tokens, passwords, or credentials found in any source file. All sensitive configuration loaded from config/environment. + +### 9. Sidecar Path Validation +**File:** `internal/handler/file.go:509–552` + +`ValidateSidecarPath` ensures `.gz`, `.br`, `.zst` sidecar files haven't escaped the root directory via symlink. + +### 10. Robust TLS Configuration +**File:** `internal/server/server.go:79–94` + +TLS 1.2+ minimum, only AEAD cipher suites (GCM, ChaCha20-Poly1305), modern curve preferences (X25519, P-256), HTTP-to-HTTPS redirect, and HSTS support. + +### 11. Security Headers on Error Responses +**File:** `internal/security/security.go:248–260` + +Security headers are set before calling the inner handler, ensuring even 400/403/404/405 responses carry X-Content-Type-Options, X-Frame-Options, CSP, etc. + +### 12. Custom 404 Page Path Validated via PathSafe +**File:** `internal/handler/file.go:450` + +Even the custom 404 page path from configuration is validated through `PathSafe`, preventing config-driven path injection. + +--- + +## Prioritized Remediation Plan + +| Priority | Finding | Severity | Effort | Impact | Status | +| -------- | ------- | -------- | ------ | ------ | ------ | +| **P1** | SEC-001: Bound the PathCache with LRU | HIGH | Low (~30 LOC) | Eliminates DoS vector | ✅ Done | +| **P2** | SEC-002: Align `config.toml` with secure code defaults | MEDIUM | Low (~10 lines TOML) | Restores secure-by-default | ✅ Done | +| **P3** | SEC-005: Add `maxCompressSize` threshold | MEDIUM | Low (~3 LOC) | Prevents memory exhaustion | ✅ Done | +| **P4** | SEC-004: Randomize multipart boundary | MEDIUM | Low (~10 LOC) | Eliminates fingerprinting | ✅ Done | +| **P5** | SEC-006: `path.Clean` in cache key function | MEDIUM | Low (~2 LOC) | Defense-in-depth | ✅ Done | +| **P6** | SEC-003: Make stack trace logging configurable | MEDIUM | Low (~10 LOC) | Reduces info leakage | ✅ Done | +| **P7** | SEC-007: Suppress server name header | LOW | Trivial (1 LOC) | Reduces fingerprinting | ✅ Done | +| **P8** | SEC-008: Sanitize log output | LOW | Low (~15 LOC) | Prevents log forgery | ✅ Done | +| **P9** | SEC-009: Remove deprecated TLS field | LOW | Trivial (1 LOC) | Code hygiene | ✅ Done | +| **P10** | SEC-010: Handle template execution errors | LOW | Low (~5 LOC) | Better error handling | ✅ Done | +| **P11** | SEC-011: Document large file memory constraint | LOW | Medium (docs + config) | Operator awareness | ✅ Done | +| **P12** | SEC-012: Expand CORS wildcard Vary comment | INFO | Trivial | Documentation | ✅ Done | +| **P13** | SEC-013: Expand ETag truncation doc comment | INFO | Trivial | Documentation | ✅ Done | +| **P14** | SEC-014: Set explicit MaxRequestBodySize | INFO | Trivial (1 LOC) | Reduces attack surface | ✅ Done | +| **P15** | SEC-015: Add MaxConnsPerIP config | INFO | Low (~15 LOC) | DoS mitigation option | ✅ Done | +| **P16** | SEC-016: Validate symlink targets in preload | INFO | Low (~10 LOC) | Closes preload escape | ✅ Done | + +--- + +*Report generated by Kai security audit pipeline. All findings verified against source code as of commit `fcfe429`. All 16 findings remediated in commits `d26183c` and `6c1948d` on branch `fix/security-audit-remediations`.* diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 4d620b8..50aa1bb 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -117,11 +117,13 @@ read_timeout = "10s" # full read deadline (covers headers; Slowloris write_timeout = "10s" idle_timeout = "75s" shutdown_timeout = "15s" # graceful drain window on SIGTERM/SIGINT +# max_conns_per_ip = 0 # max concurrent connections per IP (0 = unlimited) [files] root = "./public" # directory to serve index = "index.html" # index file for directory requests (e.g. GET /) not_found = "404.html" # custom 404 page, relative to root (optional) +# max_serve_file_size = 1073741824 # files > 1 GB get 413 (0 = no limit) [cache] enabled = true @@ -136,6 +138,7 @@ enabled = true min_size = 1024 # don't compress responses smaller than 1 KB level = 5 # gzip level 1 (fastest) – 9 (best) precompressed = true # serve .gz / .br / .zst sidecar files when available +# max_compress_size = 10485760 # skip on-the-fly gzip for bodies > 10 MB (0 = no limit) [headers] immutable_pattern = "" # glob for fingerprinted assets → Cache-Control: immutable @@ -169,9 +172,11 @@ Every config field can also be set via an environment variable, which takes prec | `STATIC_SERVER_WRITE_TIMEOUT` | `server.write_timeout` | | `STATIC_SERVER_IDLE_TIMEOUT` | `server.idle_timeout` | | `STATIC_SERVER_SHUTDOWN_TIMEOUT` | `server.shutdown_timeout` | +| `STATIC_SERVER_MAX_CONNS_PER_IP` | `server.max_conns_per_ip` | | `STATIC_FILES_ROOT` | `files.root` | | `STATIC_FILES_INDEX` | `files.index` | | `STATIC_FILES_NOT_FOUND` | `files.not_found` | +| `STATIC_FILES_MAX_SERVE_FILE_SIZE` | `files.max_serve_file_size` | | `STATIC_CACHE_ENABLED` | `cache.enabled` | | `STATIC_CACHE_PRELOAD` | `cache.preload` | | `STATIC_CACHE_MAX_BYTES` | `cache.max_bytes` | @@ -181,6 +186,7 @@ Every config field can also be set via an environment variable, which takes prec | `STATIC_COMPRESSION_ENABLED` | `compression.enabled` | | `STATIC_COMPRESSION_MIN_SIZE` | `compression.min_size` | | `STATIC_COMPRESSION_LEVEL` | `compression.level` | +| `STATIC_COMPRESSION_MAX_COMPRESS_SIZE` | `compression.max_compress_size` | | `STATIC_HEADERS_ENABLE_ETAGS` | `headers.enable_etags` | | `STATIC_SECURITY_BLOCK_DOTFILES` | `security.block_dotfiles` | | `STATIC_SECURITY_CSP` | `security.csp` | @@ -644,10 +650,11 @@ STATIC_CACHE_PRELOAD=true STATIC_CACHE_GC_PERCENT=400 ./bin/static-web ### What preloading does 1. At startup, walks every file under `files.root`. -2. Files smaller than `max_file_size` are read into the LRU cache. -3. Pre-formatted `Content-Type` and `Content-Length` response headers are computed once per file. -4. The path-safety cache (`sync.Map`) is pre-warmed — the first request for any preloaded file skips `filepath.EvalSymlinks`. -5. Preload statistics (file count, total bytes, duration) are logged at startup. +2. Symlink targets are validated — symlinks pointing outside root are skipped. +3. Files smaller than `max_file_size` are read into the LRU cache. +4. Pre-formatted `Content-Type` and `Content-Length` response headers are computed once per file. +5. The path-safety cache (bounded LRU) is pre-warmed — the first request for any preloaded file skips `filepath.EvalSymlinks`. +6. Preload statistics (file count, total bytes, duration) are logged at startup. ### When to use preload @@ -783,6 +790,17 @@ The most common causes: The server only accepts `GET`, `HEAD`, and `OPTIONS`. Any other method (POST, PUT, DELETE, PATCH, TRACE, etc.) is rejected with `405`. This is intentional — it's a static file server, not an API. If your browser is sending a `POST` request, check your HTML form actions and JavaScript fetch calls. +### `413 Payload Too Large` + +A file exceeds the `max_serve_file_size` limit (default 1 GB). Increase the limit in config: + +```toml +[files] +max_serve_file_size = 2147483648 # 2 GB +``` + +Or set to 0 to disable the limit entirely. This also applies via the `STATIC_FILES_MAX_SERVE_FILE_SIZE` environment variable. + ### Files are stale after a deploy The in-memory cache serves files from memory after the first request (or immediately if `preload = true`). After deploying new files to disk, flush both the file cache and the path-safety cache: diff --git a/cmd/static-web/main.go b/cmd/static-web/main.go index 692df00..106345c 100644 --- a/cmd/static-web/main.go +++ b/cmd/static-web/main.go @@ -207,7 +207,7 @@ func runServe(args []string) { } // Pre-warm the path cache with every URL key the file cache knows about. - pathCache = security.NewPathCache() + pathCache = security.NewPathCache(security.DefaultPathCacheSize) pathCache.PreWarm(stats.Paths, cfg.Files.Root, cfg.Security.BlockDotfiles) if !effectiveQuiet { log.Printf("path cache pre-warmed with %d entries", pathCache.Len()) diff --git a/config.toml.example b/config.toml.example index 320d561..5b2f4a1 100644 --- a/config.toml.example +++ b/config.toml.example @@ -32,6 +32,11 @@ idle_timeout = "75s" # How long to wait for in-flight requests to complete during graceful shutdown. shutdown_timeout = "15s" +# Maximum concurrent connections allowed from a single IP address. +# 0 means unlimited (default). Set to a positive value (e.g. 100) to limit +# per-IP connection count and mitigate connection exhaustion attacks. +# max_conns_per_ip = 0 + [files] # Directory to serve files from. root = "./public" @@ -42,6 +47,10 @@ index = "index.html" # Custom 404 page path, relative to root. Leave empty for the built-in 404. not_found = "404.html" +# Maximum file size (bytes) that will be served. Files exceeding this limit +# receive a 413 Payload Too Large response. Default: 1 GB. Set to 0 to disable. +# max_serve_file_size = 1073741824 + [cache] # Enable or disable the in-memory LRU file cache. enabled = true @@ -69,6 +78,11 @@ level = 5 # Encoding priority: br > zstd > gzip precompressed = true +# Maximum response body size (bytes) eligible for on-the-fly compression. +# Responses larger than this are sent uncompressed to avoid excessive memory use. +# Default: 10 MB (10485760). Set to 0 to disable the limit. +# max_compress_size = 10485760 + [headers] # Glob pattern for fingerprinted/immutable assets (gets "Cache-Control: immutable"). # Example: "**-[0-9a-f]*.{js,css}" diff --git a/docs/index.html b/docs/index.html index ec949ce..97ab19a 100644 --- a/docs/index.html +++ b/docs/index.html @@ -319,14 +319,14 @@

TLS 1.2 / 1.3

Security Hardened

- Path traversal prevention, dotfile blocking, CSP, HSTS, Referrer-Policy, Permissions-Policy — set on + Path traversal prevention, dotfile blocking, bounded LRU path-safety cache, log injection sanitisation, CSP, HSTS, Referrer-Policy, Permissions-Policy — set on every response.

Smart Caching

-

Byte-accurate LRU cache with startup preloading, configurable max size, per-file size cap, optional TTL expiry, optional ETag (disable via --no-etag), and live flush via SIGHUP without downtime.

+

Byte-accurate LRU cache with startup preloading, configurable max size, per-file size cap, optional TTL expiry, optional ETag (disable via --no-etag), bounded path-safety LRU, and live flush via SIGHUP without downtime.

@@ -478,7 +478,7 @@

Getting Started

[headers] static_max_age = 3600 # 1 hour for static assets html_max_age = 0 # no-cache for HTML -enable_etags = false # disable for max throughput +enable_etags = true # disable with --no-etag for max throughput immutable_pattern = "*.chunk.js" # fingerprinted assets only [security] @@ -518,7 +518,7 @@

Getting Started

ENV STATIC_FILES_ROOT="/public" ENV STATIC_CACHE_ENABLED="true" ENV STATIC_CACHE_PRELOAD="true" -ENV STATIC_HEADERS_ENABLE_ETAGS="false" +ENV STATIC_HEADERS_ENABLE_ETAGS="true" ENV STATIC_COMPRESSION_ENABLED="true" EXPOSE 8080 @@ -554,7 +554,7 @@

HTTP Request

Recovery Middleware

-

Panic → 500, log stack trace

+

Panic → 500, sanitised log (stack traces only with STATIC_DEBUG=1)

@@ -838,6 +843,11 @@

Configuration Reference

— Custom 404 page (relative to root) + + max_serve_file_size + 1073741824 + Max file size to serve (bytes; 0 = unlimited; default 1 GB) + @@ -932,6 +942,11 @@

Configuration Reference

true Serve .gz/.br sidecar files when available + + max_compress_size + 10485760 + Max body size for on-the-fly gzip (bytes; default 10 MB) + @@ -1156,7 +1171,8 @@

DoS Mitigations

ReadTimeout10s
WriteTimeout10s
IdleTimeout75s (keep-alive)
-
MaxRequestBodySize0 (no body accepted)
+
MaxRequestBodySize1024 bytes (static server needs no large bodies)
+
MaxConnsPerIPConfigurable (default unlimited)
Method whitelistGET, HEAD, OPTIONS only

@@ -1193,13 +1209,14 @@

CORS Policy

diff --git a/go.mod b/go.mod index 4d8a872..f949583 100644 --- a/go.mod +++ b/go.mod @@ -5,11 +5,11 @@ go 1.26 require ( github.com/BurntSushi/toml v1.6.0 github.com/hashicorp/golang-lru/v2 v2.0.7 - github.com/klauspost/compress v1.18.4 - github.com/valyala/fasthttp v1.69.0 + github.com/klauspost/compress v1.18.5 + github.com/valyala/fasthttp v1.70.0 ) require ( - github.com/andybalholm/brotli v1.2.0 // indirect + github.com/andybalholm/brotli v1.2.1 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect ) diff --git a/go.sum b/go.sum index 19fa8d4..09ffe12 100644 --- a/go.sum +++ b/go.sum @@ -1,14 +1,14 @@ github.com/BurntSushi/toml v1.6.0 h1:dRaEfpa2VI55EwlIW72hMRHdWouJeRF7TPYhI+AUQjk= github.com/BurntSushi/toml v1.6.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= -github.com/andybalholm/brotli v1.2.0 h1:ukwgCxwYrmACq68yiUqwIWnGY0cTPox/M94sVwToPjQ= -github.com/andybalholm/brotli v1.2.0/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= +github.com/andybalholm/brotli v1.2.1 h1:R+f5xP285VArJDRgowrfb9DqL18yVK0gKAW/F+eTWro= +github.com/andybalholm/brotli v1.2.1/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= -github.com/klauspost/compress v1.18.4 h1:RPhnKRAQ4Fh8zU2FY/6ZFDwTVTxgJ/EMydqSTzE9a2c= -github.com/klauspost/compress v1.18.4/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= +github.com/klauspost/compress v1.18.5 h1:/h1gH5Ce+VWNLSWqPzOVn6XBO+vJbCNGvjoaGBFW2IE= +github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= -github.com/valyala/fasthttp v1.69.0 h1:fNLLESD2SooWeh2cidsuFtOcrEi4uB4m1mPrkJMZyVI= -github.com/valyala/fasthttp v1.69.0/go.mod h1:4wA4PfAraPlAsJ5jMSqCE2ug5tqUPwKXxVj8oNECGcw= +github.com/valyala/fasthttp v1.70.0 h1:LAhMGcWk13QZWm85+eg8ZBNbrq5mnkWFGbHMUJHIdXA= +github.com/valyala/fasthttp v1.70.0/go.mod h1:oDZEHHkJ/Buyklg6uURmYs19442zFSnCIfX3j1FY3pE= github.com/xyproto/randomstring v1.0.5 h1:YtlWPoRdgMu3NZtP45drfy1GKoojuR7hmRcnhZqKjWU= github.com/xyproto/randomstring v1.0.5/go.mod h1:rgmS5DeNXLivK7YprL0pY+lTuhNQW3iGxZ18UQApw/E= diff --git a/internal/cache/preload.go b/internal/cache/preload.go index 25fd10e..3507c8d 100644 --- a/internal/cache/preload.go +++ b/internal/cache/preload.go @@ -76,6 +76,26 @@ func (c *Cache) Preload(root string, cfg PreloadConfig) PreloadStats { return nil } + // SEC-016: Validate symlink targets. If the entry is a symlink, + // resolve it and verify the target is still inside the root directory. + // This prevents preloading files that symlink outside the served root. + if d.Type()&os.ModeSymlink != 0 { + target, err := filepath.EvalSymlinks(fpath) + if err != nil { + stats.Skipped++ + return nil + } + rootWithSep := absRoot + if !strings.HasSuffix(rootWithSep, string(filepath.Separator)) { + rootWithSep += string(filepath.Separator) + } + if target != absRoot && !strings.HasPrefix(target, rootWithSep) { + stats.Skipped++ // symlink escapes root + return nil + } + fpath = target + } + // Skip dotfile components. if cfg.BlockDotfiles { rel, relErr := filepath.Rel(absRoot, fpath) diff --git a/internal/compress/compress.go b/internal/compress/compress.go index 19882af..588c273 100644 --- a/internal/compress/compress.go +++ b/internal/compress/compress.go @@ -172,6 +172,13 @@ func Middleware(cfg *config.CompressionConfig, next fasthttp.RequestHandler) fas return } + // SEC-005: Skip on-the-fly compression for bodies that exceed the + // configured maximum. This prevents excessive memory allocation and + // CPU usage from compressing very large responses in-flight. + if cfg.MaxCompressSize > 0 && len(body) > cfg.MaxCompressSize { + return + } + // Compress the body using pooled gzip.Writer and bytes.Buffer. buf := gzipBufPool.Get().(*bytes.Buffer) buf.Reset() diff --git a/internal/compress/compress_test.go b/internal/compress/compress_test.go index 0143741..f8a2100 100644 --- a/internal/compress/compress_test.go +++ b/internal/compress/compress_test.go @@ -256,6 +256,85 @@ func TestMiddleware_SkipsBelowMinSize(t *testing.T) { } } +// TestMiddleware_MaxCompressSize verifies SEC-005: bodies exceeding +// MaxCompressSize are served uncompressed, while bodies under the limit +// are still gzipped normally. +func TestMiddleware_MaxCompressSize(t *testing.T) { + const limit = 2048 + + makeHandler := func(body string) fasthttp.RequestHandler { + return func(ctx *fasthttp.RequestCtx) { + ctx.Response.Header.Set("Content-Type", "text/html; charset=utf-8") + ctx.SetBody([]byte(body)) + } + } + + t.Run("under limit is compressed", func(t *testing.T) { + cfg := &config.CompressionConfig{Enabled: true, MinSize: 10, Level: 5, MaxCompressSize: limit} + body := strings.Repeat("A", limit-1) // 2047 bytes — just under + handler := compress.Middleware(cfg, makeHandler(body)) + ctx := newTestCtx("GET", "/", map[string]string{"Accept-Encoding": "gzip"}) + handler(ctx) + + if enc := string(ctx.Response.Header.Peek("Content-Encoding")); enc != "gzip" { + t.Errorf("Content-Encoding = %q, want gzip for body under MaxCompressSize", enc) + } + // Verify decompressed content matches. + gr, err := gzip.NewReader(bytes.NewReader(ctx.Response.Body())) + if err != nil { + t.Fatalf("gzip.NewReader: %v", err) + } + got, err := io.ReadAll(gr) + if err != nil { + t.Fatalf("io.ReadAll: %v", err) + } + if string(got) != body { + t.Error("decompressed body does not match original") + } + }) + + t.Run("over limit stays uncompressed", func(t *testing.T) { + cfg := &config.CompressionConfig{Enabled: true, MinSize: 10, Level: 5, MaxCompressSize: limit} + body := strings.Repeat("B", limit+1) // 2049 bytes — just over + handler := compress.Middleware(cfg, makeHandler(body)) + ctx := newTestCtx("GET", "/", map[string]string{"Accept-Encoding": "gzip"}) + handler(ctx) + + if enc := string(ctx.Response.Header.Peek("Content-Encoding")); enc == "gzip" { + t.Error("Content-Encoding should not be gzip for body exceeding MaxCompressSize") + } + if string(ctx.Response.Body()) != body { + t.Error("body should be served uncompressed and unmodified") + } + }) + + t.Run("exactly at limit stays uncompressed", func(t *testing.T) { + cfg := &config.CompressionConfig{Enabled: true, MinSize: 10, Level: 5, MaxCompressSize: limit} + body := strings.Repeat("C", limit) // exactly 2048 bytes — not strictly less + handler := compress.Middleware(cfg, makeHandler(body)) + ctx := newTestCtx("GET", "/", map[string]string{"Accept-Encoding": "gzip"}) + handler(ctx) + + // len(body) > cfg.MaxCompressSize is false when equal, so it IS compressed. + // This tests the boundary condition: equal means "still under the limit". + if enc := string(ctx.Response.Header.Peek("Content-Encoding")); enc != "gzip" { + t.Errorf("Content-Encoding = %q, want gzip when body size equals MaxCompressSize (not strictly over)", enc) + } + }) + + t.Run("zero disables the limit", func(t *testing.T) { + cfg := &config.CompressionConfig{Enabled: true, MinSize: 10, Level: 5, MaxCompressSize: 0} + body := strings.Repeat("D", 100_000) // 100 KB — would exceed any reasonable limit + handler := compress.Middleware(cfg, makeHandler(body)) + ctx := newTestCtx("GET", "/", map[string]string{"Accept-Encoding": "gzip"}) + handler(ctx) + + if enc := string(ctx.Response.Header.Peek("Content-Encoding")); enc != "gzip" { + t.Errorf("Content-Encoding = %q, want gzip when MaxCompressSize=0 (disabled)", enc) + } + }) +} + // TestMiddleware_SkipsAlreadyEncoded ensures pre-encoded responses are passed through. func TestMiddleware_SkipsAlreadyEncoded(t *testing.T) { cfg := &config.CompressionConfig{Enabled: true, MinSize: 1, Level: 5} diff --git a/internal/config/config.go b/internal/config/config.go index 0d12c03..046be9b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -45,6 +45,10 @@ type ServerConfig struct { IdleTimeout time.Duration `toml:"idle_timeout"` // ShutdownTimeout is how long to wait for in-flight requests during shutdown. ShutdownTimeout time.Duration `toml:"shutdown_timeout"` + // MaxConnsPerIP limits the number of concurrent connections from a single IP + // address. 0 means unlimited. Default: 0 (disabled). + // SEC-015: Rate-limiting defence against connection exhaustion attacks. + MaxConnsPerIP int `toml:"max_conns_per_ip"` } // FilesConfig holds file-serving settings. @@ -55,6 +59,12 @@ type FilesConfig struct { Index string `toml:"index"` // NotFound is the path (relative to Root) of the custom 404 page. NotFound string `toml:"not_found"` + // MaxServeFileSize is the maximum file size (in bytes) that will be served. + // Files exceeding this limit receive a 413 Payload Too Large response. + // This prevents a single request from loading an arbitrarily large file + // into memory. Default: 1 GB. Set to 0 to disable the limit. + // SEC-011: Hard upper bound for large file serving. + MaxServeFileSize int64 `toml:"max_serve_file_size"` } // CacheConfig holds in-memory cache settings. @@ -89,6 +99,12 @@ type CompressionConfig struct { Level int `toml:"level"` // Precompressed enables serving pre-compressed .gz/.br sidecar files. Default: true. Precompressed bool `toml:"precompressed"` + // MaxCompressSize is the maximum response body size (in bytes) eligible for + // on-the-fly gzip compression. Bodies exceeding this limit are served + // uncompressed to avoid excessive memory allocation and CPU usage. + // Default: 10 MB. Set to 0 to disable the limit. + // SEC-005: Bounds on-the-fly compression memory usage. + MaxCompressSize int `toml:"max_compress_size"` } // HeadersConfig controls HTTP response header settings. @@ -154,6 +170,7 @@ func applyDefaults(cfg *Config) { cfg.Files.Root = "./public" cfg.Files.Index = "index.html" + cfg.Files.MaxServeFileSize = 1024 * 1024 * 1024 // 1 GB cfg.Cache.Enabled = true cfg.Cache.MaxBytes = 256 * 1024 * 1024 // 256 MB @@ -163,6 +180,7 @@ func applyDefaults(cfg *Config) { cfg.Compression.MinSize = 1024 cfg.Compression.Level = 5 cfg.Compression.Precompressed = true + cfg.Compression.MaxCompressSize = 10 * 1024 * 1024 // 10 MB cfg.Headers.StaticMaxAge = 3600 cfg.Headers.HTMLMaxAge = 0 @@ -225,6 +243,11 @@ func applyEnvOverrides(cfg *Config) { if v := os.Getenv("STATIC_FILES_NOT_FOUND"); v != "" { cfg.Files.NotFound = v } + if v := os.Getenv("STATIC_FILES_MAX_SERVE_FILE_SIZE"); v != "" { + if n, err := strconv.ParseInt(v, 10, 64); err == nil { + cfg.Files.MaxServeFileSize = n + } + } if v := os.Getenv("STATIC_CACHE_ENABLED"); v != "" { cfg.Cache.Enabled = strings.EqualFold(v, "true") || v == "1" @@ -266,6 +289,11 @@ func applyEnvOverrides(cfg *Config) { cfg.Compression.Level = n } } + if v := os.Getenv("STATIC_COMPRESSION_MAX_COMPRESS_SIZE"); v != "" { + if n, err := strconv.Atoi(v); err == nil { + cfg.Compression.MaxCompressSize = n + } + } if v := os.Getenv("STATIC_SECURITY_BLOCK_DOTFILES"); v != "" { cfg.Security.BlockDotfiles = strings.EqualFold(v, "true") || v == "1" @@ -284,4 +312,9 @@ func applyEnvOverrides(cfg *Config) { if v := os.Getenv("STATIC_HEADERS_ENABLE_ETAGS"); v != "" { cfg.Headers.EnableETags = strings.EqualFold(v, "true") || v == "1" } + if v := os.Getenv("STATIC_SERVER_MAX_CONNS_PER_IP"); v != "" { + if n, err := strconv.Atoi(v); err == nil { + cfg.Server.MaxConnsPerIP = n + } + } } diff --git a/internal/handler/dirlist.go b/internal/handler/dirlist.go index 0e852e6..6bcce52 100644 --- a/internal/handler/dirlist.go +++ b/internal/handler/dirlist.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "html/template" + "log" "os" "sort" "strings" @@ -187,8 +188,14 @@ func (h *FileHandler) serveDirectoryListing(ctx *fasthttp.RequestCtx, absDir, ur return } // Render template to a buffer then write to ctx. + // SEC-010: Handle template execution errors instead of silently discarding. var buf bytes.Buffer - _ = dirListTemplate.Execute(&buf, data) + if err := dirListTemplate.Execute(&buf, data); err != nil { + log.Printf("handler: directory listing template error for %q: %v", urlPath, err) + ctx.SetStatusCode(fasthttp.StatusInternalServerError) + ctx.SetBodyString("Internal Server Error: failed to render directory listing") + return + } ctx.SetBody(buf.Bytes()) } diff --git a/internal/handler/file.go b/internal/handler/file.go index 3292bc9..64dacdd 100644 --- a/internal/handler/file.go +++ b/internal/handler/file.go @@ -4,13 +4,16 @@ package handler import ( "bytes" + "crypto/rand" "crypto/sha256" + "encoding/binary" "encoding/hex" "errors" "fmt" "io" "io/fs" "log" + mrandv2 "math/rand/v2" "mime" "net/http" "os" @@ -335,7 +338,20 @@ func (h *FileHandler) serveFromDisk(ctx *fasthttp.RequestCtx, absPath, urlPath s // bypassing the in-memory cache but still supporting Range requests. // The file is read into memory to avoid issues with fasthttp's lazy body // evaluation closing the file descriptor before the body is consumed. +// SEC-011: Enforces MaxServeFileSize to prevent a single request from +// loading an arbitrarily large file into memory. func (h *FileHandler) serveLargeFile(ctx *fasthttp.RequestCtx, absPath, urlPath string, info os.FileInfo) { + // SEC-011: Reject files that exceed the configured hard maximum. + if h.cfg.Files.MaxServeFileSize > 0 && info.Size() > h.cfg.Files.MaxServeFileSize { + log.Printf("handler: file %q (%d bytes) exceeds max serve size (%d bytes)", + absPath, info.Size(), h.cfg.Files.MaxServeFileSize) + ctx.Error( + fmt.Sprintf("Payload Too Large: exceeds max_serve_file_size (%d bytes)", h.cfg.Files.MaxServeFileSize), + fasthttp.StatusRequestEntityTooLarge, + ) + return + } + f, err := os.Open(absPath) if err != nil { if os.IsPermission(err) { @@ -474,7 +490,13 @@ func (h *FileHandler) handleSecurityError(ctx *fasthttp.RequestCtx, err error) { } } -// computeETag returns the first 16 hex characters of sha256(data). +// computeETag returns the first 16 hex characters of sha256(data), yielding a +// 64-bit fingerprint of the file content. +// SEC-013: The truncation to 8 bytes (64 bits) is intentional. With the birthday +// paradox, collision probability reaches 1% at ~190 million files — well beyond +// practical static-server workloads. The short ETag saves bandwidth on every +// conditional request/response. If a stronger fingerprint is ever needed, the +// full sha256 sum can be used instead. // Uses hex.EncodeToString on the first 8 bytes instead of fmt.Sprintf // to avoid formatting the full 32-byte hash and then truncating (PERF-004). func computeETag(data []byte) string { @@ -572,6 +594,24 @@ func (h *FileHandler) LoadSidecar(path string) []byte { return data } +// generateBoundary returns a random MIME multipart boundary string. +// SEC-004: Using a unique boundary per response prevents attackers from +// predicting boundary values and crafting payloads that exploit multipart +// parsing in downstream proxies or clients. +// +// If crypto/rand fails (extremely unlikely on modern kernels), the function +// falls back to math/rand/v2 (auto-seeded from crypto/rand at process start) +// and logs a warning. The boundary is never all-zeroes. +func generateBoundary() string { + var buf [16]byte + if _, err := rand.Read(buf[:]); err != nil { + log.Printf("WARNING: crypto/rand.Read failed (%v), using math/rand fallback for multipart boundary", err) + binary.NativeEndian.PutUint64(buf[:8], mrandv2.Uint64()) + binary.NativeEndian.PutUint64(buf[8:], mrandv2.Uint64()) + } + return hex.EncodeToString(buf[:]) +} + // --------------------------------------------------------------------------- // Range request handling (replacement for http.ServeContent) // --------------------------------------------------------------------------- @@ -612,7 +652,7 @@ func serveRange(ctx *fasthttp.RequestCtx, data []byte, rangeHeader string) { // Multiple ranges — use multipart/byteranges. contentType := string(ctx.Response.Header.Peek("Content-Type")) - boundary := "static_web_range_boundary" + boundary := generateBoundary() // SEC-004: random boundary per response var buf bytes.Buffer for _, r := range ranges { @@ -656,7 +696,7 @@ func serveLargeFileRange(ctx *fasthttp.RequestCtx, data []byte, size int64, rang // Multiple ranges — use multipart/byteranges. contentType := string(ctx.Response.Header.Peek("Content-Type")) - boundary := "static_web_range_boundary" + boundary := generateBoundary() // SEC-004: random boundary per response var buf bytes.Buffer for _, r := range ranges { diff --git a/internal/handler/file_test.go b/internal/handler/file_test.go index 260b191..110955b 100644 --- a/internal/handler/file_test.go +++ b/internal/handler/file_test.go @@ -417,6 +417,77 @@ func TestBuildHandler_LargeFile(t *testing.T) { } } +// TestBuildHandler_MaxServeFileSize verifies SEC-011: files exceeding +// MaxServeFileSize return 413 Payload Too Large, while files under the limit +// are served normally. +func TestBuildHandler_MaxServeFileSize(t *testing.T) { + root := t.TempDir() + + // Create two files: one under the limit, one over. + smallContent := strings.Repeat("A", 500) + largeContent := strings.Repeat("B", 2048) + if err := os.WriteFile(filepath.Join(root, "small.bin"), []byte(smallContent), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "large.bin"), []byte(largeContent), 0644); err != nil { + t.Fatal(err) + } + + cfg := makeCfgWithRoot(t, root) + cfg.Cache.MaxFileSize = 256 // force both files through serveLargeFile path + cfg.Files.MaxServeFileSize = 1024 // 1 KB hard limit + cfg.Compression.Enabled = false + c := cache.NewCache(cfg.Cache.MaxBytes) + h := handler.BuildHandler(cfg, c) + + t.Run("under limit serves normally", func(t *testing.T) { + ctx := newTestCtx("GET", "/small.bin") + h(ctx) + + if ctx.Response.StatusCode() != fasthttp.StatusOK { + t.Errorf("status = %d, want 200 for file under MaxServeFileSize", ctx.Response.StatusCode()) + } + if len(ctx.Response.Body()) != 500 { + t.Errorf("body length = %d, want 500", len(ctx.Response.Body())) + } + }) + + t.Run("over limit returns 413", func(t *testing.T) { + ctx := newTestCtx("GET", "/large.bin") + h(ctx) + + if ctx.Response.StatusCode() != fasthttp.StatusRequestEntityTooLarge { + t.Errorf("status = %d, want 413 for file exceeding MaxServeFileSize", ctx.Response.StatusCode()) + } + body := string(ctx.Response.Body()) + if !strings.Contains(body, "Payload Too Large") { + t.Errorf("response body = %q, want it to contain 'Payload Too Large'", body) + } + if !strings.Contains(body, "max_serve_file_size") { + t.Errorf("response body = %q, want it to mention 'max_serve_file_size' for operator correlation", body) + } + }) + + t.Run("disabled when zero", func(t *testing.T) { + cfg2 := makeCfgWithRoot(t, root) + cfg2.Cache.MaxFileSize = 256 + cfg2.Files.MaxServeFileSize = 0 // 0 = disabled + cfg2.Compression.Enabled = false + c2 := cache.NewCache(cfg2.Cache.MaxBytes) + h2 := handler.BuildHandler(cfg2, c2) + + ctx := newTestCtx("GET", "/large.bin") + h2(ctx) + + if ctx.Response.StatusCode() != fasthttp.StatusOK { + t.Errorf("status = %d, want 200 when MaxServeFileSize=0 (disabled)", ctx.Response.StatusCode()) + } + if len(ctx.Response.Body()) != 2048 { + t.Errorf("body length = %d, want 2048", len(ctx.Response.Body())) + } + }) +} + // TestBuildHandler_CacheDisabled verifies that when Cache.Enabled=false, the // handler reads from disk on every request. func TestBuildHandler_CacheDisabled(t *testing.T) { diff --git a/internal/handler/middleware.go b/internal/handler/middleware.go index 53aeb1f..bc11764 100644 --- a/internal/handler/middleware.go +++ b/internal/handler/middleware.go @@ -2,8 +2,10 @@ package handler import ( "log" + "os" "runtime/debug" "strconv" + "strings" "sync" "time" @@ -14,6 +16,12 @@ import ( "github.com/valyala/fasthttp" ) +// debugStackTraces controls whether full goroutine stack traces are logged on panic. +// When false (default), only the panic value is logged, preventing sensitive internal +// details from leaking. Set STATIC_DEBUG=1 to enable verbose stack traces. +// SEC-003: Configurable panic stack trace verbosity. +var debugStackTraces = os.Getenv("STATIC_DEBUG") == "1" + // BuildHandler composes the full middleware chain and returns a ready-to-use // fasthttp.RequestHandler. The chain is (outer to inner): // @@ -111,19 +119,26 @@ func loggingMiddlewareWithWriter(next fasthttp.RequestHandler, logger *log.Logge } method := string(ctx.Method()) - uri := string(ctx.RequestURI()) + // SEC-008: Sanitize the URI before logging to prevent control-character + // injection (e.g. \r\n) into log files which could enable log forgery. + uri := sanitizeForLog(string(ctx.RequestURI())) logger.Print(formatAccessLogLine(method, uri, status, size, duration)) } } // recoveryMiddleware catches panics in the handler chain and returns a 500. -// It logs the panic value and the full stack trace. +// SEC-003: Full stack traces are only logged when STATIC_DEBUG=1 is set, +// preventing sensitive internal details from leaking into production logs. func recoveryMiddleware(next fasthttp.RequestHandler) fasthttp.RequestHandler { return func(ctx *fasthttp.RequestCtx) { defer func() { if rec := recover(); rec != nil { - stack := debug.Stack() - log.Printf("PANIC recovered: %v\n%s", rec, stack) + if debugStackTraces { + stack := debug.Stack() + log.Printf("PANIC recovered: %v\n%s", rec, stack) + } else { + log.Printf("PANIC recovered: %v (set STATIC_DEBUG=1 for stack trace)", rec) + } // Only write the header if not already sent. ctx.Error("Internal Server Error", fasthttp.StatusInternalServerError) } @@ -131,3 +146,35 @@ func recoveryMiddleware(next fasthttp.RequestHandler) fasthttp.RequestHandler { next(ctx) } } + +// sanitizeForLog replaces ASCII control characters (0x00–0x1F, 0x7F) with +// their hex-escaped form (e.g. \x0a) to prevent log injection attacks where +// crafted URIs containing \r\n could forge log entries. +// SEC-008: Log sanitization for untrusted request data. +func sanitizeForLog(s string) string { + // Fast path: no control characters → return as-is. + clean := true + for i := 0; i < len(s); i++ { + if s[i] < 0x20 || s[i] == 0x7f { + clean = false + break + } + } + if clean { + return s + } + + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); i++ { + c := s[i] + if c < 0x20 || c == 0x7f { + b.WriteString(`\x`) + b.WriteByte("0123456789abcdef"[c>>4]) + b.WriteByte("0123456789abcdef"[c&0x0f]) + } else { + b.WriteByte(c) + } + } + return b.String() +} diff --git a/internal/headers/headers.go b/internal/headers/headers.go index 9293ecb..6c57067 100644 --- a/internal/headers/headers.go +++ b/internal/headers/headers.go @@ -2,6 +2,7 @@ package headers import ( + "path" "path/filepath" "strconv" "strings" @@ -15,19 +16,28 @@ import ( // CacheKeyForPath normalises a URL path to the cache key used by the file // handler. Directory paths (trailing slash, or bare "/") are mapped to their // index file so that 304 checks succeed for index requests. +// SEC-006: Applies path.Clean to prevent cache key inconsistencies from +// non-canonical paths (e.g. "/a/../b" vs "/b"). // PERF-005: fast-path for the common "/" → "/index.html" case. func CacheKeyForPath(urlPath, indexFile string) string { if indexFile == "" { indexFile = "index.html" } - if urlPath == "" || urlPath == "/" { + // Remember whether the original path denotes a directory (trailing slash). + isDir := urlPath == "" || urlPath == "/" || strings.HasSuffix(urlPath, "/") + + // SEC-006: Normalise the URL path to prevent cache-key collisions caused + // by non-canonical input (e.g. "/a/../b" should map to the same key as "/b"). + urlPath = path.Clean("/" + urlPath) + + if urlPath == "/" { if indexFile == "index.html" { return "/index.html" // static string — zero alloc } return "/" + indexFile } - if strings.HasSuffix(urlPath, "/") { - return urlPath + indexFile + if isDir { + return urlPath + "/" + indexFile } return urlPath } diff --git a/internal/headers/headers_test.go b/internal/headers/headers_test.go index 2ded60d..cd79083 100644 --- a/internal/headers/headers_test.go +++ b/internal/headers/headers_test.go @@ -29,10 +29,30 @@ func TestCacheKeyForPath(t *testing.T) { indexFile string want string }{ + // --- Original basic cases --- {name: "root", urlPath: "/", indexFile: "index.html", want: "/index.html"}, {name: "directory", urlPath: "/docs/", indexFile: "home.html", want: "/docs/home.html"}, {name: "file", urlPath: "/app.js", indexFile: "index.html", want: "/app.js"}, {name: "default index", urlPath: "/", indexFile: "", want: "/index.html"}, + + // --- SEC-006: path.Clean normalization --- + {name: "dotdot traversal", urlPath: "/a/../b", indexFile: "index.html", want: "/b"}, + {name: "dotdot to root", urlPath: "/a/..", indexFile: "index.html", want: "/index.html"}, + {name: "dotdot deep", urlPath: "/a/b/../../c/d", indexFile: "index.html", want: "/c/d"}, + {name: "dot segment", urlPath: "/dir/./file.css", indexFile: "index.html", want: "/dir/file.css"}, + {name: "repeated slashes", urlPath: "/a//b///c", indexFile: "index.html", want: "/a/b/c"}, + {name: "mixed mess", urlPath: "/a/./b/../c//d", indexFile: "index.html", want: "/a/c/d"}, + + // --- Trailing-slash / directory semantics --- + {name: "dir with dot segment", urlPath: "/dir/./", indexFile: "index.html", want: "/dir/index.html"}, + {name: "dir with repeated slashes", urlPath: "/docs///", indexFile: "index.html", want: "/docs/index.html"}, + {name: "dir dotdot trailing slash", urlPath: "/a/b/../", indexFile: "index.html", want: "/a/index.html"}, + {name: "root via dotdot trailing slash", urlPath: "/a/../", indexFile: "index.html", want: "/index.html"}, + + // --- Empty and edge cases --- + {name: "empty path", urlPath: "", indexFile: "index.html", want: "/index.html"}, + {name: "empty path custom index", urlPath: "", indexFile: "home.html", want: "/home.html"}, + {name: "bare slash custom index", urlPath: "/", indexFile: "custom.html", want: "/custom.html"}, } for _, tt := range tests { diff --git a/internal/security/security.go b/internal/security/security.go index 9d5918d..5700b88 100644 --- a/internal/security/security.go +++ b/internal/security/security.go @@ -8,7 +8,8 @@ import ( "path" "path/filepath" "strings" - "sync" + + lru "github.com/hashicorp/golang-lru/v2" "github.com/BackendStack21/static-web/internal/config" "github.com/valyala/fasthttp" @@ -40,42 +41,47 @@ func SafePathFromCtx(ctx *fasthttp.RequestCtx) (string, bool) { } // --------------------------------------------------------------------------- -// PathCache — caches urlPath → safePath to avoid per-request syscalls +// PathCache — bounded LRU cache for urlPath → safePath lookups // --------------------------------------------------------------------------- +// DefaultPathCacheSize is the default maximum number of entries in the +// PathCache. 10 000 entries ≈ 1–2 MB of memory for typical URL paths. +const DefaultPathCacheSize = 10_000 + // PathCache caches the results of PathSafe so that repeated requests for the // same URL path skip the filesystem syscalls (filepath.EvalSymlinks). // It is safe for concurrent use. +// +// SEC-001: Uses a bounded LRU cache (hashicorp/golang-lru) instead of an +// unbounded sync.Map to prevent memory exhaustion via cache-flooding attacks. type PathCache struct { - m sync.Map // urlPath (string) → safePath (string) + cache *lru.Cache[string, string] } -// NewPathCache creates a new empty PathCache. -func NewPathCache() *PathCache { - return &PathCache{} +// NewPathCache creates a new PathCache with the given maximum number of entries. +// When the cache is full, the least-recently-used entry is evicted. +func NewPathCache(maxEntries int) *PathCache { + if maxEntries <= 0 { + maxEntries = DefaultPathCacheSize + } + c, _ := lru.New[string, string](maxEntries) + return &PathCache{cache: c} } // Lookup returns the cached safe path for urlPath, or ("", false) on miss. func (pc *PathCache) Lookup(urlPath string) (string, bool) { - v, ok := pc.m.Load(urlPath) - if !ok { - return "", false - } - return v.(string), true + return pc.cache.Get(urlPath) } // Store records a urlPath → safePath mapping in the cache. func (pc *PathCache) Store(urlPath, safePath string) { - pc.m.Store(urlPath, safePath) + pc.cache.Add(urlPath, safePath) } // Flush removes all entries from the cache. Call this on SIGHUP alongside // the file cache flush to ensure stale path mappings don't persist. func (pc *PathCache) Flush() { - pc.m.Range(func(key, _ any) bool { - pc.m.Delete(key) - return true - }) + pc.cache.Purge() } // PreWarm populates the cache for a set of known URL paths by running each @@ -84,19 +90,14 @@ func (pc *PathCache) PreWarm(paths []string, absRoot string, blockDotfiles bool) for _, urlPath := range paths { safePath, err := PathSafe(urlPath, absRoot, blockDotfiles) if err == nil { - pc.m.Store(urlPath, safePath) + pc.cache.Add(urlPath, safePath) } } } // Len returns the number of entries in the cache. func (pc *PathCache) Len() int { - n := 0 - pc.m.Range(func(_, _ any) bool { - n++ - return true - }) - return n + return pc.cache.Len() } // --------------------------------------------------------------------------- @@ -311,7 +312,10 @@ func Middleware(cfg *config.SecurityConfig, root string, next fasthttp.RequestHa if origin != "" { if isWildcard(cfg.CORSOrigins) { // Wildcard: emit literal "*" — never reflect the origin value. - // Do NOT set Vary: Origin since all origins receive the same header. + // SEC-012: Do NOT set Vary: Origin when using wildcard because all + // origins receive the identical "Access-Control-Allow-Origin: *" header. + // Adding Vary: Origin would cause caches to store per-origin variants + // unnecessarily, wasting storage and reducing hit rates. ctx.Response.Header.Set("Access-Control-Allow-Origin", "*") if ctx.IsOptions() { ctx.Response.Header.Set("Access-Control-Allow-Methods", "GET, HEAD, OPTIONS") diff --git a/internal/security/security_test.go b/internal/security/security_test.go index 9d5b984..56bcf0e 100644 --- a/internal/security/security_test.go +++ b/internal/security/security_test.go @@ -2,6 +2,7 @@ package security_test import ( "errors" + "fmt" "os" "path/filepath" "strings" @@ -383,6 +384,112 @@ func TestMiddleware_CORS_NoCORSConfigured(t *testing.T) { } } +// --------------------------------------------------------------------------- +// PathCache bounded LRU behaviour (SEC-001) +// --------------------------------------------------------------------------- + +func TestPathCache_BoundedLRU(t *testing.T) { + const maxEntries = 8 + + pc := security.NewPathCache(maxEntries) + + // Fill the cache to capacity. + for i := 0; i < maxEntries; i++ { + key := fmt.Sprintf("/page/%d", i) + pc.Store(key, "/safe/"+key) + } + if pc.Len() != maxEntries { + t.Fatalf("Len() = %d after filling, want %d", pc.Len(), maxEntries) + } + + // Insert more entries — Len() must never exceed maxEntries. + overflow := maxEntries * 3 + for i := maxEntries; i < maxEntries+overflow; i++ { + key := fmt.Sprintf("/page/%d", i) + pc.Store(key, "/safe/"+key) + if pc.Len() > maxEntries { + t.Fatalf("Len() = %d after inserting key %q, exceeds max %d", + pc.Len(), key, maxEntries) + } + } + + // Recently-used keys should still be retrievable. + lastKey := fmt.Sprintf("/page/%d", maxEntries+overflow-1) + if val, ok := pc.Lookup(lastKey); !ok { + t.Errorf("recently-inserted key %q missing from cache", lastKey) + } else if val != "/safe/"+lastKey { + t.Errorf("Lookup(%q) = %q, want %q", lastKey, val, "/safe/"+lastKey) + } + + // Oldest keys (from the first batch) should have been evicted. + oldKey := "/page/0" + if _, ok := pc.Lookup(oldKey); ok { + t.Errorf("oldest key %q should have been evicted but was found", oldKey) + } +} + +func TestPathCache_LookupPromotesEntry(t *testing.T) { + const maxEntries = 4 + + pc := security.NewPathCache(maxEntries) + + // Fill to capacity: keys /a, /b, /c, /d (in insertion order). + for _, k := range []string{"/a", "/b", "/c", "/d"} { + pc.Store(k, "/safe"+k) + } + + // Touch /a so it becomes most-recently-used. + if _, ok := pc.Lookup("/a"); !ok { + t.Fatal("/a should be in cache") + } + + // Insert two new keys to force two evictions. + pc.Store("/e", "/safe/e") + pc.Store("/f", "/safe/f") + + // /a should survive (it was promoted by the Lookup). + if _, ok := pc.Lookup("/a"); !ok { + t.Error("/a should still be in cache after promotion, but was evicted") + } + // /b should have been evicted (oldest untouched). + if _, ok := pc.Lookup("/b"); ok { + t.Error("/b should have been evicted but was found") + } +} + +func TestPathCache_FlushClearsAll(t *testing.T) { + pc := security.NewPathCache(16) + for i := 0; i < 10; i++ { + pc.Store(fmt.Sprintf("/k%d", i), fmt.Sprintf("/v%d", i)) + } + if pc.Len() != 10 { + t.Fatalf("Len() = %d before Flush, want 10", pc.Len()) + } + + pc.Flush() + + if pc.Len() != 0 { + t.Errorf("Len() = %d after Flush, want 0", pc.Len()) + } + if _, ok := pc.Lookup("/k0"); ok { + t.Error("Lookup should miss after Flush") + } +} + +func TestPathCache_DefaultSizeOnZero(t *testing.T) { + // Passing 0 should fall back to DefaultPathCacheSize. + pc := security.NewPathCache(0) + // We can't easily assert the internal capacity, but we can verify it + // accepts at least DefaultPathCacheSize entries without panicking and + // that Len() grows correctly. + for i := 0; i < security.DefaultPathCacheSize; i++ { + pc.Store(fmt.Sprintf("/%d", i), fmt.Sprintf("/v/%d", i)) + } + if pc.Len() != security.DefaultPathCacheSize { + t.Errorf("Len() = %d, want %d", pc.Len(), security.DefaultPathCacheSize) + } +} + // --------------------------------------------------------------------------- // Additional PathSafe edge cases // --------------------------------------------------------------------------- diff --git a/internal/server/server.go b/internal/server/server.go index 344fc90..cdb77aa 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -67,11 +67,12 @@ func New(cfg *config.ServerConfig, secCfg *config.SecurityConfig, handler fastht s.http = &fasthttp.Server{ Handler: httpHandler, - Name: "static-web", + Name: "", // SEC-007: suppress server identity disclosure ReadTimeout: cfg.ReadTimeout, WriteTimeout: cfg.WriteTimeout, IdleTimeout: cfg.IdleTimeout, - MaxRequestBodySize: 0, // no request bodies for a static server + MaxRequestBodySize: 1024, // SEC-014: explicit small limit (static server has no request bodies) + MaxConnsPerIP: cfg.MaxConnsPerIP, // SEC-015: per-IP connection rate limiting DisableKeepalive: false, } @@ -90,7 +91,9 @@ func New(cfg *config.ServerConfig, secCfg *config.SecurityConfig, handler fastht tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, }, - PreferServerCipherSuites: true, //nolint:staticcheck // intentional for TLS1.2 compat + // SEC-009: PreferServerCipherSuites removed — deprecated since Go 1.17 + // and a no-op since Go 1.22. The runtime now always selects the most + // secure cipher agreed by both parties. } // Wrap the handler to inject HSTS header on HTTPS responses. @@ -109,12 +112,13 @@ func New(cfg *config.ServerConfig, secCfg *config.SecurityConfig, handler fastht s.https = &fasthttp.Server{ Handler: httpsHandler, - Name: "static-web", + Name: "", // SEC-007: suppress server identity disclosure TLSConfig: tlsCfg, ReadTimeout: cfg.ReadTimeout, WriteTimeout: cfg.WriteTimeout, IdleTimeout: cfg.IdleTimeout, - MaxRequestBodySize: 0, + MaxRequestBodySize: 1024, // SEC-014: explicit small limit + MaxConnsPerIP: cfg.MaxConnsPerIP, // SEC-015: per-IP connection rate limiting DisableKeepalive: false, } } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 198a9ee..f93e89b 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -97,3 +97,106 @@ func TestRedirectAuthorityRejectsInvalidConfiguredHost(t *testing.T) { t.Fatal("redirectAuthority accepted invalid configured host") } } + +// --------------------------------------------------------------------------- +// Security-hardening defaults on fasthttp.Server (SEC-007, SEC-014, SEC-015) +// --------------------------------------------------------------------------- + +func TestNew_HTTPOnly_SecurityDefaults(t *testing.T) { + cfg := &config.ServerConfig{ + Addr: ":8080", + MaxConnsPerIP: 50, + } + handler := func(ctx *fasthttp.RequestCtx) { + ctx.SetStatusCode(fasthttp.StatusOK) + } + + s := New(cfg, nil, handler) + + // SEC-007: Server name must be empty to suppress identity disclosure. + if s.http.Name != "" { + t.Errorf("HTTP Name = %q, want empty (SEC-007)", s.http.Name) + } + + // SEC-014: MaxRequestBodySize must be 1024 (small explicit limit). + if s.http.MaxRequestBodySize != 1024 { + t.Errorf("HTTP MaxRequestBodySize = %d, want 1024 (SEC-014)", s.http.MaxRequestBodySize) + } + + // SEC-015: MaxConnsPerIP must match configuration. + if s.http.MaxConnsPerIP != 50 { + t.Errorf("HTTP MaxConnsPerIP = %d, want 50 (SEC-015)", s.http.MaxConnsPerIP) + } + + // No HTTPS server when TLS is not configured. + if s.https != nil { + t.Error("https server should be nil when TLS is not configured") + } +} + +func TestNew_TLS_SecurityDefaults(t *testing.T) { + cfg := &config.ServerConfig{ + Addr: ":8080", + TLSAddr: ":8443", + TLSCert: "dummy.crt", + TLSKey: "dummy.key", + RedirectHost: "example.com", + MaxConnsPerIP: 100, + } + handler := func(ctx *fasthttp.RequestCtx) { + ctx.SetStatusCode(fasthttp.StatusOK) + } + + s := New(cfg, nil, handler) + + // --- HTTP server (redirect handler) --- + + if s.http.Name != "" { + t.Errorf("HTTP Name = %q, want empty (SEC-007)", s.http.Name) + } + if s.http.MaxRequestBodySize != 1024 { + t.Errorf("HTTP MaxRequestBodySize = %d, want 1024 (SEC-014)", s.http.MaxRequestBodySize) + } + if s.http.MaxConnsPerIP != 100 { + t.Errorf("HTTP MaxConnsPerIP = %d, want 100 (SEC-015)", s.http.MaxConnsPerIP) + } + + // --- HTTPS server --- + + if s.https == nil { + t.Fatal("https server should not be nil when TLS is configured") + } + if s.https.Name != "" { + t.Errorf("HTTPS Name = %q, want empty (SEC-007)", s.https.Name) + } + if s.https.MaxRequestBodySize != 1024 { + t.Errorf("HTTPS MaxRequestBodySize = %d, want 1024 (SEC-014)", s.https.MaxRequestBodySize) + } + if s.https.MaxConnsPerIP != 100 { + t.Errorf("HTTPS MaxConnsPerIP = %d, want 100 (SEC-015)", s.https.MaxConnsPerIP) + } + + // TLS config must be present with minimum TLS 1.2. + if s.https.TLSConfig == nil { + t.Fatal("HTTPS TLSConfig should not be nil") + } + if s.https.TLSConfig.MinVersion != 0x0303 { // tls.VersionTLS12 + t.Errorf("HTTPS TLS MinVersion = %#x, want %#x (TLS 1.2)", s.https.TLSConfig.MinVersion, 0x0303) + } +} + +func TestNew_MaxConnsPerIP_Zero(t *testing.T) { + cfg := &config.ServerConfig{ + Addr: ":8080", + MaxConnsPerIP: 0, // disabled + } + handler := func(ctx *fasthttp.RequestCtx) { + ctx.SetStatusCode(fasthttp.StatusOK) + } + + s := New(cfg, nil, handler) + + if s.http.MaxConnsPerIP != 0 { + t.Errorf("HTTP MaxConnsPerIP = %d, want 0 (disabled)", s.http.MaxConnsPerIP) + } +}