mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 19:51:33 +00:00
e720474fb7
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
286 lines
8.7 KiB
Go
286 lines
8.7 KiB
Go
package main
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"crypto/sha256"
|
|
"crypto/tls"
|
|
"crypto/x509"
|
|
"encoding/json"
|
|
"encoding/pem"
|
|
"fmt"
|
|
"io"
|
|
"log/slog"
|
|
"net"
|
|
"net/http"
|
|
"time"
|
|
)
|
|
|
|
// verifyDeployment probes the live TLS endpoint for a deployment target and verifies
|
|
// that the deployed certificate matches what we expect.
|
|
//
|
|
// Parameters:
|
|
// - targetHost: the hostname or IP of the target (extracted from target config)
|
|
// - targetPort: the TLS port of the target (e.g., 443)
|
|
// - expectedCertPEM: the PEM-encoded certificate that was deployed
|
|
// - delay: wait time before probing (e.g., 2 seconds for reload to take effect)
|
|
// - timeout: overall timeout for TLS connection attempt (e.g., 10 seconds)
|
|
//
|
|
// Returns:
|
|
// - A VerificationResult if probing succeeded (even if cert doesn't match)
|
|
// - An error if the probe itself failed (network error, timeout, etc.)
|
|
//
|
|
// The function compares the SHA-256 fingerprints of the expected and actual certificates.
|
|
// If the certificate served at the endpoint differs, Verified will be false but no error
|
|
// is returned — this is an expected verification failure, not a probe failure.
|
|
func verifyDeployment(
|
|
ctx context.Context,
|
|
targetHost string,
|
|
targetPort int,
|
|
expectedCertPEM string,
|
|
delay time.Duration,
|
|
timeout time.Duration,
|
|
logger *slog.Logger,
|
|
) (*VerificationResult, error) {
|
|
// Wait for reload to take effect
|
|
if delay > 0 {
|
|
select {
|
|
case <-time.After(delay):
|
|
case <-ctx.Done():
|
|
return nil, ctx.Err()
|
|
}
|
|
}
|
|
|
|
// Parse expected certificate to compute its fingerprint
|
|
expectedFp, err := computeCertificateFingerprint(expectedCertPEM)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to parse expected certificate: %w", err)
|
|
}
|
|
|
|
// Connect to the target's TLS endpoint
|
|
address := fmt.Sprintf("%s:%d", targetHost, targetPort)
|
|
if logger != nil {
|
|
logger.Debug("probing TLS endpoint for verification",
|
|
"address", address,
|
|
"expected_fingerprint", expectedFp)
|
|
}
|
|
|
|
dialer := &net.Dialer{Timeout: timeout}
|
|
conn, err := tls.DialWithDialer(dialer, "tcp", address, &tls.Config{
|
|
// SECURITY NOTE: InsecureSkipVerify is intentionally set to true here.
|
|
// Post-deployment verification must probe the live endpoint to extract and
|
|
// compare the served certificate fingerprint, regardless of its validity
|
|
// state (expired, self-signed, internal CA, etc.). This setting is scoped
|
|
// to verification probing only — it is NEVER used for control-plane API
|
|
// calls, issuer connector communication, or any operation that trusts the
|
|
// certificate. The verification result compares SHA-256 fingerprints only.
|
|
// See TICKET-016 for full security audit rationale.
|
|
InsecureSkipVerify: true, //nolint:gosec // verification probe; documented above + docs/tls.md L-001 table
|
|
ServerName: targetHost, // For SNI
|
|
})
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to connect to %s: %w", address, err)
|
|
}
|
|
defer conn.Close()
|
|
|
|
// Extract the leaf certificate from the TLS connection
|
|
state := conn.ConnectionState()
|
|
if len(state.PeerCertificates) == 0 {
|
|
return nil, fmt.Errorf("no certificates presented by %s", address)
|
|
}
|
|
|
|
leafCert := state.PeerCertificates[0]
|
|
actualFp := fmt.Sprintf("%x", sha256.Sum256(leafCert.Raw))
|
|
|
|
if logger != nil {
|
|
logger.Debug("received certificate from endpoint",
|
|
"address", address,
|
|
"cn", leafCert.Subject.CommonName,
|
|
"actual_fingerprint", actualFp)
|
|
}
|
|
|
|
// Compare fingerprints
|
|
verified := actualFp == expectedFp
|
|
if logger != nil {
|
|
if !verified {
|
|
logger.Warn("certificate fingerprint mismatch at endpoint",
|
|
"address", address,
|
|
"expected_fingerprint", expectedFp,
|
|
"actual_fingerprint", actualFp)
|
|
} else {
|
|
logger.Info("certificate verification succeeded",
|
|
"address", address,
|
|
"fingerprint", actualFp)
|
|
}
|
|
}
|
|
|
|
return &VerificationResult{
|
|
ExpectedFingerprint: expectedFp,
|
|
ActualFingerprint: actualFp,
|
|
Verified: verified,
|
|
VerifiedAt: time.Now().UTC(),
|
|
}, nil
|
|
}
|
|
|
|
// VerificationResult represents the outcome of verifying a deployed certificate.
|
|
type VerificationResult struct {
|
|
ExpectedFingerprint string `json:"expected_fingerprint"`
|
|
ActualFingerprint string `json:"actual_fingerprint"`
|
|
Verified bool `json:"verified"`
|
|
VerifiedAt time.Time `json:"verified_at"`
|
|
Error string `json:"error,omitempty"`
|
|
}
|
|
|
|
// computeCertificateFingerprint computes the SHA-256 fingerprint of a PEM-encoded certificate.
|
|
func computeCertificateFingerprint(certPEM string) (string, error) {
|
|
block, _ := pem.Decode([]byte(certPEM))
|
|
if block == nil {
|
|
return "", fmt.Errorf("failed to decode PEM certificate")
|
|
}
|
|
|
|
cert, err := x509.ParseCertificate(block.Bytes)
|
|
if err != nil {
|
|
return "", fmt.Errorf("failed to parse x509 certificate: %w", err)
|
|
}
|
|
|
|
fp := sha256.Sum256(cert.Raw)
|
|
return fmt.Sprintf("%x", fp), nil
|
|
}
|
|
|
|
// reportVerificationResult submits the verification result back to the control plane.
|
|
// This is a best-effort operation — a failure to report doesn't block agent progress.
|
|
func (a *Agent) reportVerificationResult(
|
|
ctx context.Context,
|
|
jobID string,
|
|
targetID string,
|
|
result *VerificationResult,
|
|
) error {
|
|
if jobID == "" || targetID == "" || result == nil {
|
|
return fmt.Errorf("missing required fields for verification report")
|
|
}
|
|
|
|
// Build the request payload
|
|
payload := map[string]interface{}{
|
|
"target_id": targetID,
|
|
"expected_fingerprint": result.ExpectedFingerprint,
|
|
"actual_fingerprint": result.ActualFingerprint,
|
|
"verified": result.Verified,
|
|
"error": result.Error,
|
|
}
|
|
|
|
body, err := json.Marshal(payload)
|
|
if err != nil {
|
|
return fmt.Errorf("failed to marshal verification result: %w", err)
|
|
}
|
|
|
|
// POST to /api/v1/jobs/{id}/verify
|
|
url := fmt.Sprintf("%s/api/v1/jobs/%s/verify", a.config.ServerURL, jobID)
|
|
req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(body))
|
|
if err != nil {
|
|
return fmt.Errorf("failed to create verification request: %w", err)
|
|
}
|
|
|
|
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", a.config.APIKey))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
|
|
resp, err := a.client.Do(req)
|
|
if err != nil {
|
|
return fmt.Errorf("failed to send verification result: %w", err)
|
|
}
|
|
defer resp.Body.Close()
|
|
|
|
// Check response status
|
|
if resp.StatusCode != http.StatusOK {
|
|
bodyBytes, _ := io.ReadAll(resp.Body)
|
|
return fmt.Errorf("verification reporting failed with status %d: %s", resp.StatusCode, string(bodyBytes))
|
|
}
|
|
|
|
if a.logger != nil {
|
|
a.logger.Debug("verification result reported to control plane",
|
|
"job_id", jobID,
|
|
"verified", result.Verified)
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
// extractTargetHostAndPort extracts the host and port from target configuration.
|
|
// Common target configs include "host" or "hostname" and "port" fields.
|
|
func extractTargetHostAndPort(configJSON json.RawMessage) (string, int, error) {
|
|
var config map[string]interface{}
|
|
if err := json.Unmarshal(configJSON, &config); err != nil {
|
|
return "", 0, fmt.Errorf("invalid target config JSON: %w", err)
|
|
}
|
|
|
|
// Try common field names for hostname
|
|
var host string
|
|
for _, key := range []string{"host", "hostname", "target", "address"} {
|
|
if h, ok := config[key].(string); ok && h != "" {
|
|
host = h
|
|
break
|
|
}
|
|
}
|
|
if host == "" {
|
|
return "", 0, fmt.Errorf("target config missing host/hostname field")
|
|
}
|
|
|
|
// Try common field names for port, default to 443
|
|
port := 443
|
|
if p, ok := config["port"].(float64); ok {
|
|
port = int(p)
|
|
}
|
|
if port < 1 || port > 65535 {
|
|
return "", 0, fmt.Errorf("invalid port: %d", port)
|
|
}
|
|
|
|
return host, port, nil
|
|
}
|
|
|
|
// verifyAndReportDeployment performs TLS endpoint verification and reports the result.
|
|
// This is a best-effort operation — failures are logged but don't affect deployment status.
|
|
func (a *Agent) verifyAndReportDeployment(
|
|
ctx context.Context,
|
|
job JobItem,
|
|
targetHost string,
|
|
targetPort int,
|
|
certPEM string,
|
|
) {
|
|
// Perform verification with configured timeout and delay
|
|
result, err := verifyDeployment(ctx, targetHost, targetPort, certPEM,
|
|
2*time.Second, // delay before probing
|
|
10*time.Second, // timeout for TLS connection
|
|
a.logger)
|
|
|
|
if err != nil {
|
|
if a.logger != nil {
|
|
a.logger.Warn("verification probe failed",
|
|
"job_id", job.ID,
|
|
"target_host", targetHost,
|
|
"target_port", targetPort,
|
|
"error", err)
|
|
}
|
|
// Probe failure: report error but continue
|
|
result = &VerificationResult{
|
|
Error: err.Error(),
|
|
VerifiedAt: time.Now().UTC(),
|
|
}
|
|
}
|
|
|
|
// Report result to control plane
|
|
if job.TargetID == nil {
|
|
if a.logger != nil {
|
|
a.logger.Warn("cannot report verification: target_id is nil", "job_id", job.ID)
|
|
}
|
|
return
|
|
}
|
|
|
|
if err := a.reportVerificationResult(ctx, job.ID, *job.TargetID, result); err != nil {
|
|
if a.logger != nil {
|
|
a.logger.Warn("failed to report verification result",
|
|
"job_id", job.ID,
|
|
"error", err)
|
|
}
|
|
// Non-blocking: continue even if report fails
|
|
}
|
|
}
|