From 3ceaa38f45b643ca12c28437a7728e94d3514235 Mon Sep 17 00:00:00 2001 From: Mirko Brombin Date: Thu, 16 Jul 2026 20:57:11 +0200 Subject: [PATCH] fix: harden auth, vhost TLS, path handling, DNS and plugin startup --- .gitignore | 6 ++ README.md | 18 +++-- go.mod | 2 + internal/api/config.go | 12 ++++ internal/api/logs_explorer.go | 9 ++- internal/api/server.go | 9 +++ internal/api/sites.go | 19 ++++-- internal/api/tools.go | 2 +- internal/cli/cli.go | 105 +++++++++++++++++++++++----- internal/config/config.go | 15 +++- internal/config/dns_config.go | 4 ++ internal/config/global_config.go | 2 +- internal/config/paths.go | 70 +++++++++++++++++++ internal/config/paths_test.go | 70 +++++++++++++++++++ internal/dashboard/server.go | 10 +++ internal/dns/handler.go | 76 +++++++++++++++++++-- internal/dns/matchzone_test.go | 39 +++++++++++ internal/dns/server.go | 18 ++++- internal/logger/logger.go | 35 ++-------- internal/logger/rotating.go | 85 +++++++++++++++++++++++ internal/middleware/auth.go | 11 +-- internal/middleware/auth_test.go | 28 +++----- internal/restart/restart.go | 17 ++++- internal/safeguard/safeguard.go | 30 ++++++-- internal/server/handler.go | 46 +++++-------- internal/server/lifecycle.go | 25 ++++++- internal/server/middleware/gzip.go | 47 +++++++------ internal/server/server.go | 50 ++++++++------ internal/server/server_dns.go | 2 +- internal/server/server_utils.go | 50 ++++++++++---- internal/server/server_web.go | 37 ++++++++++ internal/server/static_handler.go | 44 +++++++++--- plugins/auth.go | 106 +++++++++++++++++++---------- plugins/docker_base.go | 69 ++++++++++++------- plugins/docker_standard.go | 34 +++++++-- plugins/nodejs.go | 45 ++++++------ plugins/php.go | 11 ++- plugins/proxy_shared.go | 3 + 38 files changed, 968 insertions(+), 293 deletions(-) create mode 100644 internal/config/paths.go create mode 100644 internal/config/paths_test.go create mode 100644 internal/dns/matchzone_test.go create mode 100644 internal/logger/rotating.go diff --git a/.gitignore b/.gitignore index 63e6431..89ddcf3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,12 @@ private.key certificate.crt test.sh + +# Local build artifacts +/goup +/goup-dev +/goup-web +/goup-dns public/node_test/app/node_modules public/node_test/app/pnpm-lock.yaml public/**/*.pyc diff --git a/README.md b/README.md index 4e0ab96..f1d33b9 100644 --- a/README.md +++ b/README.md @@ -24,12 +24,6 @@ GoUP! is a minimal, tweakable web server written in Go. You can use it to serve - [DNS Server Guide](docs/dns.md) explanation of the DNS module configuration and usage. - [Kamal Proxy Guide](docs/kamal-proxy.md) for blue/green deployments with kamal-proxy. -## Future Plans - -- API for dynamic configuration changes -- Docker/Podman support for easy deployment - - ## API & Dashboard GoUp includes a built-in REST API and a Web Dashboard for management. @@ -39,7 +33,9 @@ To enable them, edit your global configuration file (`~/.config/goup/conf.global ### Authentication -Security is mandatory when enabling the API/Dashboard. GoUp uses: +Security is mandatory when enabling the API/Dashboard, and it is enforced: +GoUp refuses to start the API without an `api_token`, and refuses to start the +Dashboard without a `username` and `password_hash`. GoUp uses: - **Basic Auth** for the Dashboard. - **Token Auth** for the API. @@ -71,7 +67,7 @@ GoUp handles compression automatically with a dual-layer strategy: GoUp includes a built-in **SafeGuard** system that monitors memory usage and automatically restarts the process if it exceeds safety limits, ensuring long-term stability. -- **Enabled by Default**: Checks memory usage every 30 seconds. +- **Enabled by Default**: Checks resident memory (RSS) every 30 seconds. - **Auto-Dump**: Saves a Pprof heap dump before restarting for easier debugging. - **Seamless Restart**: Uses `syscall.Exec` to replace the process immediately. @@ -79,14 +75,16 @@ Configuration (in `~/.config/goup/conf.global.json`): ```json { - "safe_guard": { + "safeguard": { "enable": true, "max_memory_mb": 1024, - "check_interval": 30 + "check_interval": "30s" } } ``` +`check_interval` is a Go duration string (e.g. `"30s"`, `"1m"`). + ## Installation Go is required to build the software, ensure you have it installed on your system. diff --git a/go.mod b/go.mod index 49e6ef8..a76e5cb 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,8 @@ module github.com/mirkobrombin/goup go 1.25.0 +toolchain go1.26.5 + require ( github.com/armon/go-radix v1.0.0 github.com/gorilla/mux v1.8.1 diff --git a/internal/api/config.go b/internal/api/config.go index eefebcd..4d4fc4c 100644 --- a/internal/api/config.go +++ b/internal/api/config.go @@ -30,6 +30,18 @@ func updateConfigHandler(w http.ResponseWriter, r *http.Request) { http.Error(w, "Invalid JSON", http.StatusBadRequest) return } + + // Reject changes that would leave an admin surface enabled without + // credentials (which would otherwise disable authentication entirely). + if newConf.EnableAPI && newConf.Account.APIToken == "" { + http.Error(w, "Refusing to enable the API without account.api_token", http.StatusBadRequest) + return + } + if newConf.DashboardPort != 0 && (newConf.Account.Username == "" || newConf.Account.PasswordHash == "") { + http.Error(w, "Refusing to enable the dashboard without account.username and account.password_hash", http.StatusBadRequest) + return + } + config.GlobalConfMu.Lock() config.GlobalConf = &newConf config.GlobalConfMu.Unlock() diff --git a/internal/api/logs_explorer.go b/internal/api/logs_explorer.go index b09593c..c673255 100644 --- a/internal/api/logs_explorer.go +++ b/internal/api/logs_explorer.go @@ -3,7 +3,6 @@ package api import ( "io/fs" - "io/ioutil" "net/http" "os" "path/filepath" @@ -123,12 +122,16 @@ func getLogFileHandler(w http.ResponseWriter, r *http.Request) { http.Error(w, "fileName is required", http.StatusBadRequest) return } - fullPath := filepath.Join(config.GetLogDir(), fileName) + fullPath, err := config.SafeJoin(config.GetLogDir(), fileName) + if err != nil { + http.Error(w, "Invalid log file path", http.StatusBadRequest) + return + } if _, err := os.Stat(fullPath); os.IsNotExist(err) { http.Error(w, "Log file not found", http.StatusNotFound) return } - data, err := ioutil.ReadFile(fullPath) + data, err := os.ReadFile(fullPath) if err != nil { http.Error(w, "Failed to read log file", http.StatusInternalServerError) return diff --git a/internal/api/server.go b/internal/api/server.go index fb948ff..8d989b7 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -18,6 +18,15 @@ func StartAPIServer() *http.Server { return nil } + // Fail closed: the API exposes full administrative control (create/delete + // sites, rewrite the global config, restart). Refuse to start it without a + // token instead of serving an unauthenticated admin surface on the network. + if conf.Account.APIToken == "" { + fmt.Println("[API] Refusing to start: 'account.api_token' is not set. " + + "Set a token in the global config to enable the API.") + return nil + } + router := SetupRoutes() port := conf.APIPort var handler http.Handler = router diff --git a/internal/api/sites.go b/internal/api/sites.go index 0b9cd74..faec5c1 100644 --- a/internal/api/sites.go +++ b/internal/api/sites.go @@ -4,7 +4,6 @@ import ( "encoding/json" "net/http" "os" - "path/filepath" "sort" "github.com/gorilla/mux" @@ -48,7 +47,11 @@ func createSiteHandler(w http.ResponseWriter, r *http.Request) { http.Error(w, "Invalid JSON", http.StatusBadRequest) return } - path := filepath.Join(config.GetConfigDir(), site.Domain+".json") + path, err := config.SiteConfigPath(site.Domain) + if err != nil { + http.Error(w, "Invalid domain", http.StatusBadRequest) + return + } if _, err := os.Stat(path); err == nil { http.Error(w, "Site already exists", http.StatusBadRequest) return @@ -86,7 +89,11 @@ func updateSiteHandler(w http.ResponseWriter, r *http.Request) { existing.RequestTimeout = updated.RequestTimeout existing.PluginConfigs = updated.PluginConfigs - path := filepath.Join(config.GetConfigDir(), domain+".json") + path, err := config.SiteConfigPath(domain) + if err != nil { + http.Error(w, "Invalid domain", http.StatusBadRequest) + return + } if err := existing.Save(path); err != nil { http.Error(w, "Failed to save site config", http.StatusInternalServerError) return @@ -100,7 +107,11 @@ func updateSiteHandler(w http.ResponseWriter, r *http.Request) { func deleteSiteHandler(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) domain := vars["domain"] - path := filepath.Join(config.GetConfigDir(), domain+".json") + path, err := config.SiteConfigPath(domain) + if err != nil { + http.Error(w, "Invalid domain", http.StatusBadRequest) + return + } if err := os.Remove(path); err != nil { http.Error(w, "Failed to delete site config", http.StatusInternalServerError) return diff --git a/internal/api/tools.go b/internal/api/tools.go index 57a81a3..1983594 100644 --- a/internal/api/tools.go +++ b/internal/api/tools.go @@ -46,7 +46,7 @@ func cleanupLogsHandler(w http.ResponseWriter, r *http.Request) { zipWriter.Close() backupFile := filepath.Join(logDir, "logs_backup_"+time.Now().Format("20060102_150405")+".zip") - err = os.WriteFile(backupFile, zipBuffer.Bytes(), 0644) + err = os.WriteFile(backupFile, zipBuffer.Bytes(), 0600) if err != nil { http.Error(w, "Error saving backup zip", http.StatusInternalServerError) return diff --git a/internal/cli/cli.go b/internal/cli/cli.go index bb5c186..1a41a49 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -239,7 +239,11 @@ var startDNSCmd = &cobra.Command{ } func startDNS(cmd *cobra.Command, args []string) { - configs, _ := loadConfigs() + configs, err := loadConfigs() + if err != nil { + fmt.Printf("Error loading configurations: %v\n", err) + os.Exit(1) + } if err := pidfile.Write(); err != nil { fmt.Printf("Warning: could not write PID file: %v\n", err) @@ -296,22 +300,42 @@ var stopCmd = &cobra.Command{ fmt.Printf("Error sending signal: %v\n", err) os.Exit(1) } - done := make(chan struct{}) - go func() { - proc.Wait() - close(done) - }() - select { - case <-done: + if waitForExit(pid, 10*time.Second) { fmt.Println("Server stopped.") - case <-time.After(10 * time.Second): + } else { fmt.Println("Server did not stop in time, forcing...") - proc.Kill() + _ = proc.Kill() + waitForExit(pid, 5*time.Second) } pidfile.Remove() }, } +// processAlive reports whether a process with the given PID is currently +// running. On Unix, signal 0 performs error checking without delivering a +// signal. +func processAlive(pid int) bool { + proc, err := os.FindProcess(pid) + if err != nil { + return false + } + return proc.Signal(syscall.Signal(0)) == nil +} + +// waitForExit polls until the process is gone or the timeout elapses. It +// returns true if the process exited. This works for non-child processes, +// unlike (*os.Process).Wait. +func waitForExit(pid int, timeout time.Duration) bool { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + if !processAlive(pid) { + return true + } + time.Sleep(50 * time.Millisecond) + } + return !processAlive(pid) +} + var restartCmd = &cobra.Command{ Use: "restart", Short: "Restart the server", @@ -326,15 +350,36 @@ var restartCmd = &cobra.Command{ fmt.Printf("Error finding process %d: %v\n", pid, err) os.Exit(1) } - proc.Signal(syscall.SIGTERM) - time.Sleep(500 * time.Millisecond) + if err := proc.Signal(syscall.SIGTERM); err != nil { + fmt.Printf("Error sending signal: %v\n", err) + os.Exit(1) + } + // Wait for the old process to release its PID file, ports and + // listeners before starting a fresh instance, otherwise the two + // race over the PID file and the sockets. + if !waitForExit(pid, 15*time.Second) { + fmt.Println("Previous instance did not stop in time, forcing...") + _ = proc.Kill() + waitForExit(pid, 5*time.Second) + } + exe, err := os.Executable() if err != nil { fmt.Printf("Error: %v\n", err) os.Exit(1) } - argsEnv := os.Environ() - if err := syscall.Exec(exe, os.Args, argsEnv); err != nil { + + // Re-exec as "start" (not "restart"), preserving the config flags so + // the new process actually serves traffic instead of re-running the + // restart command against a now-stopped server. + newArgs := []string{exe, "start"} + if configPath != "" { + newArgs = append(newArgs, "--config", configPath) + } + if globalConfigPath != "" { + newArgs = append(newArgs, "--global-config", globalConfigPath) + } + if err := syscall.Exec(exe, newArgs, os.Environ()); err != nil { fmt.Printf("Error restarting: %v\n", err) os.Exit(1) } @@ -348,16 +393,40 @@ var validateCmd = &cobra.Command{ } func validate(cmd *cobra.Command, args []string) { - configs, err := config.LoadAllConfigs() + configDir := config.GetConfigDir() + files, err := os.ReadDir(configDir) if err != nil { - fmt.Printf("Error loading configurations: %v\n", err) + fmt.Printf("Error reading configuration directory %s: %v\n", configDir, err) os.Exit(1) } - fmt.Printf("Validating configurations in %s:\n", config.GetConfigDir()) - for _, conf := range configs { + fmt.Printf("Validating configurations in %s:\n", configDir) + hasError := false + for _, file := range files { + if filepath.Ext(file.Name()) != ".json" { + continue + } + if file.Name() == "conf.global.json" { + continue + } + fullPath, err := config.SafeJoin(configDir, file.Name()) + if err != nil { + fmt.Printf("- %s: INVALID PATH (%v)\n", file.Name(), err) + hasError = true + continue + } + conf, err := config.LoadConfig(fullPath) + if err != nil { + fmt.Printf("- %s: FAILED (%v)\n", file.Name(), err) + hasError = true + continue + } fmt.Printf("- %s: OK\n", conf.Domain) } + + if hasError { + os.Exit(1) + } } var listCmd = &cobra.Command{ diff --git a/internal/config/config.go b/internal/config/config.go index 9a9c804..a6c3fef 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -137,12 +137,23 @@ func LoadAllConfigs() ([]SiteConfig, error) { var wg sync.WaitGroup for _, file := range files { + // Skip the global config file: it lives in the same directory but is + // not a site definition (loading it would create a phantom site with an + // empty domain on port 0). + if file.Name() == globalConfName { + continue + } if filepath.Ext(file.Name()) == ".json" { wg.Add(1) go func(fname string) { defer wg.Done() - conf, err := LoadConfig(filepath.Join(configDir, fname)) + fullPath, err := SafeJoin(configDir, fname) + if err != nil { + fmt.Printf("Error loading config %s: %v\n", fname, err) + return + } + conf, err := LoadConfig(fullPath) if err != nil { fmt.Printf("Error loading config %s: %v\n", fname, err) return @@ -168,7 +179,7 @@ func (conf *SiteConfig) Save(filePath string) error { if err != nil { return err } - return os.WriteFile(filePath, data, 0644) + return os.WriteFile(filePath, data, 0600) } // GetSiteConfigByHost returns the site configuration based on the host. diff --git a/internal/config/dns_config.go b/internal/config/dns_config.go index 4f1554b..8b6907d 100644 --- a/internal/config/dns_config.go +++ b/internal/config/dns_config.go @@ -15,6 +15,10 @@ type DNSConfig struct { Port int `json:"port"` // Default: 53 UpstreamResolvers []string `json:"upstream_resolvers"` // Optional forwarding Zones map[string][]DNSRecord `json:"zones"` // zone -> records + // AllowRecursionFrom lists CIDRs allowed to use the forwarder (recursion). + // When empty, recursion is restricted to loopback and private networks so + // the server is not an open resolver / amplification vector. + AllowRecursionFrom []string `json:"allow_recursion_from"` } // DefaultDNSConfig returns the default DNS configuration. diff --git a/internal/config/global_config.go b/internal/config/global_config.go index cf88346..d459608 100644 --- a/internal/config/global_config.go +++ b/internal/config/global_config.go @@ -82,5 +82,5 @@ func SaveGlobalConfig() error { if err != nil { return err } - return os.WriteFile(configFile, data, 0644) + return os.WriteFile(configFile, data, 0600) } diff --git a/internal/config/paths.go b/internal/config/paths.go new file mode 100644 index 0000000..f6e8e74 --- /dev/null +++ b/internal/config/paths.go @@ -0,0 +1,70 @@ +package config + +import ( + "fmt" + "path/filepath" + "strings" +) + +// ValidateDomain rejects domain names that are empty or contain characters +// that could escape the configuration directory when used to build a file +// path (path separators, "..", NUL, etc.). It accepts standard hostnames and +// wildcard labels only. +func ValidateDomain(domain string) error { + if domain == "" { + return fmt.Errorf("domain is empty") + } + if len(domain) > 253 { + return fmt.Errorf("domain is too long") + } + if strings.ContainsAny(domain, "/\\\x00") { + return fmt.Errorf("domain contains an invalid character") + } + if strings.Contains(domain, "..") { + return fmt.Errorf("domain contains '..'") + } + for _, label := range strings.Split(domain, ".") { + if label == "" { + // Allows neither a leading/trailing dot nor an empty label. + continue + } + for _, r := range label { + switch { + case r >= 'a' && r <= 'z': + case r >= 'A' && r <= 'Z': + case r >= '0' && r <= '9': + case r == '-' || r == '_' || r == '*': + default: + return fmt.Errorf("domain contains an invalid character") + } + } + } + return nil +} + +// SiteConfigPath returns the on-disk path for a site's configuration file, +// guaranteeing the result stays inside the configuration directory. +func SiteConfigPath(domain string) (string, error) { + if err := ValidateDomain(domain); err != nil { + return "", err + } + return SafeJoin(GetConfigDir(), domain+".json") +} + +// SafeJoin joins base with an untrusted relative path and verifies that the +// result does not escape base. It is the guard used for every file path built +// from request- or config-supplied data. +func SafeJoin(base, rel string) (string, error) { + cleanRel := filepath.Clean("/" + strings.ReplaceAll(rel, "\\", "/")) + cleanRel = strings.TrimPrefix(cleanRel, "/") + joined := filepath.Join(base, cleanRel) + + rel, err := filepath.Rel(base, joined) + if err != nil { + return "", err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("path escapes base directory") + } + return joined, nil +} diff --git a/internal/config/paths_test.go b/internal/config/paths_test.go new file mode 100644 index 0000000..638540d --- /dev/null +++ b/internal/config/paths_test.go @@ -0,0 +1,70 @@ +package config + +import ( + "path/filepath" + "strings" + "testing" +) + +func TestValidateDomain(t *testing.T) { + valid := []string{"example.com", "a.local", "sub.example.co.uk", "www_test-1.example.com", "*.example.com"} + for _, d := range valid { + if err := ValidateDomain(d); err != nil { + t.Errorf("expected %q to be valid, got %v", d, err) + } + } + + invalid := []string{ + "", + "../../etc/passwd", + "..", + "a/b", + "a\\b", + "a\x00b", + "foo/../bar", + } + for _, d := range invalid { + if err := ValidateDomain(d); err == nil { + t.Errorf("expected %q to be rejected", d) + } + } +} + +func TestSiteConfigPathRejectsTraversal(t *testing.T) { + if _, err := SiteConfigPath("../../../tmp/evil"); err == nil { + t.Fatal("expected traversal domain to be rejected") + } + p, err := SiteConfigPath("example.com") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.HasSuffix(p, filepath.Join("goup", "example.com.json")) { + t.Errorf("unexpected path: %s", p) + } +} + +func TestSafeJoinContainment(t *testing.T) { + base := filepath.Join("/var", "log", "goup") + + // SafeJoin clamps traversal back inside base rather than escaping it, so the + // result must always stay under base (a request for ../../etc/passwd hits a + // non-existent file inside the log dir, never /etc/passwd). + for _, rel := range []string{"../../etc/passwd", "sub/../../../etc/passwd", "/../../etc/passwd"} { + got, err := SafeJoin(base, rel) + if err != nil { + continue // rejecting is also acceptable + } + if got != base && !strings.HasPrefix(got, base+string(filepath.Separator)) { + t.Errorf("SafeJoin(%q, %q) = %q escaped base %q", base, rel, got, base) + } + } + + got, err := SafeJoin(base, "2026/07/16.log") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := filepath.Join(base, "2026", "07", "16.log") + if got != want { + t.Errorf("got %s, want %s", got, want) + } +} diff --git a/internal/dashboard/server.go b/internal/dashboard/server.go index aa176ef..3e28341 100644 --- a/internal/dashboard/server.go +++ b/internal/dashboard/server.go @@ -17,6 +17,16 @@ func StartDashboardServer() *http.Server { if conf == nil || conf.DashboardPort == 0 { return nil } + + // Fail closed: the dashboard proxies the admin API (and injects its token). + // Refuse to start without Basic Auth credentials rather than exposing it + // unauthenticated on the network. + if conf.Account.Username == "" || conf.Account.PasswordHash == "" { + fmt.Println("[Dashboard] Refusing to start: 'account.username' and " + + "'account.password_hash' must be set to secure the dashboard.") + return nil + } + port := conf.DashboardPort handler := Handler() handler = middleware.BasicAuthMiddleware(handler) diff --git a/internal/dns/handler.go b/internal/dns/handler.go index d39a370..6ac40c0 100644 --- a/internal/dns/handler.go +++ b/internal/dns/handler.go @@ -30,6 +30,9 @@ type DNSHandler struct { // names records every FQDN that exists in a zone, for NXDOMAIN vs NODATA. names map[string]bool + // allowRecursion lists the networks permitted to use the forwarder. + allowRecursion []*net.IPNet + client *dns.Client } @@ -48,9 +51,51 @@ func NewDNSHandler(conf *config.DNSConfig) (*DNSHandler, error) { client: &dns.Client{Timeout: 5 * time.Second}, } h.buildIndex() + h.allowRecursion = buildRecursionACL(conf.AllowRecursionFrom, l) return h, nil } +// buildRecursionACL parses the configured CIDRs. When none are configured it +// defaults to loopback and RFC1918/ULA private ranges, so the forwarder is +// never open to the public Internet by default. +func buildRecursionACL(cidrs []string, l *logger.Logger) []*net.IPNet { + if len(cidrs) == 0 { + cidrs = []string{ + "127.0.0.0/8", "::1/128", + "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", + "fc00::/7", "fe80::/10", + } + } + var nets []*net.IPNet + for _, c := range cidrs { + _, n, err := net.ParseCIDR(c) + if err != nil { + l.Errorf("Invalid allow_recursion_from CIDR %q: %v", c, err) + continue + } + nets = append(nets, n) + } + return nets +} + +// recursionAllowed reports whether the client address may use the forwarder. +func (h *DNSHandler) recursionAllowed(addr net.Addr) bool { + host, _, err := net.SplitHostPort(addr.String()) + if err != nil { + host = addr.String() + } + ip := net.ParseIP(host) + if ip == nil { + return false + } + for _, n := range h.allowRecursion { + if n.Contains(ip) { + return true + } + } + return false +} + // buildIndex pre-compiles every configured record into ready-to-serve RRs. func (h *DNSHandler) buildIndex() { for zone, records := range h.Config.Zones { @@ -90,10 +135,13 @@ func (h *DNSHandler) buildIndex() { }) } -// matchZone returns the longest configured zone that is a suffix of name. +// matchZone returns the longest configured zone that contains name, matching +// only on label boundaries so that "evilexample.com." does not match the zone +// "example.com". func (h *DNSHandler) matchZone(name string) string { for _, z := range h.zones { - if strings.HasSuffix(name, z+".") { + zoneDot := z + "." + if name == zoneDot || strings.HasSuffix(name, "."+zoneDot) { return z } } @@ -111,9 +159,15 @@ func (h *DNSHandler) ServeDNS(w dns.ResponseWriter, r *dns.Msg) { zone := h.matchZone(name) - // If no zone found, try forwarding if configured + // If no zone found, try forwarding if configured and permitted. if zone == "" { if len(h.Config.UpstreamResolvers) > 0 { + if !h.recursionAllowed(w.RemoteAddr()) { + m := new(dns.Msg) + m.SetRcode(r, dns.RcodeRefused) + _ = w.WriteMsg(m) + return + } h.handleForwarding(w, r) return } @@ -134,7 +188,21 @@ func (h *DNSHandler) ServeDNS(w dns.ResponseWriter, r *dns.Msg) { } } - w.WriteMsg(msg) + // Set the TC (truncated) bit when the UDP response exceeds the client's + // advertised buffer, so the client retries over TCP instead of timing out. + if _, isUDP := w.RemoteAddr().(*net.UDPAddr); isUDP { + maxSize := dns.MinMsgSize + if opt := r.IsEdns0(); opt != nil { + if sz := int(opt.UDPSize()); sz > maxSize { + maxSize = sz + } + } + msg.Truncate(maxSize) + } + + if err := w.WriteMsg(msg); err != nil { + h.Logger.Errorf("Failed to write DNS response: %v", err) + } // Log after answering, off the response latency path. if clientIP, _, err := net.SplitHostPort(w.RemoteAddr().String()); err == nil { diff --git a/internal/dns/matchzone_test.go b/internal/dns/matchzone_test.go new file mode 100644 index 0000000..b664aaa --- /dev/null +++ b/internal/dns/matchzone_test.go @@ -0,0 +1,39 @@ +package dns + +import ( + "testing" + + "github.com/mirkobrombin/goup/internal/config" +) + +// TestMatchZoneLabelBoundary guards against the suffix-match bug where a query +// for a lookalike domain (evilexample.com) would be answered authoritatively +// for the zone example.com. +func TestMatchZoneLabelBoundary(t *testing.T) { + conf := &config.DNSConfig{ + Enable: true, + Zones: map[string][]config.DNSRecord{ + "example.com": {{Type: "A", Name: "@", Value: "1.2.3.4", TTL: 3600}}, + }, + } + h, err := NewDNSHandler(conf) + if err != nil { + t.Fatalf("failed to create handler: %v", err) + } + + cases := []struct { + name string + want string + }{ + {"example.com.", "example.com"}, + {"www.example.com.", "example.com"}, + {"evilexample.com.", ""}, + {"notexample.com.", ""}, + {"example.com.evil.com.", ""}, + } + for _, c := range cases { + if got := h.matchZone(c.name); got != c.want { + t.Errorf("matchZone(%q) = %q, want %q", c.name, got, c.want) + } + } +} diff --git a/internal/dns/server.go b/internal/dns/server.go index 4f3ffca..ea85f42 100644 --- a/internal/dns/server.go +++ b/internal/dns/server.go @@ -2,14 +2,22 @@ package dns import ( "fmt" + "io" "sync" "github.com/miekg/dns" "github.com/mirkobrombin/goup/internal/config" ) -// Start initiates the DNS server(s). -func Start(conf *config.DNSConfig) { +// dnsServerCloser adapts *dns.Server (which exposes Shutdown, not Close) to +// io.Closer so the lifecycle can drain it on shutdown. +type dnsServerCloser struct{ s *dns.Server } + +func (c dnsServerCloser) Close() error { return c.s.Shutdown() } + +// Start initiates the DNS server(s). Each created server is passed to register +// (when non-nil) so the caller can close it during graceful shutdown. +func Start(conf *config.DNSConfig, register func(io.Closer)) { handler, err := NewDNSHandler(conf) if err != nil { fmt.Printf("Error initializing DNS logger: %v\n", err) @@ -28,6 +36,9 @@ func Start(conf *config.DNSConfig) { Handler: handler, ReusePort: true, } + if register != nil { + register(dnsServerCloser{srv}) + } handler.Logger.Infof("Starting DNS UDP server on port %d", conf.Port) if err := srv.ListenAndServe(); err != nil { handler.Logger.Errorf("DNS UDP Error: %v", err) @@ -44,6 +55,9 @@ func Start(conf *config.DNSConfig) { Handler: handler, ReusePort: true, } + if register != nil { + register(dnsServerCloser{srv}) + } handler.Logger.Infof("Starting DNS TCP server on port %d", conf.Port) if err := srv.ListenAndServe(); err != nil { handler.Logger.Errorf("DNS TCP Error: %v", err) diff --git a/internal/logger/logger.go b/internal/logger/logger.go index abcb707..4cad20e 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -1,14 +1,10 @@ package logger import ( - "fmt" "io" "os" - "path/filepath" "sync" - "time" - "github.com/mirkobrombin/goup/internal/config" "github.com/muesli/termenv" "github.com/rs/zerolog" ) @@ -124,19 +120,9 @@ func (l *Logger) Warnf(format string, args ...any) { // NewLogger creates a new Logger that writes JSON to files and colored // logs to stdout. func NewLogger(identifier string, fields Fields) (*Logger, error) { - logDir := filepath.Join( - config.GetLogDir(), - identifier, - fmt.Sprintf("%d", time.Now().Year()), - fmt.Sprintf("%02d", time.Now().Month()), - ) - if err := os.MkdirAll(logDir, os.ModePerm); err != nil { - return nil, err - } - - // Log file name format: 03.log (day.log) - logFile := filepath.Join(logDir, fmt.Sprintf("%02d.log", time.Now().Day())) - file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) + // A date-partitioned writer that reopens the file when the day rolls over, + // so a process running past midnight keeps writing to the right day's log. + file, err := newRotatingFileWriter(identifier, "") if err != nil { return nil, err } @@ -159,19 +145,8 @@ func NewLogger(identifier string, fields Fields) (*Logger, error) { // NewPluginLogger creates a plugin-specific log file (JSON format, no stdout). func NewPluginLogger(siteDomain, pluginName string) (*Logger, error) { - logDir := filepath.Join( - config.GetLogDir(), - siteDomain, - fmt.Sprintf("%d", time.Now().Year()), - fmt.Sprintf("%02d", time.Now().Month()), - ) - if err := os.MkdirAll(logDir, os.ModePerm); err != nil { - return nil, err - } - - // Log file format: 03-NomePlugin.log (day-PluginName.log) - logFile := filepath.Join(logDir, fmt.Sprintf("%02d-%s.log", time.Now().Day(), pluginName)) - file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) + // Date-partitioned, day-rotating writer (day-PluginName.log). + file, err := newRotatingFileWriter(siteDomain, pluginName) if err != nil { return nil, err } diff --git a/internal/logger/rotating.go b/internal/logger/rotating.go new file mode 100644 index 0000000..7332def --- /dev/null +++ b/internal/logger/rotating.go @@ -0,0 +1,85 @@ +package logger + +import ( + "fmt" + "os" + "path/filepath" + "sync" + "time" + + "github.com/mirkobrombin/goup/internal/config" +) + +// rotatingFileWriter writes log lines to a date-partitioned file and reopens a +// new file when the calendar day changes. A long-running process therefore +// keeps writing into the correct YYYY/MM/DD.log instead of the file that was +// current when the logger was first created. +type rotatingFileWriter struct { + mu sync.Mutex + identifier string + pluginName string // empty for the main access logger + file *os.File + curKey string +} + +func newRotatingFileWriter(identifier, pluginName string) (*rotatingFileWriter, error) { + w := &rotatingFileWriter{identifier: identifier, pluginName: pluginName} + if err := w.rotateIfNeeded(); err != nil { + return nil, err + } + return w, nil +} + +func (w *rotatingFileWriter) rotateIfNeeded() error { + now := time.Now() + key := fmt.Sprintf("%04d-%02d-%02d", now.Year(), now.Month(), now.Day()) + if w.file != nil && key == w.curKey { + return nil + } + + dir := filepath.Join( + config.GetLogDir(), + w.identifier, + fmt.Sprintf("%d", now.Year()), + fmt.Sprintf("%02d", now.Month()), + ) + if err := os.MkdirAll(dir, 0750); err != nil { + return err + } + + var name string + if w.pluginName == "" { + name = fmt.Sprintf("%02d.log", now.Day()) + } else { + name = fmt.Sprintf("%02d-%s.log", now.Day(), w.pluginName) + } + + f, err := os.OpenFile(filepath.Join(dir, name), os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0640) + if err != nil { + return err + } + if w.file != nil { + _ = w.file.Close() + } + w.file = f + w.curKey = key + return nil +} + +func (w *rotatingFileWriter) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + if err := w.rotateIfNeeded(); err != nil { + return 0, err + } + return w.file.Write(p) +} + +func (w *rotatingFileWriter) Close() error { + w.mu.Lock() + defer w.mu.Unlock() + if w.file != nil { + return w.file.Close() + } + return nil +} diff --git a/internal/middleware/auth.go b/internal/middleware/auth.go index d8890f0..5195bba 100644 --- a/internal/middleware/auth.go +++ b/internal/middleware/auth.go @@ -16,9 +16,11 @@ func BasicAuthMiddleware(next http.Handler) http.Handler { conf := config.GlobalConf config.GlobalConfMu.RUnlock() - // If auth is not configured, skip + // Fail closed: if credentials are not configured, deny rather than + // serving the protected surface unauthenticated. if conf == nil || conf.Account.Username == "" || conf.Account.PasswordHash == "" { - next.ServeHTTP(w, r) + w.Header().Set("WWW-Authenticate", `Basic realm="GoUp Dashboard"`) + http.Error(w, "Unauthorized: authentication is not configured", http.StatusUnauthorized) return } @@ -47,9 +49,10 @@ func TokenAuthMiddleware(next http.Handler) http.Handler { conf := config.GlobalConf config.GlobalConfMu.RUnlock() - // If token is not configured, skip + // Fail closed: if no token is configured, deny rather than serving the + // admin API unauthenticated. if conf == nil || conf.Account.APIToken == "" { - next.ServeHTTP(w, r) + http.Error(w, "Unauthorized: authentication is not configured", http.StatusUnauthorized) return } diff --git a/internal/middleware/auth_test.go b/internal/middleware/auth_test.go index 7db9f65..3abf945 100644 --- a/internal/middleware/auth_test.go +++ b/internal/middleware/auth_test.go @@ -73,17 +73,6 @@ func TestTokenAuthMiddleware(t *testing.T) { name string headers map[string]string expectedStatus int - }{ - {"ValidTokenHeader", map[string]string{"X-API-Token": token}, http.StatusOK}, - {"ValidBearerToken", map[string]string{"Authorization": "Bearer " + token}, http.StatusOK}, - {"InvalidToken", map[string]string{"X-API-Token": "wrong"}, http.StatusUnauthorized}, - {"NoToken", map[string]string{}, http.StatusOK}, - } - - tests = []struct { - name string - headers map[string]string - expectedStatus int }{ {"ValidTokenHeader", map[string]string{"X-API-Token": token}, http.StatusOK}, {"ValidBearerToken", map[string]string{"Authorization": "Bearer " + token}, http.StatusOK}, @@ -107,31 +96,32 @@ func TestTokenAuthMiddleware(t *testing.T) { } } -func TestAuthSkippedIfUnconfigured(t *testing.T) { - // Setup: Empty config +func TestAuthDeniedIfUnconfigured(t *testing.T) { + // Setup: Empty config. Auth must fail closed (deny), never fail open, so an + // admin surface is never served without credentials. config.GlobalConf = &config.GlobalConfig{} nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }) - t.Run("BasicAuthSkipped", func(t *testing.T) { + t.Run("BasicAuthDenied", func(t *testing.T) { middleware := BasicAuthMiddleware(nextHandler) req := httptest.NewRequest("GET", "/", nil) rec := httptest.NewRecorder() middleware.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Errorf("expected status %v, got %v", http.StatusOK, rec.Code) + if rec.Code != http.StatusUnauthorized { + t.Errorf("expected status %v, got %v", http.StatusUnauthorized, rec.Code) } }) - t.Run("TokenAuthSkipped", func(t *testing.T) { + t.Run("TokenAuthDenied", func(t *testing.T) { middleware := TokenAuthMiddleware(nextHandler) req := httptest.NewRequest("GET", "/", nil) rec := httptest.NewRecorder() middleware.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Errorf("expected status %v, got %v", http.StatusOK, rec.Code) + if rec.Code != http.StatusUnauthorized { + t.Errorf("expected status %v, got %v", http.StatusUnauthorized, rec.Code) } }) } diff --git a/internal/restart/restart.go b/internal/restart/restart.go index 7bc0f3a..7356345 100644 --- a/internal/restart/restart.go +++ b/internal/restart/restart.go @@ -11,11 +11,22 @@ import ( var srv *http.Server +// shutdownFn, when set, gracefully drains every running server (web, API, +// dashboard, HTTP/3, DNS) before the process re-execs. It supersedes the +// single-server srv, which only ever tracked the last-registered instance. +var shutdownFn func(time.Duration) error + // SetServer sets the server instance to be restarted. func SetServer(s *http.Server) { srv = s } +// SetShutdownFunc registers a graceful shutdown routine that closes all +// running servers. Set by the server package at startup. +func SetShutdownFunc(fn func(time.Duration) error) { + shutdownFn = fn +} + // RestartHandler handles the HTTP request to restart the server. func RestartHandler(w http.ResponseWriter, r *http.Request) { w.Write([]byte("Server is restarting...")) @@ -27,7 +38,11 @@ func RestartHandler(w http.ResponseWriter, r *http.Request) { // Restart gracefully shuts down the server and re-executes the process. func Restart() { - if srv != nil { + if shutdownFn != nil { + if err := shutdownFn(10 * time.Second); err != nil { + log.Printf("Error during shutdown: %v", err) + } + } else if srv != nil { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() if err := srv.Shutdown(ctx); err != nil { diff --git a/internal/safeguard/safeguard.go b/internal/safeguard/safeguard.go index 083dec3..6f0905c 100644 --- a/internal/safeguard/safeguard.go +++ b/internal/safeguard/safeguard.go @@ -4,15 +4,18 @@ import ( "fmt" "os" "path/filepath" - "runtime" "runtime/pprof" "time" "github.com/mirkobrombin/goup/internal/config" "github.com/mirkobrombin/goup/internal/logger" "github.com/mirkobrombin/goup/internal/restart" + "github.com/shirou/gopsutil/process" ) +// self is the current process handle, used to read the live resident set size. +var self *process.Process + var log *logger.Logger // Start starts the SafeGuard routine if enabled. @@ -45,13 +48,20 @@ func Start() { return } - interval := 10 * time.Second + interval := 30 * time.Second if conf.CheckInterval != "" { if d, err := time.ParseDuration(conf.CheckInterval); err == nil { interval = d } } + if p, err := process.NewProcess(int32(os.Getpid())); err == nil { + self = p + } else { + log.Errorf("[SafeGuard] Unable to read process memory, disabling: %v", err) + return + } + go func() { ticker := time.NewTicker(interval) defer ticker.Stop() @@ -65,10 +75,18 @@ func Start() { } func checkMemory(limitMB int) { - var m runtime.MemStats - runtime.ReadMemStats(&m) - - usageMB := int(m.Sys / 1024 / 1024) + if self == nil { + return + } + // Use the live resident set size (actual physical memory in use), not + // runtime.MemStats.Sys, which reports reserved address space and never + // meaningfully shrinks, causing false restarts. + memInfo, err := self.MemoryInfo() + if err != nil { + log.Errorf("[SafeGuard] Failed to read memory info: %v", err) + return + } + usageMB := int(memInfo.RSS / 1024 / 1024) if usageMB > limitMB { log.Errorf("CRITICAL: Memory usage %dMB exceeded limit %dMB. FORCE RESTARTING...", usageMB, limitMB) diff --git a/internal/server/handler.go b/internal/server/handler.go index 53223f2..39e1c8d 100644 --- a/internal/server/handler.go +++ b/internal/server/handler.go @@ -13,7 +13,6 @@ import ( "github.com/mirkobrombin/goup/internal/assets" "github.com/mirkobrombin/goup/internal/config" "github.com/mirkobrombin/goup/internal/logger" - "github.com/mirkobrombin/goup/internal/plugin" "github.com/mirkobrombin/goup/internal/server/middleware" ) @@ -48,18 +47,6 @@ func createHandler(conf config.SiteConfig, log *logger.Logger, identifier string // Copy the global middleware manager for this site siteMwManager := globalMwManager.Copy() - // Initialize plugins for this site - pluginManager := plugin.GetPluginManagerInstance() - if err := pluginManager.InitPluginsForSite(conf, log); err != nil { - return nil, fmt.Errorf("error initializing plugins for site %s: %v", conf.Domain, err) - } - - // Add per-site middleware - reqTimeout := conf.RequestTimeout - if reqTimeout == 0 { - reqTimeout = 60 // Default to 60 seconds - } - // Add Concurrency Middleware if conf.MaxConcurrentConnections > 0 { siteMwManager.Use(middleware.ConcurrencyMiddleware(conf.MaxConcurrentConnections)) @@ -117,19 +104,23 @@ var ( IdleConnTimeout: 90 * time.Second, TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, + // Bound the time spent waiting for a backend to start responding, so a + // hung upstream cannot pin a goroutine/connection indefinitely. + ResponseHeaderTimeout: 60 * time.Second, } - globalBytePool = &byteSlicePool{ - pool: sync.Pool{ - New: func() any { - return make([]byte, 32*1024) - }, - }, - } + globalBytePool = newByteSlicePool(32 * 1024) ) type byteSlicePool struct { pool sync.Pool + size int +} + +func newByteSlicePool(size int) *byteSlicePool { + p := &byteSlicePool{size: size} + p.pool.New = func() any { return make([]byte, size) } + return p } func (b *byteSlicePool) Get() []byte { @@ -137,8 +128,11 @@ func (b *byteSlicePool) Get() []byte { } func (b *byteSlicePool) Put(buf []byte) { - if cap(buf) == 32*1024 { - b.pool.Put(buf[:32*1024]) + // Recycle only buffers that match this pool's size, so a per-site pool with + // a custom buffer_size_kb actually reuses its buffers instead of allocating + // a fresh one on every request. + if cap(buf) >= b.size { + b.pool.Put(buf[:b.size]) } } @@ -176,13 +170,7 @@ func getSharedReverseProxy(conf config.SiteConfig, log *logger.Logger) (*httputi // Set BufferPool with custom size if specified if conf.BufferSizeKB > 0 { - rp.BufferPool = &byteSlicePool{ - pool: sync.Pool{ - New: func() any { - return make([]byte, conf.BufferSizeKB*1024) - }, - }, - } + rp.BufferPool = newByteSlicePool(conf.BufferSizeKB * 1024) } else { rp.BufferPool = globalBytePool } diff --git a/internal/server/lifecycle.go b/internal/server/lifecycle.go index e4223ec..7a7e04f 100644 --- a/internal/server/lifecycle.go +++ b/internal/server/lifecycle.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "net/http" "sync" "sync/atomic" @@ -15,6 +16,7 @@ const DefaultShutdownTimeout = 10 * time.Second var ( ready atomic.Bool serverRegistry []*http.Server + closerRegistry []io.Closer serverRegistryM sync.Mutex ) @@ -58,18 +60,27 @@ func registerServer(server *http.Server) { serverRegistry = append(serverRegistry, server) } +// registerCloser registers a resource (HTTP/3 server, DNS server, ...) that is +// not an *http.Server but must still be closed on shutdown. +func registerCloser(c io.Closer) { + serverRegistryM.Lock() + defer serverRegistryM.Unlock() + closerRegistry = append(closerRegistry, c) +} + func ShutdownServers(timeout time.Duration) error { SetReady(false) serverRegistryM.Lock() servers := append([]*http.Server(nil), serverRegistry...) + closers := append([]io.Closer(nil), closerRegistry...) serverRegistryM.Unlock() ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() var wg sync.WaitGroup - errs := make(chan error, len(servers)) + errs := make(chan error, len(servers)+len(closers)) for _, srv := range servers { wg.Add(1) @@ -81,6 +92,18 @@ func ShutdownServers(timeout time.Duration) error { }(srv) } + // HTTP/3 and DNS servers expose Close, not graceful Shutdown; close them in + // parallel so QUIC listeners and DNS sockets are released on shutdown. + for _, c := range closers { + wg.Add(1) + go func(cl io.Closer) { + defer wg.Done() + if err := cl.Close(); err != nil && !errors.Is(err, http.ErrServerClosed) { + errs <- err + } + }(c) + } + wg.Wait() close(errs) diff --git a/internal/server/middleware/gzip.go b/internal/server/middleware/gzip.go index 311df17..2a7353a 100644 --- a/internal/server/middleware/gzip.go +++ b/internal/server/middleware/gzip.go @@ -31,30 +31,6 @@ var gzipWriterPool = sync.Pool{ }, } -type gzipResponseWriter struct { - io.Writer - http.ResponseWriter - wroteHeader bool -} - -func (w *gzipResponseWriter) WriteHeader(status int) { - if w.wroteHeader { - return - } - w.wroteHeader = true - w.ResponseWriter.WriteHeader(status) -} - -func (w *gzipResponseWriter) Write(b []byte) (int, error) { - if !w.wroteHeader { - if w.Header().Get("Content-Type") == "" { - w.Header().Set("Content-Type", http.DetectContentType(b)) - } - w.WriteHeader(http.StatusOK) - } - return w.Writer.Write(b) -} - // GzipMiddleware compresses the response if the client supports it and the content is compressible. // Critical: It skips compression if "Content-Encoding" is already set (e.g. by Smart Static Handler serving .gz). func GzipMiddleware(next http.Handler) http.Handler { @@ -103,10 +79,25 @@ func (w *smartGzipWriter) WriteHeader(status int) { w.Header().Del("Content-Length") w.Header().Set("Content-Encoding", "gzip") w.Header().Set("Vary", "Accept-Encoding") + // The compressed body is a different representation than the identity + // one, so it must not share the same strong ETag (that would let a + // cache serve gzip bytes as identity or vice versa). Mark it weak and + // distinct. + if et := w.Header().Get("ETag"); et != "" { + w.Header().Set("ETag", weakGzipETag(et)) + } } w.ResponseWriter.WriteHeader(status) } +// weakGzipETag turns an ETag into a weak validator distinct from the identity +// representation's tag. +func weakGzipETag(et string) string { + trimmed := strings.TrimPrefix(et, "W/") + trimmed = strings.Trim(trimmed, "\"") + return "W/\"" + trimmed + "-gzip\"" +} + func (w *smartGzipWriter) Write(b []byte) (int, error) { if !w.checked { if w.Header().Get("Content-Type") == "" { @@ -132,6 +123,14 @@ func (w *smartGzipWriter) checkCompression() { return } + // Do not compress range responses: http.ServeContent emits 206 with + // identity byte offsets, and wrapping that in gzip produces a spec-invalid, + // corrupt response. + if w.req != nil && w.req.Header.Get("Range") != "" { + w.shouldCompress = false + return + } + ct := w.Header().Get("Content-Type") idx := strings.Index(ct, ";") if idx != -1 { diff --git a/internal/server/server.go b/internal/server/server.go index e814533..99280e9 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1,6 +1,7 @@ package server import ( + "crypto/tls" "fmt" "net" "net/http" @@ -65,15 +66,6 @@ func StartServers(configs []config.SiteConfig, enableTUI bool, enableBench bool, } } -func anyHasSSL(confs []config.SiteConfig) bool { - for _, c := range confs { - if c.SSL.Enabled { - return true - } - } - return false -} - // startSingleServer starts a server for a single site configuration. func startSingleServer(conf config.SiteConfig, mwManager *middleware.MiddlewareManager, pm *plugin.PluginManager) { identifier := conf.Domain @@ -92,11 +84,8 @@ func startSingleServer(conf config.SiteConfig, mwManager *middleware.MiddlewareM } } - // Initialize plugins for this site - if err := pm.InitPluginsForSite(conf, lg); err != nil { - lg.Errorf("Error initializing plugins for site %s: %v", conf.Domain, err) - return - } + // Plugins are initialized up front in launchWebComponents (serially, before + // any server serves) to avoid racing the shared plugin state maps. // Add plugin middleware mwManagerCopy := mwManager.Copy() @@ -129,11 +118,6 @@ func startVirtualHostServer(port int, configs []config.SiteConfig, mwManager *mi } } - if err := pm.InitPluginsForSite(conf, lg); err != nil { - lg.Errorf("Error initializing plugins for site %s: %v", conf.Domain, err) - continue - } - mwManagerCopy := mwManager.Copy() mwManagerCopy.Use(plugin.PluginMiddleware(pm)) @@ -150,7 +134,32 @@ func startVirtualHostServer(port int, configs []config.SiteConfig, mwManager *mi radixTree.Insert(conf.Domain, handler) } + // Load one certificate per SSL-enabled domain on this port. The tls + // package selects the right certificate by SNI at handshake time, so + // virtual hosts no longer silently lose TLS (which previously served them + // as plaintext HTTP on 443). + var certs []tls.Certificate + sslCount := 0 + for _, conf := range configs { + if !conf.SSL.Enabled { + continue + } + sslCount++ + cert, err := tls.LoadX509KeyPair(conf.SSL.Certificate, conf.SSL.Key) + if err != nil { + lg.Errorf("SSL certificate error for %s: %v", conf.Domain, err) + continue + } + certs = append(certs, cert) + } + if sslCount > 0 && sslCount != len(configs) { + lg.Errorf("Port %d mixes SSL and non-SSL sites; all will be served over TLS", port) + } + serverConf := config.SiteConfig{Port: port} + if len(certs) > 0 { + serverConf.SSL.Enabled = true + } mainHandler := func(w_ http.ResponseWriter, r_ *http.Request) { host, _, err := net.SplitHostPort(r_.Host) @@ -171,5 +180,8 @@ func startVirtualHostServer(port int, configs []config.SiteConfig, mwManager *mi } server := createHTTPServer(serverConf, http.HandlerFunc(mainHandler)) + if len(certs) > 0 { + server.TLSConfig.Certificates = certs + } startServerInstance(server, serverConf, lg) } diff --git a/internal/server/server_dns.go b/internal/server/server_dns.go index 9b438ef..850dc10 100644 --- a/internal/server/server_dns.go +++ b/internal/server/server_dns.go @@ -19,7 +19,7 @@ func launchDNS(wg *sync.WaitGroup) { wg.Add(1) go func() { defer wg.Done() - dns.Start(conf.DNS) + dns.Start(conf.DNS, registerCloser) }() } } diff --git a/internal/server/server_utils.go b/internal/server/server_utils.go index c027e6a..1866b0e 100644 --- a/internal/server/server_utils.go +++ b/internal/server/server_utils.go @@ -11,12 +11,31 @@ import ( "github.com/quic-go/quic-go/http3" ) +// Default timeouts applied when a site does not configure its own. They exist +// to protect an out-of-the-box deployment from slowloris-style attacks where a +// client trickles headers or a body to hold connections open indefinitely. +const ( + defaultReadTimeout = 60 * time.Second + defaultReadHeaderTimeout = 10 * time.Second + defaultIdleTimeout = 120 * time.Second +) + // createHTTPServer creates an HTTP server with the given configuration and handler. func createHTTPServer(conf config.SiteConfig, handler http.Handler) *http.Server { - readTimeout := time.Duration(0) - writeTimeout := time.Duration(0) - if conf.RequestTimeout >= 0 { + // ReadTimeout bounds the time to read the whole request (headers + body). + // Default it so a slow client cannot pin a connection forever, but let a + // site opt into a different value (0 disables it, e.g. for large uploads). + readTimeout := defaultReadTimeout + if conf.RequestTimeout > 0 { readTimeout = time.Duration(conf.RequestTimeout) * time.Second + } else if conf.RequestTimeout < 0 { + readTimeout = 0 + } + + // WriteTimeout stays opt-in: a global write deadline would break large + // downloads, proxied streams and SSE. + writeTimeout := time.Duration(0) + if conf.RequestTimeout > 0 { writeTimeout = time.Duration(conf.RequestTimeout) * time.Second } @@ -33,9 +52,11 @@ func createHTTPServer(conf config.SiteConfig, handler http.Handler) *http.Server }, } + s.ReadHeaderTimeout = defaultReadHeaderTimeout if conf.ReadHeaderTimeout > 0 { s.ReadHeaderTimeout = time.Duration(conf.ReadHeaderTimeout) * time.Second } + s.IdleTimeout = defaultIdleTimeout if conf.IdleTimeout > 0 { s.IdleTimeout = time.Duration(conf.IdleTimeout) * time.Second } @@ -52,15 +73,19 @@ func startServerInstance(server *http.Server, conf config.SiteConfig, l *logger. go func() { if conf.SSL.Enabled { - // Load the keypair once and share it between the TCP (h1/h2) and - // QUIC (h3) servers, so handshakes resume sessions from the same - // config instead of re-reading certificate files. - cert, err := tls.LoadX509KeyPair(conf.SSL.Certificate, conf.SSL.Key) - if err != nil { - l.Errorf("SSL certificate error for %s: %v", conf.Domain, err) - return + // For a virtual-host server the caller pre-loads one certificate + // per domain into TLSConfig.Certificates; the tls package selects + // the right one by SNI. For a single-site server we load its + // keypair here. Either way the same certificates are shared with + // the QUIC (h3) server. + if len(server.TLSConfig.Certificates) == 0 { + cert, err := tls.LoadX509KeyPair(conf.SSL.Certificate, conf.SSL.Key) + if err != nil { + l.Errorf("SSL certificate error for %s: %v", conf.Domain, err) + return + } + server.TLSConfig.Certificates = []tls.Certificate{cert} } - server.TLSConfig.Certificates = []tls.Certificate{cert} l.Infof("Serving %s on HTTPS port %d with HTTP/2 and HTTP/3 support", conf.Domain, conf.Port) @@ -70,9 +95,10 @@ func startServerInstance(server *http.Server, conf config.SiteConfig, l *logger. Handler: server.Handler, TLSConfig: &tls.Config{ MinVersion: tls.VersionTLS12, - Certificates: []tls.Certificate{cert}, + Certificates: server.TLSConfig.Certificates, }, } + registerCloser(h3) // Advertise HTTP/3 to TCP clients via Alt-Svc so they can switch // to QUIC on the next request. diff --git a/internal/server/server_web.go b/internal/server/server_web.go index e7ac449..dcd8e93 100644 --- a/internal/server/server_web.go +++ b/internal/server/server_web.go @@ -12,6 +12,8 @@ import ( "github.com/mirkobrombin/goup/internal/dashboard" "github.com/mirkobrombin/goup/internal/logger" "github.com/mirkobrombin/goup/internal/plugin" + "github.com/mirkobrombin/goup/internal/restart" + "github.com/mirkobrombin/goup/internal/safeguard" "github.com/mirkobrombin/goup/internal/server/middleware" "github.com/mirkobrombin/goup/internal/tui" ) @@ -70,8 +72,43 @@ func launchWebComponents(configs []config.SiteConfig, enableTUI bool, enableBenc return } + // Initialize plugins for every site up front, serially, BEFORE any server + // starts serving. The plugin config/state maps are shared across all ports; + // doing this from the per-port goroutines raced writers against readers in + // HandleRequest and could crash with "concurrent map read and map write". + for port, confs := range portConfigs { + identifier := confs[0].Domain + if len(confs) > 1 { + identifier = fmt.Sprintf("port_%d", port) + } + lg := loggers[identifier] + if lg == nil { + // Logger setup failed earlier; skip this port rather than deref a + // nil logger inside plugin init or the request path. + continue + } + for _, conf := range confs { + if err := pluginManager.InitPluginsForSite(conf, lg); err != nil { + lg.Errorf("Error initializing plugins for site %s: %v", conf.Domain, err) + } + } + } + + // Make restart drain every registered server, and let SafeGuard watch + // memory once everything is wired up. + restart.SetShutdownFunc(ShutdownServers) + safeguard.Start() + // Start servers for port, confs := range portConfigs { + identifier := confs[0].Domain + if len(confs) > 1 { + identifier = fmt.Sprintf("port_%d", port) + } + if loggers[identifier] == nil { + // No logger => plugin init was skipped; do not serve this port. + continue + } wg.Add(1) go func(port int, confs []config.SiteConfig) { defer wg.Done() diff --git a/internal/server/static_handler.go b/internal/server/static_handler.go index c0d180f..a6e3a57 100644 --- a/internal/server/static_handler.go +++ b/internal/server/static_handler.go @@ -1,15 +1,12 @@ package server import ( - "crypto/sha256" - "encoding/hex" "fmt" "mime" "net/http" "os" "path" "path/filepath" - "strconv" "strings" "github.com/mirkobrombin/goup/internal/assets" @@ -90,7 +87,16 @@ func ServeStatic(w http.ResponseWriter, r *http.Request, root string) { if isBrowser(r) { var items []assets.ListingItem for _, entry := range entries { - entryInfo, _ := entry.Info() + if isHiddenName(entry.Name()) { + continue + } + entryInfo, err := entry.Info() + if err != nil { + // The entry vanished between ReadDir and Info (or is + // otherwise unreadable); skip it instead of panicking on + // a nil FileInfo. + continue + } items = append(items, assets.ListingItem{ Name: entry.Name(), IsDir: entry.IsDir(), @@ -105,6 +111,9 @@ func ServeStatic(w http.ResponseWriter, r *http.Request, root string) { w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusOK) for _, entry := range entries { + if isHiddenName(entry.Name()) { + continue + } name := entry.Name() if entry.IsDir() { name += "/" @@ -178,13 +187,6 @@ func ServeStatic(w http.ResponseWriter, r *http.Request, root string) { http.ServeContent(w, r, filepath.Base(fullPath), serveInfo.ModTime(), file) } -// Custom ETag calculation (unused in simplified version, but kept for reference) -func calculateETag(info os.FileInfo) string { - hash := sha256.New() - hash.Write([]byte(strconv.FormatInt(info.Size(), 10))) - hash.Write([]byte(strconv.FormatInt(info.ModTime().UnixNano(), 10))) - return hex.EncodeToString(hash.Sum(nil)) -} func formatSizeBytes(b int64) string { const unit = 1024 if b < unit { @@ -203,6 +205,12 @@ func isBrowser(r *http.Request) bool { } func staticLocalPath(root, urlPath string) (string, string, error) { + // An empty root would resolve request paths against the process working + // directory, silently exposing whatever GoUp was started from. Refuse it. + if root == "" { + return "", "", fmt.Errorf("no root directory configured") + } + urlPath = strings.ReplaceAll(urlPath, "\\", "/") cleanPath := path.Clean("/" + strings.TrimPrefix(urlPath, "/")) relPath := strings.TrimPrefix(cleanPath, "/") @@ -210,9 +218,23 @@ func staticLocalPath(root, urlPath string) (string, string, error) { return cleanPath, root, nil } + // Do not serve dotfiles/dotdirs (.git, .env, ...) except the well-known + // directory used for ACME and similar protocols. + for _, seg := range strings.Split(relPath, "/") { + if strings.HasPrefix(seg, ".") && seg != ".well-known" { + return cleanPath, "", fmt.Errorf("hidden path") + } + } + localPath, err := filepath.Localize(relPath) if err != nil { return cleanPath, "", err } return cleanPath, filepath.Join(root, localPath), nil } + +// isHiddenName reports whether a directory entry should be omitted from +// listings (dotfiles other than .well-known). +func isHiddenName(name string) bool { + return strings.HasPrefix(name, ".") && name != ".well-known" +} diff --git a/plugins/auth.go b/plugins/auth.go index a511f8b..eab7044 100644 --- a/plugins/auth.go +++ b/plugins/auth.go @@ -1,7 +1,10 @@ package plugins import ( + "crypto/rand" + "crypto/subtle" "encoding/base64" + "encoding/hex" "errors" "net/http" "strings" @@ -13,6 +16,20 @@ import ( "github.com/mirkobrombin/goup/internal/plugin" ) +// sessionCookieName is the cookie that carries the opaque session token. Auth +// is bound to this unguessable token, never to the client IP (which is +// spoofable via X-Forwarded-For and shared behind NAT). +const sessionCookieName = "goup_session" + +// newSessionToken returns a cryptographically random opaque session token. +func newSessionToken() (string, error) { + b := make([]byte, 32) + if _, err := rand.Read(b); err != nil { + return "", err + } + return hex.EncodeToString(b), nil +} + // AuthPluginConfig represents the configuration for the AuthPlugin. type AuthPluginConfig struct { // Whather the plugin is enabled. @@ -172,10 +189,12 @@ func (p *AuthPlugin) HandleRequest(w http.ResponseWriter, r *http.Request) bool return false } - ip := getClientIP(r) - if sess, exists := st.getSession(ip); exists { - p.DomainLogger.Infof("[AuthPlugin] Valid session for IP=%s user=%s", ip, sess.Username) - return false + // A valid session cookie authenticates the request. + if cookie, err := r.Cookie(sessionCookieName); err == nil && cookie.Value != "" { + if sess, exists := st.getSession(cookie.Value); exists { + p.DomainLogger.Infof("[AuthPlugin] Valid session for user=%s", sess.Username) + return false + } } // No valid session, check for Authorization header. @@ -192,39 +211,52 @@ func (p *AuthPlugin) HandleRequest(w http.ResponseWriter, r *http.Request) bool return true } - expectedPassword, userExists := conf.Credentials[username] - if !userExists || expectedPassword != password { + if !credentialsValid(conf.Credentials, username, password) { unauthorized(w) return true } - st.createSession(ip, username, conf.SessionExpiration, p.PluginLogger) - p.PluginLogger.Infof("[AuthPlugin] Authenticated IP=%s user=%s", ip, username) - - return false -} - -func (p *AuthPlugin) AfterRequest(w http.ResponseWriter, r *http.Request) {} -func (p *AuthPlugin) OnExit() error { return nil } + token, err := newSessionToken() + if err != nil { + p.PluginLogger.Errorf("[AuthPlugin] Failed to generate session token: %v", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return true + } + st.createSession(token, username, conf.SessionExpiration, p.PluginLogger) -// getClientIP extracts the client's IP address from the request. -func getClientIP(r *http.Request) string { - if ip := r.Header.Get("X-Real-IP"); ip != "" { - return ip + cookie := &http.Cookie{ + Name: sessionCookieName, + Value: token, + Path: "/", + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + Secure: r.TLS != nil, } - if ips := r.Header.Get("X-Forwarded-For"); ips != "" { - // X-Forwarded-For may contain multiple IPs, take the first one - return strings.Split(ips, ",")[0] + if conf.SessionExpiration > 0 { + cookie.MaxAge = conf.SessionExpiration } + http.SetCookie(w, cookie) + p.PluginLogger.Infof("[AuthPlugin] Authenticated user=%s", username) - // Fallback to RemoteAddr - ip := r.RemoteAddr - if colonIndex := strings.LastIndex(ip, ":"); colonIndex != -1 { - ip = ip[:colonIndex] + return false +} + +// credentialsValid checks the supplied username/password against the configured +// credentials in constant time, and always performs a comparison (even for an +// unknown user) so response timing does not leak which usernames exist. +func credentialsValid(creds map[string]string, username, password string) bool { + expected, userExists := creds[username] + if !userExists { + // Compare against a dummy of equal work to equalize timing. + subtle.ConstantTimeCompare([]byte(password), []byte(password)) + return false } - return ip + return subtle.ConstantTimeCompare([]byte(expected), []byte(password)) == 1 } +func (p *AuthPlugin) AfterRequest(w http.ResponseWriter, r *http.Request) {} +func (p *AuthPlugin) OnExit() error { return nil } + // parseBasicAuth parses the Basic Authentication header. func parseBasicAuth(authHeader string) (username, password string, ok bool) { const prefix = "Basic " @@ -250,11 +282,11 @@ func unauthorized(w http.ResponseWriter) { http.Error(w, "Unauthorized", http.StatusUnauthorized) } -// getSession retrieves a session for the given IP, if it exists and is valid. -func (s *AuthPluginState) getSession(ip string) (session, bool) { +// getSession retrieves a session for the given token, if it exists and is valid. +func (s *AuthPluginState) getSession(token string) (session, bool) { s.mu.RLock() defer s.mu.RUnlock() - sess, exists := s.sessions[ip] + sess, exists := s.sessions[token] if !exists { return session{}, false } @@ -266,8 +298,8 @@ func (s *AuthPluginState) getSession(ip string) (session, bool) { return sess, true } -// createSession creates a new session for the given IP and username. -func (s *AuthPluginState) createSession(ip, username string, expiration int, l *logger.Logger) { +// createSession stores a new session under the given opaque token. +func (s *AuthPluginState) createSession(token, username string, expiration int, l *logger.Logger) { s.mu.Lock() defer s.mu.Unlock() @@ -275,12 +307,12 @@ func (s *AuthPluginState) createSession(ip, username string, expiration int, l * if expiration != -1 { expiry = time.Now().Add(time.Duration(expiration) * time.Second) } - s.sessions[ip] = session{Username: username, Expiry: expiry} + s.sessions[token] = session{Username: username, Expiry: expiry} if expiration != -1 { - l.Infof("[AuthPlugin] Created session IP=%s user=%s expires=%v", ip, username, expiry) + l.Infof("[AuthPlugin] Created session user=%s expires=%v", username, expiry) } else { - l.Infof("[AuthPlugin] Created session IP=%s user=%s never expires", ip, username) + l.Infof("[AuthPlugin] Created session user=%s never expires", username) } } @@ -290,10 +322,10 @@ func (s *AuthPluginState) cleanupExpiredSessions(interval time.Duration, l *logg defer ticker.Stop() for range ticker.C { s.mu.Lock() - for ip, sess := range s.sessions { + for token, sess := range s.sessions { if !sess.Expiry.IsZero() && sess.Expiry.Before(time.Now()) { - delete(s.sessions, ip) - l.Infof("[AuthPlugin] Session expired removed IP=%s user=%s", ip, sess.Username) + delete(s.sessions, token) + l.Infof("[AuthPlugin] Session expired removed user=%s", sess.Username) } } s.mu.Unlock() diff --git a/plugins/docker_base.go b/plugins/docker_base.go index d9f848d..2b257e0 100644 --- a/plugins/docker_base.go +++ b/plugins/docker_base.go @@ -12,7 +12,6 @@ import ( "path/filepath" "runtime" "strings" - "sync" "time" "github.com/mirkobrombin/goup/internal/config" @@ -30,11 +29,12 @@ type DockerBaseConfig struct { CLICommand string `json:"cli_command"` } -// DockerBasePlugin provides common Docker functionality. +// DockerBasePlugin provides common Docker functionality. siteConfigs is +// populated serially during startup and only read while serving, so no lock is +// required. type DockerBasePlugin struct { plugin.BasePlugin - mu sync.Mutex - Config DockerBaseConfig + siteConfigs map[string]DockerBaseConfig } func (d *DockerBasePlugin) Name() string { @@ -42,6 +42,7 @@ func (d *DockerBasePlugin) Name() string { } func (d *DockerBasePlugin) OnInit() error { + d.siteConfigs = make(map[string]DockerBaseConfig) return nil } @@ -72,41 +73,43 @@ func (d *DockerBasePlugin) OnInitForSite(conf config.SiteConfig, domainLogger *l } } } - d.Config = cfg // If the Docker plugin is not enabled (or not configured) for this site, - // skip CLI resolution entirely so that a missing docker/podman binary does - // not break plugin init on machines that don't need Docker. - if !d.Config.Enable { + // store the disabled config and skip CLI resolution entirely so that a + // missing docker/podman binary does not break plugin init on machines that + // don't need Docker. + if !cfg.Enable { + d.siteConfigs[conf.Domain] = cfg return nil } // Determine CLICommand if not set. - if d.Config.CLICommand == "" { + if cfg.CLICommand == "" { if _, err := exec.LookPath("docker"); err == nil { - d.Config.CLICommand = "docker" + cfg.CLICommand = "docker" } else if _, err := exec.LookPath("podman"); err == nil { - d.Config.CLICommand = "podman" + cfg.CLICommand = "podman" } else { return fmt.Errorf("neither 'docker' nor 'podman' found in PATH") } } // Set default SocketPath. - if runtime.GOOS != "windows" && strings.ToLower(d.Config.CLICommand) == "podman" && d.Config.SocketPath == "" { + if runtime.GOOS != "windows" && strings.ToLower(cfg.CLICommand) == "podman" && cfg.SocketPath == "" { userSocket := fmt.Sprintf("/run/user/%d/podman/podman.sock", os.Getuid()) if _, err := os.Stat(userSocket); err == nil { - d.Config.SocketPath = userSocket + cfg.SocketPath = userSocket } else { - d.Config.SocketPath = "/run/podman/podman.sock" + cfg.SocketPath = "/run/podman/podman.sock" } } - if runtime.GOOS != "windows" && d.Config.SocketPath == "" { - d.Config.SocketPath = "/var/run/docker.sock" + if runtime.GOOS != "windows" && cfg.SocketPath == "" { + cfg.SocketPath = "/var/run/docker.sock" } + d.siteConfigs[conf.Domain] = cfg d.DomainLogger.Infof("[DockerBasePlugin] Initialized for domain=%s, mode=%s, CLICommand=%s, SocketPath=%s", - conf.Domain, d.Config.Mode, d.Config.CLICommand, d.Config.SocketPath) + conf.Domain, cfg.Mode, cfg.CLICommand, cfg.SocketPath) return nil } @@ -116,7 +119,20 @@ func (d *DockerBasePlugin) HandleRequest(w http.ResponseWriter, r *http.Request) if !strings.HasPrefix(r.URL.Path, "/docker/") { return false } - output, err := d.ListContainers() + + // Gate the container-listing endpoint behind the site's own configuration: + // it must be explicitly enabled for the requested host, otherwise the + // Docker inventory would be exposed on every site. + host := r.Host + if idx := strings.Index(host, ":"); idx != -1 { + host = host[:idx] + } + cfg, ok := d.siteConfigs[host] + if !ok || !cfg.Enable { + return false + } + + output, err := d.listContainers(cfg) if err != nil { http.Error(w, fmt.Sprintf("Error listing containers: %v", err), http.StatusInternalServerError) return true @@ -132,25 +148,25 @@ func (d *DockerBasePlugin) OnExit() error { return nil } -// ListContainers lists containers via Docker API; falls back to CLI if needed. -func (d *DockerBasePlugin) ListContainers() (string, error) { - res, err := d.callDockerAPI("GET", "/containers/json", nil) +// listContainers lists containers via Docker API; falls back to CLI if needed. +func (d *DockerBasePlugin) listContainers(cfg DockerBaseConfig) (string, error) { + res, err := d.callDockerAPI(cfg, "GET", "/containers/json", nil) if err == nil { return res, nil } // CLI fallback. - if strings.ToLower(d.Config.CLICommand) == "podman" { - return RunDockerCLI(d.Config.CLICommand, d.Config.DockerfilePath, "ps", "--format", "json") + if strings.ToLower(cfg.CLICommand) == "podman" { + return RunDockerCLI(cfg.CLICommand, cfg.DockerfilePath, "ps", "--format", "json") } - return RunDockerCLI(d.Config.CLICommand, d.Config.DockerfilePath, "ps", "--format", "{{json .}}") + return RunDockerCLI(cfg.CLICommand, cfg.DockerfilePath, "ps", "--format", "{{json .}}") } -func (d *DockerBasePlugin) callDockerAPI(method, path string, body []byte) (string, error) { +func (d *DockerBasePlugin) callDockerAPI(cfg DockerBaseConfig, method, path string, body []byte) (string, error) { d.DomainLogger.Infof("[DockerBasePlugin] Calling Docker API: %s %s", method, path) if runtime.GOOS == "windows" { return "", fmt.Errorf("docker API over Unix socket is not supported on Windows") } - socket := d.Config.SocketPath + socket := cfg.SocketPath if socket == "" { socket = "/var/run/docker.sock" } @@ -159,6 +175,7 @@ func (d *DockerBasePlugin) callDockerAPI(method, path string, body []byte) (stri return net.Dial("unix", socket) }, } + defer transport.CloseIdleConnections() client := &http.Client{Transport: transport, Timeout: 5 * time.Second} urlStr := "http://unix" + path req, err := http.NewRequest(method, urlStr, bytes.NewReader(body)) diff --git a/plugins/docker_standard.go b/plugins/docker_standard.go index 3784807..ceaae17 100644 --- a/plugins/docker_standard.go +++ b/plugins/docker_standard.go @@ -97,8 +97,13 @@ func (d *DockerStandardPlugin) OnInitForSite(conf config.SiteConfig, domainLogge d.states[conf.Domain] = &dockerStandardState{config: cfg} d.DomainLogger.Infof("[DockerStandardPlugin] Initialized for domain=%s with config=%+v", conf.Domain, cfg) - if err := d.ensureContainer(conf.Domain); err != nil { - d.DomainLogger.Warnf("Container not started for domain %s: %v", conf.Domain, err) + // Only pull/build and start a container when the plugin is actually enabled + // for this site. Otherwise a disabled (or absent) config would still shell + // out to docker/podman on every site during startup. + if cfg.Enable { + if err := d.ensureContainer(conf.Domain); err != nil { + d.DomainLogger.Warnf("Container not started for domain %s: %v", conf.Domain, err) + } } return nil } @@ -110,24 +115,39 @@ func (d *DockerStandardPlugin) HandleRequest(w http.ResponseWriter, r *http.Requ if idx := strings.Index(host, ":"); idx != -1 { host = host[:idx] } + d.mu.Lock() state, ok := d.states[host] if !ok || !state.config.Enable { + d.mu.Unlock() return false } - if state.containerID == "" { + started := state.containerID != "" + proxyPaths := state.config.ProxyPaths + d.mu.Unlock() + + if !started { if err := d.ensureContainer(host); err != nil { d.PluginLogger.Errorf("Failed to start container: %v", err) http.Error(w, fmt.Sprintf("Failed to start container: %v", err), http.StatusInternalServerError) return false } } - // Build target URL using the assigned host port. - targetURL := fmt.Sprintf("http://0.0.0.0:%s", state.hostPort) - if len(state.config.ProxyPaths) == 1 && state.config.ProxyPaths[0] == "/" { + // Read the assigned host port under the lock, since ensureContainer writes + // it concurrently with other in-flight requests. + d.mu.Lock() + hostPort := state.hostPort + d.mu.Unlock() + if hostPort == "" { + http.Error(w, "Container port not available", http.StatusBadGateway) + return true + } + targetURL := fmt.Sprintf("http://127.0.0.1:%s", hostPort) + + if len(proxyPaths) == 1 && proxyPaths[0] == "/" { return d.proxyToContainer(targetURL, w, r) } - for _, prefix := range state.config.ProxyPaths { + for _, prefix := range proxyPaths { if strings.HasPrefix(r.URL.Path, prefix) { return d.proxyToContainer(targetURL, w, r) } diff --git a/plugins/nodejs.go b/plugins/nodejs.go index 3b76b03..7c11767 100644 --- a/plugins/nodejs.go +++ b/plugins/nodejs.go @@ -30,7 +30,7 @@ type NodeJSPlugin struct { plugin.BasePlugin mu sync.Mutex - process *os.Process + processes map[string]*os.Process // per-domain Node.js process siteConfigs map[string]NodeJSPluginConfig } @@ -40,6 +40,7 @@ func (p *NodeJSPlugin) Name() string { func (p *NodeJSPlugin) OnInit() error { p.siteConfigs = make(map[string]NodeJSPluginConfig) + p.processes = make(map[string]*os.Process) return nil } @@ -102,7 +103,7 @@ func (p *NodeJSPlugin) HandleRequest(w http.ResponseWriter, r *http.Request) boo } // Ensure Node.js is running if needed. - p.ensureNodeServerRunning(cfg) + p.ensureNodeServerRunning(host, cfg) // Check if path matches one of the ProxyPaths. for _, proxyPath := range cfg.ProxyPaths { @@ -120,25 +121,28 @@ func (p *NodeJSPlugin) AfterRequest(w http.ResponseWriter, r *http.Request) {} func (p *NodeJSPlugin) OnExit() error { p.mu.Lock() defer p.mu.Unlock() - if p.process != nil { - // Log to the plugin logger only - p.PluginLogger.Infof("[NodeJSPlugin] Terminating Node.js process (PID=%d).", p.process.Pid) - _ = p.process.Kill() - p.process = nil + for domain, proc := range p.processes { + if proc != nil { + // Log to the plugin logger only + p.PluginLogger.Infof("[NodeJSPlugin] Terminating Node.js process for domain=%s (PID=%d).", domain, proc.Pid) + _ = proc.Kill() + p.processes[domain] = nil + } } return nil } -// ensureNodeServerRunning starts Node.js if it is not already running. -func (p *NodeJSPlugin) ensureNodeServerRunning(cfg NodeJSPluginConfig) { +// ensureNodeServerRunning starts a Node.js process for the given domain if one +// is not already running. +func (p *NodeJSPlugin) ensureNodeServerRunning(domain string, cfg NodeJSPluginConfig) { p.mu.Lock() defer p.mu.Unlock() - if p.process != nil { + if p.processes[domain] != nil { return } - p.PluginLogger.Infof("Starting Node.js server...") + p.PluginLogger.Infof("Starting Node.js server for domain=%s...", domain) // Install dependencies if required. if cfg.InstallDeps { @@ -157,23 +161,24 @@ func (p *NodeJSPlugin) ensureNodeServerRunning(cfg NodeJSPluginConfig) { cmd.Stderr = p.PluginLogger.Writer() if err := cmd.Start(); err != nil { - p.PluginLogger.Errorf("Failed to start Node.js server: %v", err) + p.PluginLogger.Errorf("Failed to start Node.js server for domain=%s: %v", domain, err) return } - p.process = cmd.Process - p.PluginLogger.Infof("Started Node.js server (PID=%d) on port %s", p.process.Pid, cfg.Port) + p.processes[domain] = cmd.Process + p.PluginLogger.Infof("Started Node.js server for domain=%s (PID=%d) on port %s", domain, cmd.Process.Pid, cfg.Port) - // Watch for process exit. - go func() { - err := cmd.Wait() - p.PluginLogger.Infof("Node.js server exited (PID=%d), err=%v", p.process.Pid, err) + // Watch for process exit. Capture cmd so we read its Pid without racing the + // map reset below. + go func(dom string, c *exec.Cmd) { + err := c.Wait() + p.PluginLogger.Infof("Node.js server exited for domain=%s (PID=%d), err=%v", dom, c.Process.Pid, err) p.PluginLogger.Writer().Close() p.mu.Lock() - p.process = nil + p.processes[dom] = nil p.mu.Unlock() - }() + }(domain, cmd) } // proxyToNode forwards the request to Node.js, streaming both bodies through diff --git a/plugins/php.go b/plugins/php.go index 1cf2f02..bf3ae6f 100644 --- a/plugins/php.go +++ b/plugins/php.go @@ -1,6 +1,7 @@ package plugins import ( + "fmt" "net/http" "os" "path" @@ -248,5 +249,13 @@ func phpScriptPath(root, cleanPath string) (string, error) { if err != nil { return "", err } - return filepath.Join(root, localPath), nil + joined := filepath.Join(root, localPath) + + // Defense in depth: confirm the resolved path never escapes the document + // root, even if Localize behaves unexpectedly on some platform. + rel, err := filepath.Rel(root, joined) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("path escapes document root") + } + return joined, nil } diff --git a/plugins/proxy_shared.go b/plugins/proxy_shared.go index 50f9386..a40a535 100644 --- a/plugins/proxy_shared.go +++ b/plugins/proxy_shared.go @@ -26,6 +26,9 @@ var upstreamTransport = &http.Transport{ IdleConnTimeout: 90 * time.Second, TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, + // Bound the wait for a backend's response headers so a hung app process + // (Node, Python, Docker container) cannot hold the proxy goroutine open. + ResponseHeaderTimeout: 60 * time.Second, } var upstreamProxies sync.Map // target URL -> *httputil.ReverseProxy