security: use crypto/rand for agent API keys (fixes C-1)

Replaces math/rand-based agent API key generation in internal/service/agent.go
with crypto/rand.Read over a 32-byte buffer encoded with base64.RawURLEncoding,
yielding a 43-character URL-safe unpadded ASCII string (256 bits of entropy).

generateAPIKey now returns (string, error); Register and RegisterAgent propagate
entropy-source failures. hashAPIKey is unchanged — the SHA-256 hashed-at-rest
invariant is preserved.

Fixes C-1 (CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator)
from certctl-audit-report.md.

Changes:
- internal/service/agent.go: new imports (crypto/rand, encoding/base64);
  generateAPIKey rewritten to return (string, error); Register and RegisterAgent
  updated to propagate the error.
- internal/service/agent_test.go: TestGenerateAPIKey_Properties regression test
  (non-empty, length 43, valid base64url, 32 decoded bytes, no collisions over
  64 calls). No entropy-failure test — Go 1.24+ (issue #66821) makes crypto/rand
  errors fatal, so that branch is defensively unreachable.

Verification:
- go build ./cmd/server/... ./cmd/agent/... ./cmd/mcp-server/... ./cmd/cli/... → pass
- go vet ./... → pass
- go test -race (CI scope, 43 packages) → pass
- golangci-lint v2.11.4 run ./... → 0 issues
- govulncheck ./... → 0 vulnerabilities in certctl code
- Coverage: service 68.9% / handler 83.6% / domain 82.0% / middleware 63.8%
  (all above CI gates 55/60/40/30)
- grep math/rand in internal/ and cmd/ → zero production hits
- No caller assumes the old 32-char length or legacy charset
This commit is contained in:
shankar0123
2026-04-16 19:43:19 +00:00
parent 1f6cf0eafa
commit b219e5d68a
2 changed files with 62 additions and 10 deletions
+20 -10
View File
@@ -2,11 +2,12 @@ package service
import (
"context"
"crypto/rand"
"crypto/sha256"
"encoding/base64"
"encoding/hex"
"fmt"
"log/slog"
"math/rand"
"time"
"github.com/shankar0123/certctl/internal/domain"
@@ -57,8 +58,11 @@ func (s *AgentService) Register(ctx context.Context, name string, hostname strin
return nil, "", fmt.Errorf("agent name and hostname are required")
}
// Generate API key
apiKey := generateAPIKey()
// Generate API key. crypto/rand failure is non-recoverable — propagate immediately.
apiKey, err := generateAPIKey()
if err != nil {
return nil, "", fmt.Errorf("failed to generate agent api key: %w", err)
}
apiKeyHash := hashAPIKey(apiKey)
now := time.Now()
@@ -380,7 +384,10 @@ func (s *AgentService) GetAgent(ctx context.Context, id string) (*domain.Agent,
// RegisterAgent creates and registers a new agent (handler interface method).
func (s *AgentService) RegisterAgent(ctx context.Context, agent domain.Agent) (*domain.Agent, error) {
agent.ID = generateID("agent")
apiKey := generateAPIKey()
apiKey, err := generateAPIKey()
if err != nil {
return nil, fmt.Errorf("failed to generate agent api key: %w", err)
}
agent.APIKeyHash = hashAPIKey(apiKey)
agent.Status = domain.AgentStatusOnline
now := time.Now()
@@ -487,14 +494,17 @@ func (s *AgentService) CertificatePickup(ctx context.Context, agentID, certID st
return string(certPEM), nil
}
// generateAPIKey creates a random API key for an agent.
func generateAPIKey() string {
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
// generateAPIKey creates a cryptographically secure random API key for an agent.
// It fills a 32-byte buffer from crypto/rand (256 bits of entropy) and encodes it with
// base64.RawURLEncoding, yielding a 43-character URL-safe, unpadded ASCII string.
// The plaintext key is shown to the caller exactly once; only its SHA-256 hash is stored.
// Fixes C-1 (CWE-338: previously used math/rand, which is not cryptographically secure).
func generateAPIKey() (string, error) {
b := make([]byte, 32)
for i := range b {
b[i] = charset[rand.Intn(len(charset))]
if _, err := rand.Read(b); err != nil {
return "", fmt.Errorf("generate agent api key: %w", err)
}
return string(b)
return base64.RawURLEncoding.EncodeToString(b), nil
}
// hashAPIKey hashes an API key using SHA256.