Skip to content

ssl: retain WebPKI-selected certificate chains - #8653

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:ssl-verified-chain
Sep 5, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
youknowone:ssl-verified-chain

Conversation

@youknowone

@youknowone youknowone commented Sep 5, 2026

Copy link
Copy Markdown
Member

get_verified_chain() currently extends the peer's chain by matching issuer names. With two CAs sharing a subject but using different keys, it can report the wrong root and expose that certificate through get_ca_certs() after a capath handshake. It also reads the mutable SSLContext when the chain is requested.

Port Pyre's VerifiedChainBuilder approach: retain each connection's verification inputs, rebuild the path with WebPKI, and cache the selected leaf/intermediates/root. Publish the selected capath anchor on both client and server connections, with the certificate's actual CA classification. Keep verification inputs alongside cached configurations and explicit sessions so a declined resumption uses the same configuration that rustls verifies against. Resumed sessions report no new verified chain, matching CPython; CERT_NONE still attempts chain construction without making failure fatal.

The helper is independent of Python objects. Host I/O remains in rustpython-host_env; path selection belongs to the TLS backend. #8648 already supplies hashed-directory filtering and connection-time loading, which this change builds on. Outgoing certificate-chain auto-completion is unchanged.

Validation on macOS:

  • Workspace Rust tests, including eight new chain regressions; C-API tests run separately from crates/capi.
  • Workspace and C-API clippy; existing unrelated compiler-source warnings remain.
  • Release test_ssl: 196 tests, 43 skips. Also ran the three certificate-chain tests separately because their enclosing PHA class is skipped on rustls.
  • BIO checks against CPython 3.14 for context replacement, capath statistics, explicitly trusted leaves, CERT_NONE, mutual TLS, and accepted/declined session resumption.
  • Repository pre-commit hooks.

Implementation and validation assisted by Codex (GPT-6).

Summary by CodeRabbit

  • Bug Fixes
    • Improved TLS certificate-chain verification to accurately preserve and report the complete verified chain.
    • Improved trust-anchor matching for more reliable certificate validation.
    • Added stronger handling for trusted leaf certificates, revocation checks, and client certificate authentication.
    • Improved behavior across resumed and unverified TLS connections.
  • Security
    • Enhanced Rustls-based TLS validation using full certificate data rather than issuer-name matching alone.

Port the connection-owned chain builder from Pyre. Preserve verification inputs with TLS configurations and sessions, report the WebPKI-selected path, and publish only its selected capath anchor. Match CPython chain reporting for CERT_NONE and resumed sessions.

Assisted-by: Codex:GPT-6
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Team

Run ID: 13da7305-ec2d-4ffa-b380-63404fdf564e

📥 Commits

Reviewing files that changed from the base of the PR and between dde7d24 and d112860.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • Cargo.toml
  • crates/stdlib/Cargo.toml
  • crates/stdlib/src/ssl.rs
  • crates/stdlib/src/ssl/chain.rs
  • crates/stdlib/src/ssl/compat.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The SSL backend adds full certificate-chain verification with rustls-webpki. TLS configurations now retain chain builders, sockets store verified paths, and sessions preserve the builder for resumed connections.

Changes

TLS verified-chain support

Layer / File(s) Summary
Verified chain construction
Cargo.toml, crates/stdlib/Cargo.toml, crates/stdlib/src/ssl.rs, crates/stdlib/src/ssl/chain.rs
Adds VerifiedChainBuilder with server-auth, client-auth, unverified, revocation, trusted-leaf, and full-DER trust-anchor handling.
TLS configuration wiring
crates/stdlib/src/ssl/compat.rs, crates/stdlib/src/ssl.rs
Client and server configuration builders retain chain builders with root certificates, DER certificates, CRLs, and verification settings.
Handshake and session chain state
crates/stdlib/src/ssl.rs
Sockets record verified chains after handshakes, publish selected anchors, handle resumed sessions, and return stored chains. Sessions retain the chain builder.

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

Merge Risk: ⚪ Minimal · up to d1128

This change records the WebPKI-selected verified TLS certificate path per connection, including accurate anchor handling and resumed-session behavior. No concrete current-head merge blocker remains.

Suggested reviewers: joshuamegnauth54, shaharnaveh

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 3 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: retaining the certificate chains selected by WebPKI during SSL verification.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 64.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 3 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@youknowone
youknowone merged commit a8aa383 into RustPython:main Sep 5, 2026
28 of 29 checks passed
@youknowone
youknowone deleted the ssl-verified-chain branch September 5, 2026 10:35
youknowone added a commit that referenced this pull request Sep 16, 2026
Port the connection-owned chain builder from Pyre. Preserve verification inputs with TLS configurations and sessions, report the WebPKI-selected path, and publish only its selected capath anchor. Match CPython chain reporting for CERT_NONE and resumed sessions.

Assisted-by: Codex:GPT-6
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