ECDSA signing key derived from math/rand instead of crypto/rand #350

Activity

1. raballew

   added

   security

   on Mar 19, 2026
2. raballew

   added a commit that references this issue on Mar 31, 2026

   fix: use crypto/rand instead of math/rand for ECDSA key generation

   ...

   This issue will close once commit 943cefa is merged into the 'main' branch.

   943cefa
3. mangelajo commented on Apr 8, 2026

   

   mangelajo

   on Apr 8, 2026

   Member

   More actions

   This is incorrect, that key is used for validating existing tokens, and cannot be rotated.

   An alternative could be cert-based jwt tokens for the internal authentication, with a migration path, but again, if the cert key is leaked, you have the same problem as leaking the controller or router keys.
4. mangelajo

   added

   ambient-investigate

   on Apr 8, 2026
5. raballew commented on Apr 8, 2026

   

   raballew

   on Apr 8, 2026

   MemberAuthor

   More actions

   I am curious math/rand isnt cryptographically secure anyhow. I agree on the CONTROLLER_KEY part.
6. ambient-code commented on Apr 8, 2026

   

   ambient-codeBot

   on Apr 8, 2026 – with Ambient Code

   Contributor

   More actions

   Investigation: ECDSA signing key derivation

   The design is intentionally deterministic, and that's fine

   After reading controller/internal/oidc/op.go, I can confirm the signing key is derived from CONTROLLER_KEY so that controller restarts produce the same ECDSA key, allowing previously-issued JWT tokens (which have a 365-day expiry) to remain valid without requiring persistent key storage. This is a reasonable design choice — if CONTROLLER_KEY is compromised, the attacker can derive the signing key no matter what derivation method is used.

   However, there is a real entropy reduction

   The current code does:

   hash := sha256.Sum256(seed)                                         // 256 bits
   source := rand.NewSource(int64(binary.BigEndian.Uint64(hash[:8])))  // truncated to 64 bits
   reader := rand.New(source)                                          // math/rand PRNG with 64-bit state
   key, err := keygen.ECDSALegacy(elliptic.P256(), reader)

   This takes a 256-bit SHA-256 hash but only uses the first 64 bits as a seed for math/rand. The resulting ECDSA P-256 key (which normally provides ~128 bits of security) can only be one of ~2^64 possible keys. An attacker who does not know CONTROLLER_KEY but wants to brute-force the signing key only needs to search ~2^64 candidates instead of ~2^128. While 2^64 is not trivially breakable today, it is well below the security margin you'd want for a signing key.

   A fix that preserves token continuity is not straightforward

   As @mangelajo noted, changing the derivation would produce a different key, invalidating all existing tokens. If you wanted to fix this without breaking existing deployments, you could:

   1. Use the full 256-bit SHA-256 output as a seed for a CSPRNG (e.g., via golang.org/x/crypto/hkdf → crypto/ecdsa.GenerateKey), but this would change the derived key.
   2. Support both the old and new key simultaneously during a migration period — validate tokens against both keys, but only sign new tokens with the new key. After old tokens expire (up to 365 days), drop the old key.

   Practical risk assessment

   The real-world risk depends on the threat model. If CONTROLLER_KEY is a high-entropy secret (e.g., 32+ random bytes), an attacker without access to it would still need ~2^64 work to find the signing key — expensive but not impossible for well-funded attackers. If CONTROLLER_KEY is a low-entropy value, the situation is worse since SHA-256 + truncation doesn't help.

   The most practical attack vector remains compromising CONTROLLER_KEY itself, which makes this a defense-in-depth concern rather than a critical vulnerability. The fix is worth doing eventually (especially with a dual-key migration path), but it's not urgent.

   ---

   This investigation was performed by an automated agent (Ambient/Claude).
7. ambient-code

   added

   already-checked

   on Apr 8, 2026
8. raballew

   added this to the 0.10.0 milestone on Sep 24, 2026