From 2e4e85d9af0c3ca8711c0eed4f74a3f830e07fa7 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 16 Aug 2026 17:12:47 +0100 Subject: [PATCH] docs: add focused security review packet --- docs/README.md | 1 + docs/SECURITY_REVIEW.md | 12 ++ .../security-review/AUTH_CREDENTIAL_REVIEW.md | 143 ++++++++++++++++++ docs/security-review/FINDING_TEMPLATE.md | 52 +++++++ frontend-modern/public/docs/README.md | 1 + .../public/docs/SECURITY_REVIEW.md | 12 ++ .../security-review/AUTH_CREDENTIAL_REVIEW.md | 143 ++++++++++++++++++ .../docs/security-review/FINDING_TEMPLATE.md | 52 +++++++ scripts/security_review_auth_credentials.sh | 26 ++++ 9 files changed, 442 insertions(+) create mode 100644 docs/security-review/AUTH_CREDENTIAL_REVIEW.md create mode 100644 docs/security-review/FINDING_TEMPLATE.md create mode 100644 frontend-modern/public/docs/security-review/AUTH_CREDENTIAL_REVIEW.md create mode 100644 frontend-modern/public/docs/security-review/FINDING_TEMPLATE.md create mode 100755 scripts/security_review_auth_credentials.sh diff --git a/docs/README.md b/docs/README.md index 30e5a4615..0880c6f70 100644 --- a/docs/README.md +++ b/docs/README.md @@ -65,6 +65,7 @@ Pulse Pro capabilities. - [Production deployment and security](PRODUCTION_SECURITY.md) - [Security review scope](SECURITY_REVIEW.md) +- [Authentication and credential storage review packet](security-review/AUTH_CREDENTIAL_REVIEW.md) - [Security guide](../SECURITY.md) - [Privacy and telemetry disclosure](PRIVACY.md) - [OIDC and SSO](OIDC.md) diff --git a/docs/SECURITY_REVIEW.md b/docs/SECURITY_REVIEW.md index bd215c0a1..17c80d749 100644 --- a/docs/SECURITY_REVIEW.md +++ b/docs/SECURITY_REVIEW.md @@ -13,6 +13,17 @@ Before reviewing the code, read: guest access, and update verification. - [Installation](INSTALL.md) for the signed, version-pinned server installer. +## First focused review packet + +The [Authentication and Credential Storage Review Packet](security-review/AUTH_CREDENTIAL_REVIEW.md) +defines a narrow first assessment with explicit attacker capabilities, security +properties, source boundaries, manual review steps, and a reusable finding +template. Its baseline can be run with: + +```bash +./scripts/security_review_auth_credentials.sh +``` + ## Suggested review boundaries ### 1. Authentication, authorization, and tenant isolation @@ -30,6 +41,7 @@ Starting points: - `internal/api/api_token_scope_transport_integration_test.go` - `internal/api/middleware_tenant_authorization_test.go` - `pkg/auth/` +- `docs/security-review/AUTH_CREDENTIAL_REVIEW.md` ### 2. Credential storage and configuration transfer diff --git a/docs/security-review/AUTH_CREDENTIAL_REVIEW.md b/docs/security-review/AUTH_CREDENTIAL_REVIEW.md new file mode 100644 index 000000000..96b73ec89 --- /dev/null +++ b/docs/security-review/AUTH_CREDENTIAL_REVIEW.md @@ -0,0 +1,143 @@ +# Authentication and Credential Storage Review Packet + +This packet defines the first focused external review boundary for Pulse. It is +an invitation to inspect and challenge the implementation. It is not a claim +that an independent review has already happened. + +## Review objective + +Assess whether Pulse keeps authentication, authorization, tenant isolation, +credential storage, and configuration transfer within their documented trust +boundaries. Findings should identify a concrete path from attacker capability +to security impact. + +Test against a named commit so that every observation can be reproduced. + +## In scope + +- Browser session authentication and logout behavior +- Password hashing and password policy enforcement +- API token generation, hashing, comparison, scope enforcement, and revocation +- Organization binding and cross-tenant denial behavior +- Proxy authentication and forwarded request boundaries +- Authorization for configuration export and import +- Encryption key creation, permissions, migration, and validation +- AES-GCM encryption and decryption behavior +- Failure behavior when encrypted data exists without usable key material +- Secret redaction and API responses that represent credential presence + +## Out of scope + +- Security of an external identity provider itself +- Security of the operating system, reverse proxy, or container runtime +- Agent command authority, update delivery, network discovery, and SSRF +- Availability testing against a production Pulse installation +- Customer data or credentials from any real deployment + +Those boundaries remain valid review targets, but they should be handled as +separate packets so that evidence and conclusions stay precise. + +## Attacker capabilities to consider + +1. An unauthenticated network client can reach the Pulse HTTP service. +2. An authenticated user has the least privileged available role. +3. A caller holds a valid API token with one narrow scope. +4. A tenant user knows or guesses another organization identifier. +5. A reverse proxy supplies forwarded identity and network headers. +6. A local user can read a copied data directory but not the original key. +7. An operator restores encrypted configuration without the matching key. +8. A caller sends a malformed, oversized, or partial import request. + +Root access to the running Pulse host is not treated as a boundary that +application-level encryption can defeat. The review should still report any +unnecessary secret exposure that would increase the impact of host compromise. + +## Security properties to verify + +- Protected routes deny access when identity is missing or invalid. +- A credential cannot gain authority outside its assigned scopes. +- Organization selection cannot bypass tenant authorization. +- Management operations require management authority. +- Export and import authorization happens before request bodies are consumed + or state can change. +- Failed import or export requests do not mutate configuration or trigger a + reload. +- Stored API tokens and passwords are one-way hashed where the runtime only + needs comparison. +- Reversible secrets use authenticated encryption with unique nonces. +- Encryption key files reject unsafe permissions, symlinks, invalid material, + and unexpected replacement. +- Existing encrypted data without its key fails closed instead of creating a + replacement key. +- API responses expose credential presence or identifiers without returning + stored secret values. + +## Source map + +Authentication and authorization: + +- `internal/api/auth.go` +- `internal/api/authorization.go` +- `internal/api/middleware.go` +- `internal/api/middleware_tenant.go` +- `internal/api/api_token_scope_transport_integration_test.go` +- `internal/api/middleware_tenant_authorization_test.go` +- `pkg/auth/` + +Credential storage and configuration transfer: + +- `internal/crypto/crypto.go` +- `internal/crypto/crypto_test.go` +- `internal/config/persistence.go` +- `internal/config/persistence_fail_test.go` +- `internal/api/config_transfer_authorization.go` +- `internal/api/config_transfer_authorization_test.go` + +## Reproducible baseline + +From a clean checkout of the commit under review, run: + +```bash +./scripts/security_review_auth_credentials.sh +``` + +The script prints the tested commit, records the Go toolchain version in its +output, runs the focused package and authorization tests, and validates the +public documentation mirrors. A passing result is regression evidence only. +It does not prove that the implementation is free of vulnerabilities. + +## Manual review procedure + +1. Trace each authentication method from request parsing to the final user and + token context. +2. Build a route matrix for anonymous sessions, user sessions, proxy identity, + unrestricted tokens, and narrowly scoped tokens. +3. Trace organization selection before and after authorization decisions. +4. Trace export and import requests from authorization through body reads, + persistence, reload, and error handling. +5. Trace every stored secret from input through hashing or encryption to API + serialization and logging. +6. Exercise malformed credentials, stale sessions, revoked tokens, conflicting + organization identifiers, forwarded loopback claims, and missing key files. +7. Record any assumption that depends on deployment configuration rather than + application enforcement. + +Use only disposable local data. Do not test against a community member or +customer installation without the operator's explicit permission. + +## Requested output + +Use the [finding template](FINDING_TEMPLATE.md) for each confirmed issue or +material hardening gap. A useful report contains: + +- The exact tested commit and deployment mode +- Required attacker access and privileges +- Minimal reproduction steps +- Expected and observed behavior +- Security impact and affected boundary +- Relevant source locations and test evidence +- A practical containment or remediation suggestion when known + +Send suspected vulnerabilities privately to . General +hardening suggestions without an exploitable path can use the public issue +tracker after confirming that publication does not expose a weakness. diff --git a/docs/security-review/FINDING_TEMPLATE.md b/docs/security-review/FINDING_TEMPLATE.md new file mode 100644 index 000000000..ad720940a --- /dev/null +++ b/docs/security-review/FINDING_TEMPLATE.md @@ -0,0 +1,52 @@ +# Security Review Finding Template + +## Finding + +- Title: +- Reviewer: +- Review date: +- Tested commit: +- Pulse deployment mode: +- Affected boundary: +- Suggested severity: + +## Summary + +Describe the security property that fails and the resulting impact. + +## Required access + +Describe the attacker's starting access, credentials, role, organization, and +network position. + +## Reproduction + +Provide the smallest safe sequence that demonstrates the behavior. Use +disposable local data and replace all secrets with inert placeholders. + +## Expected behavior + +Describe the expected authorization, isolation, storage, or failure behavior. + +## Observed behavior + +Describe what happened, including stable logs or response fields that support +the finding. Do not include live credentials or customer data. + +## Impact + +Explain what an attacker can read, change, execute, or prevent. Separate +confirmed impact from plausible follow-on impact. + +## Source and evidence + +List relevant files, functions, tests, requests, and command output. + +## Suggested containment or remediation + +Describe immediate containment and a durable fix when known. + +## Disclosure preference + +State whether the report may be published after remediation and whether the +reviewer wants attribution. diff --git a/frontend-modern/public/docs/README.md b/frontend-modern/public/docs/README.md index 30e5a4615..0880c6f70 100644 --- a/frontend-modern/public/docs/README.md +++ b/frontend-modern/public/docs/README.md @@ -65,6 +65,7 @@ Pulse Pro capabilities. - [Production deployment and security](PRODUCTION_SECURITY.md) - [Security review scope](SECURITY_REVIEW.md) +- [Authentication and credential storage review packet](security-review/AUTH_CREDENTIAL_REVIEW.md) - [Security guide](../SECURITY.md) - [Privacy and telemetry disclosure](PRIVACY.md) - [OIDC and SSO](OIDC.md) diff --git a/frontend-modern/public/docs/SECURITY_REVIEW.md b/frontend-modern/public/docs/SECURITY_REVIEW.md index bd215c0a1..17c80d749 100644 --- a/frontend-modern/public/docs/SECURITY_REVIEW.md +++ b/frontend-modern/public/docs/SECURITY_REVIEW.md @@ -13,6 +13,17 @@ Before reviewing the code, read: guest access, and update verification. - [Installation](INSTALL.md) for the signed, version-pinned server installer. +## First focused review packet + +The [Authentication and Credential Storage Review Packet](security-review/AUTH_CREDENTIAL_REVIEW.md) +defines a narrow first assessment with explicit attacker capabilities, security +properties, source boundaries, manual review steps, and a reusable finding +template. Its baseline can be run with: + +```bash +./scripts/security_review_auth_credentials.sh +``` + ## Suggested review boundaries ### 1. Authentication, authorization, and tenant isolation @@ -30,6 +41,7 @@ Starting points: - `internal/api/api_token_scope_transport_integration_test.go` - `internal/api/middleware_tenant_authorization_test.go` - `pkg/auth/` +- `docs/security-review/AUTH_CREDENTIAL_REVIEW.md` ### 2. Credential storage and configuration transfer diff --git a/frontend-modern/public/docs/security-review/AUTH_CREDENTIAL_REVIEW.md b/frontend-modern/public/docs/security-review/AUTH_CREDENTIAL_REVIEW.md new file mode 100644 index 000000000..96b73ec89 --- /dev/null +++ b/frontend-modern/public/docs/security-review/AUTH_CREDENTIAL_REVIEW.md @@ -0,0 +1,143 @@ +# Authentication and Credential Storage Review Packet + +This packet defines the first focused external review boundary for Pulse. It is +an invitation to inspect and challenge the implementation. It is not a claim +that an independent review has already happened. + +## Review objective + +Assess whether Pulse keeps authentication, authorization, tenant isolation, +credential storage, and configuration transfer within their documented trust +boundaries. Findings should identify a concrete path from attacker capability +to security impact. + +Test against a named commit so that every observation can be reproduced. + +## In scope + +- Browser session authentication and logout behavior +- Password hashing and password policy enforcement +- API token generation, hashing, comparison, scope enforcement, and revocation +- Organization binding and cross-tenant denial behavior +- Proxy authentication and forwarded request boundaries +- Authorization for configuration export and import +- Encryption key creation, permissions, migration, and validation +- AES-GCM encryption and decryption behavior +- Failure behavior when encrypted data exists without usable key material +- Secret redaction and API responses that represent credential presence + +## Out of scope + +- Security of an external identity provider itself +- Security of the operating system, reverse proxy, or container runtime +- Agent command authority, update delivery, network discovery, and SSRF +- Availability testing against a production Pulse installation +- Customer data or credentials from any real deployment + +Those boundaries remain valid review targets, but they should be handled as +separate packets so that evidence and conclusions stay precise. + +## Attacker capabilities to consider + +1. An unauthenticated network client can reach the Pulse HTTP service. +2. An authenticated user has the least privileged available role. +3. A caller holds a valid API token with one narrow scope. +4. A tenant user knows or guesses another organization identifier. +5. A reverse proxy supplies forwarded identity and network headers. +6. A local user can read a copied data directory but not the original key. +7. An operator restores encrypted configuration without the matching key. +8. A caller sends a malformed, oversized, or partial import request. + +Root access to the running Pulse host is not treated as a boundary that +application-level encryption can defeat. The review should still report any +unnecessary secret exposure that would increase the impact of host compromise. + +## Security properties to verify + +- Protected routes deny access when identity is missing or invalid. +- A credential cannot gain authority outside its assigned scopes. +- Organization selection cannot bypass tenant authorization. +- Management operations require management authority. +- Export and import authorization happens before request bodies are consumed + or state can change. +- Failed import or export requests do not mutate configuration or trigger a + reload. +- Stored API tokens and passwords are one-way hashed where the runtime only + needs comparison. +- Reversible secrets use authenticated encryption with unique nonces. +- Encryption key files reject unsafe permissions, symlinks, invalid material, + and unexpected replacement. +- Existing encrypted data without its key fails closed instead of creating a + replacement key. +- API responses expose credential presence or identifiers without returning + stored secret values. + +## Source map + +Authentication and authorization: + +- `internal/api/auth.go` +- `internal/api/authorization.go` +- `internal/api/middleware.go` +- `internal/api/middleware_tenant.go` +- `internal/api/api_token_scope_transport_integration_test.go` +- `internal/api/middleware_tenant_authorization_test.go` +- `pkg/auth/` + +Credential storage and configuration transfer: + +- `internal/crypto/crypto.go` +- `internal/crypto/crypto_test.go` +- `internal/config/persistence.go` +- `internal/config/persistence_fail_test.go` +- `internal/api/config_transfer_authorization.go` +- `internal/api/config_transfer_authorization_test.go` + +## Reproducible baseline + +From a clean checkout of the commit under review, run: + +```bash +./scripts/security_review_auth_credentials.sh +``` + +The script prints the tested commit, records the Go toolchain version in its +output, runs the focused package and authorization tests, and validates the +public documentation mirrors. A passing result is regression evidence only. +It does not prove that the implementation is free of vulnerabilities. + +## Manual review procedure + +1. Trace each authentication method from request parsing to the final user and + token context. +2. Build a route matrix for anonymous sessions, user sessions, proxy identity, + unrestricted tokens, and narrowly scoped tokens. +3. Trace organization selection before and after authorization decisions. +4. Trace export and import requests from authorization through body reads, + persistence, reload, and error handling. +5. Trace every stored secret from input through hashing or encryption to API + serialization and logging. +6. Exercise malformed credentials, stale sessions, revoked tokens, conflicting + organization identifiers, forwarded loopback claims, and missing key files. +7. Record any assumption that depends on deployment configuration rather than + application enforcement. + +Use only disposable local data. Do not test against a community member or +customer installation without the operator's explicit permission. + +## Requested output + +Use the [finding template](FINDING_TEMPLATE.md) for each confirmed issue or +material hardening gap. A useful report contains: + +- The exact tested commit and deployment mode +- Required attacker access and privileges +- Minimal reproduction steps +- Expected and observed behavior +- Security impact and affected boundary +- Relevant source locations and test evidence +- A practical containment or remediation suggestion when known + +Send suspected vulnerabilities privately to . General +hardening suggestions without an exploitable path can use the public issue +tracker after confirming that publication does not expose a weakness. diff --git a/frontend-modern/public/docs/security-review/FINDING_TEMPLATE.md b/frontend-modern/public/docs/security-review/FINDING_TEMPLATE.md new file mode 100644 index 000000000..ad720940a --- /dev/null +++ b/frontend-modern/public/docs/security-review/FINDING_TEMPLATE.md @@ -0,0 +1,52 @@ +# Security Review Finding Template + +## Finding + +- Title: +- Reviewer: +- Review date: +- Tested commit: +- Pulse deployment mode: +- Affected boundary: +- Suggested severity: + +## Summary + +Describe the security property that fails and the resulting impact. + +## Required access + +Describe the attacker's starting access, credentials, role, organization, and +network position. + +## Reproduction + +Provide the smallest safe sequence that demonstrates the behavior. Use +disposable local data and replace all secrets with inert placeholders. + +## Expected behavior + +Describe the expected authorization, isolation, storage, or failure behavior. + +## Observed behavior + +Describe what happened, including stable logs or response fields that support +the finding. Do not include live credentials or customer data. + +## Impact + +Explain what an attacker can read, change, execute, or prevent. Separate +confirmed impact from plausible follow-on impact. + +## Source and evidence + +List relevant files, functions, tests, requests, and command output. + +## Suggested containment or remediation + +Describe immediate containment and a durable fix when known. + +## Disclosure preference + +State whether the report may be published after remediation and whether the +reviewer wants attribution. diff --git a/scripts/security_review_auth_credentials.sh b/scripts/security_review_auth_credentials.sh new file mode 100755 index 000000000..e2a96d6e8 --- /dev/null +++ b/scripts/security_review_auth_credentials.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash + +set -euo pipefail + +SCRIPT_DIR="$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(dirname -- "$SCRIPT_DIR")" + +cd "$REPO_ROOT" + +printf 'Pulse authentication and credential review baseline\n' +printf 'Commit: %s\n' "$(git rev-parse HEAD)" +go version + +printf '\nRunning cryptography, authentication, and configuration package tests\n' +go test ./internal/crypto ./pkg/auth ./internal/config -count=1 + +printf '\nRunning API authorization and configuration transfer tests\n' +./scripts/ensure_test_assets.sh +go test ./internal/api \ + -run '^(TestBearerAPITokenScopesDenyReadWriteAndExecRoutes|TestTenantMiddleware_.*|TestConfigTransfer.*|TestAllowUnprotectedExport.*|TestSecurityStatusMatchesConfigTransferPolicy|TestDeniedConfigTransferDoesNotMutateOrReload)$' \ + -count=1 + +printf '\nChecking public documentation links and mirrors\n' +python3 scripts/check_public_docs.py + +printf '\nAuthentication and credential review baseline passed\n'