Files
certctl/internal/validation/unicode.go
T
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].
2026-04-26 17:18:00 +00:00

162 lines
5.3 KiB
Go
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
package validation
import (
"fmt"
"strings"
"unicode"
)
// Bundle-9 / Audit L-012 / CWE-1007 (Insufficient Visual Distinction of
// Homoglyphs Presenting to User) + CWE-176 (Improper Handling of Unicode
// Encoding):
//
// Certificate CommonName + Subject Alternative Name fields originate from
// the CSR submitter and feed directly into:
//
// - The MCP / API surface that humans inspect ("which cert is this?")
// - The web UI that renders cert lists, deployment targets, audit events
// - Downstream relying parties that match certs by hostname
//
// An attacker who can submit a CSR (any operator with cert-create capability,
// or anonymous EST/SCEP enrollment) can plant unicode payloads that:
//
// 1. **Visually impersonate** a legitimate hostname via Cyrillic / Greek /
// Cherokee homoglyphs (e.g. CN="apple.com" with one Cyrillic 'а' that
// renders identically but routes differently via DNS or matches a
// different TLS pin).
//
// 2. **Hide content** via zero-width characters (U+200B..U+200D, U+2060,
// U+FEFF) that don't render but break naive substring matching.
//
// 3. **Reverse render order** via RTL/LTR override characters
// (U+202A..U+202E, U+2066..U+2069) that make "google.com.evil.org"
// display as "google.com.evil.org" with the suffix flipped.
//
// ValidateUnicodeSafe rejects all three categories. It does NOT NFC-normalize
// — the audit prompt's invariant is that the validator REJECTS rather than
// silently rewrites, because operators who don't know their CSR's CN was
// rewritten will get certs they didn't ask for.
// ValidateUnicodeSafe returns nil if `name` is safe to use as a certificate
// CN or SAN, or an error describing the first violation found. The error
// message includes the rune offset so operators can locate the problem in
// the CSR they submitted.
//
// Wired in: internal/connector/issuer/local/local.go (CSR-acceptance path).
// Future ride-along sites (M-029): the web frontend's CertificateStep input.
func ValidateUnicodeSafe(name string) error {
if name == "" {
// Empty is a different validation concern (handled by ValidateRequired
// in handler-side ValidateRequired). Don't double-fail here.
return nil
}
// First pass: scan for explicitly forbidden characters.
for i, r := range name {
switch {
case isRTLOverride(r):
return fmt.Errorf(
"contains bidirectional override character %U at byte offset %d — refuse (potential reverse-rendering attack, CWE-1007)",
r, i,
)
case isZeroWidth(r):
return fmt.Errorf(
"contains zero-width character %U at byte offset %d — refuse (hidden content, CWE-176)",
r, i,
)
case isControl(r):
return fmt.Errorf(
"contains control character %U at byte offset %d — refuse",
r, i,
)
}
}
// Second pass: per-label mixed-script detection. DNS labels are joined
// by '.', so we split on '.' and check each label independently. A
// label that mixes Latin with Cyrillic / Greek / Cherokee is the
// classic IDN homograph signal.
for _, label := range strings.Split(name, ".") {
if err := validateLabelSingleScript(label); err != nil {
return err
}
}
return nil
}
// isRTLOverride reports whether r is a Unicode bidirectional override
// character that an attacker could use to flip rendered text direction.
func isRTLOverride(r rune) bool {
switch r {
case 0x202A, // LEFT-TO-RIGHT EMBEDDING
0x202B, // RIGHT-TO-LEFT EMBEDDING
0x202C, // POP DIRECTIONAL FORMATTING
0x202D, // LEFT-TO-RIGHT OVERRIDE
0x202E, // RIGHT-TO-LEFT OVERRIDE
0x2066, // LEFT-TO-RIGHT ISOLATE
0x2067, // RIGHT-TO-LEFT ISOLATE
0x2068, // FIRST STRONG ISOLATE
0x2069: // POP DIRECTIONAL ISOLATE
return true
}
return false
}
// isZeroWidth reports whether r is a Unicode zero-width character that
// renders nothing but breaks substring matching.
func isZeroWidth(r rune) bool {
switch r {
case 0x200B, // ZERO WIDTH SPACE
0x200C, // ZERO WIDTH NON-JOINER
0x200D, // ZERO WIDTH JOINER
0x2060, // WORD JOINER
0xFEFF: // ZERO WIDTH NO-BREAK SPACE / BOM
return true
}
return false
}
// isControl reports whether r is a C0 or C1 control character. Tabs and
// newlines have no business in a certificate name; reject.
func isControl(r rune) bool {
return r < 0x20 || (r >= 0x7F && r <= 0x9F)
}
// validateLabelSingleScript rejects a DNS label that mixes Latin
// (az, AZ, 09, '-') with characters from a different script. Pure-
// non-Latin labels are allowed (e.g. genuine IDN domains in Cyrillic);
// the attack we're defending against is the MIX.
func validateLabelSingleScript(label string) error {
if label == "" {
return nil
}
hasASCII := false
for _, r := range label {
if r < 0x80 {
hasASCII = true
break
}
}
if !hasASCII {
// Pure non-ASCII label — could be a legitimate IDN. Don't
// reject; the homograph attack we care about is the MIX.
return nil
}
// Has ASCII — assert NO non-ASCII letters present. Non-ASCII
// non-letter chars (e.g., a digit from a different script) are
// also rejected to keep the rule simple.
for i, r := range label {
if r < 0x80 {
continue
}
if unicode.IsLetter(r) || unicode.IsDigit(r) || unicode.IsMark(r) {
return fmt.Errorf(
"label %q mixes ASCII with non-ASCII script character %U at byte offset %d — refuse (potential IDN homograph, CWE-1007)",
label, r, i,
)
}
}
return nil
}