Skip to content

feat: PQC-02 certificate-key and PQC-03 SSH host-key checks (v0.7.0) - #10

Merged
t0kubetsu merged 1 commit into
mainfrom
feat/pqc02-pqc03-cert-and-hostkey-checks
Aug 17, 2026
Merged

feat: PQC-02 certificate-key and PQC-03 SSH host-key checks (v0.7.0)#10
t0kubetsu merged 1 commit into
mainfrom
feat/pqc02-pqc03-cert-and-hostkey-checks

Conversation

@t0kubetsu

@t0kubetsu t0kubetsu commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements the two remaining v4.0 PQC spec items (Docmost, Test modules → PQC, both "Should, v4.0"):

  • PQC-02 — certificate algorithm tracking: new certificate_key check on every TLS/STARTTLS assessment. The leaf certificate is fetched with a second openssl s_client probe (the primary probe uses -brief, which suppresses cert output) via the existing probe_raw() helper, then parsed with pyca/cryptography. Flags RSA < 3072 (CNSA 2.0 / BSI TR-02102-2), EC curves outside P-256/P-384/P-521/Brainpool ≥ 256 (NIST SP 800-186), and DSA (FIPS 186-5). Ed25519/Ed448 pass.
  • PQC-03 — SSH host-key algorithm check: new host_key_algorithms check on every SSH assessment. The server_host_key_algorithms name-list is parsed from the same KEXINIT packet already read for the KEX check (RFC 4253 §7.1) — zero extra round-trips. Flags ssh-dss and ssh-rsa (plus their *-cert-v01 variants).

Backend contract (IDR-018) — deliberately preserved

  • Status and Verdict enums gained no new members; existing check names untouched.
  • Verdict stays PQC-01 (key-exchange) only — new checks FAIL without flipping SAFE/UNSAFE or the exit code.
  • Cert-fetch failure yields INFO, never ERROR, so a transient second-connection failure cannot force exit code 2 or a bogus platform finding.

Design decision

cryptography>=42 added as a dependency instead of scraping openssl x509 -text: the text output is fragile across OpenSSL versions/locales, and pyca/cryptography is the canonical maintained X.509 parser. The fetch itself still goes through tls_utils subprocess calls, so the test mock boundary is unchanged.

Testing

  • 302 tests (was 247), 100% statement coverage, ruff clean.
  • Live-verified: badssl.com (RSA-2048 → FAIL), cloudflare.com (P-256 → PASS, verdict SAFE unchanged), github.com:22 (ssh-rsa → FAIL).

Summary by CodeRabbit

  • New Features

    • Added TLS certificate key-hygiene checks, including RSA size, elliptic-curve, and algorithm validation.
    • Added SSH host-key checks for deprecated DSA and RSA-SHA1 algorithms.
    • TLS and SSH results now include certificate and host-key details.
    • Classical algorithm issues are reported independently without changing the overall PQC verdict.
  • Documentation

    • Updated CLI help, README guidance, requirements, and release notes for version 0.7.0.
  • Tests

    • Expanded coverage for certificate parsing, TLS probing, SSH detection, and validation outcomes.

PQC-02: new certificate_key check — leaf certificate fetched via a second
openssl s_client probe (probe_raw, no -brief) and parsed with
pyca/cryptography; flags RSA < 3072, unapproved EC curves, and DSA.
PQC-03: new host_key_algorithms check — server_host_key_algorithms parsed
from the same SSH KEXINIT packet (RFC 4253 §7.1); flags ssh-dss and ssh-rsa.

Neither check affects the SAFE/UNSAFE verdict (PQC-01 semantics) or the CLI
exit code; missing data yields INFO, never ERROR. Status/Verdict enums are
unchanged (platform IDR-018 contract).

New dependency: cryptography>=42. Bump version to 0.7.0.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t0kubetsu has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6d8d98d1-8358-4b88-ac4d-966855019eda

📥 Commits

Reviewing files that changed from the base of the PR and between 15945c3 and f45710a.

📒 Files selected for processing (12)
  • CHANGELOG.md
  • README.md
  • pyproject.toml
  • quantumvalidator/__init__.py
  • quantumvalidator/assessor.py
  • quantumvalidator/cli.py
  • quantumvalidator/constants.py
  • quantumvalidator/tls_utils.py
  • quantumvalidator/verdict.py
  • tests/test_assessor.py
  • tests/test_tls_utils.py
  • tests/test_verdict.py

📝 Walkthrough

Walkthrough

Version 0.7.0 adds TLS certificate-key and SSH host-key algorithm checks. Probes collect the required metadata. The new checks report independently and do not change the PQC SAFE/UNSAFE verdict.

Changes

Algorithm hygiene checks

Layer / File(s) Summary
Algorithm policies and result contracts
pyproject.toml, quantumvalidator/constants.py, quantumvalidator/tls_utils.py
Added certificate-key policies, the SSH host-key denylist, CertificateInfo, new probe result fields, and the cryptography dependency.
Certificate and SSH probe collection
quantumvalidator/tls_utils.py, tests/test_tls_utils.py
TLS probes retrieve and parse leaf certificates. SSH KEXINIT parsing exposes server host-key algorithms. Tests cover retrieval, parsing, STARTTLS behavior, and failure paths.
Independent verdict checks
quantumvalidator/assessor.py, quantumvalidator/verdict.py, tests/test_assessor.py, tests/test_verdict.py
Added certificate-key and SSH host-key checks. The checks report INFO, PASS, or FAIL without changing the PQC verdict.
Release and interface documentation
CHANGELOG.md, README.md, quantumvalidator/__init__.py, quantumvalidator/cli.py
Updated the version, release notes, CLI help, dependency documentation, algorithm hygiene documentation, and test count.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TLSProbe
  participant fetch_certificate
  participant CertificateInfo
  participant build_checks
  TLSProbe->>fetch_certificate: retrieve leaf certificate
  fetch_certificate->>CertificateInfo: parse certificate key metadata
  TLSProbe->>build_checks: pass certificate and SSH host-key data
  build_checks-->>TLSProbe: return independent hygiene checks
Loading
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/pqc02-pqc03-cert-and-hostkey-checks

Comment @coderabbitai help to get the list of available commands.

@t0kubetsu

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@t0kubetsu

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@t0kubetsu

Copy link
Copy Markdown
Contributor Author

Merging without a CodeRabbit review: CodeRabbit is failing server-side today (explicit "Review failed" above; same failure on testing-platform-backend PR #35). Two manual triggers over ~35 min got no review. Evidence backing the merge: CI green on Python 3.11/3.12/3.13 with the 100% coverage gate, ruff clean, and live verification against badssl.com (RSA-2048 → FAIL), cloudflare.com (P-256 → PASS, verdict unchanged), github.com:22 (ssh-rsa → FAIL). CodeRabbit can re-review post-merge via @coderabbitai review on main if needed.

@t0kubetsu
t0kubetsu merged commit a1a9ef5 into main Aug 17, 2026
4 checks passed
@t0kubetsu
t0kubetsu deleted the feat/pqc02-pqc03-cert-and-hostkey-checks branch August 17, 2026 14:23
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Pull request is closed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Pull request is closed.

@t0kubetsu

Copy link
Copy Markdown
Contributor Author

Post-merge ECC review (posted as comment — the GitHub reviews API is returning 503 right now, which likely also explains today's CodeRabbit "Review failed"; adversarial self-review, constants verified against primary sources).

Decision: APPROVE with comments — 0 critical, 0 high, 1 medium, 2 low.

Standards verification (2026-08-17):

  • EC_APPROVED_CURVES matches BSI TR-02102-2 (2026-01) exactly (secp256r1/384r1/521r1 + brainpoolP256r1/384r1/512r1 incl. RFC 8734 tls13 codepoints). brainpoolP320r1 is deliberately absent — it appears 0 times in TR-02102-2 (TR-02102-1 only).
  • RSA < 3072 FAIL satisfies both TR-02102-2 §3.6.1 Table 14 (≥ 3000 bit) and CNSA (3072/4096), and matches the spec text verbatim.
  • P-224 FAIL backed by TR-02102-2's ECDSA ≥ 250-bit floor; DSA FAIL backed by FIPS 186-5 (cited by TR-02102-2 itself).
  • ssh-dss / ssh-rsa (+ *-cert-v01) deprecation per FIPS 186-5 / RFC 8332 — correct.

MEDIUM (follow-up, not hotfix): the certificate fetch is a second s_client run because the primary probe uses -brief. Verified empirically that a single non--brief probe emits the PEM, Negotiated TLS1.3 group:, and Protocol: — consolidating would drop one handshake per assessment (measured 0.3–0.6 s) and remove the -ign_eof/QUIT linger risk. Needs _parse_openssl_output + fixture rework; candidate for next release.

LOW: probe_raw's -ign_eof+QUIT can linger until timeout+2 s on servers that ignore garbage input (not observed on nginx/Cloudflare/SMTP stacks, all ≤ 1.5 s total); RSA-None cosmetic value when key size unparseable.

Validation: 302 tests / 100% coverage / ruff clean / CI green 3.11–3.13 / live-verified on 4 hosts.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@t0kubetsu Thanks for documenting the review failure and the validation evidence.

The merge rationale is clear. The reported CI, coverage, lint, and live verification cover the new certificate-key and SSH host-key checks.

(ᵕ•ᴗ•) 🐇

You are interacting with an AI system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant