fix: prevent inbound QUIC connections without peer certificate verification

[sumanjeet0012]

What was wrong?

Fixes #1345

Inbound QUIC connections were being accepted and promoted to the application callback even when the remote client sent no peer certificate. This was an authentication bypass on the official QUIC listener path, allowing unauthenticated connections to appear legitimate.

The root cause involved a combination of aioquic default behavior and libp2p security requirements:

1. In py-libp2p, verify_mode is intentionally set to ssl.CERT_NONE so that aioquic does not validate self-signed libp2p certificates against the system CA store. We perform manual libp2p peer identity verification inside connection.py instead.
2. Because verify_mode is CERT_NONE, aioquic's TLS context (quic_conn.tls) defaults to _request_client_certificate = False.
3. The server was previously attempting to request a client certificate by setting quic_conn.tls._request_client_certificate = True after calling receive_datagram on the initial packet.
4. However, receive_datagram parses the initial datagram and synchronously processes the TLS ClientHello, generating and sending the ServerHello. Since our flag was not set yet, the ServerHello omitted the CertificateRequest block.
5. Consequently, clients never sent a certificate, and connection.py silently returned None when _peer_certificate was missing, failing open and invoking the user handler with an unauthenticated connection.

What changed?

libp2p/transport/quic/connection.py

• _verify_peer_identity_with_security now explicitly raises QUICPeerVerificationError when no peer certificate is present on an inbound (server-side) connection.
• Outbound connections retain the lenient path (return None) since the client may not always have the remote cert immediately.
• Added a fallback in get_peer_certificate to extract the certificate directly from the TLS context for early access before background tasks have flagged _handshake_completed.

libp2p/transport/quic/listener.py

• Added a ServerQuicConnection subclass of aioquic's QuicConnection to cleanly hook into TLS initialization.
• Why a subclass? aioquic instantiates the TLS context internally inside a private _initialize() method on the very first datagram, right before it generates the ServerHello. We override _initialize to immediately flip self.tls._request_client_certificate = True as soon as the context is created. This ensures the ServerHello accurately includes the CertificateRequest block.
• Doing this inline without a subclass proved highly brittle because manually forcing _initialize from the outside breaks aioquic's internal "first packet" state machine handling for internal variables like _version and _network_paths.
• Added a belt-and-suspenders guard in _promote_pending_connection: inbound connections with no peer certificate are explicitly rejected and closed without invoking the user handler. connections_rejected is correctly incremented.

To-Do

• Clean up commit history
• Add or update documentation related to these changes
• Add entry to the release notes

[acul71]

AI-assisted review summary

Reviewed quic-fix @ 2c4b94b7 with local validation (lint, typecheck, 2888 tests passed, docs build). Full report: maintainer copy at downloads/AI-PR-REVIEWS/1346/AI-PR-REVIEW-1346-0.md.

Verdict: Sound security fix for #1345 — request changes before merge (missing regression test).

---

What works well

• Root-cause fix: ServerQuicConnection._initialize() sets _request_client_certificate = True before ServerHello is emitted inside receive_datagram(). Preferable to switching to ssl.CERT_OPTIONAL, which could reintroduce system-CA validation issues while libp2p still relies on manual identity verification with CERT_NONE.
• Defense in depth: (1) TLS handshake requests client cert, (2) pre-callback cert guard in _promote_pending_connection, (3) fail-closed QUICPeerVerificationError for inbound connections without a peer certificate.
• Housekeeping: Fixes #1345, valid newsfragments/1345.bugfix.rst, clean merge with main (0 behind / 3 ahead), connections_rejected now incremented on verification failure.

---

Blocking

Missing regression test for the reported vulnerability

Issue #1345 documents a PoC where a raw aioquic client sends no peer certificate yet the listener callback was invoked. This PR adds no test covering that scenario.

Please add a test that:

1. Starts a QUIC listener with a real security manager.
2. Dials with a minimal client that does not present a libp2p certificate.
3. Asserts the handler is not called and connections_rejected is incremented.

For a security fix, this should be a permanent regression test mirroring the issue PoC.

---

Should address

• test_listener.py promotion mocks (lines ~362–538): Mocks lack TLS handshake state / peer certificates. With the new pre-callback guard, _promote_pending_connection tests may pass vacuously (connection_obj is None, tests skip). Update mocks (_handshake_complete, tls._peer_certificate, or patch get_peer_certificate) so happy-path promotion is still exercised alongside a new rejection test.
• CI: tox (3.11, core) failed on unrelated test_kad_dht.py::test_put_and_get_value flake; wheel/lint jobs timed out (infra). Please re-run failed jobs.

---

Minor / follow-up

• PR title typo: certifate → certificate
• Relies on aioquic private APIs (_initialize, tls._request_client_certificate) — consider noting aioquic version compatibility
• Optional doc note in QUIC transport docs that inbound connections require a peer certificate
• peer_id = remote_peer_id or local_peer_id fallback (#1345) unchanged — acceptable follow-up since unauthenticated connections are now rejected before the callback

---

Validation

Check	Result
make lint	pass
make typecheck	pass
make test	2888 passed, 16 skipped
make linux-docs	pass
Merge conflicts with main	none

---

Questions

1. Was ssl.CERT_OPTIONAL (suggested in #1345) rejected in favor of _request_client_certificate because of CA-store side effects? Worth documenting for future reviewers.
2. Which aioquic version(s) were tested — is _initialize stable across the pinned range?
3. Can you confirm the original PoC now shows callback_invoked: false on this branch?

---

Overall

Quality	Good
Security impact	Fixes high-severity auth bypass
Merge readiness	Needs regression test + green CI

Happy to re-review once the test lands.

[acul71]

Follow-up: blocking review feedback addressed

Pushed aa3da8f5 to quic-fix addressing the request changes from my earlier review (missing regression test).

What was added

Regression coverage (#1345 PoC)

• tests/core/transport/quic/test_quic_inbound_auth.py
  • test_listener_rejects_inbound_without_peer_certificate — real QUIC listener + raw aioquic client without a libp2p TLS certificate; asserts handler not called and connections_rejected >= 1
  • test_listener_accepts_authenticated_libp2p_peer — positive control (legitimate libp2p dial still accepted)
  • test_server_quic_connection_requests_client_certificate — guards the aioquic _initialize / _request_client_certificate hook

Unit / mock fixes

• test_verify_peer_identity_raises_when_inbound_has_no_certificate in test_connection.py
• Updated TestQUICListenerRaceConditions mocks with _handshake_complete + tls._peer_certificate so promotion tests exercise the happy path under the new cert guard

Small listener hardening (found while writing the integration test)

• Moved the inbound no-cert guard before connect() so rejection increments connections_rejected (previously connect() raised first and the stat stayed at 0)
• Increment connections_rejected on promotion failure in the outer error path

Docs

• Security section in docs/libp2p.transport.quic.rst (inbound cert requirement, callback rejection, connections_rejected)
• aioquic coupling note on ServerQuicConnection

Local validation (before push)

Check	Result
make lint	pass
make typecheck	pass
make test	3051 passed, 16 skipped
make linux-docs	pass

CI status

CI is in progress on aa3da8f5 — will re-review once green.

Merge readiness

• Fixes QUIC: Inbound callback exposed before peer identity verification (authentication bypass) #1345 security bypass with defense in depth (ServerQuicConnection, pre-callback cert guard, fail-closed verification)
• Newsfragment 1345.bugfix.rst present
• Regression test now mirrors the reported PoC

Pending: green CI. If checks pass, this should be ready to merge from my side.

@sumanjeet0012 @seetadev

[acul71]

Approved

CI is green on aa3da8f5. The blocking feedback from my earlier review is addressed.

Security fix (#1345)

The original fix correctly closes the inbound QUIC authentication bypass:

• ServerQuicConnection sets _request_client_certificate before ServerHello
• Pre-callback cert guard in _promote_pending_connection
• Fail-closed QUICPeerVerificationError for inbound connections without a peer certificate

Follow-up commit (aa3da8f5)

• Regression test mirroring the #1345 PoC (raw aioquic client, no cert → handler not called, connections_rejected incremented)
• Positive control for legitimate libp2p peers
• Unit test for ServerQuicConnection and inbound verification raise path
• Promotion mock fixes in test_listener.py
• Listener hardening: cert guard before connect() so connections_rejected is counted correctly
• QUIC security documentation

Validation

All CI checks pass (core/lint/wheel/docs/interop/demos/utils matrix + Windows + RTD).

Merge readiness: Ready from my side.

[sumanjeet0012]

@acul71 @seetadev Anything left in this PR,
It is good to go from my side.