feat: add cipher suite enumeration with raw TLS 1.3, QUIC, and key exchange probing

[danielewood]

Summary

• Add --ciphers flag to connect that enumerates all cipher suites a server supports across TLS 1.0–1.3, with good/weak security ratings
• Raw TLS 1.3 ClientHello prober (tls13probe.go) — byte-level packet construction, no shared state, no data races. Probes all 5 RFC 8446 cipher suites and 7 key exchange groups (including post-quantum hybrids)
• QUIC v1 Initial packet prober (quicprobe.go) — HKDF key derivation, AES-128-GCM encryption, header protection per RFC 9001. Automatically probes UDP 443 alongside TCP
• Output subgrouped by TLS version and key exchange type (ECDHE vs "RSA, no forward secrecy")
• HelloRetryRequest detection via RFC 8446 §4.1.3 sentinel

Test plan

• go test -race -count=1 ./... passes
• go vet ./... passes
• golangci-lint run passes
• Manual test: certkit connect google.com --ciphers shows TCP ciphers, QUIC ciphers, and key exchange groups
• Manual test: certkit connect gmail.com --ciphers shows "QUIC: not supported" (gmail.com rejects QUIC — confirmed with curl --http3-only)
• Manual test: certkit connect google.com --ciphers --json produces valid JSON

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR adds cipher suite enumeration functionality to the connect command via a new --ciphers flag. The implementation includes three major components:

Changes:

• Raw TLS 1.3 ClientHello prober with byte-level packet construction, eliminating data races from the previous linkname hack approach
• QUIC v1 Initial packet prober supporting cipher suite detection over UDP/443 with full RFC 9001 compliance
• Key exchange group probing for classical and post-quantum hybrid mechanisms (X25519MLKEM768, SecP256r1MLKEM768, SecP384r1MLKEM1024)

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tls13probe.go	Raw TLS 1.3 ClientHello packet builder and parser with isolated per-probe connections
quicprobe.go	QUIC v1 Initial packet builder with HKDF key derivation and AES-128-GCM encryption
connect.go	Main cipher scanning logic with concurrent probing, rating system, and result formatting
connect_test.go	Comprehensive test coverage for rating, scanning, formatting, and round-trip operations
testhelpers_test.go	New test helper for TLS servers with custom configs
cmd/certkit/connect.go	CLI integration with spinner and 60-second timeout
cmd/certkit/spinner.go	Terminal-aware progress indicator for long-running operations
go.mod	Added go-isatty dependency for terminal detection
README.md, EXAMPLES.md, CHANGELOG.md	Documentation updates

Files not reviewed (1)

• web/package-lock.json: Language not supported

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 9 comments.

Files not reviewed (1)

• web/package-lock.json: Language not supported

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• web/package-lock.json: Language not supported

[danielewood]

Fixed in 6492fa5:

• ERR-5 (bare continue in ACK handler): added slog.Debug("skipping QUIC ACK frame") before the continue in quicprobe.go
• ERR-4 (TLS/QUIC uppercase errors): addressed in comment 3975055985 — see fix there
• CL-3/CL-4 (changelog [pending]): entries now reference commit 6492fa5
• T-11 (TestBuildLegacyClientHelloMsg): removed — behavioral coverage exists through TestLegacyFallbackConnect
• Bug (QUIC-only "none detected"): FormatCipherScanResult empty check now covers both r.Ciphers and r.QUICCiphers

CS-5 on appendKeyShareExtension left as-is — the b []byte parameter acts as an append-idiom receiver and adding a struct would obscure the byte-builder pattern.

[danielewood]

Fixed in 6492fa5 — all four error strings lowercased:

• tls13probe.go: errAlertReceived = errors.New("tls alert received")
• tls13probe.go + legacyprobe.go: "tls record too large: %d bytes"
• quicprobe.go: "quic packet too short: %d bytes"
• connect.go: "tls handshake with %s: %w"

[danielewood]

Fixed in 6492fa5 — renamed emptyClientCert → emptyClientCertificate (all 3 occurrences in connect.go, including the function definition and both call sites).

[danielewood]

Fixed in 6492fa5:

1. ERR-1 (bare return err in CLI): return fmt.Errorf("connecting to %s: %w", args[0], err)
2. T-11 (TestBuildLegacyClientHelloMsg): removed
3. Logic bug (probeLegacyCipher hardcoded version): now returns (uint16, bool) with the actual negotiated version; ScanCipherSuites uses tlsVersionString(negotiatedVer)

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

• web/package-lock.json: Language not supported

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

• web/package-lock.json: Language not supported

[danielewood]

Fixed in 264281e — TestCipherSuiteNameLegacyIDs removed. Behavioral coverage exists through TestScanCipherSuites.

[danielewood]

Fixed in 264281e:

1. CS-5 — converted populateConnectResult(ctx, result, input) to (*ConnectResult).populate(ctx, input), reducing argument count to 2 (ctx + input).
2. T-11 — removed TestParseCertificateMessage; behavioral coverage exists through TestReadServerCertificates.
3. T-11 — removed TestCipherSuiteNameLegacyIDs; behavioral coverage exists through TestScanCipherSuites.
4. CL-4 — shortened full-40-char SHA URLs to 7-char SHAs in the link definitions.

[danielewood]

Fixed in 0c24518:

• ERR-4 (quicprobe.go): all 3 QUIC uppercase strings lowercased — deriving quic keys, decrypting quic payload, no crypto frame found in quic initial response
• CS-5 (tls13probe.go): extracted appendKeyShareExtensionInput{groupID, keyData} struct; updated caller

[danielewood]

Fixed in 0c24518:

• ERR-4 (quicprobe.go, legacyprobe.go, tls13probe.go): all uppercase QUIC/TLS occurrences in error strings lowercased (10 strings total)
• ERR-5 (legacyprobe_test.go): _, _ = conn.Read/Write replaced with if _, err := ...; err != nil { slog.Debug(...) } pattern

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• web/package-lock.json: Language not supported

[claude]

Code review

ERR-5 (MUST): Silent error discard in all probe functions

Affected files: tls13probe.go, connect.go, legacyprobe.go, quicprobe.go

All six probe functions (probeTLS13Cipher, probeKeyExchangeGroup, probeCipher, probeKeyExchangeGroupLegacy, probeLegacyCipher, probeQUICCipher) contain bare return false statements after every error check, with no slog.Debug call explaining why the error is being discarded.

ERR-5 (MUST): "Never silently ignore errors. Use continue in loops only with a slog.Debug explaining why."

The first sentence is a general prohibition — not limited to continue-in-loops. The project consistently applies this rule to all error-consuming patterns: slog.Debug("OCSP check failed", ...), slog.Debug("CRL fetch failed", ...), slog.Debug("failed to unmarshal acceptable CA DN", ...) in connect.go are all non-loop cases that log before swallowing an error. The CHANGELOG entry for this PR even notes that ERR-5 was explicitly applied to slog.Debug before continue in the QUIC ACK frame handler — but the probe functions themselves have the same pattern unfixed.

The probe functions conflate two distinct outcomes into the same silent false:

1. The server does not support the probed cipher/group (expected, normal)
2. A genuine error: connection refused, DNS failure, malformed response, timeout

Without any logging, unexpected failures are invisible during debugging. Example from tls13probe.go:

// https://github.com/sensiblebit/certkit/blob/0c24518d10590e91e5384bc61ce97dc518a1f86b/tls13probe.go#L391-L395
conn, err := dialer.DialContext(ctx, "tcp", input.addr)
if err != nil {
    return false  // ERR-5: no slog.Debug
}

This pattern appears ~20 times across the four probe files. The fix is to add slog.Debug before each silent return false — for example:

conn, err := dialer.DialContext(ctx, "tcp", input.addr)
if err != nil {
    slog.Debug("tls13 cipher probe dial failed", "addr", input.addr, "cipher", input.cipherID, "error", err)
    return false
}