From 3286851fdff22a62d2d82bbf95bfb879abc47a1f Mon Sep 17 00:00:00 2001 From: wweir Date: Wed, 12 Aug 2026 17:23:14 +0800 Subject: [PATCH] fix(install,config): lock down installed config, atomic binary copy, reject socks5 password, parse bare ipv6 - The installer writes /etc/sower/sowerd.toml with 0600 instead of 0644: the file holds the plaintext client-auth password. - Binary updates are atomic (temp file + rename) so an interrupted update cannot leave a truncated sowerd at /usr/local/bin. - ExecStart quotes both paths for systemd parsing. - remote.password for a socks5 upstream is rejected at validation instead of being silently ignored (the transport only negotiates no-auth). - splitRemoteAddr accepts bare IPv6 literals (default port) and rejects malformed multi-colon addresses like '::::' instead of passing them through as hosts. --- config/sower.go | 17 ++++++++-- config/sower_test.go | 54 ++++++++++++++++++++++++++++++++ internal/install/service.go | 28 ++++++++++++++--- internal/install/service_test.go | 4 +-- 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/config/sower.go b/config/sower.go index 11cd067..72af59c 100644 --- a/config/sower.go +++ b/config/sower.go @@ -116,6 +116,13 @@ func (c *SowerConfig) Validate() error { return fmt.Errorf("unsupported remote type %q", c.Remote.Type) } + if c.Remote.Type == "socks5" && c.Remote.Password != "" { + // The SOCKS5 client transport only negotiates no-auth (RFC 1928); + // a configured password would be silently ignored and the upstream + // handshake would fail only when it demands authentication. + return fmt.Errorf("remote password is not supported for socks5 upstreams; remove remote.password or use remote.type = \"sower\"") + } + remoteHost, err := validateRemoteAddr(c.Remote.Type, c.Remote.Addr) if err != nil { return err @@ -234,8 +241,14 @@ func splitRemoteAddr(addr, defaultPort string) (string, string, error) { if strings.HasPrefix(addr, "[") || strings.HasSuffix(addr, "]") { return "", "", fmt.Errorf("invalid address %q", addr) } - if strings.Count(addr, ":") == 1 { - return "", "", fmt.Errorf("missing or invalid port in %q", addr) + // A bare IPv6 literal is a valid host with the default port. Anything + // else that still contains a colon is a malformed host:port (e.g. a + // bare IPv6 with a port, which needs [addr]:port) or a garbage string. + if ip := net.ParseIP(addr); ip != nil { + return addr, defaultPort, nil + } + if strings.Contains(addr, ":") { + return "", "", fmt.Errorf("invalid address %q: IPv6 with a port must use [addr]:port", addr) } return addr, defaultPort, nil } diff --git a/config/sower_test.go b/config/sower_test.go index b457007..558df65 100644 --- a/config/sower_test.go +++ b/config/sower_test.go @@ -399,3 +399,57 @@ disable = true t.Fatalf("admin tagged base64 password = %q, want secret-pass", got) } } + +func TestSowerConfigValidateRejectsSocks5Password(t *testing.T) { + t.Parallel() + + cfg := SowerConfig{} + cfg.Remote.Type = "socks5" + cfg.Remote.Addr = "proxy.example.com:1080" + cfg.Remote.Password = "secret" + cfg.DNS.Disable = true + + if err := cfg.Validate(); err == nil { + t.Fatal("expected validation error for socks5 with password") + } +} + +func TestSowerConfigValidateAllowsSocks5WithoutPassword(t *testing.T) { + t.Parallel() + + cfg := SowerConfig{} + cfg.Remote.Type = "socks5" + cfg.Remote.Addr = "proxy.example.com:1080" + cfg.DNS.Disable = true + cfg.DNS.Fallback = "223.5.5.5" + cfg.Socks5.Disable = true + + if err := cfg.Validate(); err != nil { + t.Fatalf("validate config: %v", err) + } +} + +func TestSplitRemoteAddrBareIPv6(t *testing.T) { + t.Parallel() + + host, port, err := splitRemoteAddr("2001:db8::1", "443") + if err != nil || host != "2001:db8::1" || port != "443" { + t.Fatalf("bare ipv6: host=%q port=%q err=%v", host, port, err) + } + + if _, _, err := splitRemoteAddr(":::", "443"); err == nil { + t.Fatal("garbage multi-colon address must be rejected") + } + if _, _, err := splitRemoteAddr("1:2:3:4:5:6:7:8:9", "443"); err == nil { + t.Fatal("overlong ipv6-like address must be rejected") + } + + host, port, err = splitRemoteAddr("[2001:db8::1]:8443", "443") + if err != nil || host != "2001:db8::1" || port != "8443" { + t.Fatalf("bracketed ipv6: host=%q port=%q err=%v", host, port, err) + } + + if _, _, err := splitRemoteAddr("::::", "443"); err == nil { + t.Fatal("garbage address must be rejected") + } +} diff --git a/internal/install/service.go b/internal/install/service.go index b4da425..90dbf93 100644 --- a/internal/install/service.go +++ b/internal/install/service.go @@ -150,8 +150,28 @@ func copyFile(src, dst string, mode os.FileMode) error { if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { return fmt.Errorf("create binary directory: %w", err) } - if err := os.WriteFile(dst, data, mode); err != nil { - return fmt.Errorf("write binary to %s: %w", dst, err) + // Write via a temp file and rename so an interrupted update never leaves + // a truncated binary behind (the service is stopped at that point and + // would otherwise fail to start until the file is rewritten). + tmp, err := os.CreateTemp(filepath.Dir(dst), ".sowerd-*.tmp") + if err != nil { + return fmt.Errorf("create temp binary in %s: %w", filepath.Dir(dst), err) + } + tmpName := tmp.Name() + defer func() { _ = os.Remove(tmpName) }() + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return fmt.Errorf("write temp binary %s: %w", tmpName, err) + } + if err := tmp.Chmod(mode); err != nil { + _ = tmp.Close() + return fmt.Errorf("chmod temp binary %s: %w", tmpName, err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("close temp binary %s: %w", tmpName, err) + } + if err := os.Rename(tmpName, dst); err != nil { + return fmt.Errorf("rename temp binary to %s: %w", dst, err) } return nil } @@ -168,7 +188,7 @@ Wants=network-online.target [Service] Type=simple -ExecStart=%s -c %s +ExecStart="%s" -c "%s" Restart=always RestartSec=5s @@ -196,7 +216,7 @@ func ensureDefaultFiles(configPath, fakeSiteDir, defaultConfigTOML string) error return fmt.Errorf("stat default config path: %w", err) } - if err := os.WriteFile(configPath, []byte(defaultConfigTOML), 0o644); err != nil { + if err := os.WriteFile(configPath, []byte(defaultConfigTOML), 0o600); err != nil { return fmt.Errorf("write default config: %w", err) } fmt.Printf(" OK default config written to %s\n", configPath) diff --git a/internal/install/service_test.go b/internal/install/service_test.go index 2a79a80..0407893 100644 --- a/internal/install/service_test.go +++ b/internal/install/service_test.go @@ -76,8 +76,8 @@ func TestInstallServiceFreshInstall(t *testing.T) { t.Fatalf("read service file: %v", err) } serviceText := string(serviceContent) - if !strings.Contains(serviceText, "ExecStart="+targetBinaryPath+" -c "+configPath) { - t.Fatalf("service file missing ExecStart, content = %s", serviceText) + if !strings.Contains(serviceText, `ExecStart="`+targetBinaryPath+`" -c "`+configPath+`"`) { + t.Fatalf("service file missing quoted ExecStart, content = %s", serviceText) } configContent, err := os.ReadFile(configPath)