mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 20:01:31 +00:00
b911646e53efbb75c4a8399b6558762d25337ff4
8 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 shankar0123
|
92afe359e9 |
Bundle O (Coverage Audit Closure): test hygiene + FSM coverage tables — M-004 + M-005 + M-006 closed
Three deliverables shipped:
O.1 (M-004): t.Skip rationale audit — 65 sites, 0 orphans
O.2 (M-005): fuzz targets 9 -> 11 (+ParseNamedAPIKeys, +SanitizeForShell)
O.3 (M-006): FSM coverage tables (5 FSMs catalogued)
O.1 — t.Skip rationale audit:
Inventoried all 65 t.Skip sites in the repo (audit-time estimate
was 41; count grew via Bundle 0.7 keymem tests + Bundle M.Cloud
httptest skips). Every site carries a valid rationale —
none are orphan. Categories: OS-specific (~30), root-only (~5),
external-dep (Docker/PostgreSQL/browser/Vault/DigiCert ~15),
manual-test markers (Parts 23/24/55/56 — 4 from Bundle I),
-short mode (~6), state-dependent (~5). All class (a) per Bundle
O's classification. No edits required; the existing M-009 CI guard
catches new orphan skips going forward.
O.2 — Fuzz target additions:
internal/config/config_fuzz_test.go::FuzzParseNamedAPIKeys
Pins the CERTCTL_API_KEYS_NAMED env-var parser (dual-key
rotation, Bundle G / L-004). 16 seed inputs covering happy-path,
rotation pair, degenerate, whitespace-padded, wrong-case admin,
4-segment, adversarial chars in name, long inputs.
internal/validation/command_fuzz_test.go::FuzzSanitizeForShell
Appended to existing fuzz file. Asserts no panic + output begins+
ends with single-quote. 17 seed inputs covering plain, whitespace,
embedded quotes/backticks/dollars, newlines, NULs, shell-metachar
injection, unicode, 100x apostrophe stress, 10000x length stress.
Total fuzz-target count: 9 -> 11 (per grep verification)
O.3 — FSM coverage tables (NEW: tables/fsm-coverage.md):
Job: legal 92%, illegal 100% ✓ Existential gate
Certificate: legal 93%, illegal 100% ✓ Existential gate
Agent: legal 75%, illegal 100% △ slight Degraded gap
Notification: legal 86%, illegal 100% ✓
Health-check: legal 100% (recompute-on-tick model) ✓
4/5 FSMs meet the ≥80% legal + 100% illegal gate.
Agent's Degraded transitions are the lone gap; tracked as
M-006-extended.
Verification:
go vet ./internal/config/... ./internal/validation/... clean
go test -short -count=1 PASS
grep -rE 'func Fuzz[A-Z]' --include='*_test.go' internal/ | wc -l == 11
Audit deliverables:
gap-backlog.md: M-004 + M-005 + M-006 strikethroughs + Bundle O
closure-log entry covering all 3 sub-deliverables
closure-plan.md: Bundle O [x] closed
tables/fsm-coverage.md: NEW (5 FSMs catalogued)
CHANGELOG.md: [unreleased] Bundle O entry
|
||
|
shankar0123
|
521802f824 |
Bundle 9 follow-up: ST1018 ESC sweep + make verify pre-commit gate
CI on the bundle-9 merge (run #24962543332) failed golangci-lint with 16 staticcheck ST1018 'string literal contains the Unicode format character U+202X, consider using the \u202X escape sequence' hits — across the two test files we added (internal/validation/unicode_test.go + internal/connector/issuer/local/bundle9_coverage_test.go). Mechanical sweep, byte-identical at runtime: internal/validation/unicode_test.go (13 + 1 hits cleared) RTL/LTR overrides U+202A..U+202E + U+2066..U+2069 (lines 39-47) zero-width U+200B..U+200D + U+2060 (lines 67-70) additional U+202E in TestValidateUnicodeSafe_ErrorMentionsByteOffset internal/connector/issuer/local/bundle9_coverage_test.go (3 hits) U+202E in TestValidateCSRUnicode_RejectsDNSNameRTL U+200B in TestValidateCSRUnicode_RejectsEmailZeroWidth U+202E in TestValidateCSRUnicode_RejectsAdditionalSAN The strings now use Go \uXXXX escape sequences. Identical UTF-8 bytes hit ValidateUnicodeSafe at runtime — every test passes unchanged locally. The file-header comment in unicode_test.go that promised this convention is now actually honored. Verification: staticcheck -checks=ST1018 returns clean across the two packages. go test -count=1 -short still green. Pre-commit gate added to prevent recurrence: Makefile: new 'verify' aggregate target runs gofmt + go vet + golangci-lint run + go test -short — same set CI enforces. Run 'make verify' before every commit going forward. cowork/CLAUDE.md: new 'Pre-commit verification gate' paragraph in Operating Rules. Documents make verify as the canonical gate; explains WHY (Bundle-9 shipped green-on-vet / red-on-CI because ST1018 only fires under golangci-lint's staticcheck, not vet); documents the staticcheck-only fallback for disk-constrained sandboxes. This commit changes only: - 2 test source files (\uXXXX escapes, no behavior change) - Makefile (1 new target, 1 .PHONY entry, 1 help line) - cowork/CLAUDE.md (1 new operating-rule paragraph) |
||
|
shankar0123
|
1dcc7455cd |
Bundle 9: Local-issuer hardening — 5 findings closed + 1 partial
Closes H-010 + L-002 + L-003 + L-012 + L-014 from
comprehensive-audit-2026-04-25; partial-closes M-028 (the local.go:682
elliptic.Marshal site only).
H-010 (CWE-1257) — local-issuer coverage 68.3% -> 86.7%
* internal/connector/issuer/local/bundle9_coverage_test.go (NEW)
Adds ~30 subtests across CSR-acceptance failure paths, parsePrivateKey
four-format coverage, resolveEKUsAndKeyUsage all-EKU + fallback,
hashPublicKey RSA + ECDSA P-256/P-384/P-521 + unsupported curve,
ecdsaToECDH byte-identical round-trip pin, loadCAFromDisk
expired/non-CA/missing/happy, validateCSRUnicode all rejection arms,
marshalPrivateKeyAndZeroize / ensureKeyDirSecure all branches,
ValidateConfig 5 arms, MaxTTLSeconds cap.
* .github/workflows/ci.yml — flips local-issuer floor 60% -> 85% hard
with explicit "add tests, do not lower the gate" comment.
L-002 (CWE-226) — agent + local-CA private-key zeroization
* internal/connector/issuer/local/keymem.go (NEW)
* cmd/agent/keymem.go (NEW)
marshalPrivateKeyAndZeroize wraps x509.MarshalECPrivateKey with
defer clear(der). Agent additionally defer clear(privKeyPEM) on the
encoded buffer. Bounds heap-resident exposure of the private scalar
to the duration of PEM-encode + os.WriteFile.
L-003 (CWE-732) — 0700 key-directory hardening
* internal/connector/issuer/local/keystore.go (NEW)
* cmd/agent/keymem.go (NEW)
ensureKeyDirSecure / ensureAgentKeyDirSecure create dir tree at 0700,
accept owner-only modes, chmod-tighten permissive leaves with
re-stat verification, refuse empty/root/dot. Wired ahead of every
os.WriteFile(keyPath, ..., 0600) site in cmd/agent/main.go.
L-012 (CWE-1007 + CWE-176) — Unicode safety in CN/SAN
* internal/validation/unicode.go (NEW)
* internal/validation/unicode_test.go (NEW, 8 test functions)
ValidateUnicodeSafe rejects RTL/LTR overrides U+202A..U+202E +
U+2066..U+2069, zero-width U+200B..U+200D + U+2060 + U+FEFF,
control chars <0x20 + 0x7F..0x9F, and per-DNS-label
Latin+non-Latin-letter mixes (Cyrillic-а-in-apple homograph).
Pure-IDN labels allowed. Errors cite codepoint + byte offset.
Wired into IssueCertificate + RenewCertificate via
validateCSRUnicode covering CSR Subject CommonName + DNSNames +
EmailAddresses + request-side additional SANs.
L-014 — CA-key-in-process threat-model documentation
* internal/connector/issuer/local/local.go file-header doc comment
Documents what the bundled defense-in-depth measures DO and DO NOT
protect against; directs operators with stricter requirements to
HSM/PKCS#11/cloud-KMS-backed signing (V3 Pro KMS-issuance roadmap
entry as the source-of-truth fix).
M-028 (CWE-477) PARTIAL — 1 of 6 SA1019 sites
* internal/connector/issuer/local/local.go::ecdsaToECDH (NEW helper)
Replaces deprecated elliptic.Marshal(k.Curve, k.X, k.Y) inside
hashPublicKey with crypto/ecdh.PublicKey.Bytes(). Dispatches on
Curve.Params().Name to avoid importing crypto/elliptic for sentinel
comparisons. Supports P-256/P-384/P-521; P-224 returns
unsupported-curve error and the caller falls back to a stable X+Y
big.Int.Bytes() hash (so SKI generation never panics).
* TestHashPublicKey_ECDSA_RoundTripPin — byte-identical regression
oracle that pins the new output to the legacy elliptic.Marshal
output across all three supported curves (with explicit
//nolint:staticcheck on the SA1019 reference). Migration cannot
silently change the SubjectKeyId of every previously-issued cert.
* 5 SA1019 sites still open (test-file middleware.NewAuth × 3 +
scep.go csr.Attributes).
Audit deliverables updated:
* cowork/comprehensive-audit-2026-04-25/audit-report.md — score
20/55 -> 25/55 closed (High 6/9 -> 7/9; Low 4/19 -> 8/19).
* cowork/comprehensive-audit-2026-04-25/findings.yaml — H-010 +
L-002 + L-003 + L-012 + L-014 status open -> closed; M-028 status
open -> partial_closed; closure notes cite the Bundle-9 mechanism.
* certctl/CHANGELOG.md — Bundle-9 section under [unreleased].
|
||
|
shankar0123
|
119986fa7e |
security: add SSRF defence-in-depth for webhook notifier (fixes H-4)
The webhook notifier would previously accept any operator-configured URL
and hand it to http.Client without validation. That exposed two
SSRF classes (CWE-918):
* Reserved-address reachability — a misconfigured or adversarial
webhook URL pointing at 127.0.0.1, ::1, 169.254.169.254 (cloud
metadata), or 0.0.0.0 would succeed, exfiltrating request bodies
to local services or leaking short-lived cloud credentials.
* DNS rebinding — a hostname resolving to a public IP at validation
time and to a reserved IP at dial time would bypass any
URL-string-only check.
Fix installs two independent layers:
* validation.ValidateSafeURL runs at config-ingest time and before
every outbound POST. It rejects non-HTTP(S) schemes, empty hosts,
and literal reserved-IP hosts with a clear operator-facing error.
This is a fast early diagnostic.
* validation.SafeHTTPDialContext is installed on the webhook
http.Transport. It re-resolves the host at dial time, rejects any
resolved address whose address lies in a reserved range (loopback,
link-local, multicast, broadcast, unspecified, IPv6
link-local/multicast), and pins the resolved IP into the final
dial address so the TLS handshake targets the exact IP the guard
approved. This is the authoritative, TOCTOU-safe defence against
DNS rebinding.
The two layers are complementary — validateURL fails fast on obvious
misconfiguration; SafeHTTPDialContext fails closed when DNS changes
between validation and dial.
The existing unexported isReservedIP helper in
internal/service/network_scan.go is extracted into
internal/validation.IsReservedIP with byte-identical behaviour so the
webhook notifier and the network scanner share a single authoritative
reserved-address list. RFC 1918 ranges remain intentionally allowed
(certctl's self-hosted design). Broader unspecified / IPv6 link-local
coverage lives only in the stricter dial-time policy, where it belongs
for outbound HTTP egress.
Test seam: Connector gains an unexported validateURL func field and a
same-package newForTest constructor that installs a permissive
validator and the stdlib default transport. Production callers cannot
reach this constructor because it is unexported; only same-package
tests (package webhook) can use it. Same-package happy-path tests call
newForTest so they can point at httptest loopback servers without
being blocked by the production guard. The four SSRF-rejection tests
that verify the guard itself still call New so they exercise the real,
strict validator. This keeps the production SSRF defence
unconditionally on in real code while preserving legitimate unit-test
coverage.
Tests
-----
* internal/validation/ssrf_test.go (new) — 16-subtest pin on
IsReservedIP that is byte-identical with the original network-
scanner behaviour; ValidateSafeURL accept/reject matrix covering
HTTPS/HTTP, reserved-literal IPv4/IPv6, dangerous schemes
(file/gopher/ftp/javascript/data/ldap/dict/jar), missing hosts,
and malformed inputs; SafeHTTPDialContext rejects literal reserved
addresses and hosts resolving to reserved addresses (DNS-rebinding
coverage via localhost).
* internal/connector/notifier/webhook/webhook_test.go — happy-path
tests switched to newForTest; production-guard SSRF-rejection
tests (TestValidateConfig_RejectsReservedURLs,
TestValidateConfig_RejectsDangerousScheme,
TestPostWebhook_RejectsReservedURL,
TestPostWebhook_RejectsDangerousScheme) continue to call New so
they exercise the unconditionally-installed production validator.
Wire-format invariants preserved
--------------------------------
* Outbound HTTP request shape (method, headers, body, HMAC
signature) unchanged.
* network_scan.go behaviour unchanged — validation.IsReservedIP is
byte-identical with the deleted helper.
* RFC 1918 (10/8, 172.16/12, 192.168/16) remain allowed for both
outbound webhook and CIDR expansion, matching the self-hosted
design.
Verification
------------
* go test -race ./internal/validation/... ./internal/connector/
notifier/webhook/... ./internal/service/... — green.
* Full-suite go test -race ./... — green (GOTMPDIR=/dev/shm to
sidestep full /tmp on the sandbox host).
* Coverage gates pass: service 68.8% >= 55%, handler 83.6% >= 60%,
domain 82.0% >= 40%, middleware 63.8% >= 30%. Overall 67.8%.
Webhook package 91.5% line coverage; validation package
ValidateSafeURL/SafeHTTPDialContext 78-100% per function.
* govulncheck ./... — no vulnerabilities found.
* golangci-lint run on touched H-4 production code — clean. Pre-
existing errcheck/gosimple warnings in scope-adjacent files
(webhook_test.go:270 w.Write, network_scan.go:120/173/265/305)
verified against
|
||
|
shankar0123
|
3853b7460c |
security: reject CRLF/NUL in email headers to prevent SMTP injection (fixes H-3)
H-3 in certctl-audit-report.md: caller-supplied From/To/Subject were
interpolated directly into the SMTP DATA payload and handed to
client.Mail / client.Rcpt with no sanitization, allowing an attacker
who controls any of those values to inject extra headers (Bcc:,
Reply-To:), split the message body (CRLFCRLF), or tamper with the
SMTP envelope. CWE-113.
Fix:
- New package helper internal/validation.ValidateHeaderValue(field,
value). Rejects CR ("\r"), LF ("\n"), and NUL ("\x00") with an error
that names the offending field but does NOT echo the raw value,
so log readers cannot be attacked with injected content. Silent
stripping was considered and rejected: authentication-relevant
headers must fail visibly.
- Two-layer defense in internal/connector/notifier/email/email.go:
(1) primary guard at the top of sendEmail / sendHTMLEmail, which
blocks tampering of the SMTP envelope (client.Mail, client.Rcpt)
since net/smtp does not sanitize those arguments; and
(2) defense-in-depth guard inside formatEmailMessage /
formatHTMLEmailMessage, catching any future caller that
bypasses sendEmail. Both format functions now return an error.
- Body content is intentionally NOT validated — CR/LF in body is legal
RFC 5322 content and net/smtp handles dot-stuffing.
Tests:
- internal/validation/headers_test.go: 3 functions (AcceptsSafeInput,
RejectsControlCharacters, DefaultFieldName) covering plain ASCII,
UTF-8 multibyte, tabs, typical email addresses, CRLF injection,
lone CR, lone LF, NUL, CRLFCRLF body split, trailing CR, leading LF.
Each reject case asserts the field name IS in the error and the
raw offending value IS NOT (anti-log-injection).
- internal/connector/notifier/email/email_test.go: added
TestEmail_FormatEmailMessage_RejectsCRLFInjection and
TestEmail_FormatHTMLEmailMessage_RejectsCRLFInjection. Existing
format tests updated for the new (bytes, error) signature.
Wire-format invariants preserved:
- SMTP DATA headers still use CRLF separators and RFC 1123Z Date
(unchanged).
- Content-Type headers unchanged (text/plain for plain, text/html +
MIME-Version: 1.0 for HTML).
- No change to message encoding or transport.
Verification (Go 1.25.9 linux-arm64, parent
|
||
|
shankar0123
|
fde5b39d53 |
fix: resolve test compilation and runtime failures across codebase
- Add context.Context to handler test mocks (agent, agent_group) - Refactor scheduler to use local interfaces instead of concrete service types - Wire RevocationSvc/CAOperationsSvc sub-services in integration tests - Add context.Background() to service test calls (agent, agent_group) - Fix repo integration tests: add FK prerequisite records (team, owner, issuer, renewal_policy) before creating certificates - Set MaxOpenConns(1) on test DB to preserve SET search_path across queries - Fix Apache/HAProxy tests: replace "echo ok"/"echo reload" with "true" binary to avoid macOS exec.Command PATH resolution failure - Fix validation tests: correct error expectations for regex-first checks, replace null byte strings with strings.Repeat for length tests - Fix scheduler timeout test flakiness with t.Skip fallback - Remove unused imports (context in ca_operations_test, service in scheduler) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
shankar0123
|
a0afa7ab6f |
test(security): TICKET-018 add fuzz tests for command validation and domain parsing
Added Go native fuzz tests (testing/fuzz) for security-critical input validation: 1. FuzzValidateShellCommand in internal/validation/command_fuzz_test.go - Tests shell command validation with injection payloads (;, |, &, $, `, etc.) - Seed corpus includes valid commands and dangerous metacharacters - Ensures function never panics under fuzzing 2. FuzzValidateDomainName in internal/validation/command_fuzz_test.go - Tests RFC 1123 domain validation with wildcard support - Seed corpus includes SQL injection, path traversal, and malformed domains - Ensures function never panics under fuzzing 3. FuzzValidateACMEToken in internal/validation/command_fuzz_test.go - Tests base64url token validation - Seed corpus includes injection payloads and special characters - Ensures function never panics under fuzzing 4. FuzzIsValidRevocationReason in internal/domain/revocation_fuzz_test.go - Tests RFC 5280 revocation reason validation - Seed corpus includes case variations, injection attempts, and null bytes - Ensures function never panics and returns only valid booleans 5. FuzzCRLReasonCode in internal/domain/revocation_fuzz_test.go - Tests CRL reason code mapping - Validates return codes are within 0-9 range - Ensures invalid reasons default to 0 (unspecified) All fuzz tests follow Go 1.18+ testing/fuzz conventions with seed corpus for faster discovery of edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
shankar0123
|
3e3e68fd3a |
fix(security): TICKET-009 add HTTP timeouts to notifier clients
- Added TestSlack_ClientHasTimeout to verify 10-second timeout - Added TestTeams_ClientHasTimeout to verify 10-second timeout - Added TestPagerDuty_ClientHasTimeout to verify 10-second timeout - Added TestOpsGenie_ClientHasTimeout to verify 10-second timeout - All notifiers already configured with 10 second timeout in New() - Tests verify timeout is set and matches expected value |