mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 16:21:30 +00:00
feat(security): Sprint 5 ACQ — RED-003 deny-empty flip + SEC-009/RED-005 RFC1918 opt-in
Acquisition-audit Sprint 5 ACQ closure (2026-05-16). Two
independent findings ship together because they share Load() /
main.go wiring; the closure comments tie each line to its finding.
PART A — RED-003 (agent-bootstrap deny-empty cutover)
=====================================================
Phase 2 SEC-H1 closure (2026-05-13) introduced the
CERTCTL_AGENT_BOOTSTRAP_TOKEN_DENY_EMPTY staged feature flag with
default `false` so v2.1.x operators wouldn't get a surprise
fail-closed on upgrade. This commit flips the default to `true`
(per the staged plan in the existing CHANGELOG "Breaking changes
(scheduled for v2.2.0)" block). Operators who haven't generated a
real bootstrap token yet keep the v2.1.x warn-mode pass-through
for one upgrade window by setting
CERTCTL_AGENT_BOOTSTRAP_TOKEN_DENY_EMPTY=false explicitly.
Demo-mode escape hatch: CERTCTL_DEMO_MODE_ACK=true skips the
fail-closed gate so the screenshot/demo path stays one-command-up.
The accompanying boot-banner WARN at cmd/server/main.go:124-126
keeps demo mode visible in every log scraper, so this override
cannot silently re-enable warn-mode in production.
internal/config/config.go
- Load() default for AgentBootstrapTokenDenyEmpty flipped to true
- Validate() gate now also checks !c.Auth.DemoModeAck so the demo
override line up with the boot-banner WARN
- Closure comment block updated to cross-reference Sprint 5 ACQ
and the CHANGELOG v2.2.0 entry
cmd/server/main.go
- Updated boot-time WARN message to reflect the new default
(deny-empty=true) — the warn now fires only in the two
explicit override scenarios (warn-mode opt-back or demo mode),
and explains the operator action either way
- Info-line on configured-token path unchanged
PART B — SEC-009 + RED-005 (opt-in RFC1918 outbound block)
==========================================================
internal/validation/ssrf.go::IsReservedIP has always intentionally
left RFC 1918 ranges (10/8, 172.16/12, 192.168/16) NOT-reserved
because certctl is designed to manage certificates inside private
networks. For operators on hosted IaaS where RFC1918 IS internal
trust (kubeadm-default 10.96.0.0/12 service CIDR exposes the
Kubernetes API on 10.96.0.1; cloud-provider internal monitoring;
hosted-bastion subnets), this default is a real exposure path.
Add a package-level atomic.Bool toggle in internal/validation/ssrf.go
that, when on, extends IsReservedIP to ALSO return true for the
three RFC1918 ranges. Every IsReservedIP-derived path
(SafeHTTPDialContext, ValidateSafeURL, the network scanner, the
webhook + OIDC + ACME callers) picks up the new policy
transitively without per-call-site changes.
internal/validation/ssrf.go
- blockRFC1918Outbound atomic.Bool + SetBlockRFC1918Outbound /
BlockRFC1918OutboundEnabled accessor pair
- rfc1918Nets pre-parsed at package init (panic on parse failure
surfaces a misconfigured ssrf package immediately, not via a
silently disabled toggle)
- IsReservedIP checks the toggle after the existing reserved-IP
checks
- Header comment rewritten to document the toggle + the
transitive coverage
internal/config/config.go
- New NetworkConfig sub-config; Config gains a Network field
- Load() reads CERTCTL_BLOCK_RFC1918_OUTBOUND env var (default
false; preserves the existing self-hosted threat model)
- NetworkConfig docstring lists the operator-trap (enabling this
also blocks RFC1918 from the network scanner) so an operator
cert-discovering their own RFC1918 space doesn't get a
silently-empty scan result
cmd/server/main.go
- Wires validation.SetBlockRFC1918Outbound after config.Load and
near the demo-mode banner / agent-bootstrap-token block; emits
a one-shot INFO line when the toggle is enabled so the policy
is visible in journals
Tests
=====
internal/config/config_test.go
- TestLoad_AgentBootstrapTokenDenyEmpty_DefaultIsTrue — pins the
default flip at the boot path (Load returns the flipped value)
- TestValidate_DenyEmptyDefault_RefusesWithoutToken — pins the
fail-closed behavior under the new default
- TestValidate_DenyEmptyExplicitFalse_AllowsEmpty — pins the
v2.1.x back-compat escape hatch
- TestValidate_DenyEmpty_DemoModeAckOverride_AllowsEmpty — pins
the demo-mode override
internal/validation/ssrf_test.go
- TestIsReservedIP_RFC1918_OptIn — pins toggle-off / toggle-on
behavior across all three RFC1918 ranges, edge cases
immediately outside the ranges, and the toggle-back-off path
- TestSafeHTTPDialContext_RFC1918_OptIn — pins that the toggle
reaches the dial-time SSRF check transitively (not just
IsReservedIP in isolation)
Test-helper updates (Sprint-5-induced churn):
- internal/config/config_test.go::setMinimalValidEnv now sets
CERTCTL_AGENT_BOOTSTRAP_TOKEN to a placeholder so Load()-based
tests that don't specifically exercise the empty-token gate
keep passing under the new fail-closed default. Tests that DO
exercise the empty-token path explicitly override back to "".
- internal/config/config_est_profiles_test.go +
internal/config/config_scep_profiles_test.go: same placeholder
fix for the four Load()-based EST/SCEP profile tests.
- cmd/server/main_test.go::TestMain_ServerConfigFromEnvironment +
TestMain_AuthTypeConfiguration: same fix at the main.go test
layer with prior-value restore.
Verified locally: gofmt -l clean; go vet clean; staticcheck clean
across internal/config, internal/validation, cmd/server; short
tests green on all three packages; targeted -v run of all six new
test names confirms PASS.
This commit is contained in:
@@ -256,6 +256,18 @@ func TestMain_ServerConfigFromEnvironment(t *testing.T) {
|
||||
os.Setenv("CERTCTL_SERVER_PORT", "8080")
|
||||
os.Setenv("CERTCTL_SERVER_TLS_CERT_PATH", certPath)
|
||||
os.Setenv("CERTCTL_SERVER_TLS_KEY_PATH", keyPath)
|
||||
// Acquisition-audit RED-003 closure (Sprint 5 ACQ, 2026-05-16):
|
||||
// deny-empty default flipped to true; supply a placeholder token
|
||||
// so Load() succeeds. The defer below restores prior env.
|
||||
oldBootstrap := os.Getenv("CERTCTL_AGENT_BOOTSTRAP_TOKEN")
|
||||
os.Setenv("CERTCTL_AGENT_BOOTSTRAP_TOKEN", "test-bootstrap-token-placeholder")
|
||||
defer func() {
|
||||
if oldBootstrap != "" {
|
||||
os.Setenv("CERTCTL_AGENT_BOOTSTRAP_TOKEN", oldBootstrap)
|
||||
} else {
|
||||
os.Unsetenv("CERTCTL_AGENT_BOOTSTRAP_TOKEN")
|
||||
}
|
||||
}()
|
||||
|
||||
cfg, err := config.Load()
|
||||
if err != nil {
|
||||
@@ -317,6 +329,18 @@ func TestMain_AuthTypeConfiguration(t *testing.T) {
|
||||
|
||||
// Set auth secret for api-key mode
|
||||
os.Setenv("CERTCTL_AUTH_SECRET", "test-secret")
|
||||
// Acquisition-audit RED-003 closure (Sprint 5 ACQ, 2026-05-16):
|
||||
// deny-empty default flipped to true; supply a placeholder token
|
||||
// so Load() succeeds.
|
||||
oldBootstrap := os.Getenv("CERTCTL_AGENT_BOOTSTRAP_TOKEN")
|
||||
os.Setenv("CERTCTL_AGENT_BOOTSTRAP_TOKEN", "test-bootstrap-token-placeholder")
|
||||
defer func() {
|
||||
if oldBootstrap != "" {
|
||||
os.Setenv("CERTCTL_AGENT_BOOTSTRAP_TOKEN", oldBootstrap)
|
||||
} else {
|
||||
os.Unsetenv("CERTCTL_AGENT_BOOTSTRAP_TOKEN")
|
||||
}
|
||||
}()
|
||||
|
||||
testCases := []string{"api-key", "none"}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user