From 22844a2a67657b7dc98194f0ec4f3946744da044 Mon Sep 17 00:00:00 2001 From: s1d3sw1ped_bot Date: Mon, 31 Aug 2026 23:55:34 +0000 Subject: [PATCH] Persist a per-install JWT signing secret instead of a compiled-in default. Admin tokens were forgeable whenever JWT_SECRET was unset. Prefer the env var, otherwise write a random key to data/.jwt_secret. --- .gitignore | 1 + README.md | 1 + cmd/helix-proxy/run.go | 6 ++++- docker-compose.yml | 1 + internal/auth/jwt.go | 55 ++++++++++++++++++++++++++++++++++++--- internal/auth/jwt_test.go | 49 +++++++++++++++++++++++++++++++--- 6 files changed, 106 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 03b43da..fbf273d 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,7 @@ logs/ *.sqlite-wal # Keys / secrets +.jwt_secret keys.json *.pem *.key diff --git a/README.md b/README.md index 770d1d3..ffe3e48 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ See docker-compose.yml for full example (exposes 80/81/443, volume for data/). - `data/db.bolt` (or `DATA_DIR`) - `data/certs/`, `data/logs/`, `data/letsencrypt-acme-challenge/` - `data/www/` (default site / custom html; `WWW_DIR` or `HTML_DIR`) +- `data/.jwt_secret` (auto-generated admin API signing key if `JWT_SECRET` is unset) - etc. ## Status diff --git a/cmd/helix-proxy/run.go b/cmd/helix-proxy/run.go index ae9473b..1816691 100644 --- a/cmd/helix-proxy/run.go +++ b/cmd/helix-proxy/run.go @@ -45,7 +45,11 @@ func run(ctx context.Context) error { eng.ReloadFromStore() cm := certificate.NewManager(st) - jwtMgr := auth.NewJWTManager(os.Getenv("JWT_SECRET")) + jwtSecret, err := auth.LoadOrCreateSecret(config.DataDir()) + if err != nil { + return err + } + jwtMgr := auth.NewJWTManager(jwtSecret) go startRenewalLoop(ctx, cm, eng) diff --git a/docker-compose.yml b/docker-compose.yml index b18a144..ea50a0f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -12,6 +12,7 @@ services: # user: "0:0" # required when using PUID/PGID != built-in to allow binary to chown+drop # working_dir: /app # binary uses CWD for relative data/ + data/www/ # environment: + # - JWT_SECRET= # optional; otherwise a random secret is stored in data/.jwt_secret # - DATA_DIR=/app/data # - WWW_DIR=/app/data/www # - PUID=1000 diff --git a/internal/auth/jwt.go b/internal/auth/jwt.go index 676e2b6..3584a2a 100644 --- a/internal/auth/jwt.go +++ b/internal/auth/jwt.go @@ -2,9 +2,13 @@ package auth import ( "context" + "crypto/rand" + "encoding/hex" "errors" "fmt" "net/http" + "os" + "path/filepath" "strings" "time" @@ -35,11 +39,56 @@ type JWTManager struct { secret []byte } -// NewJWTManager creates a manager. In real use, load secret from secure store or env (never commit real secret). -// For demo we accept a secret; in production rotate and use env/JWT_SECRET or db meta. +const jwtSecretFilename = ".jwt_secret" + +// LoadOrCreateSecret returns JWT_SECRET from the environment if set, otherwise +// a per-install secret persisted at dir/.jwt_secret (created on first run). +func LoadOrCreateSecret(dir string) (string, error) { + if s := strings.TrimSpace(os.Getenv("JWT_SECRET")); s != "" { + return s, nil + } + if strings.TrimSpace(dir) == "" { + return "", fmt.Errorf("jwt secret directory is required when JWT_SECRET is unset") + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return "", fmt.Errorf("create jwt secret dir: %w", err) + } + path := filepath.Join(dir, jwtSecretFilename) + if b, err := os.ReadFile(path); err == nil { + s := strings.TrimSpace(string(b)) + if s != "" { + return s, nil + } + } else if !errors.Is(err, os.ErrNotExist) { + return "", fmt.Errorf("read jwt secret: %w", err) + } + s, err := randomSecret() + if err != nil { + return "", err + } + if err := os.WriteFile(path, []byte(s+"\n"), 0o600); err != nil { + return "", fmt.Errorf("write jwt secret: %w", err) + } + return s, nil +} + +func randomSecret() (string, error) { + b := make([]byte, 32) + if _, err := rand.Read(b); err != nil { + return "", fmt.Errorf("generate jwt secret: %w", err) + } + return hex.EncodeToString(b), nil +} + +// NewJWTManager creates a manager. Pass a secret from LoadOrCreateSecret or JWT_SECRET. +// An empty secret is replaced with a random in-memory value (tokens will not survive restart). func NewJWTManager(secret string) *JWTManager { if secret == "" { - secret = "dev-only-insecure-secret-change-in-prod" + s, err := randomSecret() + if err != nil { + panic(err) + } + secret = s } return &JWTManager{secret: []byte(secret)} } diff --git a/internal/auth/jwt_test.go b/internal/auth/jwt_test.go index faf6b68..1589d78 100644 --- a/internal/auth/jwt_test.go +++ b/internal/auth/jwt_test.go @@ -4,6 +4,8 @@ import ( "context" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "time" ) @@ -30,12 +32,53 @@ func TestJWTManager_GenerateValidate_Roundtrip(t *testing.T) { } } -func TestJWTManager_DevSecretFallback(t *testing.T) { - m := NewJWTManager("") // empty -> dev +func TestJWTManager_EmptySecretRandomRoundtrip(t *testing.T) { + m := NewJWTManager("") // empty -> random in-memory secret token, _ := m.GenerateToken(1, "a@b", "A", nil) _, err := m.ValidateToken(token) if err != nil { - t.Error("dev secret should allow roundtrip") + t.Error("random secret should allow roundtrip") + } + m2 := NewJWTManager("") + if _, err := m2.ValidateToken(token); err == nil { + t.Error("separate empty managers must not share a well-known secret") + } +} + +func TestLoadOrCreateSecret_EnvWins(t *testing.T) { + t.Setenv("JWT_SECRET", "from-env") + got, err := LoadOrCreateSecret(t.TempDir()) + if err != nil { + t.Fatal(err) + } + if got != "from-env" { + t.Fatalf("got %q", got) + } +} + +func TestLoadOrCreateSecret_Persists(t *testing.T) { + t.Setenv("JWT_SECRET", "") + dir := t.TempDir() + a, err := LoadOrCreateSecret(dir) + if err != nil { + t.Fatal(err) + } + if a == "" { + t.Fatal("empty secret") + } + b, err := LoadOrCreateSecret(dir) + if err != nil { + t.Fatal(err) + } + if a != b { + t.Fatalf("secret not persisted: %q vs %q", a, b) + } + raw, err := os.ReadFile(filepath.Join(dir, ".jwt_secret")) + if err != nil { + t.Fatal(err) + } + if string(raw) == "" { + t.Fatal("secret file empty") } }