gsadmin/certctl
1
0
Fork 0
mirror of https://github.com/shankar0123/certctl.git synced 2026-06-09 14:48:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

Bundle D: Documentation & transparency sweep — 8 findings closed

Browse Source 
Closes H-009 + L-001 + L-007 + L-008 + L-016 + L-017 + L-018 + M-027
from comprehensive-audit-2026-04-25.

H-009 — README JWT verified-already-clean
  README has zero JWT mentions at audit time. docs/architecture.md
  correctly documents JWT/OIDC integration via authenticating-gateway
  pattern (line 905-912).
  .github/workflows/ci.yml: new step
    'Forbidden README JWT advertising regression guard (H-009)'
    greps README for JWT-as-supported phrasing; passes verbatim
    (gateway / pre-G-1) but fails build on net-new advertising.

L-001 (CWE-295) — InsecureSkipVerify per-site justification
  Audit count was 8; recon found 13 production sites.
  docs/tls.md: new 'InsecureSkipVerify justifications' table
    enumerates each site by file:line with per-site rationale.
  cmd/agent/verify.go:78, internal/tlsprobe/probe.go:54,
  internal/service/network_scan.go:460: each previously-bare
    InsecureSkipVerify: true now carries //nolint:gosec.
  .github/workflows/ci.yml: new step
    'Forbidden bare InsecureSkipVerify regression guard (L-001)'
    fails build if any net-new ISV lands in non-test .go without
    nolint:gosec on the same or preceding line.

L-007 — README dependency-audit commands
  README.md: new Dependencies section with go list -m all | wc -l,
    go mod why, govulncheck ./.... Honors operating-rules invariant.

L-008 — Release-time govulncheck gate
  .github/workflows/release.yml: new 'Install govulncheck' +
    'Run govulncheck (release gate)' steps in the matrix job.
    Pinned to same install path as ci.yml. Default exit code
    semantics (fail on called-vuln only, deferred-call advisories
    tracked on master via L-021) keeps the gate appropriate.

L-016 — architecture.md drift fixes
  docs/architecture.md: system-components diagram's '21 tables'
    annotation removed (current 23; replaced with TEXT-keys
    descriptor); connector-architecture '9 connectors' prose
    replaced with grep ref + current 12-issuer list (added
    Entrust/GlobalSign/EJBCA which were missing); API-design
    '97 operations / 107 total' replaced with grep commands.
  Connector subgraphs verified-current at 12/13/6.

L-017 — workspace CLAUDE.md verified-already-clean
  Bundle B's pre-commit-gate refactor already converted current-
  state numeric claims to grep commands. Phase 0 recon confirmed
  zero remaining hardcoded counts.

L-018 — Defect age table
  cowork/comprehensive-audit-2026-04-25/defect-age.md (NEW):
    Tabulates all 9 High findings with first-mentioned commit,
    closing bundle, days-open. Methodology snippet for re-running.
    Key finding: 8 of 9 closed within 24h of audit publication.

M-027 — OpenAPI parity verified-already-clean
  Audit's 'router 121 vs OpenAPI 125 — 4-op gap' was wrong
  methodology. The 4-op 'gap' was exactly the 4 routes registered
  via r.mux.Handle (auth-exempt allowlist) instead of r.Register.
  When you count both dispatch shapes the totals match exactly.
  internal/api/router/openapi_parity_test.go (NEW):
    TestRouter_OpenAPIParity AST-walks router.go for both
    Register and mux.Handle calls + walks api/openapi.yaml's
    path/method nesting + asserts the sets match. Adding a route
    without updating the spec fails CI permanently.

Audit deliverables:
  audit-report.md: score 38/55 -> 46/55 closed
    (High 7/9 -> 8/9; Medium 20/27 -> 21/27; Low 8/19 -> 14/19)
  findings.yaml: 8 status flips open -> closed
  defect-age.md: new file
  certctl/CHANGELOG.md: Bundle D section

Verification:
  TestRouter_OpenAPIParity                                   PASS
  L-001 grep guard self-test (after //nolint:gosec adds)     PASS
  H-009 grep guard self-test                                 PASS
  go test -count=1 -short on changed packages                green
This commit is contained in:
shankar0123
2026-04-27 00:47:15 +00:00
parent 6cd3135f90
commit e720474fb7
10 changed files with 340 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/tls.md
+31
View File
@@ -175,9 +175,40 @@ The client did not trust the CA that signed the server cert. Either mount the CA
**Client side: `tls: first record does not look like a TLS handshake`**
The client is speaking plaintext HTTP to an HTTPS server (or vice-versa). Check that `CERTCTL_SERVER_URL` starts with `https://`. If you are upgrading from a pre-v2.2 release and your agents are old, they will surface this error until you roll the DaemonSet — see [`upgrade-to-tls.md`](upgrade-to-tls.md).
## InsecureSkipVerify justifications (Audit L-001)
`crypto/tls.Config.InsecureSkipVerify` short-circuits standard certificate
chain validation. Each production use site below has a justification —
the shape is "this code path is fundamentally pre-trust or
trust-from-context, and chain validation in the stdlib path is not the
right tool". Test-only sites are not enumerated here.
The CI grep guard `Forbidden bare InsecureSkipVerify regression guard
(L-001)` in `.github/workflows/ci.yml` fails the build if any new
`InsecureSkipVerify: true` lands in a non-test file without a
`//nolint:gosec` comment carrying a justification — adding a new entry
to this table is the right way to extend the surface.
| Site (file:line) | Trigger | Justification |
|---|---|---|
| `cmd/agent/main.go:59,125,136,1259,1262` | `--insecure-skip-verify` CLI flag | Dev escape hatch; docs/tls.md and the agent install script direct operators to use a real CA bundle in production. The server emits a startup WARN when set. |
| `cmd/agent/verify.go:70,78` | TLS deployment verification probe | The agent is verifying that its own freshly-deployed cert is being served. The chain may be self-signed or signed by an upstream the agent host doesn't trust; what matters is the leaf-cert match against what the agent just deployed. The verifier compares the served leaf bytes to the expected leaf, not the chain. |
| `internal/tlsprobe/probe.go:33,47,54` | Network scanner / discovery probe | Discovery's job is to find every cert on the network, including expired, self-signed, and not-yet-deployed certs. Validating the chain would silently skip the broken-cert results that are precisely what operators want to know about. |
| `internal/mcp/client.go:35` | MCP CLI `--insecure` flag | Dev escape hatch for local-only MCP testing against a self-signed control plane. |
| `internal/cli/client.go:39` | `certctl --insecure` flag | Same shape as the agent flag — local dev only. |
| `internal/connector/target/f5/f5.go:128` | F5 BIG-IP iControl REST | F5 default install ships with a self-signed cert; operators who haven't replaced it use `config.Insecure`. The connector logs this on every dial and the operator-facing config docs this. |
| `internal/connector/issuer/acme/acme.go:146` | Pebble (ACME test server) | Hard-coded for tests that drive against Pebble locally. Pebble issues self-signed; verifying the chain would defeat the purpose. |
| `internal/service/network_scan.go:460` | Network scanner probe | Same rationale as `tlsprobe/probe.go` above — discovery surfaces broken certs by design. |
**What is NOT covered by this list:** `*_test.go` files use
`InsecureSkipVerify` freely against `httptest.Server` instances; that's a
test-fixture pattern, not a production trust decision. The grep guard
ignores `_test.go`.
## Related docs
- [`upgrade-to-tls.md`](upgrade-to-tls.md) — one-step cutover from pre-HTTPS releases
- [`quickstart.md`](quickstart.md) — docker-compose walkthrough with HTTPS examples
- [`test-env.md`](test-env.md) — integration test environment (also HTTPS-only)
- [`security.md`](security.md) — overall security posture, OCSP Must-Staple guidance, encryption-at-rest spec
- Milestone spec: `prompts/https-everywhere-milestone.md` (authoritative source for locked decisions)