fix(apiserver): never sign or accept JWTs with the shipped default secret (bug-1945)
Cause: config/app.yaml shipped secretKey "xce-api-key" and resolveJWTSecret used it whenever XCE_JWT_SECRET was unset, so anyone could forge tokens for any user on such a deployment. Fix: resolveJWTSecret uses XCE_JWT_SECRET, else the config value, but treats an empty value or the old public default as unset; then it generates a random 256-bit per-process secret and InitConf logs a warning (secret never logged): tokens do not survive a restart and apiservers sharing one endpoint must set the same XCE_JWT_SECRET. The shipped app.yaml no longer carries a secret. The controllers test secret is no longer the old default. Tests: TestResolveJWTSecret_NeverUsesKnownDefaultOrEmpty, TestShippedAppYAML_NoJWTSecret, TestInitConf_NoJWTSecretGeneratesAndWarns, TestInitConf_JWTSecretFromEnvNoWarning. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c4c63f11e7
commit
19a1a6f023
4 changed files with 131 additions and 8 deletions
|
|
@ -19,7 +19,12 @@ security:
|
|||
authentication:
|
||||
token:
|
||||
type: "jwt"
|
||||
secretKey: "xce-api-key"
|
||||
# JWT signing secret. Prefer the XCE_JWT_SECRET environment variable
|
||||
# (overrides this value). If neither is set, or the value is the old
|
||||
# public default "xce-api-key", the apiserver uses a random per-process
|
||||
# secret and logs a warning: tokens then do not survive a restart, and
|
||||
# several apiservers behind one endpoint must share XCE_JWT_SECRET.
|
||||
secretKey: ""
|
||||
token_expire_time: 86400 # 1 day
|
||||
|
||||
rate_limit:
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
// Integration tests for XCE API server controllers (§2, §6, §8).
|
||||
//
|
||||
// Test signing secret: "xce-api-key" (test-only, never use in production).
|
||||
// Test signing secret: testJWTSecret (test-only, never use in production).
|
||||
// JWT tokens must embed "ip": "127.0.0.1" to match req.RemoteAddr set by httptest.
|
||||
package controllers
|
||||
|
||||
|
|
@ -26,7 +26,7 @@ import (
|
|||
"go.uber.org/zap"
|
||||
)
|
||||
|
||||
const testJWTSecret = "xce-api-key"
|
||||
const testJWTSecret = "controllers-test-only-secret"
|
||||
|
||||
// ========== Test Helpers ==========
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
package utils
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
|
|
@ -164,10 +166,18 @@ func InitConf() error {
|
|||
AppVersion: viper.GetString("app.version"),
|
||||
}
|
||||
// parse authentication section
|
||||
configuredSecret := viper.GetString("authentication.token.secretKey")
|
||||
jwtSecret := resolveJWTSecret(configuredSecret)
|
||||
if jwtSecret != configuredSecret && jwtSecret != os.Getenv("XCE_JWT_SECRET") {
|
||||
Warn("No JWT secret configured (XCE_JWT_SECRET or authentication.token.secretKey), " +
|
||||
"or it is the publicly known default: using a random per-process secret. " +
|
||||
"Tokens do not survive an apiserver restart, and apiservers sharing one endpoint " +
|
||||
"must all set the same XCE_JWT_SECRET")
|
||||
}
|
||||
AuthenticationYamlConfig = &Authentication{
|
||||
Token: TokenConfig{
|
||||
Type: viper.GetString("authentication.token.type"),
|
||||
SecretKey: resolveJWTSecret(viper.GetString("authentication.token.secretKey")),
|
||||
SecretKey: jwtSecret,
|
||||
TokenExpireTime: int(viper.GetFloat64("authentication.token.token_expire_time")),
|
||||
},
|
||||
}
|
||||
|
|
@ -340,11 +350,31 @@ func resolveTrustedProxies(entries []string) ([]string, error) {
|
|||
return proxies, nil
|
||||
}
|
||||
|
||||
// resolveJWTSecret returns the JWT secret from XCE_JWT_SECRET env var if set and non-empty,
|
||||
// otherwise falls back to the value from config file.
|
||||
// knownDefaultJWTSecret is the secret older app.yaml files shipped with; it is
|
||||
// public, so it is treated as "no secret configured" (bug-1945).
|
||||
const knownDefaultJWTSecret = "xce-api-key"
|
||||
|
||||
// usableJWTSecret reports whether s may sign tokens: non-empty and not the
|
||||
// publicly known default.
|
||||
func usableJWTSecret(s string) bool {
|
||||
return s != "" && s != knownDefaultJWTSecret
|
||||
}
|
||||
|
||||
// resolveJWTSecret returns the JWT secret: XCE_JWT_SECRET if usable, else
|
||||
// the config file value if usable, else a random per-process secret (the
|
||||
// caller detects that case because the result differs from both inputs).
|
||||
func resolveJWTSecret(configValue string) string {
|
||||
if env := os.Getenv("XCE_JWT_SECRET"); env != "" {
|
||||
if env := os.Getenv("XCE_JWT_SECRET"); usableJWTSecret(env) {
|
||||
return env
|
||||
}
|
||||
return configValue
|
||||
if usableJWTSecret(configValue) {
|
||||
return configValue
|
||||
}
|
||||
buf := make([]byte, 32)
|
||||
if _, err := rand.Read(buf); err != nil {
|
||||
// crypto/rand never fails on supported platforms; refuse to fall
|
||||
// back to a predictable secret if it ever does.
|
||||
panic(fmt.Sprintf("cannot generate a JWT secret: %v", err))
|
||||
}
|
||||
return hex.EncodeToString(buf)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -40,6 +40,94 @@ func TestResolveJWTSecret_EmptyEnvUsesConfig(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// bug-1945: the publicly known default secret and an empty secret are never
|
||||
// used; a random per-process secret replaces them.
|
||||
func TestResolveJWTSecret_NeverUsesKnownDefaultOrEmpty(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
env string
|
||||
config string
|
||||
}{
|
||||
{"default in config, no env", "", "xce-api-key"},
|
||||
{"empty config, no env", "", ""},
|
||||
{"default in env, empty config", "xce-api-key", ""},
|
||||
{"default in env and config", "xce-api-key", "xce-api-key"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Setenv("XCE_JWT_SECRET", tc.env)
|
||||
got := resolveJWTSecret(tc.config)
|
||||
if got == "" || got == "xce-api-key" {
|
||||
t.Fatalf("resolveJWTSecret(%q) with env %q = %q: must be a generated secret", tc.config, tc.env, got)
|
||||
}
|
||||
if len(got) < 32 {
|
||||
t.Errorf("generated secret too short: %d chars", len(got))
|
||||
}
|
||||
if again := resolveJWTSecret(tc.config); again == got {
|
||||
t.Errorf("generated secrets must be random, got %q twice", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1945: the shipped app.yaml carries no JWT secret.
|
||||
func TestShippedAppYAML_NoJWTSecret(t *testing.T) {
|
||||
shipped, err := os.ReadFile(filepath.Join("..", "config", "app.yaml"))
|
||||
if err != nil {
|
||||
t.Fatalf("read shipped app.yaml: %v", err)
|
||||
}
|
||||
v := viper.New()
|
||||
v.SetConfigType("yaml")
|
||||
if err := v.ReadConfig(strings.NewReader(string(shipped))); err != nil {
|
||||
t.Fatalf("parse shipped app.yaml: %v", err)
|
||||
}
|
||||
if s := v.GetString("authentication.token.secretKey"); s != "" {
|
||||
t.Errorf("shipped app.yaml must not set authentication.token.secretKey, has %q", s)
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1945: InitConf on the shipped app.yaml without XCE_JWT_SECRET runs
|
||||
// with a random secret and logs a warning (never the secret itself).
|
||||
func TestInitConf_NoJWTSecretGeneratesAndWarns(t *testing.T) {
|
||||
t.Setenv("XCE_JWT_SECRET", "")
|
||||
logs := setupInitConf(t, "job:\n max_array_size: 1000")
|
||||
oldAuth := AuthenticationYamlConfig
|
||||
t.Cleanup(func() { AuthenticationYamlConfig = oldAuth })
|
||||
if err := InitConf(); err != nil {
|
||||
t.Fatalf("InitConf: %v", err)
|
||||
}
|
||||
secret := AuthenticationYamlConfig.Token.SecretKey
|
||||
if secret == "" || secret == "xce-api-key" {
|
||||
t.Fatalf("SecretKey = %q: must be a generated secret", secret)
|
||||
}
|
||||
warns := logs.FilterLevelExact(zap.WarnLevel).FilterMessageSnippet("XCE_JWT_SECRET").All()
|
||||
if len(warns) != 1 {
|
||||
t.Fatalf("expected 1 JWT secret warning, got %d (all logs: %v)", len(warns), logs.All())
|
||||
}
|
||||
for _, e := range logs.All() {
|
||||
if strings.Contains(e.Message, secret) {
|
||||
t.Errorf("log message contains the secret: %q", e.Message)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1945: a configured secret is used as is, without a warning.
|
||||
func TestInitConf_JWTSecretFromEnvNoWarning(t *testing.T) {
|
||||
t.Setenv("XCE_JWT_SECRET", "a-deployment-secret-of-sufficient-length")
|
||||
logs := setupInitConf(t, "job:\n max_array_size: 1000")
|
||||
oldAuth := AuthenticationYamlConfig
|
||||
t.Cleanup(func() { AuthenticationYamlConfig = oldAuth })
|
||||
if err := InitConf(); err != nil {
|
||||
t.Fatalf("InitConf: %v", err)
|
||||
}
|
||||
if got := AuthenticationYamlConfig.Token.SecretKey; got != "a-deployment-secret-of-sufficient-length" {
|
||||
t.Errorf("SecretKey = %q, want the XCE_JWT_SECRET value", got)
|
||||
}
|
||||
if n := logs.FilterMessageSnippet("XCE_JWT_SECRET").Len(); n != 0 {
|
||||
t.Errorf("unexpected JWT secret warning (%d)", n)
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1948: security.trusted_proxies accepts IPs and CIDRs only; empty
|
||||
// means no trusted proxy.
|
||||
func TestResolveTrustedProxies(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue