From 9c4bb4a90ec0d42003436ae93a2eabc837dfeafd Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 22 Apr 2026 05:53:02 +0100 Subject: [PATCH] Fail closed on auth env hashing and TLS floors --- .../v6/internal/subsystems/registry.json | 23 ++++++++ .../internal/subsystems/security-privacy.md | 30 +++++++--- internal/config/config.go | 58 +++++++++++-------- internal/config/config_load_test.go | 19 ++++++ internal/config/watcher.go | 6 +- internal/config/watcher_test.go | 47 +++++++++++++-- pkg/tlsutil/fingerprint.go | 7 ++- pkg/tlsutil/tlsutil_test.go | 32 +++++++--- 8 files changed, 177 insertions(+), 45 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index 06ff9fd04..e696ee690 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -4242,9 +4242,12 @@ "internal/api/system_settings.go", "internal/cloudcp/auth/magiclink.go", "internal/cloudcp/auth/magiclink_store.go", + "internal/config/config.go", + "internal/config/watcher.go", "internal/crypto/crypto.go", "internal/securityutil/secure_storage_dir.go", "internal/telemetry/telemetry.go", + "pkg/tlsutil/fingerprint.go", "scripts/telemetry_adoption_report.py", "SECURITY.md" ], @@ -4264,7 +4267,10 @@ "internal/api/security_tokens_test.go", "internal/api/system_settings_telemetry_test.go", "internal/cloudcp/auth/magiclink_test.go", + "internal/config/config_load_test.go", + "internal/config/watcher_test.go", "internal/telemetry/telemetry_test.go", + "pkg/tlsutil/tlsutil_test.go", "scripts/tests/test_telemetry_adoption_report.py" ], "require_explicit_path_policy_coverage": true, @@ -4348,6 +4354,23 @@ "frontend-modern/src/utils/__tests__/apiTokenPresentation.test.ts" ] }, + { + "id": "security-runtime-auth-config-and-tls", + "label": "security runtime auth config and TLS proof", + "match_prefixes": [], + "match_files": [ + "internal/config/config.go", + "internal/config/watcher.go", + "pkg/tlsutil/fingerprint.go" + ], + "allow_same_subsystem_tests": false, + "test_prefixes": [], + "exact_files": [ + "internal/config/config_load_test.go", + "internal/config/watcher_test.go", + "pkg/tlsutil/tlsutil_test.go" + ] + }, { "id": "storage-directory-security", "label": "storage directory hardening proof", diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 118e031f6..fba00d486 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -47,13 +47,16 @@ visibility, and privacy controls to operators. 21. `internal/api/security.go` 22. `internal/api/security_tokens.go` 23. `internal/api/system_settings.go` -24. `internal/telemetry/telemetry.go` -25. `internal/api/router_routes_auth_security.go` -26. `internal/crypto/crypto.go` -27. `internal/securityutil/secure_storage_dir.go` -28. `internal/cloudcp/auth/magiclink.go` -29. `internal/cloudcp/auth/magiclink_store.go` -30. `scripts/telemetry_adoption_report.py` +24. `internal/config/config.go` +25. `internal/config/watcher.go` +26. `internal/telemetry/telemetry.go` +27. `internal/api/router_routes_auth_security.go` +28. `internal/crypto/crypto.go` +29. `internal/securityutil/secure_storage_dir.go` +30. `internal/cloudcp/auth/magiclink.go` +31. `internal/cloudcp/auth/magiclink_store.go` +32. `pkg/tlsutil/fingerprint.go` +33. `scripts/telemetry_adoption_report.py` ## Shared Boundaries @@ -78,6 +81,7 @@ visibility, and privacy controls to operators. 5. Change security/privacy settings presentation through the shared `frontend-modern/src/components/Settings/GeneralSettingsPanel.tsx`, `frontend-modern/src/components/Settings/SecurityAuthPanel.tsx`, `frontend-modern/src/components/Settings/SecurityOverviewPanel.tsx`, `frontend-modern/src/components/Settings/QuickSecuritySetup.tsx`, `frontend-modern/src/components/Settings/SecurityPostureSummary.tsx`, `frontend-modern/src/components/Settings/SSOProviderTypeIcon.tsx`, `frontend-modern/src/utils/securityAuthPresentation.ts`, `frontend-modern/src/utils/securityScorePresentation.ts`, `frontend-modern/src/utils/auditLogPresentation.ts`, and `frontend-modern/src/utils/auditWebhookPresentation.ts` boundary. 6. Change operator-facing telemetry/adoption reporting through `scripts/telemetry_adoption_report.py` together with the privacy disclosure whenever release-identity interpretation changes. 7. Change data-at-rest encryption-key or control-plane magic-link HMAC key and storage-root hardening semantics through `internal/crypto/crypto.go`, `internal/cloudcp/auth/magiclink.go`, `internal/cloudcp/auth/magiclink_store.go`, and `internal/securityutil/secure_storage_dir.go` together so writable-but-not-owned runtime storage mounts stay supported without weakening file-level secrecy. +8. Change auth-env password normalization or shared TLS fingerprint verification defaults through `internal/config/config.go`, `internal/config/watcher.go`, and `pkg/tlsutil/fingerprint.go` together so startup auth ingestion, live auth-env reloads, and pinned-fingerprint TLS clients keep one fail-closed security floor. ## Forbidden Paths @@ -93,6 +97,7 @@ visibility, and privacy controls to operators. 4. Keep the checked-in telemetry adoption report aligned with the same release-identity rules used by the runtime telemetry payload. 5. Update this contract whenever a new canonical security, token, auth, or privacy surface becomes part of the governed trust boundary. 6. Keep the shared storage-directory and secure storage-file hardening helper aligned with the crypto manager plus control-plane magic-link key and store handling whenever runtime data-root ownership assumptions change. +7. Keep auth-env ingestion and shared fingerprint-verifier TLS defaults aligned whenever runtime auth loading or pinned-certificate transport behavior changes. ## Current State @@ -199,6 +204,11 @@ surfaces may stay concise, but they must not claim a stronger privacy posture than the governed docs; if telemetry rows are retained for a fixed window and IP addresses are not stored rather than “never seen,” the summary copy must say that plainly. +That same shared trust boundary now also owns the TLS floor used by pinned- +fingerprint runtime clients. `pkg/tlsutil/fingerprint.go` may support +certificate-fingerprint capture and verification for self-signed deployments, +but every mode must still set an explicit minimum TLS version instead of +silently inheriting whatever older protocol floor the host runtime would allow. That same rule also applies inside shipped security guidance itself: `SECURITY.md` and the synced `frontend-modern/public/docs/SECURITY.md` copy may not bounce the operator back to GitHub `main` for section references that the @@ -357,6 +367,12 @@ state must otherwise come from the resolved `ConfigPath` / `DataPath` owner or the shared `PULSE_DATA_DIR` fallback. These surfaces may not probe `/etc/pulse` or `/data` independently and silently override the configured path authority just because those directories exist on the host. +That same auth-env boundary must also fail closed on password normalization: +`internal/config/config.go` and `internal/config/watcher.go` may auto-hash a +plaintext `PULSE_AUTH_PASS`, but they must never preserve a raw plaintext value +in runtime config just because hashing failed. Startup must return an explicit +error, and live `.env` reloads must keep the previous runtime auth password +until a valid replacement is available. That same rule also governs the auth `.env` file path itself: `router.go`, `router_routes_auth_security.go`, and `security_setup_fix.go` must derive the manual-auth env file through the shared auth-path helper instead of diff --git a/internal/config/config.go b/internal/config/config.go index b435eed6e..4cdd3967e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -46,6 +46,7 @@ const ( var ( osStat = os.Stat execCommand = exec.Command + hashPasswordFn = auth.HashPassword netDial = net.Dial netInterfaceAddrs = net.InterfaceAddrs ) @@ -105,6 +106,33 @@ func IsPasswordHashed(password string) bool { return false } +func normalizeEnvAuthPassword(raw string) (string, error) { + password := strings.Trim(raw, "'\"") + if password == "" { + return "", nil + } + + if !IsPasswordHashed(password) { + hashedPass, err := hashPasswordFn(password) + if err != nil { + return "", fmt.Errorf("hash PULSE_AUTH_PASS: %w", err) + } + return hashedPass, nil + } + + if len(password) != 60 { + log.Error(). + Int("length", len(password)). + Str("env_var", "PULSE_AUTH_PASS"). + Msg("Bcrypt hash appears truncated; expected 60 characters; authentication may fail") + log.Error(). + Str("env_var", "PULSE_AUTH_PASS"). + Msg("Ensure the full bcrypt hash is enclosed in single quotes in your .env file or Docker environment") + } + + return password, nil +} + // Config holds all application configuration // NOTE: The envconfig tags are legacy and not used - configuration is loaded from encrypted JSON files type Config struct { @@ -1295,30 +1323,14 @@ func load(initLogging bool) (*Config, error) { log.Info().Msg("Overriding auth user from env var") } if authPass := os.Getenv("PULSE_AUTH_PASS"); authPass != "" { - // Auto-hash plain text passwords for security - if !IsPasswordHashed(authPass) { - // Plain text password - hash it immediately - hashedPass, err := auth.HashPassword(authPass) - if err != nil { - log.Error().Err(err).Msg("Failed to hash password from environment variable") - // Fall back to plain text if hashing fails (shouldn't happen) - cfg.AuthPass = authPass - } else { - cfg.AuthPass = hashedPass - log.Info().Msg("Auto-hashed plain text password from environment variable") - } + normalizedAuthPass, err := normalizeEnvAuthPassword(authPass) + if err != nil { + return nil, err + } + cfg.AuthPass = normalizedAuthPass + if !IsPasswordHashed(strings.Trim(authPass, "'\"")) { + log.Info().Msg("Auto-hashed plain text password from environment variable") } else { - // Already hashed - validate it's complete - if len(authPass) != 60 { - log.Error(). - Int("length", len(authPass)). - Str("env_var", "PULSE_AUTH_PASS"). - Msg("Bcrypt hash appears truncated; expected 60 characters; authentication may fail") - log.Error(). - Str("env_var", "PULSE_AUTH_PASS"). - Msg("Ensure the full bcrypt hash is enclosed in single quotes in your .env file or Docker environment") - } - cfg.AuthPass = authPass log.Debug().Msg("Loaded pre-hashed password from env var") } } diff --git a/internal/config/config_load_test.go b/internal/config/config_load_test.go index 3fb41dbcd..26d8c5b50 100644 --- a/internal/config/config_load_test.go +++ b/internal/config/config_load_test.go @@ -2,6 +2,7 @@ package config import ( "encoding/base64" + "errors" "os" "path/filepath" "testing" @@ -241,6 +242,24 @@ func TestLoad_AuthPass_PreHashed(t *testing.T) { assert.Equal(t, hash, cfg.AuthPass) } +func TestLoad_AuthPass_HashFailureFailsClosed(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + t.Setenv("PULSE_AUTH_PASS", "plainpass") + + originalHashPasswordFn := hashPasswordFn + hashPasswordFn = func(string) (string, error) { + return "", errors.New("boom") + } + t.Cleanup(func() { + hashPasswordFn = originalHashPasswordFn + }) + + cfg, err := Load() + require.Error(t, err) + assert.Nil(t, cfg) + assert.ErrorContains(t, err, "hash PULSE_AUTH_PASS") +} + func TestLoad_Persistence(t *testing.T) { tempDir := t.TempDir() t.Setenv("PULSE_DATA_DIR", tempDir) diff --git a/internal/config/watcher.go b/internal/config/watcher.go index 01aa22ad5..45e8cffeb 100644 --- a/internal/config/watcher.go +++ b/internal/config/watcher.go @@ -365,8 +365,10 @@ func (cw *ConfigWatcher) reloadConfig() { } // Apply auth password - newPass := strings.Trim(envMap["PULSE_AUTH_PASS"], "'\"") - if newPass != oldAuthPass { + newPass, err := normalizeEnvAuthPassword(envMap["PULSE_AUTH_PASS"]) + if err != nil { + log.Error().Err(err).Msg("Failed to normalize PULSE_AUTH_PASS from watched env file; keeping existing runtime auth password") + } else if newPass != oldAuthPass { cw.config.AuthPass = newPass if newPass == "" { changes = append(changes, "auth password removed") diff --git a/internal/config/watcher_test.go b/internal/config/watcher_test.go index c18dbd9bd..c96e49068 100644 --- a/internal/config/watcher_test.go +++ b/internal/config/watcher_test.go @@ -1,6 +1,7 @@ package config import ( + "errors" "os" "path/filepath" "sync/atomic" @@ -105,7 +106,8 @@ PULSE_AUTH_PASS="secret"` // Assert assert.Equal(t, "admin", cfg.AuthUser) - assert.Equal(t, "secret", cfg.AuthPass) + assert.NotEqual(t, "secret", cfg.AuthPass) + assert.True(t, IsPasswordHashed(cfg.AuthPass)) // Test update envContentUpdated := `PULSE_AUTH_USER="newadmin" @@ -115,7 +117,8 @@ PULSE_AUTH_PASS="newsecret"` cw.reloadConfig() assert.Equal(t, "newadmin", cfg.AuthUser) - assert.Equal(t, "newsecret", cfg.AuthPass) + assert.NotEqual(t, "newsecret", cfg.AuthPass) + assert.True(t, IsPasswordHashed(cfg.AuthPass)) } func TestConfigWatcher_ReloadConfig_MockSettings(t *testing.T) { @@ -331,11 +334,14 @@ func TestConfigWatcher_ReloadConfig_Auth(t *testing.T) { cfg := &Config{ AuthUser: "oldUser", - AuthPass: "oldPass", } cw, err := NewConfigWatcher(cfg) require.NoError(t, err) + oldPass, err := hashPasswordFn("oldPass") + require.NoError(t, err) + cfg.AuthPass = oldPass + // Update auth envContent := `PULSE_AUTH_USER="newUser" PULSE_AUTH_PASS="newPass"` @@ -344,7 +350,8 @@ PULSE_AUTH_PASS="newPass"` cw.reloadConfig() assert.Equal(t, "newUser", cfg.AuthUser) - assert.Equal(t, "newPass", cfg.AuthPass) + assert.NotEqual(t, "newPass", cfg.AuthPass) + assert.True(t, IsPasswordHashed(cfg.AuthPass)) // Remove auth require.NoError(t, os.WriteFile(envPath, []byte(""), 0644)) @@ -354,6 +361,38 @@ PULSE_AUTH_PASS="newPass"` assert.Equal(t, "", cfg.AuthPass) } +func TestConfigWatcher_ReloadConfig_HashFailureKeepsExistingPassword(t *testing.T) { + tempDir := t.TempDir() + envPath := filepath.Join(tempDir, ".env") + t.Setenv("PULSE_AUTH_CONFIG_DIR", tempDir) + + existingPass, err := hashPasswordFn("existingPass") + require.NoError(t, err) + + cfg := &Config{ + AuthUser: "admin", + AuthPass: existingPass, + } + cw, err := NewConfigWatcher(cfg) + require.NoError(t, err) + + originalHashPasswordFn := hashPasswordFn + hashPasswordFn = func(string) (string, error) { + return "", errors.New("boom") + } + t.Cleanup(func() { + hashPasswordFn = originalHashPasswordFn + }) + + envContent := `PULSE_AUTH_USER="admin" +PULSE_AUTH_PASS="newPass"` + require.NoError(t, os.WriteFile(envPath, []byte(envContent), 0644)) + + cw.reloadConfig() + + assert.Equal(t, existingPass, cfg.AuthPass) +} + func TestConfigWatcher_ReloadConfig_Manual(t *testing.T) { tempDir := t.TempDir() envPath := filepath.Join(tempDir, ".env") diff --git a/pkg/tlsutil/fingerprint.go b/pkg/tlsutil/fingerprint.go index 0eff98e54..24e3453b4 100644 --- a/pkg/tlsutil/fingerprint.go +++ b/pkg/tlsutil/fingerprint.go @@ -14,6 +14,8 @@ import ( "time" ) +const minimumTLSVersion = tls.VersionTLS12 + func verifyPresentedPeerCertificates(rawCerts [][]byte, _ [][]*x509.Certificate) error { if len(rawCerts) == 0 { return fmt.Errorf("no certificates presented by server") @@ -32,6 +34,7 @@ func verifyPresentedPeerCertificates(rawCerts [][]byte, _ [][]*x509.Certificate) // blanket InsecureSkipVerify alone. func PeerCertificateCaptureTLSConfig() *tls.Config { return &tls.Config{ + MinVersion: minimumTLSVersion, InsecureSkipVerify: true, VerifyPeerCertificate: verifyPresentedPeerCertificates, } @@ -92,6 +95,7 @@ func FingerprintVerifier(fingerprint string) *tls.Config { expectedFingerprint := strings.ToLower(strings.ReplaceAll(fingerprint, ":", "")) return &tls.Config{ + MinVersion: minimumTLSVersion, InsecureSkipVerify: true, // We'll do our own verification VerifyPeerCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { if len(rawCerts) == 0 { @@ -133,6 +137,7 @@ func CreateHTTPClientWithTimeout(verifySSL bool, fingerprint string, timeout tim TLSHandshakeTimeout: 10 * time.Second, // TLS handshake timeout ResponseHeaderTimeout: 10 * time.Second, // Time to wait for response headers ExpectContinueTimeout: 1 * time.Second, + TLSClientConfig: &tls.Config{MinVersion: minimumTLSVersion}, } if !verifySSL && fingerprint == "" { @@ -142,7 +147,7 @@ func CreateHTTPClientWithTimeout(verifySSL bool, fingerprint string, timeout tim // Fingerprint verification mode transport.TLSClientConfig = FingerprintVerifier(fingerprint) } - // else: default secure mode with system CA verification + // else: default secure mode with system CA verification and explicit TLS floor // Use provided timeout, or default to 60 seconds if not specified if timeout <= 0 { diff --git a/pkg/tlsutil/tlsutil_test.go b/pkg/tlsutil/tlsutil_test.go index abe14b774..2a5f7dc1f 100644 --- a/pkg/tlsutil/tlsutil_test.go +++ b/pkg/tlsutil/tlsutil_test.go @@ -2,7 +2,6 @@ package tlsutil import ( "crypto/sha256" - "crypto/tls" "crypto/x509" "encoding/hex" "net/http" @@ -35,6 +34,12 @@ func TestFingerprintVerifier_NormalizesFingerprint(t *testing.T) { if config2.VerifyPeerCertificate == nil { t.Error("FingerprintVerifier should set VerifyPeerCertificate") } + if config1.MinVersion != minimumTLSVersion { + t.Errorf("FingerprintVerifier MinVersion = %v, want %v", config1.MinVersion, minimumTLSVersion) + } + if config2.MinVersion != minimumTLSVersion { + t.Errorf("FingerprintVerifier MinVersion = %v, want %v", config2.MinVersion, minimumTLSVersion) + } } func TestPeerCertificateCaptureTLSConfigRequiresPeerCertificate(t *testing.T) { @@ -45,6 +50,9 @@ func TestPeerCertificateCaptureTLSConfigRequiresPeerCertificate(t *testing.T) { if config.VerifyPeerCertificate == nil { t.Fatal("PeerCertificateCaptureTLSConfig should install a peer-certificate verifier") } + if config.MinVersion != minimumTLSVersion { + t.Fatalf("PeerCertificateCaptureTLSConfig MinVersion = %v, want %v", config.MinVersion, minimumTLSVersion) + } err := config.VerifyPeerCertificate(nil, nil) if err == nil || !strings.Contains(err.Error(), "no certificates") { @@ -122,6 +130,9 @@ func TestCreateHTTPClient_InsecureMode(t *testing.T) { if transport.TLSClientConfig.VerifyPeerCertificate == nil { t.Fatal("insecure mode should still validate peer certificate structure") } + if transport.TLSClientConfig.MinVersion != minimumTLSVersion { + t.Fatalf("TLSClientConfig.MinVersion = %v, want %v", transport.TLSClientConfig.MinVersion, minimumTLSVersion) + } } func TestCreateHTTPClient_FingerprintMode(t *testing.T) { @@ -145,6 +156,9 @@ func TestCreateHTTPClient_FingerprintMode(t *testing.T) { if transport.TLSClientConfig.VerifyPeerCertificate == nil { t.Error("VerifyPeerCertificate should be set in fingerprint mode") } + if transport.TLSClientConfig.MinVersion != minimumTLSVersion { + t.Fatalf("TLSClientConfig.MinVersion = %v, want %v", transport.TLSClientConfig.MinVersion, minimumTLSVersion) + } } func TestCreateHTTPClient_SecureMode(t *testing.T) { @@ -159,11 +173,15 @@ func TestCreateHTTPClient_SecureMode(t *testing.T) { t.Fatal("Transport is not *http.Transport") } - // In secure mode, TLSClientConfig should either be nil (use defaults) - // or not have InsecureSkipVerify set - if transport.TLSClientConfig != nil && transport.TLSClientConfig.InsecureSkipVerify { + if transport.TLSClientConfig == nil { + t.Fatal("secure mode should install an explicit TLS client config") + } + if transport.TLSClientConfig.InsecureSkipVerify { t.Error("InsecureSkipVerify should not be true in secure mode") } + if transport.TLSClientConfig.MinVersion != minimumTLSVersion { + t.Fatalf("TLSClientConfig.MinVersion = %v, want %v", transport.TLSClientConfig.MinVersion, minimumTLSVersion) + } } func TestCreateHTTPClientWithTimeout_DefaultTimeout(t *testing.T) { @@ -310,9 +328,7 @@ func TestFingerprintVerifier_TLSVersion(t *testing.T) { t.Fatal("FingerprintVerifier returned nil config") } - // MinVersion should not be set to an insecure version - // (default is fine, or TLS 1.2+) - if config.MinVersion != 0 && config.MinVersion < tls.VersionTLS12 { - t.Errorf("MinVersion should be 0 (default) or >= TLS 1.2, got %v", config.MinVersion) + if config.MinVersion != minimumTLSVersion { + t.Errorf("MinVersion should be %v, got %v", minimumTLSVersion, config.MinVersion) } }