Merge pull request #439 from Wikid82/feature/beta-release
Feature/beta release
This commit is contained in:
Jeremy
GitHub
@@ -165,6 +165,11 @@ coverage.out
|
||||
*.crdownload
|
||||
*.sarif
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# SBOM artifacts
|
||||
# -----------------------------------------------------------------------------
|
||||
sbom*.json
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# CodeQL & Security Scanning (large, not needed)
|
||||
# -----------------------------------------------------------------------------
|
||||
|
||||
1
.github/agents/Manegment.agent.md
vendored
1
.github/agents/Manegment.agent.md
vendored
@@ -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/*.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
|
||||
|
||||
4
.github/agents/QA_Security.agent.md
vendored
4
.github/agents/QA_Security.agent.md
vendored
@@ -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)
|
||||
|
||||
|
||||
21
.github/workflows/docker-build.yml
vendored
21
.github/workflows/docker-build.yml
vendored
@@ -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: |
|
||||
|
||||
2
.github/workflows/docs-to-issues.yml
vendored
2
.github/workflows/docs-to-issues.yml
vendored
@@ -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)'
|
||||
|
||||
5
.gitignore
vendored
5
.gitignore
vendored
@@ -229,6 +229,11 @@ test-results/local.har
|
||||
# -----------------------------------------------------------------------------
|
||||
/trivy-*.txt
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# SBOM artifacts
|
||||
# -----------------------------------------------------------------------------
|
||||
sbom*.json
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# Docker Overrides (new location)
|
||||
# -----------------------------------------------------------------------------
|
||||
|
||||
@@ -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
|
||||
|
||||
21
backend/internal/util/crypto.go
Normal file
21
backend/internal/util/crypto.go
Normal file
@@ -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
|
||||
}
|
||||
82
backend/internal/util/crypto_test.go
Normal file
82
backend/internal/util/crypto_test.go
Normal file
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -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!
|
||||
|
||||
110
docs/issues/issue-365-manual-test-plan.md
Normal file
110
docs/issues/issue-365-manual-test-plan.md
Normal file
@@ -0,0 +1,110 @@
|
||||
---
|
||||
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
|
||||
**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
|
||||
@@ -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 |
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
## Detailed Analysis by File
|
||||
## Codebase Research Findings
|
||||
|
||||
---
|
||||
### 1. Cookie/Session Implementation
|
||||
|
||||
### 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%
|
||||
|
||||
**Test File:** `testdb_test.go` (exists - 200+ lines)
|
||||
|
||||
#### 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
|
||||
**Assessment**: ✅ **SECURE** - All cookie security attributes properly configured:
|
||||
- `HttpOnly: true` - Prevents XSS access
|
||||
- `Secure: true` (production + HTTPS)
|
||||
- `SameSite: Strict` (HTTPS) / `Lax` (HTTP dev)
|
||||
|
||||
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
|
||||
### 2. Security Headers Implementation
|
||||
|
||||
**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
|
||||
**Location**: [backend/internal/api/middleware/security.go](backend/internal/api/middleware/security.go)
|
||||
|
||||
**Recommendation:** P3 - Lower priority, accept current coverage for test utilities.
|
||||
**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
|
||||
|
||||
**Assessment**: ✅ **COMPREHENSIVE** - All major security headers present.
|
||||
|
||||
### 3. Token Comparison Methods
|
||||
|
||||
**Locations Analyzed**:
|
||||
|
||||
| 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 |
|
||||
|
||||
**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
|
||||
|
||||
### 4. Current Security Documentation
|
||||
|
||||
**Location**: [docs/security.md](docs/security.md)
|
||||
|
||||
**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)
|
||||
|
||||
### 5. CI/CD Pipeline Analysis
|
||||
|
||||
**Location**: [.github/workflows/docker-build.yml](.github/workflows/docker-build.yml)
|
||||
|
||||
**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)
|
||||
|
||||
**Best Integration Point**: After `build-and-push` step, before Trivy scan (around line 130)
|
||||
|
||||
### 6. Dockerfile Security Analysis
|
||||
|
||||
**Location**: [Dockerfile](Dockerfile)
|
||||
|
||||
**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)
|
||||
|
||||
---
|
||||
|
||||
### 2. `backend/internal/api/handlers/proxy_host_handler.go` (25 Missing, 4 Partials)
|
||||
## Phase 1: Documentation Enhancements (Estimated: 1-2 hours)
|
||||
|
||||
**File Purpose:** CRUD operations for proxy hosts including bulk security header updates.
|
||||
### 1.1 TLS Downgrade Attack Documentation
|
||||
|
||||
**Current Coverage:** 75.00%
|
||||
**File**: `docs/security.md`
|
||||
|
||||
**Test Files:**
|
||||
- `proxy_host_handler_test.go`
|
||||
- `proxy_host_handler_security_headers_test.go`
|
||||
**Add Section**:
|
||||
```markdown
|
||||
## TLS Security
|
||||
|
||||
#### Uncovered Code Paths (New in PR #434)
|
||||
### TLS Version Enforcement
|
||||
|
||||
| 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 |
|
||||
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.
|
||||
|
||||
#### Test Scenarios to Add
|
||||
**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)
|
||||
```
|
||||
|
||||
**Complexity**: Low | **Dependencies**: None
|
||||
|
||||
---
|
||||
|
||||
### 1.2 DNS Hijacking Documentation
|
||||
|
||||
**File**: `docs/security.md`
|
||||
|
||||
**Add Section**:
|
||||
```markdown
|
||||
## DNS Security
|
||||
|
||||
### Protecting Against DNS Hijacking
|
||||
|
||||
Configure your upstream resolver to use DNS-over-HTTPS (DoH) or DNS-over-TLS (DoT):
|
||||
|
||||
**Docker Host Configuration:**
|
||||
```bash
|
||||
# /etc/systemd/resolved.conf
|
||||
[Resolve]
|
||||
DNS=1.1.1.1#cloudflare-dns.com
|
||||
DNSOverTLS=yes
|
||||
```
|
||||
|
||||
**Additional Protections:**
|
||||
1. **DNSSEC**: Ensure your domain registrar supports DNSSEC
|
||||
2. **CAA Records**: Restrict which CAs can issue certs for your domain
|
||||
```
|
||||
|
||||
**Complexity**: Low | **Dependencies**: None
|
||||
|
||||
---
|
||||
|
||||
### 1.3 Security Incident Response Plan
|
||||
|
||||
**New File**: `docs/security-incident-response.md`
|
||||
|
||||
**Content**: Incident classification, detection, containment, recovery procedures
|
||||
|
||||
**Complexity**: Low | **Dependencies**: None
|
||||
|
||||
---
|
||||
|
||||
### 1.4 Security Update Notifications
|
||||
|
||||
**Files**: `docs/getting-started.md`, `docs/security.md`
|
||||
|
||||
**Add**: GitHub Watch instructions, Watchtower/Diun configuration examples
|
||||
|
||||
**Complexity**: Low | **Dependencies**: None
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Code Changes (Estimated: 4-6 hours)
|
||||
|
||||
### 2.1 SBOM Generation
|
||||
|
||||
**File**: `.github/workflows/docker-build.yml`
|
||||
|
||||
**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
|
||||
|
||||
- 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
|
||||
```
|
||||
|
||||
**Additional Files**:
|
||||
- `.gitignore`: Add `sbom*.json`
|
||||
- `.dockerignore`: Add `sbom*.json`
|
||||
|
||||
**Complexity**: Medium | **Dependencies**: None
|
||||
|
||||
---
|
||||
|
||||
### 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
|
||||
```
|
||||
|
||||
**Complexity**: Medium | **Dependencies**: Testing with read-only FS
|
||||
|
||||
---
|
||||
|
||||
### 2.3 Constant-Time Token Comparison
|
||||
|
||||
**New File**: `backend/internal/util/crypto.go`
|
||||
```go
|
||||
// File: backend/internal/api/handlers/proxy_host_handler_update_test.go
|
||||
package util
|
||||
|
||||
func TestProxyHostUpdate_EnableStandardHeaders_Null(t *testing.T) {
|
||||
// Create host, then update with enable_standard_headers: null
|
||||
// Verify host.EnableStandardHeaders becomes nil
|
||||
}
|
||||
import "crypto/subtle"
|
||||
|
||||
func TestProxyHostUpdate_EnableStandardHeaders_True(t *testing.T) {
|
||||
// Create host, then update with enable_standard_headers: true
|
||||
// Verify host.EnableStandardHeaders is pointer to true
|
||||
}
|
||||
|
||||
func TestProxyHostUpdate_EnableStandardHeaders_False(t *testing.T) {
|
||||
// Create host, then update with enable_standard_headers: false
|
||||
// Verify host.EnableStandardHeaders is pointer to false
|
||||
}
|
||||
|
||||
func TestProxyHostUpdate_ForwardAuthEnabled(t *testing.T) {
|
||||
// Create host with forward_auth_enabled: false
|
||||
// Update to forward_auth_enabled: true
|
||||
// Verify change persisted
|
||||
}
|
||||
|
||||
func TestProxyHostUpdate_WAFDisabled(t *testing.T) {
|
||||
// Create host with waf_disabled: false
|
||||
// Update to waf_disabled: true
|
||||
// Verify change persisted
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
|
||||
func TestProxyHostUpdate_SecurityHeaderProfileID_NegativeFloat(t *testing.T) {
|
||||
// Create host
|
||||
// Update with security_header_profile_id: -1.0 (float64)
|
||||
// Expect 400 Bad Request
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
func TestProxyHostUpdate_SecurityHeaderProfileID_InvalidString(t *testing.T) {
|
||||
// Create host
|
||||
// Update with security_header_profile_id: "not-a-number"
|
||||
// Expect 400 Bad Request
|
||||
}
|
||||
|
||||
func TestProxyHostUpdate_SecurityHeaderProfileID_UnsupportedType(t *testing.T) {
|
||||
// Create host
|
||||
// Send security_header_profile_id as boolean (true) or array
|
||||
// Expect 400 Bad Request
|
||||
}
|
||||
|
||||
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
|
||||
// ConstantTimeCompare compares two strings in constant time.
|
||||
func ConstantTimeCompare(a, b string) bool {
|
||||
return subtle.ConstantTimeCompare([]byte(a), []byte(b)) == 1
|
||||
}
|
||||
```
|
||||
|
||||
#### Recommended Actions
|
||||
**New Test File**: `backend/internal/util/crypto_test.go`
|
||||
|
||||
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%
|
||||
**Modify**: `backend/internal/api/handlers/user_handler.go` - Use constant-time comparison in AcceptInvite
|
||||
|
||||
**Complexity**: Medium | **Dependencies**: None
|
||||
|
||||
---
|
||||
|
||||
### 3. `backend/internal/api/handlers/security_headers_handler.go` (8 Missing, 4 Partials)
|
||||
## Phase 3: Testing & Validation (Estimated: 2-3 hours)
|
||||
|
||||
**File Purpose:** CRUD for security header profiles, presets, CSP validation.
|
||||
### Required Tests
|
||||
|
||||
**Current Coverage:** 93.75%
|
||||
| File | Purpose |
|
||||
|------|---------|
|
||||
| `backend/internal/util/crypto_test.go` | Constant-time comparison tests + benchmarks |
|
||||
| Integration test additions | Security header verification |
|
||||
|
||||
**Test File:** `security_headers_handler_test.go` (extensive - 500+ lines)
|
||||
### Coverage Requirements
|
||||
|
||||
#### Uncovered Code Paths
|
||||
|
||||
| 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` |
|
||||
|
||||
#### Test Scenarios to Add
|
||||
|
||||
```go
|
||||
// File: backend/internal/api/handlers/security_headers_handler_coverage_test.go
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
```
|
||||
|
||||
**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
|
||||
- All new code must achieve 85% coverage
|
||||
- Run: `scripts/go-test-coverage.sh`
|
||||
|
||||
---
|
||||
|
||||
### 4. `backend/internal/api/handlers/test_helpers.go` (2 Missing)
|
||||
## File Change Summary
|
||||
|
||||
**File Purpose:** Polling helpers for test synchronization (`waitForCondition`).
|
||||
### Files to Create
|
||||
|
||||
**Current Coverage:** 87.50%
|
||||
| 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 |
|
||||
|
||||
**Test File:** `test_helpers_test.go` (exists)
|
||||
### Files to Modify
|
||||
|
||||
#### Uncovered Code Paths
|
||||
| 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 |
|
||||
|
||||
| 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 |
|
||||
### Files Verified Secure (No Changes)
|
||||
|
||||
#### Analysis
|
||||
|
||||
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
|
||||
|
||||
**Current tests already cover:**
|
||||
- `TestWaitForCondition_PassesImmediately`
|
||||
- `TestWaitForCondition_PassesAfterIterations`
|
||||
- `TestWaitForCondition_Timeout` (uses mockTestingT)
|
||||
- `TestWaitForConditionWithInterval_*` variants
|
||||
|
||||
#### Recommended Actions
|
||||
|
||||
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
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
### 5. `backend/internal/api/routes/routes.go` (1 Missing, 1 Partial)
|
||||
## Out of Scope (Future Issues)
|
||||
|
||||
**File Purpose:** API route registration and middleware wiring.
|
||||
|
||||
**Current Coverage:** 66.66% (but only 1 new line missing)
|
||||
|
||||
**Test File:** `routes_test.go` (exists)
|
||||
|
||||
#### Uncovered Code Paths
|
||||
|
||||
| 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.
|
||||
|
||||
#### Test Scenarios to Add
|
||||
|
||||
```go
|
||||
// File: backend/internal/api/routes/routes_security_headers_test.go
|
||||
|
||||
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
|
||||
|
||||
---
|
||||
|
||||
### 6. `backend/internal/caddy/config.go` (1 Missing, 1 Partial)
|
||||
|
||||
**File Purpose:** Caddy JSON configuration generation.
|
||||
|
||||
**Current Coverage:** 98.82% (excellent)
|
||||
|
||||
**Test Files:** Multiple test files `config_security_headers_test.go`
|
||||
|
||||
#### Uncovered Code Path
|
||||
|
||||
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
|
||||
|
||||
#### Recommended Actions
|
||||
|
||||
1. **Review coverage report details** to identify exact line
|
||||
2. **Likely already covered** by `TestBuildSecurityHeadersHandler_APIFriendlyPreset`
|
||||
3. **Estimated effort:** 15 minutes to verify
|
||||
|
||||
---
|
||||
|
||||
### 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
|
||||
|
||||
| Lines | Function | Issue | Solution |
|
||||
|-------|----------|-------|----------|
|
||||
| ~67 | `Delete()` | ID=0 validation check | Already tested |
|
||||
|
||||
#### Analysis
|
||||
|
||||
Looking at the test file, `TestCertificateHandler_Delete_InvalidID` tests the "invalid" ID case but may not specifically test ID=0.
|
||||
|
||||
```go
|
||||
// Validate ID range
|
||||
if id == 0 {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid id"})
|
||||
return
|
||||
}
|
||||
```
|
||||
|
||||
#### Test Scenarios to Add
|
||||
|
||||
```go
|
||||
func TestCertificateHandler_Delete_ZeroID(t *testing.T) {
|
||||
// DELETE /api/certificates/0
|
||||
// Expect 400 Bad Request with "invalid id" error
|
||||
}
|
||||
```
|
||||
|
||||
#### Recommended Actions
|
||||
|
||||
1. **Add single test for ID=0 case**
|
||||
2. **Estimated effort:** 10 minutes
|
||||
|
||||
---
|
||||
|
||||
## 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** |
|
||||
|
||||
---
|
||||
|
||||
## Test File Locations
|
||||
|
||||
### New Test Files to Create
|
||||
|
||||
1. `backend/internal/api/handlers/proxy_host_handler_update_test.go` - Update field coverage
|
||||
|
||||
### Existing Test Files to Extend
|
||||
|
||||
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
|
||||
|
||||
---
|
||||
|
||||
## Dependencies Between Tests
|
||||
|
||||
```
|
||||
None identified - all tests can be implemented independently
|
||||
```
|
||||
Per Issue #365:
|
||||
- ❌ Certificate Transparency Log Monitoring
|
||||
- ❌ MFA via Authentik
|
||||
- ❌ SSO for Charon admin
|
||||
- ❌ Audit logging for GDPR/SOC2
|
||||
|
||||
---
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
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
|
||||
- [ ] 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
|
||||
|
||||
---
|
||||
|
||||
## Mock Requirements
|
||||
|
||||
### For proxy_host_handler.go Tests
|
||||
|
||||
- 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)
|
||||
|
||||
### For security_headers_handler.go Tests
|
||||
|
||||
- Same as above
|
||||
- Close DB connection to simulate errors
|
||||
|
||||
### For certificate_handler.go Tests
|
||||
|
||||
- Use existing test setup patterns
|
||||
- No mocks needed for ID=0 test
|
||||
|
||||
---
|
||||
|
||||
## 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`)
|
||||
|
||||
**Total Estimated Effort:** ~4 hours for all improvements
|
||||
|
||||
---
|
||||
|
||||
**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
|
||||
|
||||
@@ -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)
|
||||
|
||||
**Failed Test Details:**
|
||||
|
||||
- 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 ✅
|
||||
|
||||
- **Tool:** TypeScript Check
|
||||
- **Status:** ✅ PASSED
|
||||
- **Result:** No type errors detected
|
||||
|
||||
### 3. Pre-commit Hooks ✅
|
||||
|
||||
- **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)
|
||||
|
||||
### 4. Security Scans ✅
|
||||
|
||||
#### Go Vulnerability Check
|
||||
|
||||
- **Tool:** govulncheck
|
||||
- **Status:** ✅ PASSED
|
||||
- **Result:** No vulnerabilities found
|
||||
|
||||
#### 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
|
||||
|
||||
### 5. Linting ✅
|
||||
|
||||
#### Go Vet
|
||||
|
||||
- **Status:** ✅ PASSED
|
||||
- **Result:** No issues detected
|
||||
|
||||
#### Frontend Lint
|
||||
|
||||
- **Status:** ✅ PASSED
|
||||
- **Tool:** ESLint
|
||||
- **Result:** No issues detected
|
||||
|
||||
#### Markdownlint
|
||||
|
||||
- **Status:** ⚠️ FIXED
|
||||
- **Initial Issues:** 6 line-length violations in VERSION.md and WEBSOCKET_FIX_SUMMARY.md
|
||||
- **Action:** Ran auto-fix
|
||||
- **Final Status:** ✅ PASSED
|
||||
|
||||
### 6. Integration Testing ✅
|
||||
|
||||
#### Docker Container Build
|
||||
|
||||
- **Status:** ✅ PASSED
|
||||
- **Build Time:** 303.7s (full rebuild with --no-cache)
|
||||
- **Image Size:** Optimized
|
||||
- **Container Status:** Running successfully
|
||||
|
||||
#### Caddy Configuration Verification
|
||||
|
||||
- **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
|
||||
|
||||
#### Live Proxy Traffic Analysis
|
||||
|
||||
- **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
|
||||
|
||||
#### Functional Testing
|
||||
|
||||
**Test Scenario:** Toggle "Enable Standard Proxy Headers" on existing proxy hosts
|
||||
|
||||
- **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
|
||||
|
||||
---
|
||||
|
||||
## Issues Found
|
||||
|
||||
### 1. Frontend Test Flakiness (RESOLVED)
|
||||
|
||||
- **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
|
||||
|
||||
### 2. Markdown Linting (RESOLVED)
|
||||
|
||||
- **Severity:** TRIVIAL
|
||||
- **Files:** VERSION.md, WEBSOCKET_FIX_SUMMARY.md
|
||||
- **Issue:** Line length > 120 characters
|
||||
- **Resolution:** Auto-fix applied
|
||||
- **Status:** ✅ RESOLVED
|
||||
|
||||
---
|
||||
|
||||
## Security Analysis
|
||||
|
||||
### Threat Model
|
||||
|
||||
**Original Issue:**
|
||||
|
||||
- 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
|
||||
|
||||
- **Go Dependencies:** ✅ CLEAN (0 vulnerabilities)
|
||||
- **Container Base Image:** ✅ CLEAN (0 high/critical)
|
||||
- **Secrets Detection:** ✅ CLEAN (test keys only, expected)
|
||||
|
||||
---
|
||||
|
||||
## Performance Impact
|
||||
|
||||
- **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)
|
||||
|
||||
---
|
||||
|
||||
## Compliance Checklist
|
||||
|
||||
- [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)
|
||||
|
||||
---
|
||||
|
||||
## Recommendations
|
||||
|
||||
### Immediate Actions
|
||||
|
||||
None required. All issues resolved.
|
||||
|
||||
### Future Improvements
|
||||
|
||||
1. **Test Stability**
|
||||
- Consider adding retry logic for concurrent operation tests
|
||||
- Use more deterministic wait conditions instead of timeouts
|
||||
|
||||
2. **CI/CD Enhancement**
|
||||
- Add automated proxy host CRUD tests to CI pipeline
|
||||
- Include Caddy config validation in pre-deploy checks
|
||||
|
||||
3. **Monitoring**
|
||||
- Add alerting for 500 errors on proxy host API endpoints
|
||||
- Track Caddy config reload success/failure rates
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
The Caddy `trusted_proxies` fix has been thoroughly verified and is production-ready. All quality gates have been passed:
|
||||
|
||||
- ✅ Code coverage maintained
|
||||
- ✅ Type safety enforced
|
||||
- ✅ Security scans clean
|
||||
- ✅ Linting passed
|
||||
- ✅ Integration tests successful
|
||||
- ✅ Live container verification confirmed
|
||||
|
||||
**The 500 error when saving proxy hosts with "Enable Standard Proxy Headers" toggled has been resolved.
|
||||
The fix is validated and safe for deployment.**
|
||||
|
||||
---
|
||||
|
||||
## 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": [...]
|
||||
}
|
||||
```bash
|
||||
cd backend && go test -coverprofile=coverage.out ./... && go tool cover -func=coverage.out
|
||||
```
|
||||
|
||||
**Note:** No `trusted_proxies` field in reverse_proxy handler (correct).
|
||||
### Results
|
||||
|
||||
#### Container Health
|
||||
| Metric | Value | Threshold | Status |
|
||||
|--------|-------|-----------|--------|
|
||||
| Total Coverage | **85.3%** | 85% | ✅ PASS |
|
||||
| Test Failures | **0** | 0 | ✅ PASS |
|
||||
|
||||
```json
|
||||
{
|
||||
"build_time": "unknown",
|
||||
"git_commit": "unknown",
|
||||
"internal_ip": "172.20.0.9",
|
||||
"service": "Charon",
|
||||
"status": "ok",
|
||||
"version": "dev"
|
||||
}
|
||||
```
|
||||
### 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% |
|
||||
|
||||
---
|
||||
|
||||
**Audited by:** QA_Security Agent - The Auditor
|
||||
**Signature:** ✅ APPROVED FOR PRODUCTION
|
||||
## 2. Frontend Coverage Tests
|
||||
|
||||
### Command Executed
|
||||
|
||||
```bash
|
||||
cd frontend && npm run test:coverage
|
||||
```
|
||||
|
||||
### Results
|
||||
|
||||
| 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 |
|
||||
|
||||
### Duration
|
||||
|
||||
- Total Duration: 108.12s
|
||||
|
||||
---
|
||||
|
||||
## 3. TypeScript Type Safety Check
|
||||
|
||||
### Command Executed
|
||||
|
||||
```bash
|
||||
cd frontend && npm run type-check
|
||||
```
|
||||
|
||||
### Results
|
||||
|
||||
| Metric | Value | Threshold | Status |
|
||||
|--------|-------|-----------|--------|
|
||||
| Type Errors | **0** | 0 | ✅ PASS |
|
||||
|
||||
---
|
||||
|
||||
## 4. Pre-commit Hooks
|
||||
|
||||
### Command Executed
|
||||
|
||||
```bash
|
||||
pre-commit run --all-files
|
||||
```
|
||||
|
||||
### 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 |
|
||||
|
||||
---
|
||||
|
||||
## 5. Security Scans
|
||||
|
||||
### Trivy Filesystem Scan
|
||||
|
||||
#### Command Executed
|
||||
|
||||
```bash
|
||||
docker run --rm -v "$(pwd):/app:ro" -w /app aquasec/trivy:latest fs \
|
||||
--scanners vuln,misconfig --severity HIGH,CRITICAL .
|
||||
```
|
||||
|
||||
#### Results
|
||||
|
||||
| Target | Type | Vulnerabilities | Misconfigurations |
|
||||
|--------|------|-----------------|-------------------|
|
||||
| package-lock.json | npm | **0** | - |
|
||||
|
||||
| Severity | Count | Threshold | Status |
|
||||
|----------|-------|-----------|--------|
|
||||
| CRITICAL | **0** | 0 | ✅ PASS |
|
||||
| HIGH | **0** | 0 | ✅ PASS |
|
||||
|
||||
---
|
||||
|
||||
## 6. Go Vulnerability Check
|
||||
|
||||
### Command Executed
|
||||
|
||||
```bash
|
||||
govulncheck ./...
|
||||
```
|
||||
|
||||
### Results
|
||||
|
||||
| Metric | Value | Threshold | Status |
|
||||
|--------|-------|-----------|--------|
|
||||
| Known Vulnerabilities | **0** | 0 | ✅ PASS |
|
||||
|
||||
---
|
||||
|
||||
## 7. Crypto Utility Tests (Issue #365 Specific)
|
||||
|
||||
### Command Executed
|
||||
|
||||
```bash
|
||||
cd backend && go test -v -cover ./internal/util/...
|
||||
```
|
||||
|
||||
### 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%** |
|
||||
|
||||
---
|
||||
|
||||
## 8. Files Changed in Issue #365
|
||||
|
||||
| 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 |
|
||||
|
||||
---
|
||||
|
||||
## 9. Issues Found
|
||||
|
||||
**None.** All tests pass, coverage thresholds are met, and no security vulnerabilities were detected.
|
||||
|
||||
---
|
||||
|
||||
## 10. Summary
|
||||
|
||||
### Pass/Fail Counts
|
||||
|
||||
| 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 |
|
||||
|
||||
### Coverage Percentages
|
||||
|
||||
| Component | Coverage | Threshold | Delta |
|
||||
|-----------|----------|-----------|-------|
|
||||
| Backend | 85.3% | 85% | +0.3% |
|
||||
| Frontend | 87.59% | 85% | +2.59% |
|
||||
|
||||
### Security Scan Results
|
||||
|
||||
| Scanner | Critical | High | Medium | Low |
|
||||
|---------|----------|------|--------|-----|
|
||||
| Trivy | 0 | 0 | - | - |
|
||||
| govulncheck | 0 | 0 | 0 | 0 |
|
||||
|
||||
---
|
||||
|
||||
## Final Verdict
|
||||
|
||||
# ✅ PASS
|
||||
|
||||
All QA and security testing requirements have been met for Issue #365 - Additional Security Enhancements:
|
||||
|
||||
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
|
||||
|
||||
**The branch `feature/issue-365-additional-security` is ready for merge.**
|
||||
|
||||
---
|
||||
|
||||
*Report generated: 2025-12-21*
|
||||
*QA Agent: QA_Security*
|
||||
|
||||
400
docs/security-incident-response.md
Normal file
400
docs/security-incident-response.md
Normal file
@@ -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 <ATTACKER_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 <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 <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
|
||||
```
|
||||
@@ -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?"
|
||||
|
||||
Reference in New Issue
Block a user