From 8bd91fc0445aaba8bedb5d73ae17d818e6c2681e Mon Sep 17 00:00:00 2001 From: Khalid Saifullah Khan Date: Thu, 22 Oct 2026 01:47:08 +0530 Subject: [PATCH] fix: reports on real servers say the right thing (PRD-015, eval/12) From the owner's review of their own scans (badssl.com and others), as a cryptographer, QA and PM: Scanner - When Go's TLS client cannot complete a handshake (3DES, RC4, NULL or small-DH suites only; an RSA certificate kept for RSA-only clients), the served chain is read from the raw TLS 1.2-and-earlier flights. Before, the no-SNI fallback certificate stood in for it. - The DH modulus is tested for primality and for being a safe prime. Evaluation (eval/12; stored scans are re-evaluated on start) - KX.DH-COMPOSITE (critical), KX.DH-NOT-SAFE-PRIME (low). - A missing issuer is an intermediate only when the leaf names its download URL (AIA); otherwise a root the stores do not hold. - CERT.COMPROMISED-ROOT for Superfish, eDellRoot, DSDTestProvider. - Revocation is "unknown" unless a verified staple vouches for it. - The untrusted-root wording covers public roots since removed. - The list's certificate expiry is the served leaf's, refreshed on re-evaluation (it showed the expired no-SNI placeholder). Screens - Fix first: three rows, one "when" chip each, the rest folded. - Every TLS version starts folded; version heads, suite rows and Fix first rows line up (measured: E2E-VIS-ALIGN). CRYPTO-REVIEW-003 and QA-REPORT-014 sign off; journey 240/0. --- CHANGELOG.md | 16 +++ docs/methodology.md | 17 +++ docs/product/CRYPTO-REVIEW-003-real-scans.md | 82 ++++++++++++ docs/product/PRD-013-visual-rebuild.md | 2 +- .../product/PRD-015-accuracy-on-real-scans.md | 54 ++++++++ docs/program/QA-REPORT-014.md | 71 ++++++++++ docs/program/RTM.md | 15 +++ internal/archtest/rtm_test.go | 1 + internal/eval/eval.go | 2 +- internal/eval/eval_t1_test.go | 59 ++++++++- internal/eval/passtext.go | 7 +- internal/eval/pkiview.go | 12 +- internal/eval/rules.go | 3 + internal/eval/rules_t1.go | 125 +++++++++++++++--- internal/eval/sections.go | 25 +++- internal/pki/pkitest/pkitest.go | 11 ++ internal/reportpdf/endpoint.go | 2 + internal/server/certexpiry_internal_test.go | 26 ++++ internal/server/evaluate.go | 1 + internal/server/runner.go | 7 +- internal/store/anchors.go | 4 +- internal/tlsprobe/certs.go | 61 +++++++-- internal/tlsprobe/primality_internal_test.go | 32 +++++ internal/tlsprobe/report.go | 9 +- internal/tlsprobe/scan.go | 50 ++++++- internal/tlsprobe/scan_legacy_test.go | 17 +++ internal/tlsprobe/testserver/legacy.go | 26 ++++ testing/e2e/t1-phase.mjs | 2 +- testing/e2e/visual-phase.mjs | 63 ++++++++- web/static/js/views/help.js | 2 +- web/static/js/views/report.js | 75 +++++++---- web/static/js/views/tls-config.js | 18 ++- web/static/style.css | 45 +++++++ 33 files changed, 856 insertions(+), 86 deletions(-) create mode 100644 docs/product/CRYPTO-REVIEW-003-real-scans.md create mode 100644 docs/product/PRD-015-accuracy-on-real-scans.md create mode 100644 docs/program/QA-REPORT-014.md create mode 100644 internal/server/certexpiry_internal_test.go create mode 100644 internal/tlsprobe/primality_internal_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 371428c..6eb4179 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,22 @@ the CBOM/report/eval schema versions, the database) can change in a minor releas ### Fixed +- **Reports on real servers say the right thing** ([PRD-015](docs/product/PRD-015-accuracy-on-real-scans.md), + [CRYPTO-REVIEW-003](docs/product/CRYPTO-REVIEW-003-real-scans.md); eval/12, stored scans are re-evaluated on start): + - a server whose only suites Go's TLS client will not offer (3DES, RC4, NULL, small DH) had its no-SNI fallback + certificate reported as its own; the served chain is now read from the scanner's raw TLS 1.2 handshakes, which + also finds an RSA certificate a server keeps for RSA-only clients. Scan such servers again; + - a leaf signed straight by an unknown root is no longer called a missing intermediate; + - a chain under Superfish, eDellRoot or DSDTestProvider is *The certificate chains to a root whose private key is + public* (critical); + - revocation reads *Status unknown*, not *Good*, unless a stapled OCSP response vouches for the certificate; + - an untrusted root is described as a private CA or a public root since removed, not only as a private one; + - a composite DH modulus is critical, and a DH prime that is not a safe prime is low; + - at equal bits, the weakest classical strength picks a disallowed algorithm (3DES) over an allowed one. +- **Fix first** shows the three things to fix first, each with one chip saying when, and folds the rest under one + line that counts them by severity. +- **Every TLS version on Configuration starts folded**, its head saying whether it should be on and how its suites + rate; version heads, suite rows and Fix first rows line up. - **SSH evaluation (eval/11; stored scans are re-evaluated on start):** - a truncated MAC is as strong as its tag: hmac-sha1-96 is 96 bits, not 128 (NIST SP 800-107 Rev. 1); - the next steps name OpenSSH releases only to a server that identifies as OpenSSH; diff --git a/docs/methodology.md b/docs/methodology.md index 414bf2a..eb7d567 100644 --- a/docs/methodology.md +++ b/docs/methodology.md @@ -102,6 +102,23 @@ chain. For internal endpoints, add the private CA to the system's CA bundle (or result is re-evaluated at once and reads "trusted by your CA bundle". Private chains are exempt from the public lifetime limit and the CT requirement. The readiness level is unaffected by trust. +### Rules and changes in eval/12 (PRD-015) + +| Rule | Severity | Fires when | Source | +|---|---|---|---| +| `KX.DH-COMPOSITE` | critical | the DHE modulus fails a primality test (Miller-Rabin with 20 bases plus Baillie-PSW); `KX.DH-SMALL` is then not raised | RFC 7919, NIST SP 800-56A Rev. 3 | +| `KX.DH-NOT-SAFE-PRIME` | low | the DHE prime is not a safe prime ((p-1)/2 not prime) and is not a published group | RFC 7919, CVE-2016-0701 | +| `CERT.COMPROMISED-ROOT` | critical | the chain is issued under Superfish, eDellRoot or DSDTestProvider, roots installed with their private keys; `CERT.UNTRUSTED` is then not raised | CVE-2015-2077, RFC 5280 | + +- **The certificate a server serves is always read.** When Go's TLS client cannot complete a handshake (3DES, RC4, + NULL or small-DH suites only; an RSA certificate a server keeps for RSA-only clients), the chain is read from the + plaintext Certificate message of the scanner's own TLS 1.2-and-earlier handshakes (source `raw-tlsv1.2`). +- **A missing issuer** is called a missing intermediate only when the served path stops at a certificate that names + where its issuer can be downloaded (AIA caIssuers); otherwise it is a root the stores checked do not hold: a + private CA, or a public root since removed. +- **Revocation** is *good* only when a verified stapled OCSP response says so. Otherwise its status is *unknown*: the + scan does not contact the CA's OCSP responder or CRL. The revocation checks still count as run. + ### Rules added in eval/3 (PRD-004) | Rule | Severity | Fires when | Source | diff --git a/docs/product/CRYPTO-REVIEW-003-real-scans.md b/docs/product/CRYPTO-REVIEW-003-real-scans.md new file mode 100644 index 0000000..fb726e8 --- /dev/null +++ b/docs/product/CRYPTO-REVIEW-003-real-scans.md @@ -0,0 +1,82 @@ +# CRYPTO-REVIEW-003 · The owner's real scans, finding by finding (PRD-015) + +Date: 2026-10-22. Reviewer role: cryptographer (PRD-015 REQ-ACC-10). Data: a copy of the owner's database +(`dist/cipherhorizon.db`, 55 TLS endpoints, one SSH server), re-evaluated with eval/12, and eight hosts scanned +again with this build (3des, rc4, dh-composite, dh-small-subgroup, superfish, revoked, null at badssl.com, and +www.google.com). badssl.com publishes what each host demonstrates, so each report is checked against a known +answer. The owner's own database was not changed. + +## Verdict + +**Signed off** after the fixes below. Every badssl.com host now reports the defect it was built to show, under the +right name and severity; no host reports a defect it does not have, with the two limits stated at the end. + +## Host by host + +| Host | It demonstrates | Before | Now | +|---|---|---|---| +| expired | an expired leaf | CERT.EXPIRED | same: correct | +| wrong.host | a name mismatch | CERT.NAME-MISMATCH | same | +| self-signed | a self-signed leaf | CERT.UNTRUSTED ("self-signed") | same | +| untrusted-root | an unknown root | CERT.UNTRUSTED | same, with "a root the stores checked do not hold" | +| revoked | a revoked certificate | Revocation **Good**, nothing said | Revocation **unknown**: no staple, the CA is not contacted (REQ-ACC-5) | +| incomplete-chain | a missing intermediate | CERT.CHAIN-INCOMPLETE | same: the leaf names its issuer's URL (AIA), so the intermediate is truly missing | +| sha384, sha512, extended-validation, 1000-sans, 10000-sans | (now) an expired leaf under a retired root | EXPIRED, and UNTRUSTED saying "a private or unknown root: add it to your CA bundle" | EXPIRED, and UNTRUSTED saying "either a private CA or a public root since removed" (REQ-ACC-6). DigiCert Global Root CA is absent from both bundled store snapshots. | +| superfish, edellroot, dsdtestprovider | a root whose private key is public | EXPIRED, **CHAIN-INCOMPLETE** (wrong) and UNTRUSTED | **CERT.COMPROMISED-ROOT** (critical) and EXPIRED (REQ-ACC-3, 4) | +| preact-cli, webpack-dev-server | a dev-server certificate with a published key | CHAIN-INCOMPLETE (wrong), UNTRUSTED | UNTRUSTED with "a root the stores checked do not hold" (the published key itself is not detectable: see limits) | +| 3des, rc4, rc4-md5, null, dh480, dh512 | weak suites | correct suite findings, but the **certificate was the no-SNI fallback** | on a new scan, the served `*.badssl.com` certificate, read from the raw TLS 1.2 flight (REQ-ACC-1); checked on 3des, rc4 and null, scanned again | +| dh480, dh512, dh1024 | a small DH prime | KX.DH-SMALL (high, high, medium) | same | +| dh2048 | a 2048-bit group (good) | nothing | nothing | +| dh-composite | a composite modulus | KX.DH-SMALL "a small prime, 2047 bits" (wrong word) | **KX.DH-COMPOSITE** (critical), no "small prime" (REQ-ACC-2) | +| dh-small-subgroup | a prime that is not a safe prime | nothing | **KX.DH-NOT-SAFE-PRIME** (low) | +| static-rsa | RSA key transport only | NO-FS, plus NULL and RC4 | same: the host's `kRSA` cipher string does accept the NULL and RC4 RSA suites | +| null | NULL and anonymous suites | CIPHER.NULL, CIPHER.ANON (critical) | same, now with the served certificate | +| mozilla-modern, -intermediate, -old | Mozilla's three profiles | as expected (old: DH-SMALL, 3DES; modern: HSTS short only) | same | +| no-sct | no CT proof | CT.NO-SCTS (with EXPIRED: the certificate has since expired) | same | +| client, client-cert-missing | a client-certificate request | FEAT.CLIENT-CERT-REQUESTED (info) | same | +| hsts, preloaded-hsts | HSTS | HTTP.HSTS-SHORT where max-age < 1 year | same | +| tls-v1-0, tls-v1-1, tls-v1-2 | one version each | PROTO.NO-TLS12 for 1.0 and 1.1 | same | +| www.google.com | (a large site) | RSA suites with "?" key exchange and signature | the RSA-2048 certificate is now read; TLS 1.3 suites rate *Acceptable* (112-bit authentication for a client offering only RSA signatures), consistent with rating every suite by the weakest key it can be authenticated with | + +## Numbers and claims checked + +- **Primality.** `math/big.ProbablyPrime(20)`: Miller-Rabin with 20 random bases plus Baillie-PSW, which has no + known counterexample. A safe prime is p with (p-1)/2 prime; RFC 7919's groups are safe primes, and RFC 5114's + are not but publish their subgroup order, so a prime matching a published group is not flagged. +- **A composite modulus.** The discrete logarithm modulo a composite reduces to logarithms modulo its factors + (Pohlig-Hellman over the factorisation), so a 2047-bit composite gives far less than 2047-bit security. + Critical, and Logjam-labelled: an attacker able to factor or who chose the modulus can read the sessions. +- **Not a safe prime.** Small subgroups exist; a server that reuses its exponent and does not check the peer's + value leaks it piece by piece (CVE-2016-0701, Lim-Lee). Low: modern servers use fresh exponents. +- **Compromised roots.** Superfish (CVE-2015-2077), eDellRoot and DSDTestProvider (Dell, 2015) were installed with + their private keys. Matched by the issuer's common name, which is enough here: the chain is untrusted by every + store either way, and the finding names *why* that matters (any machine that installed the root trusts + certificates anyone can make). +- **The missing-issuer guess.** Public CAs put an AIA caIssuers URL in every leaf they issue from an intermediate + (the CA/Browser Forum Baseline Requirements' certificate profiles); a leaf with none was most likely signed by a root directly. On the owner's data + this separates incomplete-chain (has AIA) from the five rogue roots (none) without error. +- **Revocation.** Without a verified stapled response saying *good*, revocation is unknown to a scanner that does + not contact the CA. Let's Encrypt no longer runs OCSP at all (2025), so a CRL URL alone is now common. + +## Defects found and fixed in this branch + +1. The no-SNI fallback certificate stood in for the served one on servers Go's client cannot reach (scanner). +2. Google's RSA certificate was never read (scanner): every RSA suite had unknown parts. +3. Rogue-root leaves were "missing an intermediate" (evaluation). +4. Compromised roots were not named (evaluation). +5. Revocation read "good" with no evidence (evaluation, a new section status `unknown`). +6. The untrusted-root advice was wrong for public roots since removed (wording). +7. A composite DH modulus passed as "a small prime"; a non-safe prime was not seen (scanner and evaluation). +8. The weakest-strength figure chose RSA-2048 over 3DES at equal bits; at equal bits a disallowed algorithm is + now the weaker (screen). + +## Limits, stated on the page or in Help + +- The CA's OCSP responder and CRL are not contacted (PRD-015, out of scope); revocation says *unknown*. +- A leaf whose private key was published with a tool (preact-cli, webpack-dev-server) is not recognised as such; + it reads *untrusted root*, which is true. +- Scans made before this version keep the certificate they recorded; scanning again reads the served one. + +## Sign-off + +Signed off for PRD-015 REQ-ACC-10, cryptography. diff --git a/docs/product/PRD-013-visual-rebuild.md b/docs/product/PRD-013-visual-rebuild.md index b470a4b..e51dd0f 100644 --- a/docs/product/PRD-013-visual-rebuild.md +++ b/docs/product/PRD-013-visual-rebuild.md @@ -21,4 +21,4 @@ Results: QA-REPORT-012. | **REQ-VIS-10** | **SSH report** algorithm tables use the bits bar and the post-quantum glyph. | | `script:testing/e2e/visual-phase.mjs#E2E-VIS-6` | P2 | | **REQ-VIS-11** | **Fit and access** (§4): no sideways scroll at 1440, 1100, 860 and 375 px; every control reachable by Tab; dark mode; contrast AA. | | `script:testing/e2e/visual-phase.mjs#E2E-VIS-7` | P0 | | **REQ-VIS-12** | **Sign-off.** A cryptographer checks every number and claim the new screens show, against its source (CRYPTO-REVIEW-001); the PM signs off the walk (QA-REPORT-012). | | `report:docs/product/CRYPTO-REVIEW-001-visual-rebuild.md` | P0 | -| **REQ-VIS-13** | **Versions fold** (owner request, 2026-10-21): each TLS version that is on is open only when it needs a look (an old version on, a suite below Strong, or the only version); folded, its head says how its suites rate; one button shows or hides every version and says which it will do. | A reviewer sees first what there is to do | `script:testing/e2e/visual-phase.mjs#E2E-VIS-FOLD` | P1 | +| **REQ-VIS-13** | **Versions fold** (owner request, 2026-10-21): every TLS version that is on starts folded (the owner's second request, 2026-10-22; the only version opens by itself); folded, its head says whether it should be on and how its suites rate; one button shows or hides every version and says which it will do. | A reviewer sees first what there is to do | `script:testing/e2e/visual-phase.mjs#E2E-VIS-FOLD` | P1 | diff --git a/docs/product/PRD-015-accuracy-on-real-scans.md b/docs/product/PRD-015-accuracy-on-real-scans.md new file mode 100644 index 0000000..82559bd --- /dev/null +++ b/docs/product/PRD-015-accuracy-on-real-scans.md @@ -0,0 +1,54 @@ +# PRD-015 · Accuracy on real scans + +Status: draft 2026-10-22, from the owner's review of their own database (55 scans, mostly the badssl.com test +hosts, plus google.com, qualys.com and their own sites): "a lot of things don't make sense", "the alignment on many +of them is so bad", "things to fix looks broken", "the menus of TLS should be collapsed rather than expanded". +Reviewed as a cryptographer ([CRYPTO-REVIEW-003](CRYPTO-REVIEW-003-real-scans.md)), as QA and as the PM +([QA-REPORT-014](../program/QA-REPORT-014.md)). + +## Why + +badssl.com is built so that each host shows one defect; it is the fairest test of whether the report says the +right thing. On the owner's scans it did not, in six ways a cryptographer would catch at once: + +1. **The wrong certificate.** On every server whose only suites Go's TLS client will not offer (3des, rc4, rc4-md5, + null, dh480, dh512 at badssl.com), the only certificate the report held was the server's *no-SNI fallback*. The + certificate section described a certificate the host never serves. Google's RSA certificate was never seen + either, so every RSA suite showed "?" for its key exchange and signature. +2. **The wrong diagnosis.** A leaf signed straight by an unknown root (superfish, edellroot, dsdtestprovider, + preact-cli, webpack-dev-server) was reported as a *missing intermediate*, with the advice to serve one. +3. **A known-compromised root not named.** Superfish, eDellRoot and DSDTestProvider are roots whose private keys + were published; the report said only "untrusted". +4. **"Good" for revocation that was never checked.** revoked.badssl.com read Revocation: Good. The scan never + contacts the CA, so the status is unknown. +5. **Advice that is wrong.** A chain ending at DigiCert Global Root CA (sha384, sha512, extended-validation), a + public root absent from both bundled store snapshots (Mozilla retires website trust for roots past 15 years + under its root-age policy), was "a private or unknown root: add it to your CA bundle". +6. **DH parameters judged by size alone.** dh-composite (a composite modulus) and dh-small-subgroup (a prime that + is not a safe prime) passed or were called "a small prime". + +And on the screens: Fix first listed every finding with two labels each (severity and urgency), rows and version +heads did not line up, and every TLS version opened by default. + +## Requirements + +| ID | Requirement | Acceptance | Test | Pri | +|---|---|---|---|---| +| **REQ-ACC-1** | **The served certificate is always read.** When crypto/tls cannot complete a handshake (3DES, RC4, NULL, small DH, an RSA certificate a server keeps for RSA-only clients), the chain is read from the raw TLS 1.2-and-earlier flights the suite enumeration receives, where the Certificate message is plaintext. The no-SNI comparison runs after. | A server with only 3DES and RC4 suites has its SNI chain, sourced `raw-tlsv1.2` | `TestCertificateIsReadFromARawFlight` | P0 | +| **REQ-ACC-2** | **DH parameters are tested, not just measured.** The modulus is tested for primality and, when prime, for being a safe prime. A composite modulus is a critical finding (and not also "a small prime"); a prime that is not a safe prime and not a published group is a low one. | ffdhe2048 is a safe prime; 2^127-1 is prime, not safe; a product of two primes is composite | `TestDHPrimality`, `TestEveryRuleFiresAndIsCited` | P0 | +| **REQ-ACC-3** | **The missing issuer is named for what it most likely is.** "Missing intermediate" only when the served path stops at a certificate that names where its issuer can be downloaded (AIA caIssuers, as public CAs' leaves do); otherwise "a root the stores checked do not hold". | A private root's leaf served alone is untrusted, not incomplete | `TestT1TrustMatrix` | P0 | +| **REQ-ACC-4** | **Known-compromised roots are named.** A chain issued under Superfish, eDellRoot or DSDTestProvider is CERT.COMPROMISED-ROOT (critical, clients fail), in place of "untrusted". | | `TestCompromisedRootIsItsOwnFinding` | P0 | +| **REQ-ACC-5** | **Revocation is "unknown" unless a staple vouches for it.** With no stapled, verified, good OCSP response, the revocation area's status is *unknown* (a new section status), with the reason; its checks still count as run. The page says "not contacted: revocation status unknown". | | `TestRevocationIsUnknownWithoutAStaple` | P0 | +| **REQ-ACC-6** | **An untrusted root is described truthfully**: "a root the stores checked do not hold: either a private CA, or a public root since removed from them". | | `TestT1TrustMatrix` | P1 | +| **REQ-ACC-7** | **Fix first is a list of priorities**: the first three rows, each with one chip saying when to act (coloured by severity, the severity in its tooltip); the rest folded under one line that counts them by severity; numbering runs on. | | `script:testing/e2e/visual-phase.mjs#E2E-FIX-1` | P0 | +| **REQ-ACC-8** | **Every TLS version starts folded**, its head saying whether it should be on and how its suites rate (the only version opens by itself). | | `script:testing/e2e/visual-phase.mjs#E2E-VIS-FOLD` | P0 | +| **REQ-ACC-9** | **Things on one line are on one line**: Fix first chips beside their titles, titles in one column; each strength chip centred on its suite row; version heads on one grid; no empty rank column when the client picks the order; the weakest strength prefers a disallowed algorithm at equal bits. Measured in the browser. | | `script:testing/e2e/visual-phase.mjs#E2E-VIS-ALIGN` | P0 | +| **REQ-ACC-10** | **Sign-off**: the cryptographer checks every finding the owner's scans produce against what each badssl.com host demonstrates (CRYPTO-REVIEW-003); QA and the PM walk the screens on the owner's data (QA-REPORT-014). The evaluation is eval/12, so stored scans are re-evaluated on start. | | `report:docs/product/CRYPTO-REVIEW-003-real-scans.md`, `report:docs/program/QA-REPORT-014.md` | P0 | + +## Out of scope + +- Contacting the CA's OCSP responder or CRL. The URLs come from the certificate, which the target chose; fetching + them is a request to an address an attacker picks, and stays a separate, opt-in decision. +- Detecting a published private key for a leaf (the preact-cli and webpack-dev-server certificates): it needs a + list of key fingerprints this product does not carry. Those hosts read "untrusted root", which is true. +- Reports scanned before this version keep the certificate they recorded: scan them again to read the served one. diff --git a/docs/program/QA-REPORT-014.md b/docs/program/QA-REPORT-014.md new file mode 100644 index 0000000..cf72de3 --- /dev/null +++ b/docs/program/QA-REPORT-014.md @@ -0,0 +1,71 @@ +# QA-REPORT-014 · Accuracy on real scans (PRD-015) + +Date: 2026-10-22. Build: branch `fix/real-scan-review` on top of `5109981` (main, PR #24). Requirements: +[PRD-015](../product/PRD-015-accuracy-on-real-scans.md), [RTM](RTM.md) §PRD-015. Cryptography: +[CRYPTO-REVIEW-003](../product/CRYPTO-REVIEW-003-real-scans.md). + +Data: a copy of the owner's database (`dist/cipherhorizon.db`: 55 TLS endpoints, 54 with a completed scan, one SSH +server), served by this build so every stored scan was re-evaluated with eval/12, and eight hosts scanned again. +The owner's own database was not written to. + +## Result + +| Gate | Command | Result | +|---|---|---| +| Unit and integration | `go test ./...` | pass | +| New tests | `TestCertificateIsReadFromARawFlight`, `TestDHPrimality`, `TestRevocationIsUnknownWithoutAStaple`, `TestCompromisedRootIsItsOwnFinding`, `TestCertExpiryIgnoresTheNoSNIChain` | pass | +| Traceability | `TestRequirementsTraceability` | 10 new rows (REQ-ACC-1 to 10), all done, each with evidence | +| Lint | `golangci-lint run ./...`, and with `GOOS=linux` | 0 issues, 0 issues | +| UI scripts parse | `node --check` on every module | all parse | +| Browser journey | `testing/e2e/journey.mjs`, the CI setup | **240 passed, 0 failed** | +| Screen walk | below, on the owner's data | done | +| Cryptography | CRYPTO-REVIEW-003 | signed off | + +## What the owner saw, and what was wrong + +| The owner's complaint | Cause | Fix | +|---|---|---| +| "A lot of things don't make sense" | Six evaluation and scanner errors (CRYPTO-REVIEW-003): the no-SNI fallback certificate reported as the server's on legacy-only servers; Google's RSA certificate never read; rogue roots called missing intermediates; compromised roots not named; revocation "good" unchecked; DH moduli judged by size only | REQ-ACC-1 to 6, eval/12 | +| The same, on the endpoints list | "Certificate expiry: expired 2996 days ago" on nearly every badssl.com host: the list took the earliest expiry of *every* chain, the no-SNI placeholder (expired 2018) included | The served leaf only; re-evaluation now refreshes the stored date (31 of the 54 now read valid, 13 expired, 12 have no served chain on record until scanned again) | +| "Things to fix looks broken" | Every finding was a "fix first" row, each with a severity badge *and* an urgency word, on a grid several old rules fought over | Three rows, one "when" chip each (coloured by severity), the rest folded with a count by severity; one clean grid (REQ-ACC-7) | +| "The alignment is so bad" | The strength chip sat 9 px above its row (an old `display: inline-flex` on the table cell); version heads wrapped differently per version; an empty rank column; area tiles centred | Measured, then fixed: 0 px; version heads on one grid; the rank column hidden when the client picks the order (REQ-ACC-9) | +| "TLS should be collapsed" | Versions opened when they needed a look | Every version starts folded, its head saying whether it should be on and how its suites rate (REQ-ACC-8) | + +## The new checks + +- **E2E-FIX-1** Fix first: the first three rows, one chip each, the rest folded under "N more things to fix" with a + count by severity, numbering running on, and no old severity badge or urgency word left. +- **E2E-VIS-ALIGN** In the browser: each Fix first chip's centre within 4 px of its title's first line, every title + at one x; every strength chip centred on its suite row within 2 px; every version head's name, facts and + verdict at one x down the section. +- **E2E-VIS-FOLD** (rewritten) Every version starts folded unless it is the only one; each folded head's tally + equals the evaluation's words per version; the show/hide button opens and closes them all and says which. +- **E2E-VIS-2** (changed) The Summary has four figures; a fifth, *Not checked*, only when a part of the scan did not + run. Revocation's *unknown* is the same on every endpoint and is said in its area, not counted here. +- **T1-E2E.6** (widened) A section's status word may also be *Status unknown*. + +## Screen walk (QA and PM), on the owner's data + +At 1440, 1100 (dark) and 375 px, by eye; 860 px by the journey's fit check. + +- **Summary** (superfish, ecc384, google): the headline and *First:* name the right thing (superfish: the + compromised root, not "expired"); Fix first shows three rows and folds the rest; the weakest strength names 3DES + where 3DES and RSA-2048 tie at 112; Revocation reads *Status unknown* with the reason. +- **Configuration** (google, ecc384): every version folded with its verdict; opened, the strength chips sit on the + row; the class lanes' values never collide with *weakest*; on a phone a long value ("112 bits, disallowed") + wraps instead of pushing the page 3 px wide (found by the walk, fixed). +- **Endpoints list**: expiry dates are the served certificates'. + +## PM: is the story met? + +The product says it is *a scanner that shows its work*. Before this, on the most standard test site there is, it +showed the wrong certificate for six hosts, the wrong diagnosis for five, and called unchecked revocation "good". +After it, every badssl.com host reports the defect it was built to show, under its right name, and what the scan +cannot know is said as *unknown*, never as *good*. Fix first is again a list of priorities. The story is met, with +two limits stated in PRD-015 (no CA contact; published leaf keys not recognised). + +## Sign-off + +- **QA:** all gates above pass on this build. Signed off. +- **Cryptography:** CRYPTO-REVIEW-003. Signed off. +- **PM:** PRD-015 REQ-ACC-1 to 10 met. Signed off for merge. **Not released.** diff --git a/docs/program/RTM.md b/docs/program/RTM.md index 62c455b..f14b380 100644 --- a/docs/program/RTM.md +++ b/docs/program/RTM.md @@ -421,6 +421,21 @@ WP-B to WP-G (SSH) ship in the second pull request, amended 2026-10-13 per [STRA | REQ-SSV-8 | P0 | — | UX-DESIGN-012 | — | script:testing/e2e/visual-phase.mjs#E2E-VIS-7, script:testing/e2e/visual-phase.mjs#E2E-VIS-9, script:testing/e2e/ssh-phase.mjs#E2E-H.2b | done | contrast measured on the SSH instruments too | | REQ-SSV-9 | P0 | — | UX-DESIGN-012 | — | report:docs/product/CRYPTO-REVIEW-002-ssh-instruments.md, report:docs/program/QA-REPORT-013.md | done | | +## PRD-015 · Accuracy on real scans ([PRD-015](../product/PRD-015-accuracy-on-real-scans.md); results in [QA-REPORT-014](QA-REPORT-014.md)) + +| Requirement | Pri | WP | Source | SR | Evidence | Status | Notes | +|---|---|---|---|---|---|---|---| +| REQ-ACC-1 | P0 | — | owner review | — | TestCertificateIsReadFromARawFlight | done | badssl.com 3des, rc4, null; google.com's RSA certificate | +| REQ-ACC-2 | P0 | — | owner review | — | TestDHPrimality, TestEveryRuleFiresAndIsCited | done | KX.DH-COMPOSITE, KX.DH-NOT-SAFE-PRIME | +| REQ-ACC-3 | P0 | — | owner review | — | TestT1TrustMatrix | done | AIA caIssuers decides intermediate or root | +| REQ-ACC-4 | P0 | — | owner review | — | TestCompromisedRootIsItsOwnFinding | done | Superfish, eDellRoot, DSDTestProvider | +| REQ-ACC-5 | P0 | — | owner review | — | TestRevocationIsUnknownWithoutAStaple | done | the new section status unknown | +| REQ-ACC-6 | P1 | — | owner review | — | TestT1TrustMatrix | done | | +| REQ-ACC-7 | P0 | — | owner review | — | script:testing/e2e/visual-phase.mjs#E2E-FIX-1 | done | | +| REQ-ACC-8 | P0 | — | owner review | — | script:testing/e2e/visual-phase.mjs#E2E-VIS-FOLD | done | supersedes REQ-VIS-13's open-when-needed rule | +| REQ-ACC-9 | P0 | — | owner review | — | script:testing/e2e/visual-phase.mjs#E2E-VIS-ALIGN | done | measured in the browser | +| REQ-ACC-10 | P0 | — | owner review | — | report:docs/product/CRYPTO-REVIEW-003-real-scans.md, report:docs/program/QA-REPORT-014.md | done | eval/12 | + ## Security requirements (SECARCH-001) coverage Each SR must be exercised by at least one requirement above; `TestRequirementsTraceability` checks that every `SR-nn` in SECARCH-001 appears in the SR column at least once, except those listed here as cross-cutting. diff --git a/internal/archtest/rtm_test.go b/internal/archtest/rtm_test.go index 4d9ca94..4cbdae3 100644 --- a/internal/archtest/rtm_test.go +++ b/internal/archtest/rtm_test.go @@ -28,6 +28,7 @@ var prdPaths = []string{ "../../docs/product/PRD-012-certificate-viewer.md", "../../docs/product/PRD-013-visual-rebuild.md", "../../docs/product/PRD-014-ssh-instruments.md", + "../../docs/product/PRD-015-accuracy-on-real-scans.md", } const ( diff --git a/internal/eval/eval.go b/internal/eval/eval.go index 387913e..3a18861 100644 --- a/internal/eval/eval.go +++ b/internal/eval/eval.go @@ -18,7 +18,7 @@ import ( // Version identifies the rule set that produced an Evaluation. Stored evaluations // keep the version they were made with, so history shows what was concluded then. -const Version = "eval/11" +const Version = "eval/12" // eval/7 (PRD-008): a post-quantum signature scheme proved accepted by the signature stage counts // toward L4, and SSH endpoints are judged into the same shape (Protocol "ssh"). diff --git a/internal/eval/eval_t1_test.go b/internal/eval/eval_t1_test.go index 6930a40..517f489 100644 --- a/internal/eval/eval_t1_test.go +++ b/internal/eval/eval_t1_test.go @@ -288,7 +288,9 @@ func TestT1TrustMatrix(t *testing.T) { nb := time.Date(2026, 4, 1, 0, 0, 0, 0, time.UTC) pleaf := priv.Leaf(t, pkitest.LeafOpts{NotBefore: nb, NotAfter: nb.AddDate(2, 0, 0)}) rep := v2(chainResult("tls-default", pleaf)) - expect(t, "private CA without the bundle", evalAt(rep, epoch, f.stores), "CERT.CHAIN-INCOMPLETE", "CERT.UNTRUSTED") + // A leaf signed straight by the root names no issuer URL: the root is unknown, not an + // intermediate missing (badssl.com superfish, preact-cli). + expect(t, "private CA without the bundle", evalAt(rep, epoch, f.stores), "CERT.UNTRUSTED") anchors, _ := pki.NewStore("private", "", false, []pki.Root{{DER: priv.Cert.Raw}}) ev = eval.EvaluateWith(rep, eval.Input{Now: epoch, Stores: f.stores, Anchors: anchors}) expect(t, "private CA with the bundle", ev) @@ -304,7 +306,7 @@ func TestT1TrustMatrix(t *testing.T) { if m := ev.PKI[0].Served.Missing; m == nil || m.Likely != "root" { t.Errorf("missing view %+v", m) } - if fs := findID(ev, "CERT.UNTRUSTED"); !strings.Contains(fs[0].Detail, "private or unknown root") { + if fs := findID(ev, "CERT.UNTRUSTED"); !strings.Contains(fs[0].Detail, "either a private CA") { t.Errorf("untrusted detail: %s", fs[0].Detail) } @@ -663,5 +665,58 @@ func t1Firing(t *testing.T) []eval.Evaluation { } out = append(out, evalAt(stapled(t, c.leaf, b), c.at, st)) } + // A root whose private key is public, and DH moduli that are not prime or not safe primes. + sf := pkitest.NewRoot(t, "Superfish, Inc.", pkitest.CAOpts{}) + out = append(out, evalAt(v2(chainResult("tls-default", sf.Leaf(t, pkitest.LeafOpts{}))), epoch, f.stores)) + no, yes := false, true + for _, d := range []*tlsprobe.DHResult{{PrimeBits: 2048, Suite: "TLS_DHE_RSA_WITH_AES_128_GCM_SHA256", IsPrime: &no}, + {PrimeBits: 2048, Suite: "TLS_DHE_RSA_WITH_AES_128_GCM_SHA256", IsPrime: &yes, SafePrime: &no}} { + r := v2(chainResult("tls-default", leaf, in)) + r.DH = d + out = append(out, evalAt(r, epoch, f.stores)) + } return out } + +// QA of real scans (badssl.com revoked): with no stapled response vouching for the certificate, +// and the CA never contacted, revocation is "unknown", never "good"; its checks still count as run. +// A good, verified staple makes it good. +func TestRevocationIsUnknownWithoutAStaple(t *testing.T) { + f := newPKI(t) + sectionOf := func(ev eval.Evaluation) eval.Section { + for _, s := range ev.Sections { + if s.ID == eval.SecRevocation { + return s + } + } + return eval.Section{} + } + ev := evalAt(v2(chainResult("tls-default", f.inter.Leaf(t, pkitest.LeafOpts{SCTs: sctOK}), f.inter.Cert)), epoch, f.stores) + if s := sectionOf(ev); s.Status != eval.StatusUnknown || s.ReasonCode != "not-contacted" || !strings.Contains(s.Reason, "not known") { + t.Errorf("no staple: section %+v", s) + } + for _, c := range ev.Checks { + if c.Section == eval.SecRevocation && c.Result == "not-checked" { + t.Errorf("check %s reads not-checked; it ran", c.ID) + } + } + good := evalAt(stapled(t, "leaf.pem", osslStaple(t, "ocsp-good")), osslAt(t), osslStore(t)) + if s := sectionOf(good); s.Status == eval.StatusUnknown { + t.Errorf("good staple: section %+v", s) + } +} + +// QA of real scans (badssl.com superfish): a chain to a root whose private key is public is that +// finding, critical and failing clients, not also "untrusted" for the same reason. +func TestCompromisedRootIsItsOwnFinding(t *testing.T) { + f := newPKI(t) + sf := pkitest.NewRoot(t, "eDellRoot", pkitest.CAOpts{}) + ev := evalAt(v2(chainResult("tls-default", sf.Leaf(t, pkitest.LeafOpts{}))), epoch, f.stores) + fs := findID(ev, "CERT.COMPROMISED-ROOT") + if len(fs) != 1 || fs[0].Severity != eval.Critical || !fs[0].Availability || !strings.Contains(fs[0].Detail, "eDellRoot") { + t.Fatalf("compromised root: %+v", fs) + } + if len(findID(ev, "CERT.UNTRUSTED")) != 0 || len(findID(ev, "CERT.CHAIN-INCOMPLETE")) != 0 { + t.Errorf("also untrusted or incomplete: %v", t1IDs(ev)) + } +} diff --git a/internal/eval/passtext.go b/internal/eval/passtext.go index d33896c..ee22141 100644 --- a/internal/eval/passtext.go +++ b/internal/eval/passtext.go @@ -23,8 +23,11 @@ var passText = map[string]string{ "CIPHER.IANA-DISCOURAGED": "No accepted suite is marked Discouraged by IANA", "CIPHER.CBC": "No CBC-mode suite is accepted", - "KX.DH-SMALL": "No small Diffie-Hellman prime is used", - "KX.DH-COMMON-PRIME": "No small, widely shared Diffie-Hellman prime is used", + "KX.DH-SMALL": "No small Diffie-Hellman prime is used", + "KX.DH-COMMON-PRIME": "No small, widely shared Diffie-Hellman prime is used", + "KX.DH-COMPOSITE": "The Diffie-Hellman modulus is a prime", + "KX.DH-NOT-SAFE-PRIME": "The Diffie-Hellman prime is a safe prime or a published group", + "CERT.COMPROMISED-ROOT": "The chain does not end at a root whose private key is public", "CERT.EXPIRED": "The certificate has not expired", "CERT.NOT-YET-VALID": "The certificate is already valid", diff --git a/internal/eval/pkiview.go b/internal/eval/pkiview.go index 229530d..0a7862a 100644 --- a/internal/eval/pkiview.go +++ b/internal/eval/pkiview.go @@ -69,10 +69,12 @@ type ServedView struct { type MissingView struct { IssuerDN string `json:"issuer"` AIA []string `json:"aia,omitempty"` // caIssuers URLs, not fetched - // Likely is "intermediate" when only the leaf was served (a server that - // forgot its intermediate), and "root" when the served path already ends at - // a CA certificate: then the unknown issuer is most likely a root no store - // holds (a private CA), which servers do not send anyway. + // Likely is "intermediate" when the served path stops at a certificate that + // names where its issuer can be downloaded (AIA caIssuers, as public CAs' + // leaves do): a server that forgot its intermediate. It is "root" when the + // path already ends at a CA certificate, or the leaf names no issuer URL (a + // leaf signed straight by a root): then the unknown issuer is most likely a + // root no store holds, which servers do not send anyway. Likely string `json:"likely"` } @@ -190,7 +192,7 @@ func (c *ctx) analyseChains() { ch.view.Served.ExpiresFirst = s.ExpiresFirst() if m, dn, aia := s.MissingIssuer(all...); m { likely := "intermediate" - if top := s.Top(); top != leaf && top.IsCA { + if top := s.Top(); (top != leaf && top.IsCA) || len(aia) == 0 { likely = "root" } ch.missing = likely == "intermediate" diff --git a/internal/eval/rules.go b/internal/eval/rules.go index 89af5a3..5e25dbf 100644 --- a/internal/eval/rules.go +++ b/internal/eval/rules.go @@ -263,6 +263,9 @@ var rules = []rule{ if c.rep.DH == nil || c.rep.DH.PrimeBits >= 2048 { return nil } + if d := c.rep.DH; d.IsPrime != nil && !*d.IsPrime { + return nil // not a prime at all: KX.DH-COMPOSITE says so + } sev := Medium if c.rep.DH.PrimeBits < 1024 { sev = High diff --git a/internal/eval/rules_t1.go b/internal/eval/rules_t1.go index 9879f65..764811a 100644 --- a/internal/eval/rules_t1.go +++ b/internal/eval/rules_t1.go @@ -13,21 +13,32 @@ import ( // the staple or the SCTs run only on tls-report/2 (v2Only). var ( - citRFC5246 = rfc("5246", "The Transport Layer Security (TLS) Protocol Version 1.2") - citRFC6960 = rfc("6960", "X.509 Internet Public Key Infrastructure Online Certificate Status Protocol - OCSP") - citRFC7633 = rfc("7633", "X.509v3 Transport Layer Security (TLS) Feature Extension") - citRFC6066 = rfc("6066", "Transport Layer Security (TLS) Extensions: Extension Definitions") - citRFC6962 = rfc("6962", "Certificate Transparency") - citRFC7919 = rfc("7919", "Negotiated Finite Field Diffie-Hellman Ephemeral Parameters for TLS") - citSC081 = Citation{Title: "CA/Browser Forum Ballot SC-081v3: schedule of reducing validity periods (BR §6.3.2)", URL: "https://cabforum.org/2025/04/11/ballot-sc081v3-introduce-schedule-of-reducing-validity-and-data-reuse-periods/"} - citChromeRP = Citation{Title: "Chrome Root Program Policy (dedicated serverAuth hierarchies)", URL: "https://www.chromium.org/Home/chromium-security/root-ca-policy/"} - citChromeCT = Citation{Title: "Chrome Certificate Transparency Policy", URL: "https://googlechrome.github.io/CertificateTransparency/ct_policy.html"} - citLEOCSP = Citation{Title: "Let's Encrypt: ending OCSP support (2025)", URL: "https://letsencrypt.org/2024/12/05/ending-ocsp/"} - citHSTSPre = Citation{Title: "HSTS preload list submission requirements", URL: "https://hstspreload.org/"} - citLucky13 = cve("CVE-2013-0169", "Lucky Thirteen: CBC padding timing") - citDROWN = cve("CVE-2016-0800", "DROWN: SSLv2 cross-protocol attack") + citRFC5246 = rfc("5246", "The Transport Layer Security (TLS) Protocol Version 1.2") + citRFC6960 = rfc("6960", "X.509 Internet Public Key Infrastructure Online Certificate Status Protocol - OCSP") + citRFC7633 = rfc("7633", "X.509v3 Transport Layer Security (TLS) Feature Extension") + citRFC6066 = rfc("6066", "Transport Layer Security (TLS) Extensions: Extension Definitions") + citRFC6962 = rfc("6962", "Certificate Transparency") + citRFC7919 = rfc("7919", "Negotiated Finite Field Diffie-Hellman Ephemeral Parameters for TLS") + citSC081 = Citation{Title: "CA/Browser Forum Ballot SC-081v3: schedule of reducing validity periods (BR §6.3.2)", URL: "https://cabforum.org/2025/04/11/ballot-sc081v3-introduce-schedule-of-reducing-validity-and-data-reuse-periods/"} + citChromeRP = Citation{Title: "Chrome Root Program Policy (dedicated serverAuth hierarchies)", URL: "https://www.chromium.org/Home/chromium-security/root-ca-policy/"} + citChromeCT = Citation{Title: "Chrome Certificate Transparency Policy", URL: "https://googlechrome.github.io/CertificateTransparency/ct_policy.html"} + citLEOCSP = Citation{Title: "Let's Encrypt: ending OCSP support (2025)", URL: "https://letsencrypt.org/2024/12/05/ending-ocsp/"} + citHSTSPre = Citation{Title: "HSTS preload list submission requirements", URL: "https://hstspreload.org/"} + citLucky13 = cve("CVE-2013-0169", "Lucky Thirteen: CBC padding timing") + citDROWN = cve("CVE-2016-0800", "DROWN: SSLv2 cross-protocol attack") + citSP80056A = Citation{Title: "NIST SP 800-56A Rev. 3: Pair-Wise Key-Establishment Using Discrete Logarithm Cryptography (domain parameters, section 5.5)", URL: "https://csrc.nist.gov/pubs/sp/800/56/a/r3/final"} + citDHSubgroup = cve("CVE-2016-0701", "OpenSSL: DH small-subgroup key recovery with primes that are not safe primes") + citSuperfish = cve("CVE-2015-2077", "Superfish: a root certificate installed with its private key") ) +// compromisedRoots are roots whose private keys were published with the software that installed +// them, matched by the issuer's common name: anyone can issue a certificate under them. +var compromisedRoots = map[string]string{ + "Superfish, Inc.": "Superfish, installed on Lenovo laptops in 2014 and 2015 with its private key", + "eDellRoot": "eDellRoot, installed on Dell computers in 2015 with its private key", + "DSDTestProvider": "DSDTestProvider, installed by Dell System Detect in 2015 with its private key", +} + // chainRule runs fn on every analysed chain of a tls-report/2. func chainRule(id, title, category string, sev Severity, cites []Citation, remed string, fn func(c *ctx, ch *chain) (string, Severity, bool)) rule { return rule{ @@ -206,8 +217,8 @@ var t1Rules = []rule{ []Citation{citRFC5280}, "Install a certificate from a CA your clients trust and serve the full chain, or add your private CA to this system's CA bundle if the certificate is meant to be private.", func(c *ctx, ch *chain) (string, Severity, bool) { - if len(ch.trustedBy) > 0 || ch.private || ch.expired { - return "", "", false // trusted somewhere, or expiry has its own finding + if len(ch.trustedBy) > 0 || ch.private || ch.expired || compromisedRootOf(ch) != "" { + return "", "", false // trusted somewhere, or expiry or a compromised root has its own finding } why := ch.trustSummary() m := ch.view.Served.Missing @@ -215,7 +226,7 @@ var t1Rules = []rule{ case ch.missing: why = "an intermediate is missing (see CERT.CHAIN-INCOMPLETE); " + why case m != nil && m.Likely == "root": - why = fmt.Sprintf("the chain ends at a CA issued by %q, which no trusted store holds (a private or unknown root: add it to the system's CA bundle if it is yours); ", m.IssuerDN) + why + why = fmt.Sprintf("the chain ends at a certificate issued by %q, a root the stores checked do not hold: either a private CA (add it to this system's CA bundle if it is yours) or a public root since removed from them; ", m.IssuerDN) + why case pki.SelfSigned(ch.served.Leaf()): why = "self-signed; " + why } @@ -224,6 +235,15 @@ var t1Rules = []rule{ } return ch.label() + ": " + why, "", true }), + chainRule("CERT.COMPROMISED-ROOT", "The certificate chains to a root whose private key is public", "certificate", Critical, + []Citation{citSuperfish, citRFC5280}, + "Remove this root from every machine that trusts it and replace the certificate with one from a CA your clients trust. Anyone can issue certificates under this root, so a client that trusts it can be intercepted.", + func(c *ctx, ch *chain) (string, Severity, bool) { + if what := compromisedRootOf(ch); what != "" { + return fmt.Sprintf("%s: issued under %s; no root store trusts it, but any machine that installed it does", ch.label(), what), "", true + } + return "", "", false + }), chainRule("CERT.WEAK-EXPONENT", "RSA public exponent is smaller than 65537", "certificate", Low, []Citation{citCABF}, "Reissue with a new key whose public exponent is 65537.", @@ -353,6 +373,33 @@ var t1Rules = []rule{ }, }, + { + id: "KX.DH-COMPOSITE", title: "The Diffie-Hellman modulus is not a prime", category: "key-exchange", severity: Critical, evidence: Observed, + citations: []Citation{citRFC7919, citSP80056A}, + remediation: "Replace the DH parameters with an RFC 7919 group (ffdhe2048 or larger), or drop DHE in favour of ECDHE. Treat every session made with these parameters as exposed.", + check: func(c *ctx) []hit { + if c.rep.DH == nil || c.rep.DH.IsPrime == nil || *c.rep.DH.IsPrime { + return nil + } + return one(fmt.Sprintf("the %d-bit modulus the server sent with %s failed a primality test: the discrete logarithm modulo a composite is far easier than its size suggests", c.rep.DH.PrimeBits, c.rep.DH.Suite)) + }, + }, + { + id: "KX.DH-NOT-SAFE-PRIME", title: "The Diffie-Hellman prime is not a safe prime", category: "key-exchange", severity: Low, evidence: Observed, + citations: []Citation{citRFC7919, citDHSubgroup}, + remediation: "Use an RFC 7919 group (its primes are safe primes), or make sure the server uses a fresh exponent for every handshake and checks the peer's public value.", + check: func(c *ctx) []hit { + d := c.rep.DH + if d == nil || d.IsPrime == nil || !*d.IsPrime || d.SafePrime == nil || *d.SafePrime { + return nil + } + if _, known := refdata.LookupDHPrime(d.PrimeSHA256); known { + return nil // a published group (RFC 5114's come with their subgroup order) + } + return one(fmt.Sprintf("(p-1)/2 is not prime for the %d-bit prime sent with %s, so small subgroups exist; a server that reuses its exponent can leak it to a client that sends a value in one (small-subgroup attack)", d.PrimeBits, d.Suite)) + }, + }, + // ---- HTTP ---- { id: "HTTP.HSTS-DISABLED", title: "HSTS is switched off (max-age=0)", category: "http", severity: Medium, evidence: Observed, @@ -436,3 +483,49 @@ var t1Rules = []rule{ }, }, } + +// commonName is the CN of an RFC 4514 distinguished name ("CN=Superfish\, Inc.,O=..." gives +// "Superfish, Inc."), unescaped. +func commonName(dn string) string { + var cur strings.Builder + esc := false + var parts []string + for _, r := range dn { + switch { + case esc: + cur.WriteRune(r) + esc = false + case r == '\\': + esc = true + case r == ',': + parts = append(parts, cur.String()) + cur.Reset() + default: + cur.WriteRune(r) + } + } + parts = append(parts, cur.String()) + for _, p := range parts { + if v, ok := strings.CutPrefix(strings.TrimSpace(p), "CN="); ok { + return v + } + } + return "" +} + +// compromisedRootOf names the compromised root a chain ends at, or "". +func compromisedRootOf(ch *chain) string { + var dns []string + if m := ch.view.Served.Missing; m != nil { + dns = append(dns, m.IssuerDN) + } + for _, x := range ch.cr.Chain { + dns = append(dns, x.Issuer, x.Subject) + } + for _, dn := range dns { + if what, ok := compromisedRoots[commonName(dn)]; ok { + return what + } + } + return "" +} diff --git a/internal/eval/sections.go b/internal/eval/sections.go index f21e9f7..ba71a00 100644 --- a/internal/eval/sections.go +++ b/internal/eval/sections.go @@ -35,6 +35,9 @@ const ( // or LDAP endpoint), so there was nothing to check; it is not a gap in the scan. StatusNotApplicable = "not-applicable" StatusInconclusive = "inconclusive" + // StatusUnknown: every check in the area ran, but the property the area is about cannot be + // known from the endpoint alone (revocation, without contacting the CA). Its checks still count. + StatusUnknown = "unknown" ) // Section is one part of the endpoint page with its status. @@ -100,13 +103,13 @@ func sectionOf(f Finding) string { // availabilityRules make clients fail to connect (REQ-TUI-2). var availabilityRules = map[string]bool{ "CERT.EXPIRED": true, "CERT.UNTRUSTED": true, "CERT.NAME-MISMATCH": true, "CERT.CHAIN-INCOMPLETE": true, - "CERT.NO-SERVERAUTH": true, "OCSP.MUST-STAPLE-MISSING": true, "OCSP.REVOKED": true, + "CERT.NO-SERVERAUTH": true, "OCSP.MUST-STAPLE-MISSING": true, "OCSP.REVOKED": true, "CERT.COMPROMISED-ROOT": true, } // attackLabels name the attacks a finding enables (REQ-TVLN-1). The detail // decides between FREAK and Logjam for export suites. var attackLabels = map[string][]string{ - "KX.DH-SMALL": {"Logjam"}, "KX.DH-COMMON-PRIME": {"Logjam"}, "PROTO.SSL2": {"DROWN"}, + "KX.DH-SMALL": {"Logjam"}, "KX.DH-COMMON-PRIME": {"Logjam"}, "PROTO.SSL2": {"DROWN"}, "KX.DH-COMPOSITE": {"Logjam"}, "FEAT.COMPRESSION": {"CRIME"}, "CIPHER.SWEET32": {"SWEET32"}, "CIPHER.RC4": {"RC4 biases"}, "VULN.POODLE": {"POODLE"}, "VULN.BEAST": {"BEAST"}, "CIPHER.CBC": {"LUCKY13"}, "VULN.DROWN-SHARED-KEY": {"DROWN"}, } @@ -177,6 +180,10 @@ func sections(c *ctx, findings []Finding) []Section { s.Status = StatusAttention } + if id == SecRevocation && s.Status == StatusGood && !c.legacy && !c.revocationVouched() { + s.Status, s.ReasonCode = StatusUnknown, "not-contacted" + s.Reason = "No stapled OCSP response vouches for the certificate, and this scan does not contact the CA's OCSP responder or CRL, so whether it has been revoked is not known." + } if c.legacy { switch id { case SecRevocation: @@ -319,3 +326,17 @@ func referenceData(now time.Time) []RefInfo { } return out } + +// revocationVouched reports whether a stapled OCSP response that verifies says the served leaf is +// good: the only revocation status this scan can know without contacting the CA. +func (c *ctx) revocationVouched() bool { + for _, ch := range c.chains { + if ch.cr.NoSNI { + continue + } + if o := ch.view.OCSP; o != nil && o.ForThisCert && o.Error == "" && o.Status == "good" && o.Signature == "valid" { + return true + } + } + return false +} diff --git a/internal/pki/pkitest/pkitest.go b/internal/pki/pkitest/pkitest.go index 741bae6..6494314 100644 --- a/internal/pki/pkitest/pkitest.go +++ b/internal/pki/pkitest/pkitest.go @@ -4,6 +4,7 @@ package pkitest import ( + "bytes" "crypto" "crypto/ecdsa" "crypto/elliptic" @@ -154,6 +155,11 @@ func (ca *CA) LeafKey(t testing.TB, o LeafOpts) (*x509.Certificate, crypto.Signe if o.EKU == nil && !o.NoEKU { o.EKU = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth} } + // A leaf from an intermediate names where its issuer can be downloaded (AIA caIssuers), as a + // public CA's leaves do; a leaf signed straight by a root names none. + if o.AIA == nil && !ca.selfSigned() { + o.AIA = []string{"http://ca.test/" + ca.Cert.Subject.CommonName + ".cer"} + } tmpl := &x509.Certificate{ SerialNumber: big.NewInt(serial.Add(1)), Subject: pkix.Name{CommonName: o.CN}, NotBefore: o.NotBefore, NotAfter: o.NotAfter, DNSNames: o.DNS, IPAddresses: o.IPs, @@ -225,3 +231,8 @@ func DER(certs ...*x509.Certificate) [][]byte { } return out } + +// selfSigned reports whether the CA is a root. +func (ca *CA) selfSigned() bool { + return bytes.Equal(ca.Cert.RawIssuer, ca.Cert.RawSubject) +} diff --git a/internal/reportpdf/endpoint.go b/internal/reportpdf/endpoint.go index ff765fa..d906558 100644 --- a/internal/reportpdf/endpoint.go +++ b/internal/reportpdf/endpoint.go @@ -193,6 +193,8 @@ func statusOf(st string) (string, tone) { return "Not applicable", toneInfo case "inconclusive": return "Inconclusive", toneWarn + case "unknown": + return "Status unknown", toneInfo } return capitalise(st), toneInfo } diff --git a/internal/server/certexpiry_internal_test.go b/internal/server/certexpiry_internal_test.go new file mode 100644 index 0000000..43363cb --- /dev/null +++ b/internal/server/certexpiry_internal_test.go @@ -0,0 +1,26 @@ +package server + +import ( + "testing" + "time" + + "github.com/offsetsecurity/cipherhorizon/internal/tlsprobe" +) + +// QA of real scans (badssl.com): the list's certificate expiry is the served leaf's, never the +// placeholder a server gives clients that send no server name (badssl.com's expired 2018 fallback). +func TestCertExpiryIgnoresTheNoSNIChain(t *testing.T) { + served := time.Date(2027, 1, 1, 0, 0, 0, 0, time.UTC) + fallback := time.Date(2018, 1, 1, 0, 0, 0, 0, time.UTC) + rep := &tlsprobe.Report{Certificates: []tlsprobe.CertResult{ + {Chain: []tlsprobe.CertInfo{{NotAfter: served}}}, + {NoSNI: true, Chain: []tlsprobe.CertInfo{{NotAfter: fallback}}}, + }} + if got := leafNotAfter(rep); got == nil || !got.Equal(served) { + t.Errorf("expiry %v, want the served leaf's %v", got, served) + } + only := &tlsprobe.Report{Certificates: []tlsprobe.CertResult{{NoSNI: true, Chain: []tlsprobe.CertInfo{{NotAfter: fallback}}}}} + if got := leafNotAfter(only); got != nil { + t.Errorf("only a no-SNI chain: expiry %v, want none", got) + } +} diff --git a/internal/server/evaluate.go b/internal/server/evaluate.go index bafb620..6b8e830 100644 --- a/internal/server/evaluate.go +++ b/internal/server/evaluate.go @@ -112,6 +112,7 @@ func resultOf(rep *tlsprobe.Report, ev eval.Evaluation) (store.Result, error) { Report: repJSON, Evaluation: evJSON, Band: ev.Hygiene.Band, Score: ev.Hygiene.Score, ReadinessLevel: ev.Readiness.Level, HNDLExposed: ev.Readiness.HNDLExposed, Critical: ev.Counts[eval.Critical], High: ev.Counts[eval.High], Medium: ev.Counts[eval.Medium], Low: ev.Counts[eval.Low], Info: ev.Counts[eval.Info], Incomplete: rep.Incomplete, + CertNotAfter: leafNotAfter(rep), }, nil } diff --git a/internal/server/runner.go b/internal/server/runner.go index d13b69f..5884bba 100644 --- a/internal/server/runner.go +++ b/internal/server/runner.go @@ -373,9 +373,6 @@ func (r *Runner) execute(parent context.Context, sc store.ScanSummary) { } res.FinalEvent = fmt.Sprintf("done: TLS hygiene %s (%d), PQC readiness L%d (%s), %d connections", ev.Hygiene.Band, ev.Hygiene.Score, ev.Readiness.Level, ev.Readiness.Label, rep.Connections) - if na := leafNotAfter(rep); na != nil { - res.CertNotAfter = na - } switch err := r.st.FinishScan(sc.ID, res); { case errors.Is(err, store.ErrNotFound): // canceled while evaluating: nothing to add @@ -394,7 +391,9 @@ func (r *Runner) canceledByUser(id int64) bool { func leafNotAfter(rep *tlsprobe.Report) *time.Time { var earliest *time.Time for _, cr := range rep.Certificates { - if len(cr.Chain) == 0 { + // The no-SNI chain is what clients that send no server name get, often a placeholder + // (badssl.com's expired 2018 fallback): it is not the endpoint's certificate. + if len(cr.Chain) == 0 || cr.NoSNI { continue } t := cr.Chain[0].NotAfter diff --git a/internal/store/anchors.go b/internal/store/anchors.go index e3346bb..77d3d79 100644 --- a/internal/store/anchors.go +++ b/internal/store/anchors.go @@ -167,9 +167,9 @@ func (s *Store) ReplaceEvaluation(scanID int64, r Result, event string) error { } defer func() { _ = tx.Rollback() }() res, err := tx.Exec(`UPDATE scans SET eval_json = ?, band = ?, score = ?, readiness_level = ?, hndl_exposed = ?, crit = ?, high = ?, medium = ?, low = ?, info = ?, - baseline = ?, baseline_meets = ?, baseline_changes = ? WHERE id = ? AND status = 'done'`, + baseline = ?, baseline_meets = ?, baseline_changes = ?, cert_not_after = ? WHERE id = ? AND status = 'done'`, r.Evaluation, r.Band, r.Score, r.ReadinessLevel, b2i(r.HNDLExposed), r.Critical, r.High, r.Medium, r.Low, r.Info, - r.Baseline, nullBool(r.BaselineMeets), r.BaselineChanges, scanID) + r.Baseline, nullBool(r.BaselineMeets), r.BaselineChanges, nullTS(r.CertNotAfter), scanID) if err != nil { return err } diff --git a/internal/tlsprobe/certs.go b/internal/tlsprobe/certs.go index bab6196..4cc44d9 100644 --- a/internal/tlsprobe/certs.go +++ b/internal/tlsprobe/certs.go @@ -7,6 +7,7 @@ import ( "crypto/x509" "encoding/base64" "encoding/hex" + "strings" "github.com/offsetsecurity/cipherhorizon/internal/certinfo" "github.com/offsetsecurity/cipherhorizon/internal/tlsprobe/registry" @@ -89,6 +90,8 @@ func (p *prober) certificates(ctx context.Context, legacy map[wire.Version]*lega byLeaf = map[string]int{} defaultFP string noSNI string + noSNIState *tls.ConnectionState + noSNIKey string ) for i, a := range attempts { if ctx.Err() != nil { @@ -111,14 +114,8 @@ func (p *prober) certificates(ctx context.Context, legacy map[wire.Version]*lega defaultFP = key } if a.source == "no-sni" { - if key == defaultFP { - noSNI = "same" - continue - } - noSNI = "different" - r := p.describeChain(a.source, state) - r.NoSNI = true - results = append(results, r) + // Judged after the raw chains are in: the default handshake may have failed. + noSNIState, noSNIKey = state, key continue } if idx, seen := byLeaf[key]; seen { @@ -130,6 +127,54 @@ func (p *prober) certificates(ctx context.Context, legacy map[wire.Version]*lega results = append(results, p.describeChain(a.source, state)) } + // Chains Go's TLS client could not reach (suites it does not offer, or a + // certificate the server keeps for clients that offer only RSA) are read from + // the raw flights of the suite enumeration, newest version first. + for _, v := range []wire.Version{wire.VersionTLS12, wire.VersionTLS11, wire.VersionTLS10, wire.VersionSSL30} { + e := legacy[v] + if e == nil { + continue + } + for _, rc := range e.chains { + fp := sha256.Sum256(rc.ders[0]) + key := hex.EncodeToString(fp[:]) + if _, seen := byLeaf[key]; seen { + continue + } + var certs []*x509.Certificate + for _, der := range rc.ders { + c, err := x509.ParseCertificate(der) + if err != nil { + break + } + certs = append(certs, c) + } + if len(certs) == 0 { + p.note("%s: a certificate in a raw flight could not be parsed", v) + continue + } + source := "raw-" + strings.ToLower(v.String()) + if s, ok := registry.SuiteByID(rc.suite); ok { + source += ", " + s.Name + } + byLeaf[key] = len(results) + if defaultFP == "" { + defaultFP = key + } + results = append(results, p.describeChain(source, &tls.ConnectionState{PeerCertificates: certs})) + } + } + if noSNIState != nil { + if _, sameAsSNI := byLeaf[noSNIKey]; sameAsSNI || noSNIKey == defaultFP { + noSNI = "same" + } else { + noSNI = "different" + r := p.describeChain("no-sni", noSNIState) + r.NoSNI = true + results = append(results, r) + } + } + var requested *bool switch { case clientCertRequested: diff --git a/internal/tlsprobe/primality_internal_test.go b/internal/tlsprobe/primality_internal_test.go new file mode 100644 index 0000000..7fb9b27 --- /dev/null +++ b/internal/tlsprobe/primality_internal_test.go @@ -0,0 +1,32 @@ +package tlsprobe + +import ( + "math/big" + "testing" +) + +// QA of real scans (badssl.com dh-composite, dh-small-subgroup): the DH modulus is tested for +// primality and, when prime, for being a safe prime. +func TestDHPrimality(t *testing.T) { + ffdhe2048, _ := new(big.Int).SetString("FFFFFFFFFFFFFFFFADF85458A2BB4A9AAFDC5620273D3CF1D8B9C583CE2D3695A9E13641146433FBCC939DCE249B3EF97D2FE363630C75D8F681B202AEC4617AD3DF1ED5D5FD65612433F51F5F066ED0856365553DED1AF3B557135E7F57C935984F0C70E0E68B77E2A689DAF3EFE8721DF158A136ADE73530ACCA4F483A797ABC0AB182B324FB61D108A94BB2C8E3FBB96ADAB760D7F4681D4F42A3DE394DF4AE56EDE76372BB190B07A7C8EE0A6D709E02FCE1CDF7E2ECC03404CD28342F619172FE9CE98583FF8E4F1232EEF28183C3FE3B1B4C6FAD733BB5FCBC2EC22005C58EF1837D1683B2C6F34A26C1B2EFFA886B423861285C97FFFFFFFFFFFFFFFF", 16) + notSafe := new(big.Int).Add(new(big.Int).Lsh(big.NewInt(1), 127), big.NewInt(0)) + notSafe.Sub(notSafe, big.NewInt(1)) // 2^127 - 1 is prime; (p-1)/2 = 2^126 - 1 is not + composite := new(big.Int).Mul(big.NewInt(1000003), big.NewInt(999983)) + for name, tc := range map[string]struct { + p *big.Int + prime, safe *bool + }{ + "ffdhe2048 (a safe prime)": {ffdhe2048, ptrB(true), ptrB(true)}, + "2^127-1 (prime, not safe)": {notSafe, ptrB(true), ptrB(false)}, + "a composite": {composite, ptrB(false), nil}, + } { + ip, sp := primality(tc.p.Bytes()) + if !eqB(ip, tc.prime) || !eqB(sp, tc.safe) { + t.Errorf("%s: prime %v safe %v", name, ip, sp) + } + } +} + +func ptrB(b bool) *bool { return &b } + +func eqB(a, b *bool) bool { return (a == nil) == (b == nil) && (a == nil || *a == *b) } diff --git a/internal/tlsprobe/report.go b/internal/tlsprobe/report.go index 1b6b9d5..69ea9ba 100644 --- a/internal/tlsprobe/report.go +++ b/internal/tlsprobe/report.go @@ -228,7 +228,14 @@ type DHResult struct { PrimeBits int `json:"primeBits"` GeneratorBits int `json:"generatorBits"` PrimeSHA256 string `json:"primeSha256"` // fingerprint of the prime, for matching known groups - Suite string `json:"suite"` // the suite the sample came from + // IsPrime: the modulus passed a Miller-Rabin and Baillie-PSW test. A composite modulus makes + // the discrete logarithm easy (badssl.com dh-composite). nil on reports from earlier versions. + IsPrime *bool `json:"isPrime,omitempty"` + // SafePrime: (p-1)/2 is prime too, so every element but 1 and p-1 generates a large subgroup; + // otherwise small subgroups exist and a server that reuses its exponent can leak it (RFC 7919 + // section 1, RFC 8446 section 4.2.8.1). Only judged when IsPrime. + SafePrime *bool `json:"safePrime,omitempty"` + Suite string `json:"suite"` // the suite the sample came from } // Features are server behaviours read from hello messages. diff --git a/internal/tlsprobe/scan.go b/internal/tlsprobe/scan.go index 4ced241..8b48eac 100644 --- a/internal/tlsprobe/scan.go +++ b/internal/tlsprobe/scan.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "errors" "fmt" + "math/big" "net" "sort" "strconv" @@ -477,8 +478,20 @@ type legacyEnum struct { pref string // "server" | "client" | "unknown" first *wire.ServerHello small bool + // chains are the distinct certificate chains the server sent in these + // plaintext flights (DER, leaf first), with the suite that drew each. + chains []rawChain } +// rawChain is a certificate chain read from a raw TLS 1.2-and-earlier flight. +type rawChain struct { + suite uint16 + ders [][]byte +} + +// maxRawChains bounds how many distinct chains one version's enumeration keeps. +const maxRawChains = 4 + func (p *prober) legacyGroups(small bool) []uint16 { if small { return commonGroups @@ -518,6 +531,7 @@ func (p *prober) enumerateLegacy(ctx context.Context, v wire.Version, small bool if e.first == nil { e.first = sh } + e.keepChain(sh.Suite, o.flight.Certificates) e.suites = append(e.suites, sh.Suite) remaining = without(remaining, sh.Suite) } @@ -536,6 +550,19 @@ func (p *prober) enumerateLegacy(ctx context.Context, v wire.Version, small bool return e } +// keepChain records a chain the first time its leaf is seen. +func (e *legacyEnum) keepChain(suite uint16, ders [][]byte) { + if len(ders) == 0 || len(e.chains) >= maxRawChains { + return + } + for _, c := range e.chains { + if string(c.ders[0]) == string(ders[0]) { + return + } + } + e.chains = append(e.chains, rawChain{suite: suite, ders: ders}) +} + // preference asks who decides between accepted items. For adjacent pairs // (items[i], items[i+1]) it offers them in REVERSE order: a server with its own // preference still returns items[i]; one that follows the client returns @@ -684,7 +711,28 @@ func (p *prober) sampleDH(ctx context.Context, v wire.Version, s registry.Suite, return nil } sum := sha256.Sum256(ke.Prime) - return &DHResult{PrimeBits: ke.PrimeBits, GeneratorBits: ke.GeneratorBits, PrimeSHA256: hex.EncodeToString(sum[:]), Suite: s.Name} + r := &DHResult{PrimeBits: ke.PrimeBits, GeneratorBits: ke.GeneratorBits, PrimeSHA256: hex.EncodeToString(sum[:]), Suite: s.Name} + r.IsPrime, r.SafePrime = primality(ke.Prime) + return r +} + +// maxPrimalityBits bounds the moduli tested for primality (a few milliseconds at 8192 bits). +const maxPrimalityBits = 8192 + +// primality says whether p is prime and, if so, whether it is a safe prime. ProbablyPrime(20) is +// Miller-Rabin with 20 random bases plus Baillie-PSW, which has no known counterexample. +func primality(b []byte) (isPrime, safe *bool) { + p := new(big.Int).SetBytes(b) + if p.BitLen() < 2 || p.BitLen() > maxPrimalityBits { + return nil, nil + } + ip := p.ProbablyPrime(20) + if !ip { + return &ip, nil + } + q := new(big.Int).Rsh(new(big.Int).Sub(p, big.NewInt(1)), 1) + sp := q.ProbablyPrime(20) + return &ip, &sp } // ---- stage 4: features --------------------------------------------------------------------- diff --git a/internal/tlsprobe/scan_legacy_test.go b/internal/tlsprobe/scan_legacy_test.go index 1398310..5501e99 100644 --- a/internal/tlsprobe/scan_legacy_test.go +++ b/internal/tlsprobe/scan_legacy_test.go @@ -225,3 +225,20 @@ func TestHeartbeatIsProbed(t *testing.T) { t.Errorf("a server that does not: Heartbeat = %v, want false", without.Features.Heartbeat) } } + +// QA of real scans (badssl.com 3des, rc4, null): a server whose only suites crypto/tls will not +// offer still has its certificate read, from the raw TLS 1.2 flight, and it is the chain the +// server sends with SNI, not a fallback. +func TestCertificateIsReadFromARawFlight(t *testing.T) { + _, rsaCert := testserver.Certificates(t) + order := []uint16{0x000A, 0x0005} // TLS_RSA_WITH_3DES_EDE_CBC_SHA, TLS_RSA_WITH_RC4_128_SHA + srv := testserver.StartScript(t, testserver.LegacyWithCertificate(0x0303, order, rsaCert.Certificate)) + rep := scan(t, srv, opts()) + if len(rep.Certificates) != 1 { + t.Fatalf("%d certificate results, want 1: %+v", len(rep.Certificates), rep.Certificates) + } + c := rep.Certificates[0] + if c.NoSNI || !strings.HasPrefix(c.Source, "raw-tlsv1.2") || len(c.Chain) == 0 || c.Chain[0].PublicKey.Algorithm != "RSA" { + t.Errorf("certificate %+v, want the RSA chain read from a raw TLS 1.2 flight", c) + } +} diff --git a/internal/tlsprobe/testserver/legacy.go b/internal/tlsprobe/testserver/legacy.go index 292801a..caffa98 100644 --- a/internal/tlsprobe/testserver/legacy.go +++ b/internal/tlsprobe/testserver/legacy.go @@ -209,3 +209,29 @@ func AnswerAnyTLS13() func([]byte) []byte { return append(rec, msg...) } } + +// LegacyWithCertificate is Legacy (server order), but its flight carries the certificate chain +// (DER, leaf first) between the ServerHello and the ServerHelloDone: a server whose only suites +// crypto/tls will not offer (3DES, RC4, NULL), so its certificate is seen only in raw flights. +func LegacyWithCertificate(version uint16, order []uint16, chain [][]byte) func([]byte) []byte { + plain := Legacy(version, order, false) + return func(in []byte) []byte { + out := plain(in) + if len(out) < 9 || out[0] != 22 { + return out + } + // Rebuild the handshake messages: the ServerHello, the Certificate, the ServerHelloDone. + msgs := out[5 : len(out)-4] + var list []byte + for _, der := range chain { + list = append(list, byte(len(der)>>16), byte(len(der)>>8), byte(len(der))) //nolint:gosec // 24-bit TLS lengths of a test chain + list = append(list, der...) + } + body := append([]byte{byte(len(list) >> 16), byte(len(list) >> 8), byte(len(list))}, list...) //nolint:gosec // as above + cert := append([]byte{11, byte(len(body) >> 16), byte(len(body) >> 8), byte(len(body))}, body...) //nolint:gosec // as above + all := append(append(append([]byte(nil), msgs...), cert...), 14, 0, 0, 0) + rec := binary.BigEndian.AppendUint16([]byte{22}, version) + rec = binary.BigEndian.AppendUint16(rec, uint16(len(all))) //nolint:gosec // a test chain of a few KiB + return append(rec, all...) + } +} diff --git a/testing/e2e/t1-phase.mjs b/testing/e2e/t1-phase.mjs index 30ac59a..ed4e81d 100644 --- a/testing/e2e/t1-phase.mjs +++ b/testing/e2e/t1-phase.mjs @@ -69,7 +69,7 @@ export async function t1Phase(t) { /^post-quantum readiness\|tls hygiene\|weakest classical strength\|certificate(\|not checked)?$/.test(page.tiles.join('|').toLowerCase()), page.tiles.join('|')); check('T1-E2E.5', 'What to fix lists the actionable findings, at least one', page.fix >= 1, page.fix); check('T1-E2E.6', 'every check row states its status as a symbol and a word', - page.index.length === 8 && page.index.every((s) => /^[✖▲✔○?] (Problem|Needs attention|Good|Not checked|Not collected|Inconclusive)$/.test(s)), page.index.join(' / ')); + page.index.length === 8 && page.index.every((s) => /^[✖▲✔○?] (Problem|Needs attention|Good|Not checked|Not collected|Inconclusive|Status unknown)$/.test(s)), page.index.join(' / ')); // PRD-013 REQ-VIS-3..6: the reviewer's order, each section named, and a jump row to each with its state. const ORDER = ['sec-protocols', 'sec-certificates', 'sec-key-exchange', 'sec-revocation', 'sec-features', 'sec-algorithms']; check('T1-E2E.7', 'Configuration has its sections in the reviewer’s order, each with a heading, and the jump row leads to each with its state', diff --git a/testing/e2e/visual-phase.mjs b/testing/e2e/visual-phase.mjs index 9e99344..e342c5e 100644 --- a/testing/e2e/visual-phase.mjs +++ b/testing/e2e/visual-phase.mjs @@ -86,6 +86,48 @@ export async function visualPhase({ check, js, go, sleep, shot, viewport }) { && sum.opened.tab === 'Configuration' && sum.opened.inView, JSON.stringify(sum)); await shot('vis-summary'); + // E2E-FIX-1 (QA-REPORT-014): Fix first is the three problems to fix first, one chip each saying when, + // and the rest folded under one line that counts them by severity; numbering runs on across both. + // (The tile VIS-2 opened left the page on Configuration: ask for the Summary by name.) + await go(`#/endpoints/${target.id}?tab=summary`); + await sleep(1500); + const fix = await js(`const first = [...document.querySelectorAll('#fix > ol.fixlist > li')]; + const more = document.querySelector('#fix details.fixmore'); + const rest = more ? [...more.querySelectorAll('ol.fixlist > li')] : []; + const all = [...first, ...rest]; + return { first: first.length, rest: rest.length, closed: more ? !more.open : null, sum: more ? more.querySelector('summary').textContent : '', + nums: all.map((li) => li.querySelector('.fix-n').textContent), chips: all.map((li) => li.querySelectorAll('.fixwhen').length), + oldBadges: document.querySelectorAll('#fix .sev, #fix .urgency').length }`); + const actN = (ev.findings || []).filter((f) => f.severity !== 'info').reduce((m, f) => m.add(f.id), new Set()).size; + check('E2E-FIX-1', 'Fix first shows the three problems to fix first, one "when" chip each, and folds the rest under one line that counts them; numbering runs on', + fix.first === Math.min(3, actN) && fix.first + fix.rest === actN && (actN <= 3 || (fix.closed && new RegExp(`^${actN - 3} more`).test(fix.sum))) + && fix.nums.every((n, i) => n === String(i + 1)) && fix.chips.every((c) => c === 1) && fix.oldBadges === 0, JSON.stringify(fix).slice(0, 400)); + + // E2E-VIS-ALIGN (QA-REPORT-014): things that belong on one line are on one line. Fix first: each + // chip sits beside its title and every title starts at one x. Suites: each strength chip is centred + // on its row's part cells. Versions: every head's name, facts and verdict line up down the section. + const alignSummary = await js(`const rows = [...document.querySelectorAll('#fix ol.fixlist > li')].filter((li) => li.offsetParent); + return rows.map((li) => { const c = li.querySelector('.fixwhen').getBoundingClientRect(), t = li.querySelector('.action-title').getBoundingClientRect(); + return { dy: Math.round(Math.abs((c.top + c.height / 2) - (t.top + Math.min(t.height, 22) / 2))), x: Math.round(t.left) }; })`); + await go(`#/endpoints/${target.id}?tab=configuration`); + await sleep(1500); + const alignCfg = await js(`document.querySelectorAll('#sec-protocols details.ver-block').forEach((d) => { d.open = true; }); + await new Promise((r) => setTimeout(r, 200)); + const rows = [...document.querySelectorAll('#sec-protocols table.suites tbody tr')].map((tr) => { + const chip = tr.querySelector('.strength-cell .schip').getBoundingClientRect(), cell = tr.querySelector('.pcell').getBoundingClientRect(); + return Math.round(Math.abs((chip.top + chip.height / 2) - (cell.top + cell.height / 2))); }); + document.querySelectorAll('#sec-protocols details.ver-block').forEach((d) => { d.open = false; }); + await new Promise((r) => setTimeout(r, 200)); + const heads = [...document.querySelectorAll('#sec-protocols details.ver-block > summary')].map((s) => ({ + name: Math.round(s.querySelector('.ver-title').getBoundingClientRect().left), meta: Math.round(s.querySelector('.ver-meta').getBoundingClientRect().left), + right: Math.round((s.querySelector('.ver-verdict') || s).getBoundingClientRect().right) })); + return { rows, heads }`); + const same = (xs) => xs.every((x) => Math.abs(x - xs[0]) <= 1); + check('E2E-VIS-ALIGN', 'Fix first chips sit beside their titles, which share one column; every strength chip is centred on its suite row; every version head lines up', + alignSummary.length > 0 && alignSummary.every((r) => r.dy <= 4) && same(alignSummary.map((r) => r.x)) + && alignCfg.rows.length > 0 && alignCfg.rows.every((d) => d <= 2) && same(alignCfg.heads.map((h) => h.name)) && same(alignCfg.heads.map((h) => h.meta)) && same(alignCfg.heads.map((h) => h.right)), + JSON.stringify({ alignSummary, rows: alignCfg.rows.filter((d) => d > 2).slice(0, 5), heads: alignCfg.heads }).slice(0, 400)); + // E2E-VIS-3: each version's suites are the report's, in the server's order, each with its five cells // and the evaluation's word; the banner is what a current client negotiates. await go(`#/endpoints/${target.id}?tab=configuration`); @@ -121,8 +163,7 @@ export async function visualPhase({ check, js, go, sleep, shot, viewport }) { const btn = document.querySelector('#sec-protocols .ver-toggle'); const tick = () => new Promise((r) => setTimeout(r, 120)); const labels = [btn && btn.textContent]; - const closed = blocks.find((b) => !b.open); - if (closed) { closed.querySelector('summary').click(); await tick(); } + for (const b of blocks.filter((x) => !x.open)) { b.querySelector('summary').click(); await tick(); } labels.push(btn && btn.textContent); if (btn) { btn.click(); await tick(); } const afterHide = blocks.map((b) => b.open); @@ -130,10 +171,18 @@ export async function visualPhase({ check, js, go, sleep, shot, viewport }) { if (btn) { btn.click(); await tick(); } return { state, labels, afterHide, afterShow: blocks.map((b) => b.open) }`); const legacyV = (v) => ['SSLv2', 'SSLv3', 'TLSv1.0', 'TLSv1.1'].includes(v); - const wantOpen = (v) => legacyV(v) || fold.state.length === 1 || (evH.suiteStrength || []).some((x) => x.version === v && x.word !== 'strong'); - const foldOk = fold.state.every((b) => b.open === wantOpen(b.v)) && fold.state.filter((b) => !b.open).every((b) => /^Strong all \d+$/.test(b.verdict)); - check('E2E-VIS-FOLD', 'a TLS version is open only when it needs a look, folded ones say how their suites rate, and one button shows or hides every version and says which', - fold.state.length > 1 && fold.state.some((b) => !b.open) && foldOk && fold.labels.join('|') === 'Show every version|Hide every version|Show every version' + // Every version starts folded (owner request after PR #24), unless it is the only one; a folded head + // says how its suites rate, one chip per word with its count, as the evaluation states them. + const tallyOf = (v) => ['insecure', 'weak', 'acceptable', 'strong'].map((w) => [w, (evH.suiteStrength || []).filter((x) => x.version === v && x.word === w).length]).filter(([, n]) => n); + const verdictOk = (b) => { + const t = tallyOf(b.v); + if (t.length === 1 && t[0][0] === 'strong') return b.verdict === `Strong all ${t[0][1]}`; + return t.every(([w, n]) => b.verdict.includes(`${w[0].toUpperCase()}${w.slice(1)} ${n}`)); + }; + void legacyV; + const foldOk = fold.state.every((b) => b.open === (fold.state.length === 1)) && fold.state.every(verdictOk); + check('E2E-VIS-FOLD', 'every TLS version starts folded, its head says how its suites rate, and one button shows or hides every version and says which', + fold.state.length > 1 && foldOk && fold.labels.join('|') === 'Show every version|Hide every version|Show every version' && fold.afterHide.every((o) => !o) && fold.afterShow.every((o) => o), JSON.stringify(fold).slice(0, 400)); await go(`#/endpoints/${target.id}?tab=configuration`); await sleep(1500); @@ -273,7 +322,7 @@ export async function visualPhase({ check, js, go, sleep, shot, viewport }) { } check('E2E-VIS-10', 'the SSH Summary has readiness with the stepper, the baseline, the weakest classical strength (the evaluation’s weakest) and the host key; the baseline is Fix first line 1 in the shared row shape, numbered on from it, with no “Why it matters” (PRD-014 REQ-SSV-7)', ss.labels.join() === 'Post-quantum readiness,Baseline,Weakest classical strength,Host key' && ss.stepper && !!w && ss.weakest === `${w.name}: ${words(w.strength)}` - && ss.first && ss.first.n === '1' && /^Meet /.test(ss.first.title) && /sshd_config/.test(ss.first.link) && ss.nums.every((x, i) => x === String(i + 1)) && ss.lists === 1 && !ss.prose + && ss.first && ss.first.n === '1' && /^Meet /.test(ss.first.title) && /sshd_config/.test(ss.first.link) && ss.nums.every((x, i) => x === String(i + 1)) && ss.lists <= 2 && !ss.prose && (!good || (/✓ Meets/.test(good.tile) && !/^Meet /.test(good.first))), JSON.stringify({ ss, weakest: w && w.name, good }).slice(0, 500)); await shot('vis-ssh-summary'); diff --git a/web/static/js/views/help.js b/web/static/js/views/help.js index aefd5f0..89fb9ef 100644 --- a/web/static/js/views/help.js +++ b/web/static/js/views/help.js @@ -76,7 +76,7 @@ export async function render(root) { h('p', null, 'Mail, directory and database services that start in plain text are scanned through STARTTLS. It is chosen by port (25, 587, 143, 110, 21, 5222, 389 and 5432), by a scheme such as ', code('smtp://mail.example.com'), ', or when you add the endpoint (', h('i', null, 'How it starts TLS'), ').'), h('p', null, 'To add many at once, use ', h('b', null, 'TLS endpoints → Add TLS endpoints → Paste a list'), '. Each endpoint can be rescanned on a schedule from its page.'), - h('p', null, h('b', null, 'Reading a report. '), 'The Summary leads with four figures (post-quantum readiness, the TLS hygiene score, the weakest classical strength and the certificate), then ', h('i', null, 'Fix first'), ', one numbered line per problem, a bar of every check’s result and a tile per area that opens it. Configuration is the one place to check the whole setup, in the order a reviewer reads it: protocols and cipher suites, the certificate chain, key exchange and signatures, revocation, protocol features, and every algorithm by class. A TLS version whose suites are all Strong is folded to one line; select it to see its suites. The row of chips at its top jumps to each, its dot showing that part’s state, and each part ends with the issues found in it. Select any certificate in the chain to open the whole X.509 document: its validity, its key and signature strength, every extension, decoded, and the PEM to copy or download.'), + h('p', null, h('b', null, 'Reading a report. '), 'The Summary leads with four figures (post-quantum readiness, the TLS hygiene score, the weakest classical strength and the certificate), then ', h('i', null, 'Fix first'), ', the three problems to fix first with the rest folded under them, a bar of every check’s result and a tile per area that opens it. Configuration is the one place to check the whole setup, in the order a reviewer reads it: protocols and cipher suites, the certificate chain, key exchange and signatures, revocation, protocol features, and every algorithm by class. Each TLS version is folded to one line that says whether it should be on and how its suites rate; select it to see its suites, or show every version at once. The row of chips at its top jumps to each, its dot showing that part’s state, and each part ends with the issues found in it. Select any certificate in the chain to open the whole X.509 document: its validity, its key and signature strength, every extension, decoded, and the PEM to copy or download.'), h('p', null, h('b', null, 'Reading the figures. '), 'A bar of bits runs from 0 to 256 over four bands (red under 80, amber to 111, grey to 127, green from 128), filled to the strength, with the bits in words beside it; a ring marks the weakest. A shield with a check is quantum-safe and one with a cross quantum-vulnerable; a question mark means no source classified it; a dot means there is no post-quantum question to ask. Every figure names its source when you hover it.'), h('p', { class: 'callout' }, h('b', null, 'Scan only systems you own or are authorised to test. '), 'A scan makes a few hundred short TLS handshakes to its target and contacts nothing else.')), sec('ssh', 'Scan an SSH server', diff --git a/web/static/js/views/report.js b/web/static/js/views/report.js index f9abe32..a30466d 100644 --- a/web/static/js/views/report.js +++ b/web/static/js/views/report.js @@ -23,7 +23,7 @@ export const SECTIONS = [ const STATUS = { problem: ['✖', 'Problem'], attention: ['▲', 'Needs attention'], good: ['✔', 'Good'], 'not-checked': ['○', 'Not checked'], 'not-collected': ['○', 'Not collected'], inconclusive: ['?', 'Inconclusive'], - 'not-applicable': ['·', 'Not applicable'], + 'not-applicable': ['·', 'Not applicable'], unknown: ['?', 'Status unknown'], }; export function statusChip(st) { @@ -149,25 +149,30 @@ export function summaryView(ctx) { if (!act.length && !extra) { items.appendChild(h('p', { class: 'muted' }, 'Nothing needs fixing. The checks that found nothing are listed under Weaknesses.')); } else { - // One numbered line per finding, worst first: what it is and what to do. Why it matters is in - // its evidence (UX-DESIGN-011 §3.1). A profile's own action (SSH: the baseline) is line 1. - const off = extra ? 1 : 0; - items.appendChild(h('ol', { class: 'fixlist actions' }, - extra ? h('li', { class: 'action sev-medium', id: `issue-${extra.id}` }, - h('span', { class: 'fix-n' }, '1'), - badge(extra.chip, 'warn'), - h('span', { class: 'fix-main' }, h('span', { class: 'action-head' }, h('span', { class: 'action-title' }, extra.title)), - h('span', { class: 'action-fix', title: extra.why || null }, extra.fix)), - h('a', { href: `#sec-${extra.id}`, class: 'xlink action-more', onclick: (e) => { e.preventDefault(); extra.go(); } }, extra.link)) : null, - act.map((f, i) => h('li', { class: `action sev-${f.severity}`, id: `issue-${f.id}` }, - h('span', { class: 'fix-n' }, String(i + 1 + off)), - sevBadge(f.severity), - h('span', { class: 'fix-main' }, - h('span', { class: 'action-head' }, h('span', { class: 'action-title' }, f.title), h('span', { class: `urgency u-${f.severity}` }, URGENCY[f.severity]), - f.availability ? badge('clients fail to connect', 'bad', 'Clients that hit this refuse the connection') : null, - ctx.triage && ctx.triage[f.id] ? badge(ctx.triage[f.id].status === 'accepted-risk' ? 'accepted risk' : 'acknowledged', 'neutral') : null), - h('span', { class: 'action-fix', title: whyOf(f, profileOf(ctx)) }, f.remediation)), - h('a', { href: `#chk-${f.id}`, class: 'xlink action-more', onclick: (e) => { e.preventDefault(); ctx.goIssue(f.id); } }, 'Evidence'))))); + // One numbered row per finding, worst first: when to act (one chip, coloured by severity), what + // it is, and what to do; why it matters is in the row's tooltip and its evidence one click away + // (UX-DESIGN-011 §3.1). A profile's own action (SSH: the baseline) is row 1. The first three are + // what to fix first; the rest fold under one line, so the list stays a list of priorities. + const rows = []; + if (extra) { + rows.push(fixRow(rows.length + 1, extra.id, 'medium', extra.chip, extra.title, [], extra.fix, extra.why, + h('a', { href: `#sec-${extra.id}`, class: 'xlink action-more', onclick: (e) => { e.preventDefault(); extra.go(); } }, extra.link))); + } + for (const f of act) { + const tags = [f.availability ? h('span', { class: 'fixtag fixtag-bad', title: 'Clients that hit this refuse the connection' }, 'clients fail to connect') : null, + ctx.triage && ctx.triage[f.id] ? h('span', { class: 'fixtag' }, ctx.triage[f.id].status === 'accepted-risk' ? 'accepted risk' : 'acknowledged') : null]; + rows.push(fixRow(rows.length + 1, f.id, f.severity, URGENCY[f.severity], f.title, tags, f.remediation, whyOf(f, profileOf(ctx)), + h('a', { href: `#chk-${f.id}`, class: 'xlink action-more', onclick: (e) => { e.preventDefault(); ctx.goIssue(f.id); } }, 'Evidence'))); + } + const FIRST = 3; + items.appendChild(h('ol', { class: 'fixlist actions' }, rows.slice(0, FIRST))); + if (rows.length > FIRST) { + const more = rows.length - FIRST; + items.appendChild(h('details', { class: 'fixmore' }, + h('summary', null, h('span', { class: 'ver-chev', 'aria-hidden': 'true' }), `${plural(more, 'more thing')} to fix`, + h('span', { class: 'fixmore-sum' }, tallyOf(act.slice(FIRST - (extra ? 1 : 0))))), + h('ol', { class: 'fixlist actions', start: String(FIRST + 1) }, rows.slice(FIRST)))); + } } wrap.appendChild(items); @@ -212,6 +217,23 @@ const versionLabel = (v) => v.replace('TLSv', 'TLS ').replace('SSLv', 'SSL '); +/** One Fix first row: its number, when to act, what it is, what to do, and its evidence. */ +function fixRow(n, id, severity, when, title, tags, fix, why, link) { + return h('li', { class: `action fixrow sev-${severity}`, id: `issue-${id}` }, + h('span', { class: 'fix-n' }, String(n)), + h('span', { class: `fixwhen fixwhen-${severity}`, title: `${capital(severity)} severity` }, when), + h('span', { class: 'fix-main' }, + h('span', { class: 'action-head' }, h('span', { class: 'action-title' }, title), tags), + h('span', { class: 'action-fix', title: why || null }, fix)), + link); +} + +/** The severities of the folded rows, as "2 medium · 3 low". */ +function tallyOf(list) { + const order = ['critical', 'high', 'medium', 'low']; + return order.map((s) => [s, list.filter((f) => f.severity === s).length]).filter(([, n]) => n).map(([s, n]) => `${n} ${s}`).join(' · '); +} + function versionState([tone, sym, state, judgement]) { return h('span', { class: `vstate vs-${tone}` }, h('span', { class: 'vs-sym', 'aria-hidden': 'true' }, sym), h('b', null, state), judgement ? h('span', { class: 'vs-judge' }, ` · ${judgement}`) : null); @@ -429,6 +451,8 @@ function summaryStrip(ctx, sections) { const { ev } = ctx; const r = ev.readiness; const g = ev.hygiene; + // Revocation's 'unknown' is the same on every endpoint (the CA is never contacted): it is said in its + // area, not counted here as a gap in this scan. const notChecked = Object.values(sections).filter((s) => ['not-checked', 'not-collected', 'inconclusive'].includes(s.status)); const pki = (ev.pki || []).find((v) => !v.noSni) || (ev.pki || [])[0]; // Each instrument opens the section it summarises. @@ -477,7 +501,7 @@ function summaryStrip(ctx, sections) { // ---- section anatomy ------------------------------------------------------------------------ function notChecked(sec) { - if (!['not-checked', 'not-collected', 'inconclusive'].includes(sec.status)) return null; + if (!['not-checked', 'not-collected', 'inconclusive', 'unknown'].includes(sec.status)) return null; const how = { 'option-off': sec.id === 'http' ? 'It is on by default; the server was started with --http-checks=false.' : 'An option turned it off for this scan.', 'not-applicable': 'It does not apply to this service.', @@ -533,8 +557,9 @@ function revocationBody(ctx) { const o = v.ocsp; let ocspLine; if (!o) { + // Nothing vouches for the certificate, and this scan never contacts the CA: say the status is unknown. ocspLine = leaf.ocspMustStaple ? 'No OCSP response was stapled, although the certificate requires one (Must-Staple).' - : (v.ocspUrls || []).length ? 'No OCSP response was stapled.' : 'No OCSP response was stapled, and the certificate names no OCSP responder.'; + : 'Revocation status unknown: no OCSP response was stapled, and this scan does not contact the CA.'; } else if (o.error) { ocspLine = `A stapled OCSP response was sent but is not usable: ${o.error}.`; } else if (!o.forThisCert) { @@ -579,9 +604,9 @@ function protocolsBody(ctx) { const on = rep.protocols.filter((p) => p.supported).map((p) => p.version); const old = on.filter(legacyVersion); const facts = [ - on.length ? `Accepts ${on.join(', ')}.` : 'No protocol version was accepted.', - old.length ? `${old.join(' and ')} ${old.length === 1 ? 'is' : 'are'} deprecated.` : on.length ? 'No deprecated version is accepted.' : null, - rep.negotiated && rep.negotiated.version ? `A current client negotiates ${rep.negotiated.version}.` : null, + on.length ? `Accepts ${on.map(versionLabel).join(', ')}.` : 'No protocol version was accepted.', + old.length ? `${old.map(versionLabel).join(' and ')} ${old.length === 1 ? 'is' : 'are'} deprecated.` : on.length ? 'No deprecated version is accepted.' : null, + rep.negotiated && rep.negotiated.version ? `A current client negotiates ${versionLabel(rep.negotiated.version)}.` : null, ]; const lead = h('div', { class: 'pillrow', role: 'list' }, rep.protocols.map((p) => h('span', { role: 'listitem', class: `protopill ${p.evidence === 'inconclusive' ? 'p-unknown' : p.supported ? (legacyVersion(p.version) ? 'p-bad' : 'p-on') : 'p-off'}` }, diff --git a/web/static/js/views/tls-config.js b/web/static/js/views/tls-config.js index d1f197b..27e2608 100644 --- a/web/static/js/views/tls-config.js +++ b/web/static/js/views/tls-config.js @@ -107,9 +107,9 @@ function protocolsSection(ctx, tone) { const words = suites.map((x) => (((ev.suiteStrength || []).find((r) => r.version === x.version && r.id === x.id)) || {}).word || ''); const order = ['insecure', 'weak', 'acceptable', 'strong', '']; const tally = order.map((w) => [w, words.filter((x) => x === w).length]).filter(([, c]) => c); - // Open when it needs a look: an old version that is on, or any suite below Strong; folded when - // every suite is Strong, so the page shows first what there is to do (owner request, PRD-013). - const needsLook = bad || words.some((w) => w !== 'strong') || on.length === 1; + // Every version starts folded (owner request): its head already says whether it should be on + // and how its suites rate, and a click opens its suites. The only version opens by itself. + const needsLook = on.length === 1; const verdict = h('span', { class: 'ver-verdict' }, tally.length === 1 && tally[0][0] === 'strong' ? [strengthChip('strong'), ` all ${suites.length}`] : tally.map(([w, c]) => h('span', { class: 'ver-tally' }, strengthChip(w), ` ${c}`))); const head = h('summary', { class: `ver-head-row${bad ? ' is-bad' : ''}`, title: 'Show or hide this version’s suites' }, @@ -120,8 +120,9 @@ function protocolsSection(ctx, tone) { suites.length ? (fs === suites.length ? 'forward secrecy on all' : `forward secrecy on ${fs} of ${suites.length}`) : null].filter(Boolean).join(' · ')), groups.length ? h('span', { class: 'ver-groups' }, h('span', { class: 'muted' }, 'Groups'), groups.map((g) => groupChip(ev, g))) : null, suites.length ? verdict : null); - const table = suites.length ? h('div', { class: 'tablewrap ver-body' }, h('table', { class: 'suites' }, - h('thead', null, h('tr', null, h('th', { scope: 'col', class: 'num' }, '#'), h('th', { scope: 'col' }, 'Cipher suite'), + const ranked = suites.some((s) => s.rank); + const table = suites.length ? h('div', { class: 'tablewrap ver-body' }, h('table', { class: `suites${ranked ? '' : ' unranked'}` }, + h('thead', null, h('tr', null, h('th', { scope: 'col', class: 'num', title: 'The server\u2019s order' }, '#'), h('th', { scope: 'col' }, 'Cipher suite'), PART_COLUMNS.map(([, w]) => h('th', { scope: 'col', class: 'pcol' }, w)), h('th', { scope: 'col' }, 'Strength'))), h('tbody', null, suites.map((s) => suiteRow(ev, s, rep))))) : h('p', { class: 'muted' }, 'No cipher suites were recorded for this version.'); return h('details', { class: `ver-block${bad ? ' is-bad' : ''}`, id: `ver-${v}`, open: needsLook }, head, table); @@ -268,7 +269,7 @@ function revocationSection(ctx, tone) { : o.error || !o.forThisCert ? statusTile('OCSP stapling', o.error ? `Stapled but unusable: ${o.error}` : 'Stapled, for another certificate', 'problem') : statusTile('OCSP stapling', `Stapled: ${o.status}${o.nextUpdate ? `, until ${fmtDay(o.nextUpdate)}` : ''}`, o.status === 'good' && o.signature === 'valid' ? 'good' : 'problem'), statusTile('Must-staple', leaf.ocspMustStaple ? 'Required by the certificate' : 'Not set', 'na'), - statusTile('OCSP and CRL', urls.length ? `${plural(urls.length, 'pointer')}: ${urls.join(', ')}` : 'The certificate names none', urls.length ? 'good' : (v.public ? 'attention' : 'na')), + statusTile('OCSP and CRL', urls.length ? 'Named by the certificate, not contacted: revocation status unknown' : 'The certificate names none', urls.length ? 'na' : (v.public ? 'attention' : 'na'), null, null, urls.length ? urls.join('\n') : null), statusTile('Certificate Transparency', scts.length ? `${plural(scts.length, 'SCT')}: ${scts.map((s) => s.logName || 'unknown log').join(', ')}` : v.public ? 'No SCT in any of the three places' : 'No SCT: not needed for a private certificate', scts.length ? 'good' : v.public ? 'problem' : 'na'), ]; @@ -384,6 +385,9 @@ export function tlsConfiguration(ctx, about) { export function weakestAlgorithm(ev) { const rank = { broken: 0, weak: 1, acceptable: 2, strong: 3 }; const list = (ev.algorithms || []).filter((a) => bandOf(a.strength) !== 'na'); - return list.slice().sort((a, b) => (rank[bandOf(a.strength)] - rank[bandOf(b.strength)]) || ((a.strength.bits || 0) - (b.strength.bits || 0)))[0] || null; + // At equal bits, one NIST no longer allows (3DES at 112) is weaker than one it does (RSA-2048). + const allowed = (a) => (a.strength.status === 'disallowed' ? 0 : 1); + return list.slice().sort((a, b) => (rank[bandOf(a.strength)] - rank[bandOf(b.strength)]) || ((a.strength.bits || 0) - (b.strength.bits || 0)) + || (allowed(a) - allowed(b)))[0] || null; } diff --git a/web/static/style.css b/web/static/style.css index 89b54d3..9dcf245 100644 --- a/web/static/style.css +++ b/web/static/style.css @@ -1931,3 +1931,48 @@ details.ver-block[open] > summary .ver-chev { transform: rotate(90deg); } .ver-tally { display: inline-flex; align-items: center; gap: 4px; } details.ver-block[open] > summary .ver-verdict { display: none; } .ver-toggle { margin-left: 8px; vertical-align: middle; } + +/* Review of real scans (QA-REPORT-014): Fix first rows on one grid, top-aligned; one urgency chip + per row coloured by severity; the rest folded. Suite rows centre every cell on one line. */ +.fixlist .action.fixrow { display: grid; grid-template-columns: 28px 118px minmax(0, 1fr) auto; align-items: start; column-gap: 14px; padding: 14px 16px; } +.fixrow > .fix-n { padding-top: 3px; text-align: right; } +.fixrow > .action-more { padding-top: 2px; margin: 0; white-space: nowrap; } +.fixwhen { justify-self: start; display: inline-flex; align-items: center; height: 22px; padding: 0 8px; border-radius: 3px; font-size: 12px; font-weight: 600; white-space: nowrap; } +.fixwhen-critical, .fixwhen-high { color: var(--bad); background: var(--bad-bg); } +.fixwhen-medium { color: var(--warn); background: var(--warn-bg); } +.fixwhen-low, .fixwhen-info { color: var(--text-2); background: var(--surface-2); } +.fixrow .action-head { display: flex; flex-wrap: wrap; align-items: baseline; gap: 4px 10px; } +.fixrow .action-fix { display: block; margin-top: 4px; } +.fixtag { font-size: 11.5px; font-weight: 600; padding: 1px 6px; border-radius: 3px; color: var(--text-2); background: var(--surface-2); } +.fixtag-bad { color: var(--bad); background: var(--bad-bg); } +.fixmore { margin-top: 8px; } +.fixmore > summary { list-style: none; cursor: pointer; display: flex; align-items: center; gap: 10px; padding: 10px 16px; border: 1px solid var(--border); border-radius: 4px; font-size: 13.5px; font-weight: 600; color: var(--text); } +.fixmore > summary::-webkit-details-marker { display: none; } +.fixmore[open] > summary { border-bottom-left-radius: 0; border-bottom-right-radius: 0; } +.fixmore[open] > summary .ver-chev { transform: rotate(90deg); } +.fixmore-sum { margin-left: auto; font-weight: 400; color: var(--text-2); font-size: 12.5px; } +.fixmore > .fixlist { border-top: 0; border-top-left-radius: 0; border-top-right-radius: 0; } +@media (max-width: 640px) { + .fixlist .action.fixrow { grid-template-columns: 22px minmax(0, 1fr); row-gap: 6px; } + .fixrow > .fixwhen { grid-column: 2; } .fixrow > .fix-main { grid-column: 2; } .fixrow > .action-more { grid-column: 2; } +} +table.suites tbody td, table.suites tbody th { vertical-align: middle; } +table.suites .strength-cell { display: table-cell; white-space: nowrap; } +table.suites .strength-cell > .schip { vertical-align: middle; } +table.suites .strength-bits { display: inline-block; margin-left: 8px; vertical-align: middle; font-size: 12px; color: var(--text-2); font-family: var(--font-mono); } +table.suites.unranked .num { display: none; } +/* Version heads on one grid, so every version's title, facts and verdict line up down the section: + chevron, name and state, the facts, the verdict; the groups on a second line under the facts. */ +details.ver-block > summary.ver-head-row { display: grid; grid-template-columns: 12px 210px minmax(0, 1fr) auto; column-gap: 12px; row-gap: 6px; align-items: center; } +summary.ver-head-row > .ver-chev { grid-column: 1; grid-row: 1; } +summary.ver-head-row > .ver-title { grid-column: 2; grid-row: 1; } +summary.ver-head-row > .ver-meta { grid-column: 3; grid-row: 1; } +summary.ver-head-row > .ver-verdict { grid-column: 4; grid-row: 1; margin-left: 0; justify-content: flex-end; } +summary.ver-head-row > .ver-groups { grid-column: 3 / 5; grid-row: 2; } +.ver-tally { min-width: 0; } .ver-tally .schip { min-width: 84px; justify-content: center; } +@media (max-width: 860px) { + details.ver-block > summary.ver-head-row { grid-template-columns: 12px minmax(0, 1fr); } + summary.ver-head-row > .ver-meta, summary.ver-head-row > .ver-verdict, summary.ver-head-row > .ver-groups { grid-column: 2; grid-row: auto; justify-content: flex-start; } +} +@media (min-width: 521px) { .lane-row .bitsbar, .lane-axis .bb-axis-row { grid-template-columns: minmax(60px, 1fr) 13em; } } +@media (max-width: 520px) { .bb-value { white-space: normal; overflow-wrap: anywhere; } .bb-weakest { display: inline-block; margin-left: 0; margin-top: 2px; } }