Fix release-blocking defects found by the fresh-user release test - #3
Merged
Merged
Conversation
The pre-release fresh-user pass found five defects that each silently produced a confident wrong answer from a documented code path. --port was declared and documented as overriding the port in the target, but was never read, so 'tlsanalyzer host -p 8443' scanned port 443 and graded a port the user never asked about. Only the host:port form worked. An unreachable host produced a fabricated report: a name that did not resolve, or a refused port, yielded F (0/100), risk CRITICAL, and exit 0 with no error field. A host that was never contacted was indistinguishable from one measured to be insecure, and in a --targets sweep every typo became a confident failing row. Both are now errors with a non-zero exit. Only the negotiated cipher suite was reported, then presented as the server's whole configuration. That produced a grade rationale of 'all ciphers have forward secrecy' after a single handshake, and a false CNSA 2.0 violation against servers that do offer AES-256. TLS 1.2 and earlier suites are now enumerated by probing each one. TLS 1.3 suites cannot be, because Go ignores per-suite configuration for TLS 1.3, and that limit is now stated in scanWarnings rather than left implicit. --targets with --format json emitted concatenated objects rather than an array, so the documented batch workflow produced JSON that strict parsers reject. --skip-quantum still charged the full 25-point maximum while scoring zero, so declining to run an analysis dropped the grade a whole band and reported a quantum score of 0 for servers that verifiably negotiate a hybrid ML-KEM group. A check that did not run now contributes neither points nor maximum. Enumerating the full cipher list then exposed two further problems it had been masking. ChaCha20 and 3DES suites reported a key size of 0, making ChaCha20-Poly1305 read as weaker than AES-128. And the cipher score awarded bonuses per suite and saturated at the maximum, so a server offering non-forward-secret RSA suites scored 25/25 while the policy evaluator listed sixteen violations against the same configuration. Cipher strength is now graded by the weakest accepted suite, since negotiation can be steered toward it. Verified against openssl: github.com does accept AES128-SHA with no forward secrecy, and example.com does offer AES-256, which the previous single-suite enumeration had reported as a violation. README sample output regenerated from the fixed build.
The re-test confirmed three of five fixes but blocked on two.
--skip-quantum still fabricated a negative post-quantum finding. Excluding the
assessment from the grade was not enough: the report continued to print
Quantum Ready QV, a failed hybrid PQC checklist item, hybridPqcReady false, and
a priority-1 recommendation to enable hybrid key exchange, for servers that
demonstrably negotiate X25519MLKEM768 and whose CNSA section in the same report
listed that group as approved. A suppressed check now yields no quantum
findings, no quantum recommendations, and a quantum grade of 'not assessed'.
IPv6 targets could not be scanned at all. Connection addresses were built with
fmt.Sprintf("%s:%d", host, port), producing ::1:19502 rather than [::1]:19502,
so every IPv6 target failed as unreachable and looked like a real finding.
Verified against a local TLS server on [::1] that OpenSSL also reaches.
Also from the same pass: scan coverage warnings existed only in JSON and were
invisible in text, the default format; the cipher enumeration warning claimed
TLS 1.2 suites were 'enumerated in full' when Go does not implement the
CBC-SHA256 and CBC-SHA384 families, so a server can accept suites that do not
appear; and an unrecognized --format silently produced text with exit 0, so a
pipeline asking for --format JSON got text and a success code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The pre-release fresh-user pass on v0.3.0 found five P1 defects. Each one silently produced a confident wrong answer from a documented code path, so the release was blocked. All five verified independently before fixing.
P1 findings
--portnever worked. Declared and documented as "overrides port in target", but never read.tlsanalyzer host -p 8443scanned port 443 and graded a port the user never asked about. Only thehost:portform worked.Unreachable hosts produced fabricated reports. A name that did not resolve, or a refused port, yielded
F (0/100),Risk: CRITICAL,HNDL: CRITICAL - no forward secrecy, and exit 0 with no error field anywhere. A host that was never contacted was indistinguishable from one measured to be insecure. In a--targetssweep, every typo or decommissioned host became a confident failing row. Now an error with a non-zero exit.Only the negotiated cipher suite was reported, then presented as the whole configuration. Consequences: a grade rationale of "all ciphers have forward secrecy" after a single handshake, and a false CNSA 2.0 violation (
key size below minimum, 128 bits) against servers that do offer AES-256. TLS 1.2 and earlier suites are now enumerated by probing each one.TLS 1.3 suites cannot be enumerated this way, because Go ignores
Config.CipherSuitesfor TLS 1.3 and always offers all three. Rather than leave that implicit, it is now stated inscanWarnings, so an absent suite is never read as an unsupported one.--targets --format jsonemitted invalid JSON, as concatenated objects rather than an array. This is a headline example in--help; every strict parser rejected it.--skip-quantumlowered the grade and printed false claims. The skipped assessment still counted zero points against a full 25-point maximum, so declining to run an analysis dropped the grade a whole band and reported a quantum score of 0 for servers that verifiably negotiate a hybrid ML-KEM group.Found because of the fixes
Enumerating the full cipher list exposed two problems the single-suite view had been masking:
Cipher Strength 25/25while the policy evaluator listed sixteen violations against that same configuration. Cipher strength is now graded by the weakest accepted suite, since negotiation can be steered toward it.Verification
Each claim cross-checked against openssl rather than the tool itself:
github.comdoes acceptAES128-SHAwith no forward secrecy (New, SSLv3, Cipher is AES128-SHA), so its reduced cipher score is honest.example.comdoes offerTLS_AES_256_GCM_SHA384, confirming the previous CNSA violation was false.Regression tests added for all five P1s plus the two follow-on defects, including IPv6 handling in the port override.
go test -race ./...clean. The README sample output was regenerated from the fixed build, since the previous one displayed the false CNSA violation.