Pulse/docs/SECURITY_AUDIT_2025-12-18.md
rcourtman ae70d9e423 feat: add --kube-include-all-deployments flag for Kubernetes agent
Adds IncludeAllDeployments option to show all deployments, not just
problem ones (where replicas don't match desired). This provides parity
with the existing --kube-include-all-pods flag.

- Add IncludeAllDeployments to kubernetesagent.Config
- Add --kube-include-all-deployments flag and PULSE_KUBE_INCLUDE_ALL_DEPLOYMENTS env var
- Update collectDeployments to respect the new flag
- Add test for IncludeAllDeployments functionality
- Update UNIFIED_AGENT.md documentation

Addresses feedback from PR #855
2025-12-18 20:58:30 +00:00

375 lines
15 KiB
Markdown
Raw Blame History

This file contains invisible Unicode characters

This file contains invisible Unicode characters that are indistinguishable to humans but may be processed differently by a computer. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Security Audit Report - Pulse Application
## Date: 2025-12-18
## Auditor: Claude (Gemini)
---
## Executive Summary
This document presents the findings from a comprehensive security audit of the Pulse monitoring application. The audit examined authentication, authorization, cryptography, input validation, SSRF prevention, command execution, and general security practices.
### Overall Assessment
**Security Posture: A- (Excellent with minor recommendations)**
The codebase demonstrates a mature security posture with:
- ✅ Strong cryptographic practices (bcrypt, SHA3-256, AES-256-GCM)
- ✅ Comprehensive SSRF protection for webhooks
- ✅ CSRF protection for session-based authentication
- ✅ Rate limiting and account lockout
- ✅ Command execution policy with blocklist/allowlist
- ✅ Proper input sanitization and validation
- ✅ Security headers implementation
- ✅ Audit logging
A prior security audit on 2025-11-07 addressed 9 critical to low severity issues in the sensor-proxy component, all of which were successfully remediated.
---
## Audit Scope
### Components Reviewed
1. **Authentication System** (`internal/api/auth.go`, `internal/auth/`)
2. **Session Management** (`internal/api/security.go`, session stores)
3. **Cryptography** (`internal/crypto/crypto.go`)
4. **API Token Management** (`internal/api/security_tokens.go`)
5. **OIDC Integration** (`internal/api/security_oidc.go`)
6. **Webhook/Notification Security** (`internal/notifications/`)
7. **Command Execution** (`internal/agentexec/policy.go`)
8. **Database Operations** (`internal/metrics/store.go`)
9. **Configuration & Secrets** (`internal/config/`)
---
## Strengths Identified
### 1. Authentication & Password Security ✅
- **bcrypt hashing** with cost factor 12 for passwords
- **SHA3-256** for API token hashing
- **Constant-time comparison** for token validation (prevents timing attacks)
- **12-character minimum** password length requirement
- **Automatic hashing** of plain-text passwords on startup
### 2. Session Security ✅
- **HttpOnly cookies** for session tokens
- **Secure flag** set based on HTTPS detection
- **SameSite policy** properly configured (Lax/None based on proxy detection)
- **24-hour session expiry** with sliding window extension
- **Session invalidation** on password change
### 3. Rate Limiting & Account Lockout ✅
- **10 attempts/minute** for auth endpoints
- **5 failed attempts** triggers 15-minute lockout
- **Per-username AND per-IP** tracking
- **Lockout bypass prevention** (both must be clear)
### 4. CSRF Protection ✅
- **CSRF tokens** generated per session
- **Separate CSRF cookie** (not HttpOnly, readable by JS)
- **Header/form validation** for state-changing requests
- **Safe methods** (GET, HEAD, OPTIONS) exempted
- **API token auth** correctly bypasses CSRF (not vulnerable)
### 5. SSRF Prevention ✅
- **Webhook URL validation** with DNS resolution check
- **Private IP blocking** (RFC1918, link-local, loopback)
- **Cloud metadata endpoint blocking** (169.254.169.254, etc.)
- **Configurable allowlist** for internal webhooks
- **DNS rebinding protection** via IP resolution verification
### 6. Encryption at Rest ✅
- **AES-256-GCM** for credential encryption
- **Unique nonce** generation per encryption operation
- **Key file protections** with existence validation before encryption
- **Orphaned data prevention** (refuses to encrypt if key deleted)
### 7. Security Headers ✅
- Content-Security-Policy
- X-Frame-Options (DENY by default)
- X-Content-Type-Options: nosniff
- X-XSS-Protection
- Referrer-Policy
- Permissions-Policy
### 8. Command Execution Policy ✅
- **Blocklist** for dangerous commands (rm -rf, mkfs, dd, etc.)
- **Auto-approve list** for read-only inspection commands
- **Require approval** for service control, package management
- **Sudo normalization** for consistent policy application
### 9. SQL Injection Prevention ✅
- **Parameterized queries** used throughout metrics store
- **Prepared statements** for batch operations
- No string concatenation in SQL queries
### 10. XSS Prevention ✅
- **DOMPurify** for markdown rendering
- **HTML entity encoding** in tooltips
- **Allowed tag/attribute lists** for sanitized content
- **LLM output sanitization** (AI chat)
---
## Findings & Recommendations
### HIGH SEVERITY: None Identified
### MEDIUM SEVERITY
#### M1. Admin Bypass Debug Mode 🟡
**Location:** `internal/api/auth.go:675-691`
**Finding:**
The `adminBypassEnabled()` function allows bypassing authentication when both `ALLOW_ADMIN_BYPASS=1` and `PULSE_DEV=true` are set. While properly gated for development only:
```go
if os.Getenv("ALLOW_ADMIN_BYPASS") != "1" {
return
}
if os.Getenv("PULSE_DEV") == "true" || strings.EqualFold(os.Getenv("NODE_ENV"), "development") {
log.Warn().Msg("Admin authentication bypass ENABLED (development mode)")
adminBypassState.enabled = true
}
```
**Risk:** Accidental production deployment with these env vars could expose full admin access.
**Recommendation:**
1. Add prominent warning log at startup if either var is set
2. Consider disallowing in Docker `PULSE_DOCKER=true` mode
3. Document this explicitly as a development-only feature
---
#### M2. Recovery Token Exposure Window 🟡
**Location:** Session and recovery token stores
**Finding:**
Recovery tokens for password reset appear to be stored in JSON files. While tokens are hashed:
- File permissions should be verified as 0600
- Token expiration should be enforced server-side (appears to be implemented)
**Recommendation:**
1. Verify file permissions are set correctly (0600) during token store initialization
2. Add cleanup routine for expired tokens
---
### LOW SEVERITY
#### L1. Cookie Security in HTTP Proxies 🔵
**Location:** `internal/api/auth.go:75-107`
**Finding:**
When behind an HTTP (non-HTTPS) proxy, cookies fall back to `SameSite=Lax` with `Secure=false`. This is functionally necessary but reduces security.
**Recommendation:**
1. Log a warning when cookies are set without Secure flag
2. Add documentation recommending HTTPS termination at proxy
---
#### L2. Session Token Entropy 🔵
**Location:** `internal/api/auth.go:109-118`
**Finding:**
Session tokens are 32 bytes (256 bits) of entropy via `crypto/rand`, which is excellent. However, the error handling falls back to empty string:
```go
if _, err := cryptorand.Read(b); err != nil {
log.Error().Err(err).Msg("Failed to generate secure session token")
return "" // Fallback - should never happen
}
```
**Recommendation:**
Consider returning an error or panicking rather than returning empty string, as an empty session token could have undefined behavior.
---
#### L3. OIDC State Parameter Validation 🔵
**Location:** `internal/api/security_oidc.go`
**Finding:**
OIDC configuration is properly validated and state parameters should be verified during the OAuth flow. This should be confirmed in the callback handler.
**Recommendation:**
1. Verify state parameter is generated with sufficient entropy
2. Ensure state parameter has short expiration (5-10 minutes)
---
#### L4. Apprise CLI Command Execution 🔵
**Location:** `internal/notifications/notifications.go`
**Finding:**
The Apprise CLI path and targets are passed to `exec.CommandContext`. While the CLI path is configurable:
```go
args := []string{"-t", title, "-b", body}
args = append(args, cfg.Targets...)
execFn := n.appriseExec
```
**Risk:** If an attacker can control `cfg.Targets`, they might inject malicious arguments.
**Recommendation:**
1. Validate that targets match expected Apprise URL format
2. Consider sanitizing or escaping special characters in targets
---
### INFORMATIONAL
#### I1. Dependencies
The `go.mod` shows modern, well-maintained dependencies:
- Go 1.24.0 (latest stable)
- `golang.org/x/crypto v0.45.0` (current)
- `github.com/coreos/go-oidc/v3 v3.17.0` (current)
**Recommendation:**
Run `govulncheck` periodically to scan for known vulnerabilities.
---
#### I2. GitGuardian Integration
The `.gitguardian.yaml` is properly configured to:
- Ignore documentation and example files
- Block placeholder patterns
- Scan actual code and configuration
---
#### I3. Existing Security Audit
The previous audit (2025-11-07) addressed critical vulnerabilities in the sensor-proxy:
- Socket directory tampering (CRITICAL) ✅ Fixed
- SSRF via get_temperature (CRITICAL) ✅ Fixed
- Connection exhaustion DoS (CRITICAL) ✅ Fixed
- Multi-UID rate limit bypass (CRITICAL) ✅ Fixed
- Incomplete GID authorization (MEDIUM) ✅ Fixed
- Unbounded SSH output (MEDIUM) ✅ Fixed
- Weak host key validation (MEDIUM) ✅ Fixed
- Insufficient capability separation (MEDIUM) ✅ Fixed
- Missing systemd hardening (LOW) ✅ Fixed
---
## Security Architecture Summary
### Data Flow Security
```
┌─────────────────────────────────────────────────────────────────┐
│ Client (Browser/API) │
│ ┌─────────────────────────────────────────────────────────────┐│
│ │ HTTPS + TLS │ Session Cookie (HttpOnly, Secure, SameSite) ││
│ │ │ CSRF Token (Cookie + Header validation) ││
│ │ │ API Token (Header: X-API-Token) ││
│ └─────────────────────────────────────────────────────────────┘│
└─────────────────────────────────────────────────────────────────┘
┌─────────────────────────────────────────────────────────────────┐
│ Pulse Server │
│ ┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐ │
│ │ Rate Limiter │ │ Auth Middleware │ │ CSRF Handler │ │
│ │ 10 auth/min │ │ Session/Token │ │ State-changing │ │
│ │ 500 api/min │ │ Validation │ │ operations │ │
│ └─────────────────┘ └─────────────────┘ └─────────────────┘ │
│ │
│ ┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐ │
│ │ Account Lockout │ │ Command Policy │ │ SSRF Prevention │ │
│ │ 5 attempts/15m │ │ Block/Allow/ │ │ Private IP │ │
│ │ │ │ Require Approval│ │ blocklist │ │
│ └─────────────────┘ └─────────────────┘ └─────────────────┘ │
│ │
│ ┌─────────────────────────────────────────────────────────────┐│
│ │ Encryption at Rest (AES-256-GCM) ││
│ │ - Node credentials: /etc/pulse/nodes.enc ││
│ │ - Email settings: /etc/pulse/email.enc ││
│ │ - Webhooks: /etc/pulse/webhooks.enc ││
│ │ - OIDC config: /etc/pulse/oidc.enc ││
│ └─────────────────────────────────────────────────────────────┘│
└─────────────────────────────────────────────────────────────────┘
```
### Password/Token Hashing
| Credential Type | Algorithm | Parameters |
|-----------------|-----------|------------|
| User Passwords | bcrypt | Cost factor 12 |
| API Tokens | SHA3-256 | - |
| Encryption Key | AES-256-GCM | 32-byte random key |
| Session Tokens | Random | 32 bytes (256-bit) |
---
## Compliance Checklist
| Requirement | Status | Notes |
|-------------|--------|-------|
| Password hashing | ✅ | bcrypt, cost 12 |
| Session management | ✅ | Secure cookies, 24h expiry |
| CSRF protection | ✅ | Token-based |
| Rate limiting | ✅ | Auth + API endpoints |
| Encryption at rest | ✅ | AES-256-GCM |
| HTTPS support | ✅ | TLS configurable |
| Security headers | ✅ | CSP, X-Frame-Options, etc. |
| Audit logging | ✅ | Auth events logged |
| Input validation | ✅ | SQL params, webhook URLs |
| Command execution control | ✅ | Policy-based |
---
## Recommendations Summary
### Priority 1 (Consider Addressing)
- [ ] M1: Add additional safeguards for dev mode bypass
- [ ] M2: Verify recovery token file permissions
### Priority 2 (Optional Improvements)
- [ ] L1: Add warning logs for non-secure cookies
- [ ] L2: Improve session token generation error handling
- [ ] L3: Document OIDC state parameter security
- [ ] L4: Add Apprise target validation
### Priority 3 (Ongoing)
- [ ] I1: Run `govulncheck` regularly
- [ ] Keep dependencies updated
- [ ] Review GitGuardian alerts
---
## Conclusion
The Pulse application demonstrates a **strong security posture** with comprehensive protections against common web application vulnerabilities. The codebase shows evidence of security-conscious development practices:
1. **Defense in depth** with multiple layers of authentication and authorization
2. **Secure defaults** requiring explicit configuration to reduce security
3. **Modern cryptography** using industry-standard algorithms
4. **Comprehensive validation** of user inputs and external URLs
5. **Audit trails** for security-relevant events
The identified findings are primarily of low to medium severity and represent opportunities for hardening rather than critical vulnerabilities.
**Final Security Grade: A-**
---
## References
- **Previous Audit:** `docs/SECURITY_AUDIT_2025-11-07.md`
- **Security Policy:** `SECURITY.md`
- **Security Changelog:** `docs/SECURITY_CHANGELOG.md`
---
## Audit Team
**Auditor:** Claude (Gemini 2.5)
**Methodology:** Static code analysis and architecture review
**Audit Duration:** 2025-12-18 (single session)
**Files Reviewed:** ~50 source files across 10 packages
---
**For security concerns or questions:**
https://github.com/rcourtman/Pulse/issues