-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Additional Security Enhancements (Issue #365) #436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
Wikid82
wants to merge
15
commits into
development
Choose a base branch
from
feature/issue-365-additional-security
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
42fcb0f
Merge pull request #435 from Wikid82/feature/beta-release
Wikid82 7e4b3a4
docs: add planning document for Issue #365 Additional Security
Wikid82 28aa28c
feat: add manual testing guidelines for tracking potential bugs in Cl…
actions-user 8e9766e
feat: update implementation specification for additional security enh…
actions-user 84a8c1f
feat: update execution steps and security scan requirements in QA_Sec…
actions-user 2dfe7ee
feat: add additional security enhancements (Issue #365)
actions-user fcdc941
Merge pull request #438 from Wikid82/feature/issue-365-additional-sec…
Wikid82 834f593
feat: update manual testing guidelines and add test plan for security…
actions-user 66db28e
Merge branch 'development' into feature/issue-365-additional-security
Wikid82 ede0f65
Merge branch 'feature/issue-365-additional-security' into feature/bet…
Wikid82 393260e
Merge pull request #439 from Wikid82/feature/beta-release
Wikid82 4270aa3
Merge branch 'main' into feature/issue-365-additional-security
Wikid82 5cd578b
Merge branch 'development' into feature/issue-365-additional-security
Wikid82 606acb1
Merge branch 'development' into feature/issue-365-additional-security
Wikid82 1be2892
Update docs/security-incident-response.md
Wikid82 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } | ||
| }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.