diff --git a/internal/config/defaults_helpers.go b/internal/config/defaults_helpers.go index e71921b83..4a022f0b8 100644 --- a/internal/config/defaults_helpers.go +++ b/internal/config/defaults_helpers.go @@ -133,19 +133,37 @@ func UseRandSource(r io.Reader) (restore func()) { // from the scoped source (crypto/rand by default, see UseRandSource). Used for // auto-generating JWT keys and other secrets in dev mode. The source is never // logged or included in an error. +// +// A generated secret must pass the validators it will later face: a random +// string occasionally spells one of insecurePatterns ("test", "admin", ...) or +// a placeholder, and in staging/prod the validator then rejects a value the +// CLI itself just made (and build has already persisted it to .env.secrets, so +// the project stays broken). Such a draw is discarded and the next bytes of the +// source are used, so a seeded source (plan and apply) still yields one value. func generateSecureRandom(length int) (string, error) { src := io.Reader(rand.Reader) if p := randSource.Load(); p != nil { src = *p } - // Allocate enough bytes to guarantee the encoded output is >= length. - b := make([]byte, length) - if _, err := io.ReadFull(src, b); err != nil { - return "", fmt.Errorf("generating secret: %w", err) - } - encoded := base64.RawURLEncoding.EncodeToString(b) - if len(encoded) < length { - return encoded, nil + for attempt := 0; attempt < secretAttempts; attempt++ { + // Allocate enough bytes to guarantee the encoded output is >= length. + b := make([]byte, length) + if _, err := io.ReadFull(src, b); err != nil { + return "", fmt.Errorf("generating secret: %w", err) + } + encoded := base64.RawURLEncoding.EncodeToString(b) + if len(encoded) >= length { + encoded = encoded[:length] + } + if !isInsecurePassword(encoded) && !isPlaceholderSecret(encoded) { + return encoded, nil + } } - return encoded[:length], nil + return "", fmt.Errorf("generating secret: %d draws in a row matched a blocked pattern", secretAttempts) } + +// secretAttempts bounds the resampling in generateSecureRandom. A fresh draw is +// rejected with probability ~1e-4, so a second rejection is already a +// one-in-100-million event; a source that keeps producing rejected values (a +// stuck fixed reader) fails loudly instead of looping. +const secretAttempts = 32 diff --git a/internal/config/defaults_helpers_test.go b/internal/config/defaults_helpers_test.go index 55918b1bf..67d46e59a 100644 --- a/internal/config/defaults_helpers_test.go +++ b/internal/config/defaults_helpers_test.go @@ -7,6 +7,7 @@ package config // Constraints: tests in this file must not run in parallel with each other. import ( + "encoding/base64" "errors" "strings" "testing" @@ -68,3 +69,92 @@ func TestUseRandSourceErrorHidesSource(t *testing.T) { t.Fatalf("want generating-secret error, got %v", err) } } + +// fixedThenReader serves head once, then falls through to the countReader. +type fixedThenReader struct { + head []byte + then countReader +} + +func (r *fixedThenReader) Read(p []byte) (int, error) { + if len(r.head) > 0 { + n := copy(p, r.head) + r.head = r.head[n:] + return n, nil + } + return r.then.Read(p) +} + +// encodedBytes returns the raw bytes whose base64url form starts with prefix +// (padded with 'A' to a multiple of four), so a test can force the exact +// string generateSecureRandom first sees. +func encodedBytes(t *testing.T, prefix string, length int) []byte { + t.Helper() + enc := prefix + strings.Repeat("A", (4-len(prefix)%4)%4) + for len(enc) < length*4/3 { + enc += "AAAA" + } + raw, err := base64.RawURLEncoding.DecodeString(enc[:len(enc)/4*4]) + if err != nil { + t.Fatal(err) + } + return raw[:length] +} + +// A random draw that spells a blocked pattern (about 1 in 10^4 per project, the +// flake in TestSurfaceIdentity) is discarded, so the secret always passes the +// staging/prod validators. +func TestGenerateSecureRandomSkipsBlockedPatterns(t *testing.T) { + for _, bad := range []string{"xxTeStxx", "AdMiNxx", "xxhasuraxx", "12345xxx", "change-me-"} { + for _, n := range []int{24, 44} { + src := &fixedThenReader{head: encodedBytes(t, bad, n)} + restore := UseRandSource(src) + s, err := generateSecureRandom(n) + restore() + if err != nil || len(s) != n { + t.Fatalf("%q len %d: %q, %v", bad, n, s, err) + } + if len(src.head) != 0 { + t.Fatalf("%q len %d: the forced draw was not consumed", bad, n) + } + if isInsecurePassword(s) || isPlaceholderSecret(s) { + t.Errorf("%q len %d: generated %q, which the validators reject", bad, n, s) + } + } + } +} + +// Plan and apply read one seeded stream: skipping a bad draw must be the same +// decision both times, so the two still produce one secret. +func TestGenerateSecureRandomSkipIsDeterministic(t *testing.T) { + gen := func() string { + restore := UseRandSource(&fixedThenReader{head: encodedBytes(t, "xxtestxx", 24)}) + defer restore() + s, err := generateSecureRandom(24) + if err != nil { + t.Fatal(err) + } + return s + } + if a, b := gen(), gen(); a != b { + t.Errorf("seeded source, different secrets: %q vs %q", a, b) + } +} + +// A source that only ever yields a blocked value fails instead of looping. +func TestGenerateSecureRandomStuckSourceErrors(t *testing.T) { + restore := UseRandSource(&repeatReader{b: encodedBytes(t, "xxtestxx", 24)}) + defer restore() + if s, err := generateSecureRandom(24); err == nil { + t.Fatalf("want an error, got %q", s) + } +} + +type repeatReader struct{ b []byte } + +func (r *repeatReader) Read(p []byte) (int, error) { + for i := range p { + p[i] = r.b[i%len(r.b)] + } + return len(p), nil +}