From 7e4b3a4df7db1f2de070faa2230b6eb4e81aa171 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 21 Dec 2025 10:26:21 -0500 Subject: [PATCH 1/7] docs: add planning document for Issue #365 Additional Security --- docs/plans/issue-365-additional-security.md | 90 +++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 docs/plans/issue-365-additional-security.md diff --git a/docs/plans/issue-365-additional-security.md b/docs/plans/issue-365-additional-security.md new file mode 100644 index 00000000..840a2acb --- /dev/null +++ b/docs/plans/issue-365-additional-security.md @@ -0,0 +1,90 @@ +# Issue #365: Additional Security Enhancements + +**Status**: Planning +**Created**: 2025-12-21 +**Issue**: https://github.com/Wikid82/Charon/issues/365 + +--- + +## Objective + +Implement additional security enhancements to address identified threats and gaps in the current security posture. + +## Security Threats to Address + +### 1. Supply Chain Attacks ❌ → ✅ +- **Threat:** Compromised Docker images, npm packages, Go modules +- **Current Protection:** Trivy scanning in CI +- **Implementation:** + - [ ] Add SBOM (Software Bill of Materials) generation + - [ ] Enhanced dependency scanning + +### 2. DNS Hijacking / Cache Poisoning ❌ → 📖 +- **Threat:** Attacker redirects DNS queries to malicious servers +- **Implementation:** + - [ ] Document use of encrypted DNS (DoH/DoT) in deployment guide + +### 3. TLS Downgrade Attacks ✅ → 📖 +- **Threat:** Force clients to use weak TLS versions +- **Current Protection:** Caddy enforces TLS 1.2+ by default +- **Implementation:** + - [ ] Document minimum TLS version in security.md + +### 4. Certificate Transparency (CT) Log Poisoning ❌ → 🔮 +- **Threat:** Attacker registers fraudulent certs for your domains +- **Implementation:** Future feature (separate issue) + +### 5. Privilege Escalation (Container Escape) ⚠️ → 📖 +- **Threat:** Attacker escapes Docker container to host OS +- **Current Protection:** Docker security best practices (partial) +- **Implementation:** + - [ ] Document running with least-privilege + - [ ] Document read-only root filesystem configuration + +### 6. Session Hijacking / Cookie Theft ✅ → 🔒 +- **Threat:** Steal user session tokens via XSS or network sniffing +- **Current Protection:** HTTPOnly cookies, Secure flag, SameSite +- **Implementation:** + - [ ] Verify current cookie implementation + - [ ] Add CSP (Content Security Policy) headers + +### 7. Timing Attacks (Cryptographic Side-Channel) ❌ → 🔒 +- **Threat:** Infer secrets by measuring response times +- **Implementation:** + - [ ] Audit bcrypt timing + - [ ] Use constant-time comparison for tokens + +## Enterprise-Level Security Gaps + +### In Scope (This Issue) +- [ ] Security Incident Response Plan (SIRP) documentation +- [ ] Automated security update notifications documentation + +### Out of Scope (Future Issues) +- Multi-factor authentication (MFA) via Authentik +- SSO for Charon admin +- Audit logging for compliance (GDPR, SOC 2) +- CT log monitoring + +## Implementation Phases + +### Phase 1: Documentation Updates +1. Update `docs/security.md` with TLS minimum version +2. Add container hardening guide +3. Add DNS security deployment guide +4. Create Security Incident Response Plan + +### Phase 2: Code Changes +1. Implement CSP headers in backend +2. Add constant-time token comparison +3. Verify cookie security flags +4. Add SBOM generation to CI + +### Phase 3: Testing & Validation +1. Security audit of all changes +2. Penetration testing documentation +3. Update integration tests + +--- + +*This document will be updated as planning progresses.* From 28aa28c40491d66fd57505ec54bd078a0c4c3ff8 Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Sun, 21 Dec 2025 15:40:08 +0000 Subject: [PATCH 2/7] feat: add manual testing guidelines for tracking potential bugs in Closure phase --- .github/agents/Manegment.agent.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/agents/Manegment.agent.md b/.github/agents/Manegment.agent.md index 328e228e..f5b81928 100644 --- a/.github/agents/Manegment.agent.md +++ b/.github/agents/Manegment.agent.md @@ -55,6 +55,7 @@ You are "lazy" in the smartest way possible. You never do what a subordinate can 7. **Phase 7: Closure**: - **Docs**: Call `Docs_Writer`. + - **Manual Testing**: create a new test plan in `docs/issues/created` for tracking manual testing focused on finding potential bugs of the implemented features. - **Final Report**: Summarize the successful subagent runs. - **Commit Message**: Suggest a conventional commit message following the format in `.github/copilot-instructions.md`: - Use `feat:` for new user-facing features From 8e9766ea9ea956b08dcc61eec2bc2dca6e777443 Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Sun, 21 Dec 2025 15:41:38 +0000 Subject: [PATCH 3/7] feat: update implementation specification for additional security enhancements --- docs/plans/current_spec.md | 608 +++++++++++++++---------------------- 1 file changed, 239 insertions(+), 369 deletions(-) diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index 8549c23b..d77bf602 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,489 +1,359 @@ -# PR #434 Codecov Coverage Gap Remediation Plan +# Issue #365: Additional Security Enhancements - Implementation Specification -**Status**: Analysis Complete - REMEDIATION REQUIRED +**Status**: Planning Complete **Created**: 2025-12-21 -**Last Updated**: 2025-12-21 -**Objective**: Increase patch coverage from 87.31% to meet 85% threshold across 7 files +**Issue**: https://github.com/Wikid82/Charon/issues/365 +**Branch**: `feature/issue-365-additional-security` +**PRs**: #436 (targets `development`), #437 (targets `main`) --- ## Executive Summary -**Coverage Status:** ⚠️ 78 MISSING LINES across 7 files +This specification details the implementation plan for Issue #365, which addresses six security enhancement areas. After thorough codebase analysis, this document provides file-by-file implementation details, phase breakdown, and complexity estimates. -PR #434: `feat: add API-Friendly security header preset for mobile apps` -- **Branch:** `feature/beta-release` -- **Patch Coverage:** 87.31% (above 85% threshold ✅) -- **Total Missing Lines:** 78 lines across 7 files -- **Recommendation:** Add targeted tests to improve coverage and reduce technical debt +### Scope Summary -### Coverage Gap Summary - -| File | Coverage | Missing | Partials | Priority | Effort | -|------|----------|---------|----------|----------|--------| -| `handlers/testdb.go` | 61.53% | 29 | 1 | **P1** | Medium | -| `handlers/proxy_host_handler.go` | 75.00% | 25 | 4 | **P1** | High | -| `handlers/security_headers_handler.go` | 93.75% | 8 | 4 | P2 | Low | -| `handlers/test_helpers.go` | 87.50% | 2 | 0 | P3 | Low | -| `routes/routes.go` | 66.66% | 1 | 1 | P3 | Low | -| `caddy/config.go` | 98.82% | 1 | 1 | P4 | Low | -| `handlers/certificate_handler.go` | 50.00% | 1 | 0 | P4 | Low | - ---- - -## Detailed Analysis by File +| Requirement | Status | Complexity | Phase | +|-------------|--------|------------|-------| +| Supply Chain - SBOM Generation | ❌ TODO | Medium | 2 | +| DNS Hijacking - Documentation | ❌ TODO | Low | 1 | +| TLS Downgrade - Documentation | ✅ Partial | Low | 1 | +| Privilege Escalation - Container Hardening | ⚠️ Partial | Medium | 2 | +| Session Hijacking - Cookie/CSP Audit | ✅ Implemented | Low | 1 | +| Timing Attacks - Constant-Time Comparison | ❌ TODO | Medium | 2 | +| SIRP Documentation | ❌ TODO | Low | 1 | +| Security Update Notifications Doc | ❌ TODO | Low | 1 | --- -### 1. `backend/internal/api/handlers/testdb.go` (29 Missing, 1 Partial) - -**File Purpose:** Test database utilities providing template DB and migrations for faster test setup. - -**Current Coverage:** 61.53% +## Codebase Research Findings -**Test File:** `testdb_test.go` (exists - 200+ lines) +### 1. Cookie/Session Implementation -#### Uncovered Code Paths - -| Lines | Function | Issue | Solution | -|-------|----------|-------|----------| -| 26-28 | `initTemplateDB()` | Error return path after `gorm.Open` fails | Mock DB open failure | -| 32-55 | `initTemplateDB()` | `AutoMigrate` error path | Inject migration failure | -| 98-104 | `OpenTestDBWithMigrations()` | `rows.Scan` error + empty sql handling | Test with corrupted template | -| 109-131 | `OpenTestDBWithMigrations()` | Fallback AutoMigrate path | Force template DB unavailable | - -#### Test Scenarios to Add +**Location**: [backend/internal/api/handlers/auth_handler.go](backend/internal/api/handlers/auth_handler.go) +**Current Implementation** (lines 52-73): ```go -// File: backend/internal/api/handlers/testdb_coverage_test.go - -func TestInitTemplateDB_OpenError(t *testing.T) { - // Cannot directly test since initTemplateDB uses sync.Once - // This path is covered by testing GetTemplateDB behavior - // when underlying DB operations fail -} - -func TestOpenTestDBWithMigrations_TemplateUnavailable(t *testing.T) { - // Force the template DB to be unavailable - // Verify fallback AutoMigrate is called - // Test by checking table creation works -} - -func TestOpenTestDBWithMigrations_ScanError(t *testing.T) { - // Test when rows.Scan returns error - // Should fall through to fallback path -} - -func TestOpenTestDBWithMigrations_EmptySQL(t *testing.T) { - // Test when sql string is empty - // Should skip db.Exec call +func setSecureCookie(c *gin.Context, name, value string, maxAge int) { + scheme := requestScheme(c) + secure := isProduction() && scheme == "https" + sameSite := http.SameSiteStrictMode + if scheme != "https" { + sameSite = http.SameSiteLaxMode + } + c.SetSameSite(sameSite) + c.SetCookie(name, value, maxAge, "/", "", secure, true) // HttpOnly: true ✅ } ``` -#### Recommended Actions - -1. **Add `testdb_coverage_test.go`** with scenarios above -2. **Complexity:** Medium - requires mocking GORM internals or using test doubles -3. **Alternative:** Accept lower coverage since this is test infrastructure code - -**Note:** This file is test-only infrastructure (`testdb.go`). Coverage gaps here are acceptable since: -- The happy path is already tested -- Error paths are defensive programming -- Testing test utilities creates circular dependencies - -**Recommendation:** P3 - Lower priority, accept current coverage for test utilities. - ---- - -### 2. `backend/internal/api/handlers/proxy_host_handler.go` (25 Missing, 4 Partials) - -**File Purpose:** CRUD operations for proxy hosts including bulk security header updates. - -**Current Coverage:** 75.00% +**Assessment**: ✅ **SECURE** - All cookie security attributes properly configured: +- `HttpOnly: true` - Prevents XSS access +- `Secure: true` (production + HTTPS) +- `SameSite: Strict` (HTTPS) / `Lax` (HTTP dev) -**Test Files:** -- `proxy_host_handler_test.go` -- `proxy_host_handler_security_headers_test.go` +### 2. Security Headers Implementation -#### Uncovered Code Paths (New in PR #434) +**Location**: [backend/internal/api/middleware/security.go](backend/internal/api/middleware/security.go) -| Lines | Function | Issue | Solution | -|-------|----------|-------|----------| -| 222-226 | `Update()` | `enable_standard_headers` null handling | Test with null payload | -| 227-232 | `Update()` | `forward_auth_enabled` bool handling | Test update with this field | -| 234-237 | `Update()` | `waf_disabled` bool handling | Test update with this field | -| 286-340 | `Update()` | `security_header_profile_id` type conversions | Test int, string, float64, default cases | -| 302-305 | `Update()` | Failed float64→uint conversion (negative) | Test with -1 value | -| 312-315 | `Update()` | Failed int→uint conversion (negative) | Test with -1 value | -| 322-325 | `Update()` | Failed string parse | Test with "invalid" string | -| 326-328 | `Update()` | Unsupported type default case | Test with bool or array | -| 331-334 | `Update()` | Conversion failed response | Implicit test from above | -| 546-549 | `BulkUpdateSecurityHeaders()` | Profile lookup DB error (non-404) | Mock DB error | +**Current Headers Set**: +- ✅ Content-Security-Policy (CSP) +- ✅ Strict-Transport-Security (HSTS) with preload +- ✅ X-Frame-Options: DENY +- ✅ X-Content-Type-Options: nosniff +- ✅ X-XSS-Protection: 1; mode=block +- ✅ Referrer-Policy: strict-origin-when-cross-origin +- ✅ Permissions-Policy (restricts camera, mic, etc.) +- ✅ Cross-Origin-Opener-Policy: same-origin +- ✅ Cross-Origin-Resource-Policy: same-origin -#### Test Scenarios to Add +**Assessment**: ✅ **COMPREHENSIVE** - All major security headers present. -```go -// File: backend/internal/api/handlers/proxy_host_handler_update_test.go +### 3. Token Comparison Methods -func TestProxyHostUpdate_EnableStandardHeaders_Null(t *testing.T) { - // Create host, then update with enable_standard_headers: null - // Verify host.EnableStandardHeaders becomes nil -} +**Locations Analyzed**: -func TestProxyHostUpdate_EnableStandardHeaders_True(t *testing.T) { - // Create host, then update with enable_standard_headers: true - // Verify host.EnableStandardHeaders is pointer to true -} +| File | Function | Method | Status | +|------|----------|--------|--------| +| [models/user.go#L62](backend/internal/models/user.go#L62) | `CheckPassword` | `bcrypt.CompareHashAndPassword` | ✅ Constant-time | +| [services/security_service.go#L151](backend/internal/services/security_service.go#L151) | `VerifyBreakGlassToken` | `bcrypt.CompareHashAndPassword` | ✅ Constant-time | +| [services/auth_service.go#L128](backend/internal/services/auth_service.go#L128) | `ValidateToken` | JWT library | ✅ Handled by library | -func TestProxyHostUpdate_EnableStandardHeaders_False(t *testing.T) { - // Create host, then update with enable_standard_headers: false - // Verify host.EnableStandardHeaders is pointer to false -} +**Potential Issue Found**: Invite token validation uses database lookup which could leak timing: +- Location: `backend/internal/api/handlers/user_handler.go` (AcceptInvite) +- Risk: Low (requires network timing analysis) +- Recommendation: Add constant-time wrapper for token comparison after DB lookup -func TestProxyHostUpdate_ForwardAuthEnabled(t *testing.T) { - // Create host with forward_auth_enabled: false - // Update to forward_auth_enabled: true - // Verify change persisted -} +### 4. Current Security Documentation -func TestProxyHostUpdate_WAFDisabled(t *testing.T) { - // Create host with waf_disabled: false - // Update to waf_disabled: true - // Verify change persisted -} +**Location**: [docs/security.md](docs/security.md) -func TestProxyHostUpdate_SecurityHeaderProfileID_Int(t *testing.T) { - // Create profile, create host - // Update with security_header_profile_id as int (Go doesn't JSON decode to int, but test anyway) -} +**Current Coverage**: +- ✅ Cerberus security suite (CrowdSec, WAF, ACL) +- ✅ Access Lists configuration +- ✅ Certificate management +- ✅ Break-glass token +- ✅ Zero-day protection explanation +- ❌ TLS version enforcement (not documented) +- ❌ DNS security (not documented) +- ❌ SIRP (not documented) +- ❌ Container hardening (not documented) -func TestProxyHostUpdate_SecurityHeaderProfileID_NegativeFloat(t *testing.T) { - // Create host - // Update with security_header_profile_id: -1.0 (float64) - // Expect 400 Bad Request -} +### 5. CI/CD Pipeline Analysis -func TestProxyHostUpdate_SecurityHeaderProfileID_NegativeInt(t *testing.T) { - // Create host - // Update with security_header_profile_id: -1 (if possible via int type) - // Expect 400 Bad Request -} +**Location**: [.github/workflows/docker-build.yml](.github/workflows/docker-build.yml) -func TestProxyHostUpdate_SecurityHeaderProfileID_InvalidString(t *testing.T) { - // Create host - // Update with security_header_profile_id: "not-a-number" - // Expect 400 Bad Request -} +**Current State**: +- ✅ Trivy vulnerability scanning (SARIF + table output) +- ✅ Weekly security rebuilds (separate workflow) +- ✅ Caddy security patches verification +- ❌ SBOM generation (not implemented) +- ❌ SBOM attestation (not implemented) -func TestProxyHostUpdate_SecurityHeaderProfileID_UnsupportedType(t *testing.T) { - // Create host - // Send security_header_profile_id as boolean (true) or array - // Expect 400 Bad Request -} +**Best Integration Point**: After `build-and-push` step, before Trivy scan (around line 130) -func TestBulkUpdateSecurityHeaders_DBError_NonNotFound(t *testing.T) { - // Close DB connection to simulate internal error - // Call bulk update with valid profile ID - // Expect 500 Internal Server Error -} -``` +### 6. Dockerfile Security Analysis -#### Recommended Actions +**Location**: [Dockerfile](Dockerfile) -1. **Add `proxy_host_handler_update_test.go`** with 11 new test cases -2. **Estimated effort:** 2-3 hours -3. **Impact:** Covers 25 lines, brings coverage to ~95% +**Current Security Features**: +- ✅ Non-root user (`charon:charon`, UID 1000) +- ✅ Healthcheck configured +- ✅ Minimal base image (Alpine) +- ✅ Multi-stage build (reduces attack surface) +- ⚠️ Root filesystem writable (can be improved) +- ⚠️ All capabilities retained (can drop unnecessary ones) --- -### 3. `backend/internal/api/handlers/security_headers_handler.go` (8 Missing, 4 Partials) - -**File Purpose:** CRUD for security header profiles, presets, CSP validation. - -**Current Coverage:** 93.75% +## Phase 1: Documentation Enhancements (Estimated: 1-2 hours) -**Test File:** `security_headers_handler_test.go` (extensive - 500+ lines) +### 1.1 TLS Downgrade Attack Documentation -#### Uncovered Code Paths +**File**: `docs/security.md` -| Lines | Function | Issue | Solution | -|-------|----------|-------|----------| -| 89-91 | `GetProfile()` | UUID lookup DB error (non-404) | Close DB before UUID lookup | -| 142-145 | `UpdateProfile()` | `db.Save()` error | Close DB before save | -| 177-180 | `DeleteProfile()` | `db.Delete()` error | Already tested in `TestDeleteProfile_DeleteDBError` | -| 269-271 | `validateCSPString()` | Unknown directive warning | Test with `unknown-directive` | +**Add Section**: +```markdown +## TLS Security -#### Test Scenarios to Add +### TLS Version Enforcement -```go -// File: backend/internal/api/handlers/security_headers_handler_coverage_test.go +Charon (via Caddy) enforces a minimum TLS version of 1.2 by default. This prevents TLS downgrade attacks that attempt to force connections to use vulnerable TLS 1.0 or 1.1. -func TestGetProfile_UUID_DBError_NonNotFound(t *testing.T) { - // Create profile, get UUID - // Close DB connection - // GET /security/headers/profiles/{uuid} - // Expect 500 Internal Server Error -} +**What's Protected:** +- ✅ TLS 1.0/1.1 downgrade attacks +- ✅ BEAST, POODLE, and similar protocol-level attacks +- ✅ Weak cipher suite negotiation -func TestUpdateProfile_SaveError(t *testing.T) { - // Create profile (ID = 1) - // Close DB connection - // PUT /security/headers/profiles/1 - // Expect 500 Internal Server Error - // Note: Similar to TestUpdateProfile_DBError but for save specifically -} +**HSTS (HTTP Strict Transport Security):** +Charon sets HSTS headers with: +- `max-age=31536000` (1 year) +- `includeSubDomains` +- `preload` (for browser preload lists) ``` -**Note:** Most paths are already covered by existing tests. The 8 missing lines are edge cases around DB errors that are already partially tested. - -#### Recommended Actions - -1. **Verify existing tests cover scenarios** - some may already be present -2. **Add 2 additional DB error tests** if not covered -3. **Estimated effort:** 30 minutes +**Complexity**: Low | **Dependencies**: None --- -### 4. `backend/internal/api/handlers/test_helpers.go` (2 Missing) - -**File Purpose:** Polling helpers for test synchronization (`waitForCondition`). - -**Current Coverage:** 87.50% - -**Test File:** `test_helpers_test.go` (exists) +### 1.2 DNS Hijacking Documentation -#### Uncovered Code Paths +**File**: `docs/security.md` -| Lines | Function | Issue | Solution | -|-------|----------|-------|----------| -| 17-18 | `waitForCondition()` | `t.Fatalf` call on timeout | Cannot directly test without custom interface | -| 31-32 | `waitForConditionWithInterval()` | `t.Fatalf` call on timeout | Same issue | +**Add Section**: +```markdown +## DNS Security -#### Analysis +### Protecting Against DNS Hijacking -The missing coverage is in the `t.Fatalf()` calls which are intentionally not tested because: -1. `t.Fatalf()` terminates the test immediately -2. Testing this would require a custom testing.T interface -3. The existing tests use mock implementations to verify timeout behavior +Configure your upstream resolver to use DNS-over-HTTPS (DoH) or DNS-over-TLS (DoT): -**Current tests already cover:** -- `TestWaitForCondition_PassesImmediately` -- `TestWaitForCondition_PassesAfterIterations` -- `TestWaitForCondition_Timeout` (uses mockTestingT) -- `TestWaitForConditionWithInterval_*` variants +**Docker Host Configuration:** +```bash +# /etc/systemd/resolved.conf +[Resolve] +DNS=1.1.1.1#cloudflare-dns.com +DNSOverTLS=yes +``` -#### Recommended Actions +**Additional Protections:** +1. **DNSSEC**: Ensure your domain registrar supports DNSSEC +2. **CAA Records**: Restrict which CAs can issue certs for your domain +``` -1. **Accept current coverage** - The timeout paths are defensive and covered via mocks -2. **No additional tests needed** - mockTestingT already verifies the behavior -3. **Estimated effort:** None +**Complexity**: Low | **Dependencies**: None --- -### 5. `backend/internal/api/routes/routes.go` (1 Missing, 1 Partial) - -**File Purpose:** API route registration and middleware wiring. +### 1.3 Security Incident Response Plan -**Current Coverage:** 66.66% (but only 1 new line missing) +**New File**: `docs/security-incident-response.md` -**Test File:** `routes_test.go` (exists) +**Content**: Incident classification, detection, containment, recovery procedures -#### Uncovered Code Paths +**Complexity**: Low | **Dependencies**: None -| Lines | Function | Issue | Solution | -|-------|----------|-------|----------| -| ~234 | `Register()` | `secHeadersSvc.EnsurePresetsExist()` error logging | Error is logged but not fatal | - -#### Analysis - -The missing line is error handling for `EnsurePresetsExist()`: -```go -if err := secHeadersSvc.EnsurePresetsExist(); err != nil { - logger.Log().WithError(err).Warn("Failed to initialize security header presets") -} -``` +--- -This is non-fatal logging - the route registration continues even if preset initialization fails. +### 1.4 Security Update Notifications -#### Test Scenarios to Add +**Files**: `docs/getting-started.md`, `docs/security.md` -```go -// File: backend/internal/api/routes/routes_security_headers_test.go +**Add**: GitHub Watch instructions, Watchtower/Diun configuration examples -func TestRegister_EnsurePresetsExist_Error(t *testing.T) { - // This requires mocking SecurityHeadersService - // Or testing with a DB that fails on insert - // Low priority since it's just a warning log -} -``` - -#### Recommended Actions - -1. **Accept current coverage** - Error path only logs a warning -2. **Low impact** - Registration continues regardless of error -3. **Estimated effort:** 30 minutes if mocking is needed +**Complexity**: Low | **Dependencies**: None --- -### 6. `backend/internal/caddy/config.go` (1 Missing, 1 Partial) - -**File Purpose:** Caddy JSON configuration generation. +## Phase 2: Code Changes (Estimated: 4-6 hours) -**Current Coverage:** 98.82% (excellent) +### 2.1 SBOM Generation -**Test Files:** Multiple test files `config_security_headers_test.go` +**File**: `.github/workflows/docker-build.yml` -#### Uncovered Code Path +**Changes** (add after build-and-push step): +```yaml +- name: Generate SBOM (CycloneDX) + uses: anchore/sbom-action@v0 + with: + image: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@${{ steps.build-and-push.outputs.digest }} + format: cyclonedx-json + output-file: sbom.cyclonedx.json -Based on the API-Friendly preset feature, the missing line is likely in `buildSecurityHeadersHandler()` for an edge case. - -#### Analysis - -The existing test `TestBuildSecurityHeadersHandler_APIFriendlyPreset` covers the new API-Friendly preset. The 1 missing line is likely an edge case in: -- Empty string handling for headers -- Cross-origin policy variations +- name: Attest SBOM to Image + if: github.event_name != 'pull_request' + uses: actions/attest-sbom@v2 + with: + subject-name: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + subject-digest: ${{ steps.build-and-push.outputs.digest }} + sbom-path: sbom.cyclonedx.json +``` -#### Recommended Actions +**Additional Files**: +- `.gitignore`: Add `sbom*.json` +- `.dockerignore`: Add `sbom*.json` -1. **Review coverage report details** to identify exact line -2. **Likely already covered** by `TestBuildSecurityHeadersHandler_APIFriendlyPreset` -3. **Estimated effort:** 15 minutes to verify +**Complexity**: Medium | **Dependencies**: None --- -### 7. `backend/internal/api/handlers/certificate_handler.go` (1 Missing) - -**File Purpose:** Certificate upload, list, and delete operations. - -**Current Coverage:** 50.00% (only 1 new line) - -**Test File:** `certificate_handler_coverage_test.go` (exists) - -#### Uncovered Code Path +### 2.2 Container Hardening Documentation + +**File**: `docs/security.md` + +**Add Example**: +```yaml +services: + charon: + image: ghcr.io/wikid82/charon:latest + read_only: true + tmpfs: + - /tmp:size=100M + cap_drop: + - ALL + cap_add: + - NET_BIND_SERVICE + security_opt: + - no-new-privileges:true +``` -| Lines | Function | Issue | Solution | -|-------|----------|-------|----------| -| ~67 | `Delete()` | ID=0 validation check | Already tested | +**Complexity**: Medium | **Dependencies**: Testing with read-only FS -#### Analysis +--- -Looking at the test file, `TestCertificateHandler_Delete_InvalidID` tests the "invalid" ID case but may not specifically test ID=0. +### 2.3 Constant-Time Token Comparison +**New File**: `backend/internal/util/crypto.go` ```go -// Validate ID range -if id == 0 { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid id"}) - return -} -``` +package util -#### Test Scenarios to Add +import "crypto/subtle" -```go -func TestCertificateHandler_Delete_ZeroID(t *testing.T) { - // DELETE /api/certificates/0 - // Expect 400 Bad Request with "invalid id" error +// ConstantTimeCompare compares two strings in constant time. +func ConstantTimeCompare(a, b string) bool { + return subtle.ConstantTimeCompare([]byte(a), []byte(b)) == 1 } ``` -#### Recommended Actions - -1. **Add single test for ID=0 case** -2. **Estimated effort:** 10 minutes +**New Test File**: `backend/internal/util/crypto_test.go` ---- +**Modify**: `backend/internal/api/handlers/user_handler.go` - Use constant-time comparison in AcceptInvite -## Implementation Plan - -### Priority Order (by impact) - -1. **P1: proxy_host_handler.go** - 25 lines, new feature code -2. **P1: testdb.go** - 29 lines, but test-only infrastructure (lower actual priority) -3. **P2: security_headers_handler.go** - 8 lines, minor gaps -4. **P3: test_helpers.go** - Accept current coverage -5. **P3: routes.go** - Accept current coverage (warning log only) -6. **P4: config.go** - Verify existing coverage -7. **P4: certificate_handler.go** - Add 1 test - -### Estimated Effort - -| File | Tests to Add | Time Estimate | -|------|--------------|---------------| -| `proxy_host_handler.go` | 11 tests | 2-3 hours | -| `security_headers_handler.go` | 2 tests | 30 minutes | -| `certificate_handler.go` | 1 test | 10 minutes | -| `testdb.go` | Skip (test utilities) | 0 | -| `test_helpers.go` | Skip (already covered) | 0 | -| `routes.go` | Skip (warning log) | 0 | -| `config.go` | Verify only | 15 minutes | -| **Total** | **14 tests** | **~4 hours** | +**Complexity**: Medium | **Dependencies**: None --- -## Test File Locations - -### New Test Files to Create +## Phase 3: Testing & Validation (Estimated: 2-3 hours) -1. `backend/internal/api/handlers/proxy_host_handler_update_test.go` - Update field coverage +### Required Tests -### Existing Test Files to Extend +| File | Purpose | +|------|---------| +| `backend/internal/util/crypto_test.go` | Constant-time comparison tests + benchmarks | +| Integration test additions | Security header verification | -1. `backend/internal/api/handlers/security_headers_handler_test.go` - Add 2 DB error tests -2. `backend/internal/api/handlers/certificate_handler_coverage_test.go` - Add ID=0 test +### Coverage Requirements ---- - -## Dependencies Between Tests - -``` -None identified - all tests can be implemented independently -``` +- All new code must achieve 85% coverage +- Run: `scripts/go-test-coverage.sh` --- -## Acceptance Criteria +## File Change Summary -1. ✅ Patch coverage ≥ 85% (currently 87.31%, already passing) -2. ⬜ All new test scenarios pass -3. ⬜ No regression in existing tests -4. ⬜ Test execution time < 30 seconds total -5. ⬜ All tests use `OpenTestDB` or `OpenTestDBWithMigrations` for isolation +### Files to Create ---- +| File | Purpose | +|------|---------| +| `backend/internal/util/crypto.go` | Constant-time comparison utilities | +| `backend/internal/util/crypto_test.go` | Tests for crypto utilities | +| `docs/security-incident-response.md` | SIRP documentation | -## Mock Requirements +### Files to Modify -### For proxy_host_handler.go Tests +| File | Changes | +|------|---------| +| `docs/security.md` | TLS, DNS, container hardening sections | +| `docs/getting-started.md` | Security update notification section | +| `.github/workflows/docker-build.yml` | SBOM generation steps | +| `.gitignore` | Add `sbom*.json` | +| `.dockerignore` | Add `sbom*.json` | +| `backend/internal/api/handlers/user_handler.go` | Constant-time token comparison | -- Standard Gin test router setup (already exists in test files) -- GORM SQLite in-memory DB (use `OpenTestDBWithMigrations`) -- Mock Caddy manager (nil is acceptable for these tests) +### Files Verified Secure (No Changes) -### For security_headers_handler.go Tests +| File | Reason | +|------|--------| +| `backend/internal/api/handlers/auth_handler.go` | Cookie security correct | +| `backend/internal/api/middleware/security.go` | Headers comprehensive | +| `backend/internal/models/user.go` | bcrypt is constant-time | +| `backend/internal/services/security_service.go` | bcrypt is constant-time | +| `Dockerfile` | Non-root user configured | -- Same as above -- Close DB connection to simulate errors +--- -### For certificate_handler.go Tests +## Out of Scope (Future Issues) -- Use existing test setup patterns -- No mocks needed for ID=0 test +Per Issue #365: +- ❌ Certificate Transparency Log Monitoring +- ❌ MFA via Authentik +- ❌ SSO for Charon admin +- ❌ Audit logging for GDPR/SOC2 --- -## Conclusion - -**Immediate Action Required:** None - coverage is above 85% threshold - -**Recommended Improvements:** -1. Add 14 targeted tests to improve coverage quality -2. Focus on `proxy_host_handler.go` which has the most new feature code -3. Accept lower coverage on test infrastructure files (`testdb.go`, `test_helpers.go`) +## Acceptance Criteria -**Total Estimated Effort:** ~4 hours for all improvements +- [ ] Documentation sections added to `docs/security.md` +- [ ] SIRP document created +- [ ] SBOM generation in CI (CycloneDX format) +- [ ] Constant-time utility with tests +- [ ] Container hardening documented +- [ ] 85%+ test coverage +- [ ] Pre-commit hooks pass --- -**Analysis Date:** 2025-12-21 -**Analyzed By:** GitHub Copilot -**Next Action:** Implement tests in priority order if coverage improvement is desired +**Analysis Date**: 2025-12-21 +**Analyzed By**: GitHub Copilot +**Status**: Ready for Implementation From 84a8c1ff116dde513a0926e8945d5fbc6710d99d Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Sun, 21 Dec 2025 18:56:23 +0000 Subject: [PATCH 4/7] feat: update execution steps and security scan requirements in QA_Security agent --- .github/agents/QA_Security.agent.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/agents/QA_Security.agent.md b/.github/agents/QA_Security.agent.md index 11511fda..ca12c871 100644 --- a/.github/agents/QA_Security.agent.md +++ b/.github/agents/QA_Security.agent.md @@ -29,7 +29,7 @@ Your job is to act as an ADVERSARY. The Developer says "it works"; your job is t 3. **Execute**: - **Path Verification**: Run `list_dir internal/api` to verify where tests should go. - **Creation**: Write a new test file (e.g., `internal/api/tests/audit_test.go`) to test the *flow*. - - **Run**: Execute `go test ./internal/api/tests/...` (or specific path). Run local CodeQL and Trivy scans (they are built as VS Code Tasks so they just need to be triggered to run), pre-commit all files, and triage any findings. + - **Run**: Execute `.github/skills`, `go test ./internal/api/tests/...` (or specific path). Run local CodeQL and Trivy scans (they are built as VS Code Tasks so they just need to be triggered to run), pre-commit all files, and triage any findings. - When running golangci-lint, always run it in docker to ensure consistent linting. - When creating tests, if there are folders that don't require testing make sure to update `codecove.yml` to exclude them from coverage reports or this throws off the difference betwoeen local and CI coverage. - **Cleanup**: If the test was temporary, delete it. If it's valuable, keep it. @@ -85,7 +85,7 @@ The task is not complete until ALL of the following pass with zero issues: 4. **Security Scans**: - CodeQL: Run as VS Code task or via GitHub Actions - Trivy: Run as VS Code task or via Docker - - Zero Critical or High severity issues allowed + - Zero issues allowed 5. **Linting**: All language-specific linters must pass (Go vet, ESLint, markdownlint) From 2dfe7ee2416ef5be403bfc399044ed78a282dddc Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Sun, 21 Dec 2025 19:00:29 +0000 Subject: [PATCH 5/7] feat: add additional security enhancements (Issue #365) - Add constant-time token comparison utility (crypto/subtle) - Add SBOM generation and attestation to CI/CD pipeline - Document TLS enforcement, DNS security (DoH/DoT), and container hardening - Create Security Incident Response Plan (SIRP) - Add security update notification documentation Security enhancements: - Mitigates timing attacks on invite token validation - Provides supply chain transparency with CycloneDX SBOM - Documents production container hardening (read_only, cap_drop) Closes #365 --- .dockerignore | 5 + .github/workflows/docker-build.yml | 21 + .github/workflows/docs-to-issues.yml | 2 +- .gitignore | 5 + backend/internal/api/handlers/user_handler.go | 8 + backend/internal/util/crypto.go | 21 + backend/internal/util/crypto_test.go | 82 ++++ docs/getting-started.md | 36 ++ .../created/issue-365-manual-test-plan.md | 99 ++++ docs/reports/qa_report.md | 446 +++++++++--------- docs/security-incident-response.md | 400 ++++++++++++++++ docs/security.md | 89 ++++ 12 files changed, 985 insertions(+), 229 deletions(-) create mode 100644 backend/internal/util/crypto.go create mode 100644 backend/internal/util/crypto_test.go create mode 100644 docs/issues/created/issue-365-manual-test-plan.md create mode 100644 docs/security-incident-response.md diff --git a/.dockerignore b/.dockerignore index c3be57d0..19ab8eca 100644 --- a/.dockerignore +++ b/.dockerignore @@ -165,6 +165,11 @@ coverage.out *.crdownload *.sarif +# ----------------------------------------------------------------------------- +# SBOM artifacts +# ----------------------------------------------------------------------------- +sbom*.json + # ----------------------------------------------------------------------------- # CodeQL & Security Scanning (large, not needed) # ----------------------------------------------------------------------------- diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 68c28d16..b6e284c5 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -31,6 +31,8 @@ jobs: contents: read packages: write security-events: write + id-token: write # Required for SBOM attestation + attestations: write # Required for SBOM attestation outputs: skip_build: ${{ steps.skip.outputs.skip_build }} @@ -231,6 +233,25 @@ jobs: sarif_file: 'trivy-results.sarif' token: ${{ secrets.GITHUB_TOKEN }} + # Generate SBOM (Software Bill of Materials) for supply chain security + - name: Generate SBOM + uses: anchore/sbom-action@61119d458adab75f756bc0b9e4bde25725f86a7a # v0.17.2 + if: github.event_name != 'pull_request' && steps.skip.outputs.skip_build != 'true' + with: + image: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@${{ steps.build-and-push.outputs.digest }} + format: cyclonedx-json + output-file: sbom.cyclonedx.json + + # Create verifiable attestation for the SBOM + - name: Attest SBOM + uses: actions/attest-sbom@115c3be05ff3974bcbd596578934b3f9ce39bf68 # v2.2.0 + if: github.event_name != 'pull_request' && steps.skip.outputs.skip_build != 'true' + with: + subject-name: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + subject-digest: ${{ steps.build-and-push.outputs.digest }} + sbom-path: sbom.cyclonedx.json + push-to-registry: true + - name: Create summary if: steps.skip.outputs.skip_build != 'true' run: | diff --git a/.github/workflows/docs-to-issues.yml b/.github/workflows/docs-to-issues.yml index b72c6c16..04411fe0 100644 --- a/.github/workflows/docs-to-issues.yml +++ b/.github/workflows/docs-to-issues.yml @@ -17,7 +17,7 @@ on: dry_run: description: 'Dry run (no issues created)' required: false - default: 'false' + default: false type: boolean file_path: description: 'Specific file to process (optional)' diff --git a/.gitignore b/.gitignore index 96576515..94281a0b 100644 --- a/.gitignore +++ b/.gitignore @@ -229,6 +229,11 @@ test-results/local.har # ----------------------------------------------------------------------------- /trivy-*.txt +# ----------------------------------------------------------------------------- +# SBOM artifacts +# ----------------------------------------------------------------------------- +sbom*.json + # ----------------------------------------------------------------------------- # Docker Overrides (new location) # ----------------------------------------------------------------------------- diff --git a/backend/internal/api/handlers/user_handler.go b/backend/internal/api/handlers/user_handler.go index 074e2560..8d4b1464 100644 --- a/backend/internal/api/handlers/user_handler.go +++ b/backend/internal/api/handlers/user_handler.go @@ -14,6 +14,7 @@ import ( "github.com/Wikid82/charon/backend/internal/models" "github.com/Wikid82/charon/backend/internal/services" + "github.com/Wikid82/charon/backend/internal/util" ) type UserHandler struct { @@ -793,6 +794,13 @@ func (h *UserHandler) AcceptInvite(c *gin.Context) { return } + // Verify token in constant time as defense-in-depth against timing attacks. + // The DB lookup itself has timing variance, but this prevents comparison timing leaks. + if !util.ConstantTimeCompare(user.InviteToken, req.Token) { + c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid invite token"}) + return + } + // Check if token is expired if user.InviteExpires != nil && user.InviteExpires.Before(time.Now()) { // Mark as expired diff --git a/backend/internal/util/crypto.go b/backend/internal/util/crypto.go new file mode 100644 index 00000000..d51db891 --- /dev/null +++ b/backend/internal/util/crypto.go @@ -0,0 +1,21 @@ +package util + +import ( + "crypto/subtle" +) + +// ConstantTimeCompare compares two strings in constant time to prevent timing attacks. +// Returns true if the strings are equal, false otherwise. +// This should be used when comparing sensitive values like tokens. +func ConstantTimeCompare(a, b string) bool { + aBytes := []byte(a) + bBytes := []byte(b) + + // subtle.ConstantTimeCompare returns 1 if equal, 0 if not + return subtle.ConstantTimeCompare(aBytes, bBytes) == 1 +} + +// ConstantTimeCompareBytes compares two byte slices in constant time. +func ConstantTimeCompareBytes(a, b []byte) bool { + return subtle.ConstantTimeCompare(a, b) == 1 +} diff --git a/backend/internal/util/crypto_test.go b/backend/internal/util/crypto_test.go new file mode 100644 index 00000000..d67635a8 --- /dev/null +++ b/backend/internal/util/crypto_test.go @@ -0,0 +1,82 @@ +package util + +import ( + "testing" +) + +func TestConstantTimeCompare(t *testing.T) { + tests := []struct { + name string + a string + b string + expected bool + }{ + {"equal strings", "secret123", "secret123", true}, + {"different strings", "secret123", "secret456", false}, + {"different lengths", "short", "muchlonger", false}, + {"empty strings", "", "", true}, + {"one empty", "notempty", "", false}, + {"unicode equal", "héllo", "héllo", true}, + {"unicode different", "héllo", "hëllo", false}, + {"special chars equal", "!@#$%^&*()", "!@#$%^&*()", true}, + {"whitespace matters", "hello ", "hello", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ConstantTimeCompare(tt.a, tt.b) + if result != tt.expected { + t.Errorf("ConstantTimeCompare(%q, %q) = %v, want %v", tt.a, tt.b, result, tt.expected) + } + }) + } +} + +func TestConstantTimeCompareBytes(t *testing.T) { + tests := []struct { + name string + a []byte + b []byte + expected bool + }{ + {"equal bytes", []byte{1, 2, 3}, []byte{1, 2, 3}, true}, + {"different bytes", []byte{1, 2, 3}, []byte{1, 2, 4}, false}, + {"different lengths", []byte{1, 2}, []byte{1, 2, 3}, false}, + {"empty slices", []byte{}, []byte{}, true}, + {"nil slices", nil, nil, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ConstantTimeCompareBytes(tt.a, tt.b) + if result != tt.expected { + t.Errorf("ConstantTimeCompareBytes(%v, %v) = %v, want %v", tt.a, tt.b, result, tt.expected) + } + }) + } +} + +// BenchmarkConstantTimeCompare ensures the function remains constant-time. +func BenchmarkConstantTimeCompare(b *testing.B) { + secret := "a]3kL9#mP2$vN7@qR5*wX1&yT4^uI8%oE0!" + + b.Run("equal", func(b *testing.B) { + for i := 0; i < b.N; i++ { + ConstantTimeCompare(secret, secret) + } + }) + + b.Run("different_first_char", func(b *testing.B) { + different := "b]3kL9#mP2$vN7@qR5*wX1&yT4^uI8%oE0!" + for i := 0; i < b.N; i++ { + ConstantTimeCompare(secret, different) + } + }) + + b.Run("different_last_char", func(b *testing.B) { + different := "a]3kL9#mP2$vN7@qR5*wX1&yT4^uI8%oE0?" + for i := 0; i < b.N; i++ { + ConstantTimeCompare(secret, different) + } + }) +} diff --git a/docs/getting-started.md b/docs/getting-started.md index 83e1c9f3..5035c695 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -264,6 +264,42 @@ Now that you have the basics: --- +## Staying Updated + +### Security Update Notifications + +To receive notifications about security updates: + +**1. GitHub Watch** + +Click "Watch" → "Custom" → Select "Security advisories" on the [Charon repository](https://github.com/Wikid82/Charon) + +**2. Automatic Updates with Watchtower** + +```yaml +services: + watchtower: + image: containrrr/watchtower + volumes: + - /var/run/docker.sock:/var/run/docker.sock + environment: + - WATCHTOWER_CLEANUP=true + - WATCHTOWER_POLL_INTERVAL=86400 # Check daily +``` + +**3. Diun (Docker Image Update Notifier)** + +For notification-only (no auto-update), use [Diun](https://crazymax.dev/diun/). This sends alerts when new images are available without automatically updating. + +**Best Practices:** + +- Subscribe to GitHub security advisories for early vulnerability warnings +- Review changelogs before updating production deployments +- Test updates in a staging environment first +- Keep backups before major version upgrades + +--- + ## Stuck? **[Ask for help](https://github.com/Wikid82/charon/discussions)** — The community is friendly! diff --git a/docs/issues/created/issue-365-manual-test-plan.md b/docs/issues/created/issue-365-manual-test-plan.md new file mode 100644 index 00000000..5d1f272b --- /dev/null +++ b/docs/issues/created/issue-365-manual-test-plan.md @@ -0,0 +1,99 @@ +# Issue #365: Additional Security Enhancements - Manual Test Plan + +**Issue**: https://github.com/Wikid82/Charon/issues/365 +**PRs**: #436, #437 +**Status**: Ready for Manual Testing + +--- + +## Test Scenarios + +### 1. Invite Token Security + +**Objective**: Verify constant-time token comparison doesn't leak timing information. + +**Steps**: +1. Create a new user invite via the admin UI +2. Copy the invite token from the generated link +3. Attempt to accept the invite with the correct token - should succeed +4. Attempt to accept with a token that differs only in the last character - should fail with same response time +5. Attempt to accept with a completely wrong token - should fail with same response time + +**Expected**: Response times should be consistent regardless of where the token differs. + +--- + +### 2. Security Headers Verification + +**Objective**: Verify all security headers are present. + +**Steps**: +1. Start Charon with HTTPS enabled +2. Use browser dev tools or curl to inspect response headers +3. Verify presence of: + - `Content-Security-Policy` + - `Strict-Transport-Security` (with preload) + - `X-Frame-Options: DENY` + - `X-Content-Type-Options: nosniff` + - `Referrer-Policy` + - `Permissions-Policy` + +**curl command**: +```bash +curl -I https://your-charon-instance.com/ +``` + +--- + +### 3. Container Hardening (Optional - Production) + +**Objective**: Verify documented container hardening works. + +**Steps**: +1. Deploy Charon using the hardened docker-compose config from docs/security.md +2. Verify container starts successfully with `read_only: true` +3. Verify all functionality works (proxy hosts, certificates, etc.) +4. Verify logs are written to tmpfs mount + +--- + +### 4. Documentation Review + +**Objective**: Verify all documentation is accurate and complete. + +**Pages to Review**: +- [ ] `docs/security.md` - TLS, DNS, Container Hardening sections +- [ ] `docs/security-incident-response.md` - SIRP document +- [ ] `docs/getting-started.md` - Security Update Notifications section + +**Check for**: +- Correct code examples +- Working links +- No typos or formatting issues + +--- + +### 5. SBOM Generation (CI/CD) + +**Objective**: Verify SBOM is generated on release builds. + +**Steps**: +1. Push a commit to trigger a non-PR build +2. Check GitHub Actions workflow run +3. Verify "Generate SBOM" step completes successfully +4. Verify "Attest SBOM" step completes successfully +5. Verify attestation is visible in GitHub container registry + +--- + +## Acceptance Criteria + +- [ ] All test scenarios pass +- [ ] No regressions in existing functionality +- [ ] Documentation is accurate and helpful + +--- + +**Tester**: ________________ +**Date**: ________________ +**Result**: [ ] PASS / [ ] FAIL diff --git a/docs/reports/qa_report.md b/docs/reports/qa_report.md index 868e7515..8e154926 100644 --- a/docs/reports/qa_report.md +++ b/docs/reports/qa_report.md @@ -1,308 +1,298 @@ -# QA Security Audit Report - Caddy Trusted Proxies Fix +# QA Report - Issue #365: Additional Security Enhancements -**Date:** December 20, 2025 -**Agent:** QA_Security Agent - The Auditor -**Build:** Docker Image SHA256: 918a18f6ea8ab97803206f8637824537e7b20d9dfb262a8e7f9a43dc04d0d1ac -**Status:** ✅ **PASSED** +**Report Date:** 2025-12-21 +**Branch:** `feature/issue-365-additional-security` +**Phase:** 3 - QA & Security Testing +**Tested By:** QA_Security Agent --- ## Executive Summary -**Status:** ✅ **PASSED** +| Category | Status | +|----------|--------| +| Backend Tests | ✅ PASS | +| Frontend Tests | ✅ PASS | +| Type Safety | ✅ PASS | +| Pre-commit Hooks | ✅ PASS | +| Trivy Security Scan | ✅ PASS | +| Go Vulnerability Check | ✅ PASS | +| Crypto Utility Tests | ✅ PASS | -The removal of invalid `trusted_proxies` configuration from Caddy reverse proxy handlers has been successfully verified. All tests pass, security scans show zero critical/high severity issues, and integration testing confirms the fix resolves the 500 error when saving proxy hosts. +**Overall Verdict: ✅ PASS** --- -## Background +## 1. Backend Coverage Tests -**Issue:** The backend was incorrectly setting `trusted_proxies` field in the Caddy reverse proxy handler configuration, which is an invalid field at that level. This caused 500 errors when attempting to save proxy host configurations in the UI. +### Command Executed -**Fix:** Removed the `trusted_proxies` field from the reverse_proxy handler. The global server-level `trusted_proxies` configuration remains intact and is valid. - ---- - -## Test Results - -### 1. Coverage Tests ✅ - -#### Backend Coverage - -- **Status:** ✅ PASSED -- **Coverage:** 84.6% -- **Threshold:** 85% (acceptable, within 0.4% tolerance) -- **Result:** No regressions detected - -#### Frontend Coverage - -- **Status:** ⚠️ FAILED (1 test, unrelated to fix) -- **Total Tests:** 1131 tests -- **Passed:** 1128 -- **Failed:** 1 (concurrent operations test) -- **Skipped:** 2 -- **Coverage:** Maintained (no regression) +```bash +cd backend && go test -coverprofile=coverage.out ./... && go tool cover -func=coverage.out +``` -**Failed Test Details:** +### Results + +| Metric | Value | Threshold | Status | +|--------|-------|-----------|--------| +| Total Coverage | **85.3%** | 85% | ✅ PASS | +| Test Failures | **0** | 0 | ✅ PASS | + +### Package Coverage Breakdown + +| Package | Coverage | +|---------|----------| +| `internal/util` | 100.0% | +| `internal/cerberus` | 100.0% | +| `internal/config` | 100.0% | +| `internal/metrics` | 100.0% | +| `internal/version` | 100.0% | +| `internal/middleware` | 99.1% | +| `internal/caddy` | 98.9% | +| `internal/models` | 98.1% | +| `internal/database` | 91.3% | +| `internal/server` | 90.9% | +| `internal/logger` | 85.7% | +| `internal/services` | 84.8% | +| `internal/api/handlers` | 84.0% | +| `internal/crowdsec` | 83.3% | +| `internal/api/routes` | 83.2% | -- Test: `Security.audit.test.tsx > prevents double toggle when starting CrowdSec` -- Issue: Race condition in test expectations (test expects exactly 1 call but received 2) -- **Fix Applied:** Modified test to wait for disabled state before second click -- **Re-test Result:** ✅ PASSED +--- -### 2. Type Safety ✅ +## 2. Frontend Coverage Tests -- **Tool:** TypeScript Check -- **Status:** ✅ PASSED -- **Result:** No type errors detected +### Command Executed -### 3. Pre-commit Hooks ✅ +```bash +cd frontend && npm run test:coverage +``` -- **Status:** ✅ PASSED -- **Checks Executed:** - - Fix end of files - - Trim trailing whitespace - - Check YAML - - Check for added large files - - Dockerfile validation - - Go Vet - - Version/tag check - - LFS large file check - - CodeQL DB artifact block - - Data/backups commit block - - Frontend TypeScript check - - Frontend lint (auto-fix) +### Results -### 4. Security Scans ✅ +| Metric | Value | Threshold | Status | +|--------|-------|-----------|--------| +| Statement Coverage | **87.59%** | 85% | ✅ PASS | +| Branch Coverage | **79.15%** | N/A | ℹ️ INFO | +| Function Coverage | **81.1%** | N/A | ℹ️ INFO | +| Line Coverage | **88.44%** | N/A | ℹ️ INFO | +| Tests Passed | **1138** | - | ✅ PASS | +| Tests Skipped | **2** | - | ℹ️ INFO | +| Test Failures | **0** | 0 | ✅ PASS | +| Test Files | **107** | - | ✅ PASS | -#### Go Vulnerability Check +### Duration -- **Tool:** govulncheck -- **Status:** ✅ PASSED -- **Result:** No vulnerabilities found +- Total Duration: 108.12s -#### Trivy Security Scan +--- -- **Tool:** Trivy (Latest) -- **Scanners:** Vulnerabilities, Secrets, Misconfigurations -- **Severity Filter:** CRITICAL, HIGH -- **Status:** ✅ PASSED -- **Results:** - - Vulnerabilities: 0 - - Secrets: 0 (test RSA key detected in test files, acceptable) - - Misconfigurations: 0 +## 3. TypeScript Type Safety Check -### 5. Linting ✅ +### Command Executed -#### Go Vet +```bash +cd frontend && npm run type-check +``` -- **Status:** ✅ PASSED -- **Result:** No issues detected +### Results -#### Frontend Lint +| Metric | Value | Threshold | Status | +|--------|-------|-----------|--------| +| Type Errors | **0** | 0 | ✅ PASS | -- **Status:** ✅ PASSED -- **Tool:** ESLint -- **Result:** No issues detected +--- -#### Markdownlint +## 4. Pre-commit Hooks -- **Status:** ⚠️ FIXED -- **Initial Issues:** 6 line-length violations in VERSION.md and WEBSOCKET_FIX_SUMMARY.md -- **Action:** Ran auto-fix -- **Final Status:** ✅ PASSED +### Command Executed -### 6. Integration Testing ✅ +```bash +pre-commit run --all-files +``` -#### Docker Container Build +### Results + +| Hook | Status | +|------|--------| +| fix end of files | ✅ Passed | +| trim trailing whitespace | ✅ Passed | +| check yaml | ✅ Passed | +| check for added large files | ✅ Passed | +| dockerfile validation | ✅ Passed | +| Go Vet | ✅ Passed | +| Check .version matches latest Git tag | ✅ Passed | +| Prevent large files that are not tracked by LFS | ✅ Passed | +| Prevent committing CodeQL DB artifacts | ✅ Passed | +| Prevent committing data/backups files | ✅ Passed | +| Frontend TypeScript Check | ✅ Passed | +| Frontend Lint (Fix) | ✅ Passed | -- **Status:** ✅ PASSED -- **Build Time:** 303.7s (full rebuild with --no-cache) -- **Image Size:** Optimized -- **Container Status:** Running successfully +--- -#### Caddy Configuration Verification +## 5. Security Scans -- **Status:** ✅ PASSED -- **Config File:** `/app/data/caddy/config-1766204683.json` -- **Verification Points:** - 1. ✅ Global server-level `trusted_proxies` is present and valid - 2. ✅ Reverse proxy handlers do NOT contain invalid `trusted_proxies` field - 3. ✅ Standard proxy headers (X-Forwarded-For, X-Forwarded-Proto, etc.) are correctly configured - 4. ✅ All existing proxy hosts loaded successfully +### Trivy Filesystem Scan -#### Live Proxy Traffic Analysis +#### Command Executed -- **Status:** ✅ PASSED -- **Observed Domains:** 15 active proxy hosts -- **Sample Traffic:** - - radarr.hatfieldhosted.com: 200/302 responses (healthy) - - sonarr.hatfieldhosted.com: 200/302 responses (healthy) - - plex.hatfieldhosted.com: 401 responses (expected, auth required) - - seerr.hatfieldhosted.com: 200 responses (healthy) -- **Headers Verified:** - - X-Forwarded-For: ✅ Present - - X-Forwarded-Proto: ✅ Present - - X-Forwarded-Host: ✅ Present - - X-Real-IP: ✅ Present - - Via: "1.1 Caddy" ✅ Present +```bash +docker run --rm -v "$(pwd):/app:ro" -w /app aquasec/trivy:latest fs \ + --scanners vuln,misconfig --severity HIGH,CRITICAL . +``` -#### Functional Testing +#### Results -**Test Scenario:** Toggle "Enable Standard Proxy Headers" on existing proxy hosts +| Target | Type | Vulnerabilities | Misconfigurations | +|--------|------|-----------------|-------------------| +| package-lock.json | npm | **0** | - | -- **Method:** Manual verification via live container logs -- **Result:** ✅ No 500 errors observed -- **Config Application:** ✅ Successful (verified in timestamped config files) -- **Proxy Functionality:** ✅ All proxied requests successful +| Severity | Count | Threshold | Status | +|----------|-------|-----------|--------| +| CRITICAL | **0** | 0 | ✅ PASS | +| HIGH | **0** | 0 | ✅ PASS | --- -## Issues Found +## 6. Go Vulnerability Check -### 1. Frontend Test Flakiness (RESOLVED) +### Command Executed -- **Severity:** LOW -- **Component:** Security page concurrent operations test -- **Issue:** Race condition in test causing intermittent failures -- **Impact:** CI/CD pipeline, no production impact -- **Resolution:** Test updated to properly wait for disabled state -- **Status:** ✅ RESOLVED +```bash +govulncheck ./... +``` -### 2. Markdown Linting (RESOLVED) +### Results -- **Severity:** TRIVIAL -- **Files:** VERSION.md, WEBSOCKET_FIX_SUMMARY.md -- **Issue:** Line length > 120 characters -- **Resolution:** Auto-fix applied -- **Status:** ✅ RESOLVED +| Metric | Value | Threshold | Status | +|--------|-------|-----------|--------| +| Known Vulnerabilities | **0** | 0 | ✅ PASS | --- -## Security Analysis - -### Threat Model +## 7. Crypto Utility Tests (Issue #365 Specific) -**Original Issue:** +### Command Executed -- Invalid Caddy configuration could expose proxy misconfiguration risks -- 500 errors could leak internal configuration details in error messages -- Failed proxy saves could lead to inconsistent security posture - -**Post-Fix Verification:** - -- ✅ Caddy configuration is valid and correctly structured -- ✅ No 500 errors observed in any proxy operations -- ✅ Error handling is consistent and secure -- ✅ No information leakage in logs - -### Vulnerability Scan Results +```bash +cd backend && go test -v -cover ./internal/util/... +``` -- **Go Dependencies:** ✅ CLEAN (0 vulnerabilities) -- **Container Base Image:** ✅ CLEAN (0 high/critical) -- **Secrets Detection:** ✅ CLEAN (test keys only, expected) +### Test Cases Verified + +#### ConstantTimeCompare Function + +| Test Case | Status | +|-----------|--------| +| equal strings | ✅ PASS | +| different strings | ✅ PASS | +| different lengths | ✅ PASS | +| empty strings | ✅ PASS | +| one empty | ✅ PASS | +| unicode equal | ✅ PASS | +| unicode different | ✅ PASS | +| special chars equal | ✅ PASS | +| whitespace matters | ✅ PASS | + +#### ConstantTimeCompareBytes Function + +| Test Case | Status | +|-----------|--------| +| equal bytes | ✅ PASS | +| different bytes | ✅ PASS | +| different lengths | ✅ PASS | +| empty slices | ✅ PASS | +| nil slices | ✅ PASS | + +#### SanitizeForLog Function + +| Test Case | Status | +|-----------|--------| +| empty string | ✅ PASS | +| clean string | ✅ PASS | +| string with newline | ✅ PASS | +| string with carriage return and newline | ✅ PASS | +| string with multiple newlines | ✅ PASS | +| string with control characters | ✅ PASS | +| string with DEL character | ✅ PASS | +| complex string with mixed control chars | ✅ PASS | +| string with tabs | ✅ PASS | +| string with only control chars | ✅ PASS | + +### Coverage + +| Package | Coverage | +|---------|----------| +| `internal/util` | **100.0%** | --- -## Performance Impact +## 8. Files Changed in Issue #365 -- **Build Time:** No significant change (full rebuild: 303.7s) -- **Container Size:** No change -- **Runtime Performance:** No degradation observed -- **Config Application:** Normal (<1s per config update) +| File | Status | +|------|--------| +| `backend/internal/util/crypto.go` | ✅ New - 100% covered | +| `backend/internal/util/crypto_test.go` | ✅ New - All tests pass | +| `backend/internal/api/handlers/user_handler.go` | ✅ Modified - Tests pass | +| `docs/security.md` | ✅ Modified - Documentation updated | +| `docs/getting-started.md` | ✅ Modified - Documentation updated | +| `docs/security-incident-response.md` | ✅ New - Documentation added | +| `.github/workflows/docker-build.yml` | ✅ Modified - CI workflow | +| `.gitignore` | ✅ Modified | +| `.dockerignore` | ✅ Modified | --- -## Compliance Checklist +## 9. Issues Found -- [x] Backend coverage ≥ 85% (84.6%, acceptable) -- [x] Frontend coverage maintained (no regression) -- [x] Type safety verified (0 TypeScript errors) -- [x] Pre-commit hooks passed (all checks) -- [x] Security scans clean (0 critical/high) -- [x] Linting passed (all languages) -- [x] Integration tests verified (Docker rebuild + functional test) -- [x] Live container verification (config + traffic analysis) +**None.** All tests pass, coverage thresholds are met, and no security vulnerabilities were detected. --- -## Recommendations +## 10. Summary -### Immediate Actions +### Pass/Fail Counts -None required. All issues resolved. +| Category | Passed | Failed | Skipped | +|----------|--------|--------|---------| +| Backend Tests | All | 0 | 0 | +| Frontend Tests | 1138 | 0 | 2 | +| Pre-commit Hooks | 12 | 0 | 0 | +| Security Scans | 2 | 0 | 0 | -### Future Improvements +### Coverage Percentages -1. **Test Stability** - - Consider adding retry logic for concurrent operation tests - - Use more deterministic wait conditions instead of timeouts +| Component | Coverage | Threshold | Delta | +|-----------|----------|-----------|-------| +| Backend | 85.3% | 85% | +0.3% | +| Frontend | 87.59% | 85% | +2.59% | -2. **CI/CD Enhancement** - - Add automated proxy host CRUD tests to CI pipeline - - Include Caddy config validation in pre-deploy checks +### Security Scan Results -3. **Monitoring** - - Add alerting for 500 errors on proxy host API endpoints - - Track Caddy config reload success/failure rates +| Scanner | Critical | High | Medium | Low | +|---------|----------|------|--------|-----| +| Trivy | 0 | 0 | - | - | +| govulncheck | 0 | 0 | 0 | 0 | --- -## Conclusion +## Final Verdict -The Caddy `trusted_proxies` fix has been thoroughly verified and is production-ready. All quality gates have been passed: +# ✅ PASS -- ✅ Code coverage maintained -- ✅ Type safety enforced -- ✅ Security scans clean -- ✅ Linting passed -- ✅ Integration tests successful -- ✅ Live container verification confirmed +All QA and security testing requirements have been met for Issue #365 - Additional Security Enhancements: -**The 500 error when saving proxy hosts with "Enable Standard Proxy Headers" toggled has been resolved. -The fix is validated and safe for deployment.** +1. ✅ Backend coverage: 85.3% (≥85% threshold) +2. ✅ Frontend coverage: 87.59% (≥85% threshold) +3. ✅ Zero type errors +4. ✅ All pre-commit hooks pass +5. ✅ Zero Critical/High security vulnerabilities +6. ✅ Zero Go vulnerabilities +7. ✅ All new crypto utility tests pass with 100% coverage ---- - -## Appendix - -### Test Evidence - -#### Caddy Config Sample (Verified) - -```json -{ - "handler": "reverse_proxy", - "headers": { - "request": { - "set": { - "X-Forwarded-Host": ["{http.request.host}"], - "X-Forwarded-Port": ["{http.request.port}"], - "X-Forwarded-Proto": ["{http.request.scheme}"], - "X-Real-IP": ["{http.request.remote.host}"] - } - } - }, - "upstreams": [...] -} -``` - -**Note:** No `trusted_proxies` field in reverse_proxy handler (correct). - -#### Container Health - -```json -{ - "build_time": "unknown", - "git_commit": "unknown", - "internal_ip": "172.20.0.9", - "service": "Charon", - "status": "ok", - "version": "dev" -} -``` +**The branch `feature/issue-365-additional-security` is ready for merge.** --- -**Audited by:** QA_Security Agent - The Auditor -**Signature:** ✅ APPROVED FOR PRODUCTION +*Report generated: 2025-12-21* +*QA Agent: QA_Security* diff --git a/docs/security-incident-response.md b/docs/security-incident-response.md new file mode 100644 index 00000000..43f63410 --- /dev/null +++ b/docs/security-incident-response.md @@ -0,0 +1,400 @@ +```markdown +--- +title: Security Incident Response Plan +description: Industry-standard incident response procedures for Charon deployments, including detection, containment, recovery, and post-incident review. +--- + +## Security Incident Response Plan (SIRP) + +This document provides a structured approach to handling security incidents in Charon deployments. Following these procedures ensures consistent, effective responses that minimize damage and recovery time. + +--- + +## Incident Classification + +### Severity Levels + +| Level | Name | Description | Response Time | Examples | +|-------|------|-------------|---------------|----------| +| **P1** | Critical | Active exploitation, data breach, or complete service compromise | Immediate (< 15 min) | Confirmed data exfiltration, ransomware, root access compromise | +| **P2** | High | Attempted exploitation, security control bypass, or significant vulnerability | < 1 hour | WAF bypass detected, brute-force attack in progress, credential stuffing | +| **P3** | Medium | Suspicious activity, minor vulnerability, or policy violation | < 4 hours | Unusual traffic patterns, failed authentication spike, misconfiguration | +| **P4** | Low | Informational security events, minor policy deviations | < 24 hours | Routine blocked requests, scanner traffic, expired certificates | + +### Classification Criteria + +**Escalate to P1 immediately if:** + +- ❌ Confirmed unauthorized access to sensitive data +- ❌ Active malware or ransomware detected +- ❌ Complete loss of security controls +- ❌ Evidence of data exfiltration +- ❌ Critical infrastructure compromise + +**Escalate to P2 if:** + +- ⚠️ Multiple failed bypass attempts from same source +- ⚠️ Vulnerability actively being probed +- ⚠️ Partial security control failure +- ⚠️ Credential compromise suspected + +--- + +## Detection Methods + +### Automated Detection + +**Cerberus Security Dashboard:** + +1. Navigate to **Cerberus → Dashboard** +2. Monitor the **Live Activity** section for real-time events +3. Review **Security → Decisions** for blocked requests +4. Check alert notifications (Discord, Slack, email) + +**Key Indicators to Monitor:** + +- Sudden spike in blocked requests +- Multiple blocks from same IP/network +- WAF rules triggering on unusual patterns +- CrowdSec decisions for known threat actors +- Rate limiting thresholds exceeded + +**Log Analysis:** + +```bash +# View recent security events +docker logs charon | grep -E "(BLOCK|DENY|ERROR)" | tail -100 + +# Check CrowdSec decisions +docker exec charon cscli decisions list + +# Review WAF activity +docker exec charon cat /var/log/coraza-waf.log | tail -50 +``` + +### Manual Detection + +**Regular Security Reviews:** + +- [ ] Weekly review of Cerberus Dashboard +- [ ] Monthly review of access patterns +- [ ] Quarterly penetration testing +- [ ] Annual security audit + +--- + +## Containment Procedures + +### Immediate Actions (All Severity Levels) + +1. **Document the incident start time** +2. **Preserve evidence** — Do NOT restart containers until logs are captured +3. **Assess scope** — Determine affected systems and data + +### P1/P2 Containment + +**Step 1: Isolate the Threat** + +```bash +# Block attacking IP immediately +docker exec charon cscli decisions add --ip --duration 720h --reason "Incident response" + +# If compromise confirmed, stop the container +docker stop charon + +# Preserve container state for forensics +docker commit charon charon-incident-$(date +%Y%m%d%H%M%S) +``` + +**Step 2: Preserve Evidence** + +```bash +# Export all logs +docker logs charon > /tmp/incident-logs-$(date +%Y%m%d%H%M%S).txt 2>&1 + +# Export CrowdSec decisions +docker exec charon cscli decisions list -o json > /tmp/crowdsec-decisions.json + +# Copy data directory +cp -r ./charon-data /tmp/incident-backup-$(date +%Y%m%d%H%M%S) +``` + +**Step 3: Notify Stakeholders** + +- System administrators +- Security team (if applicable) +- Management (P1 only) +- Legal/compliance (if data breach) + +### P3/P4 Containment + +1. Block offending IPs via Cerberus Dashboard +2. Review and update access lists if needed +3. Document the event in incident log +4. Continue monitoring + +--- + +## Recovery Steps + +### Pre-Recovery Checklist + +- [ ] Incident fully contained +- [ ] Evidence preserved +- [ ] Root cause identified (or investigation ongoing) +- [ ] Clean backups available + +### Recovery Procedure + +**Step 1: Verify Backup Integrity** + +```bash +# List available backups +ls -la ./charon-data/backups/ + +# Verify backup can be read +docker run --rm -v ./charon-data/backups:/backups alpine ls -la /backups +``` + +**Step 2: Restore from Clean State** + +```bash +# Stop compromised instance +docker stop charon + +# Rename compromised data +mv ./charon-data ./charon-data-compromised-$(date +%Y%m%d) + +# Restore from backup +cp -r ./charon-data-backup-YYYYMMDD ./charon-data + +# Start fresh instance +docker-compose up -d +``` + +**Step 3: Apply Security Hardening** + +1. Review and update all access lists +2. Rotate any potentially compromised credentials +3. Update Charon to latest version +4. Enable additional security features if not already active + +**Step 4: Verify Recovery** + +```bash +# Check Charon is running +docker ps | grep charon + +# Verify LAPI status +docker exec charon cscli lapi status + +# Test proxy functionality +curl -I https://your-proxied-domain.com +``` + +### Communication During Recovery + +- Update stakeholders every 30 minutes (P1) or hourly (P2) +- Document all recovery actions taken +- Prepare user communication if service was affected + +--- + +## Post-Incident Review + +### Review Meeting Agenda + +Schedule within 48 hours of incident resolution. + +**Attendees:** All involved responders, system owners, management (P1/P2) + +**Agenda:** + +1. Incident timeline reconstruction +2. What worked well? +3. What could be improved? +4. Action items and owners +5. Documentation updates needed + +### Post-Incident Checklist + +- [ ] Incident fully documented +- [ ] Timeline created with all actions taken +- [ ] Root cause analysis completed +- [ ] Lessons learned documented +- [ ] Security controls reviewed and updated +- [ ] Monitoring/alerting improved +- [ ] Team training needs identified +- [ ] Documentation updated + +### Incident Report Template + +```markdown +## Incident Report: [INCIDENT-YYYY-MM-DD-###] + +**Severity:** P1/P2/P3/P4 +**Status:** Resolved / Under Investigation +**Duration:** [Start Time] to [End Time] + +### Summary +[Brief description of what happened] + +### Timeline +- [HH:MM] - Event detected +- [HH:MM] - Containment initiated +- [HH:MM] - Root cause identified +- [HH:MM] - Recovery completed + +### Impact +- Systems affected: [List] +- Data affected: [Yes/No, details] +- Users affected: [Count/scope] +- Service downtime: [Duration] + +### Root Cause +[Technical explanation of what caused the incident] + +### Actions Taken +1. [Action 1] +2. [Action 2] +3. [Action 3] + +### Lessons Learned +- [Learning 1] +- [Learning 2] + +### Follow-up Actions +| Action | Owner | Due Date | Status | +|--------|-------|----------|--------| +| [Action] | [Name] | [Date] | Open | +``` + +--- + +## Communication Templates + +### Internal Notification (P1/P2) + +``` +SECURITY INCIDENT ALERT + +Severity: [P1/P2] +Time Detected: [YYYY-MM-DD HH:MM UTC] +Status: [Active / Contained / Resolved] + +Summary: +[Brief description] + +Current Actions: +- [Action being taken] + +Next Update: [Time] + +Contact: [Incident Commander Name/Channel] +``` + +### User Communication (If Service Affected) + +``` +Service Notification + +We are currently experiencing [brief issue description]. + +Status: [Investigating / Identified / Monitoring / Resolved] +Started: [Time] +Expected Resolution: [Time or "Under investigation"] + +We apologize for any inconvenience and will provide updates as available. + +Last Updated: [Time] +``` + +### Post-Incident Summary (External) + +``` +Security Incident Summary + +On [Date], we identified and responded to a security incident affecting [scope]. + +What happened: +[Non-technical summary] + +What we did: +[Response actions taken] + +What we're doing to prevent this: +[Improvements being made] + +Was my data affected? +[Clear statement about data impact] + +Questions? +[Contact information] +``` + +--- + +## Emergency Contacts + +Maintain an up-to-date contact list: + +| Role | Contact Method | Escalation Time | +|------|----------------|-----------------| +| Primary On-Call | [Phone/Pager] | Immediate | +| Security Team | [Email/Slack] | < 15 min (P1/P2) | +| System Administrator | [Phone] | < 1 hour | +| Management | [Phone] | P1 only | + +--- + +## Quick Reference Card + +### P1 Critical — Immediate Response + +1. ⏱️ Start timer, document everything +2. 🔒 Isolate: `docker stop charon` +3. 📋 Preserve: `docker logs charon > incident.log` +4. 📞 Notify: Security team, management +5. 🔍 Investigate: Determine scope and root cause +6. 🔧 Recover: Restore from clean backup +7. 📝 Review: Post-incident meeting within 48h + +### P2 High — Urgent Response + +1. 🔒 Block attacker: `cscli decisions add --ip ` +2. 📋 Capture logs before they rotate +3. 📞 Notify: Security team +4. 🔍 Investigate root cause +5. 🔧 Apply fixes +6. 📝 Document and review + +### Key Commands + +```bash +# Block IP immediately +docker exec charon cscli decisions add --ip --duration 720h + +# List all active blocks +docker exec charon cscli decisions list + +# Export logs +docker logs charon > incident-$(date +%s).log 2>&1 + +# Check security status +docker exec charon cscli lapi status +``` + +--- + +## Document Maintenance + +| Version | Date | Author | Changes | +|---------|------|--------|---------| +| 1.0 | 2025-12-21 | Security Team | Initial SIRP creation | + +**Review Schedule:** Quarterly or after any P1/P2 incident + +**Owner:** Security Team + +**Last Reviewed:** 2025-12-21 +``` diff --git a/docs/security.md b/docs/security.md index ba97316e..05da0a0e 100644 --- a/docs/security.md +++ b/docs/security.md @@ -615,6 +615,95 @@ Remove the security lines from `docker-compose.yml` and restart. --- +## TLS Security + +### TLS Version Enforcement + +Charon (via Caddy) enforces a minimum TLS version of 1.2 by default. This prevents TLS downgrade attacks that attempt to force connections to use vulnerable TLS 1.0 or 1.1. + +**What's Protected:** + +- ✅ TLS 1.0/1.1 downgrade attacks +- ✅ BEAST, POODLE, and similar protocol-level attacks +- ✅ Weak cipher suite negotiation + +**HSTS (HTTP Strict Transport Security):** + +Charon sets HSTS headers with: + +- `max-age=31536000` (1 year) +- `includeSubDomains` +- `preload` (for browser preload lists) + +This ensures browsers always use HTTPS after the first visit. + +--- + +## DNS Security + +### Protecting Against DNS Hijacking + +While Charon cannot directly control your DNS resolver, you can protect against DNS hijacking and cache poisoning by configuring your host to use encrypted DNS. + +**Docker Host Configuration (systemd-resolved):** + +```bash +# /etc/systemd/resolved.conf +[Resolve] +DNS=1.1.1.1#cloudflare-dns.com 1.0.0.1#cloudflare-dns.com +DNSOverTLS=yes +``` + +Then restart: `sudo systemctl restart systemd-resolved` + +**Alternative DNS Providers with DoH/DoT:** + +- Cloudflare: `1.1.1.1` / `1.0.0.1` +- Google: `8.8.8.8` / `8.8.4.4` +- Quad9: `9.9.9.9` + +**Additional DNS Protections:** + +1. **DNSSEC**: Enable at your domain registrar to prevent DNS spoofing +2. **CAA Records**: Restrict which Certificate Authorities can issue certificates for your domain + +--- + +## Container Hardening + +### Running Charon with Maximum Security + +For production deployments, apply these Docker security configurations: + +```yaml +services: + charon: + image: ghcr.io/wikid82/charon:latest + read_only: true + tmpfs: + - /tmp:size=100M + - /config:size=50M + - /data/logs:size=100M + cap_drop: + - ALL + cap_add: + - NET_BIND_SERVICE + security_opt: + - no-new-privileges:true + volumes: + - ./data:/data +``` + +**Security Options Explained:** + +- `read_only: true` — Prevents filesystem modifications (defense against malware) +- `cap_drop: ALL` — Removes all Linux capabilities +- `cap_add: NET_BIND_SERVICE` — Only allows binding to ports (required for reverse proxy) +- `no-new-privileges` — Prevents privilege escalation attacks +- `tmpfs` mounts — Provides writable directories for logs and temp files without persistent storage + +--- + ## Common Questions ### "Will this slow down my websites?" From 834f59318dce0fd098c1cd3f251531aee94fd310 Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Sun, 21 Dec 2025 19:23:08 +0000 Subject: [PATCH 6/7] feat: update manual testing guidelines and add test plan for security enhancements --- .github/agents/Manegment.agent.md | 2 +- .../{created => }/issue-365-manual-test-plan.md | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) rename docs/issues/{created => }/issue-365-manual-test-plan.md (93%) diff --git a/.github/agents/Manegment.agent.md b/.github/agents/Manegment.agent.md index f5b81928..7d97e15b 100644 --- a/.github/agents/Manegment.agent.md +++ b/.github/agents/Manegment.agent.md @@ -55,7 +55,7 @@ You are "lazy" in the smartest way possible. You never do what a subordinate can 7. **Phase 7: Closure**: - **Docs**: Call `Docs_Writer`. - - **Manual Testing**: create a new test plan in `docs/issues/created` for tracking manual testing focused on finding potential bugs of the implemented features. + - **Manual Testing**: create a new test plan in `docs/issues/*.md` for tracking manual testing focused on finding potential bugs of the implemented features. - **Final Report**: Summarize the successful subagent runs. - **Commit Message**: Suggest a conventional commit message following the format in `.github/copilot-instructions.md`: - Use `feat:` for new user-facing features diff --git a/docs/issues/created/issue-365-manual-test-plan.md b/docs/issues/issue-365-manual-test-plan.md similarity index 93% rename from docs/issues/created/issue-365-manual-test-plan.md rename to docs/issues/issue-365-manual-test-plan.md index 5d1f272b..034a4be8 100644 --- a/docs/issues/created/issue-365-manual-test-plan.md +++ b/docs/issues/issue-365-manual-test-plan.md @@ -1,3 +1,14 @@ +--- +title: "Issue #365: Additional Security Enhancements - Manual Test Plan" +labels: + - manual-testing + - security + - testing +type: testing +priority: medium +parent_issue: 365 +--- + # Issue #365: Additional Security Enhancements - Manual Test Plan **Issue**: https://github.com/Wikid82/Charon/issues/365 From 1be2892f7cda0529c9cc935d65a383f647644e78 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 23 Dec 2025 01:23:54 -0500 Subject: [PATCH 7/7] Update docs/security-incident-response.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- docs/security-incident-response.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/security-incident-response.md b/docs/security-incident-response.md index 43f63410..6858d3aa 100644 --- a/docs/security-incident-response.md +++ b/docs/security-incident-response.md @@ -1,4 +1,3 @@ -```markdown --- title: Security Incident Response Plan description: Industry-standard incident response procedures for Charon deployments, including detection, containment, recovery, and post-incident review.