From b8f776f4f16e0f2f3f54f5b65d3f3291f06ad02e Mon Sep 17 00:00:00 2001 From: sumanjeet0012 Date: Sat, 13 Jun 2026 20:10:21 +0530 Subject: [PATCH] fix: prevent inbound QUIC connections from proceeding without peer certificate verification --- libp2p/transport/quic/connection.py | 28 +++++++++++++++-- libp2p/transport/quic/listener.py | 48 ++++++++++++++++++++++------- newsfragments/1345.bugfix.rst | 1 + 3 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 newsfragments/1345.bugfix.rst diff --git a/libp2p/transport/quic/connection.py b/libp2p/transport/quic/connection.py index f0cd45369..d2d9127c2 100644 --- a/libp2p/transport/quic/connection.py +++ b/libp2p/transport/quic/connection.py @@ -593,6 +593,17 @@ async def _verify_peer_identity_with_security(self) -> ID | None: await self._extract_peer_certificate() if not self._peer_certificate: + # Inbound (server-side) connections MUST present a peer + # certificate so the remote libp2p identity can be established. + # Accepting a connection without one would allow unauthenticated + # peers to reach the application callback — the libp2p QUIC + # trust boundary requires verified peer identity before promotion. + if not self._is_initiator: + raise QUICPeerVerificationError( + "Inbound QUIC connection has no peer certificate: " + "remote peer identity cannot be established. " + "Connection rejected." + ) logger.debug("No peer certificate available for verification") return None @@ -674,8 +685,21 @@ async def get_peer_certificate(self) -> x509.Certificate | None: The peer's X.509 certificate, or None if not available """ - # If we don't have a certificate yet, try to extract it - if not self._peer_certificate and self._handshake_completed: + # Fast path: already extracted + if self._peer_certificate: + return self._peer_certificate + + # If our high-level handshake flag is set, extract now + if self._handshake_completed: + await self._extract_peer_certificate() + return self._peer_certificate + + # For server-side connections promoted before background tasks have + # processed the HandshakeCompleted event, the aioquic-level TLS + # handshake is already done (quic_conn._handshake_complete is True) + # but our _handshake_completed flag hasn't been set yet. Read the + # certificate directly from the TLS context in that case. + if self._quic and getattr(self._quic, "_handshake_complete", False): await self._extract_peer_certificate() return self._peer_certificate diff --git a/libp2p/transport/quic/listener.py b/libp2p/transport/quic/listener.py index bf25e4e26..cf562bd4b 100644 --- a/libp2p/transport/quic/listener.py +++ b/libp2p/transport/quic/listener.py @@ -51,6 +51,21 @@ logger.setLevel(logging.DEBUG) +class ServerQuicConnection(QuicConnection): + """ + A custom QuicConnection that ensures the server requests a client certificate. + aioquic's tls.Context defaults _request_client_certificate to False. Since + the ServerHello is generated synchronously inside receive_datagram, we must + override _initialize to set the flag immediately after the TLS context is created. + """ + + def _initialize(self, peer_cid: bytes) -> None: + super()._initialize(peer_cid) + if hasattr(self, "tls") and self.tls: + self.tls._request_client_certificate = True + logger.debug("ServerQuicConnection: Set _request_client_certificate=True") + + class QUICPacketInfo: """Information extracted from a QUIC packet header.""" @@ -587,7 +602,7 @@ async def _handle_new_connection( f"{packet_info.destination_cid.hex()}" ) - quic_conn = QuicConnection( + quic_conn = ServerQuicConnection( configuration=server_config, original_destination_connection_id=packet_info.destination_cid, ) @@ -636,17 +651,9 @@ async def _handle_new_connection( initial_dcid, quic_conn, addr, sequence ) - # Process initial packet + # Process initial packet. Since we use ServerQuicConnection, the + # TLS context will be initialized with _request_client_certificate=True quic_conn.receive_datagram(data, addr, now=time.time()) - if quic_conn.tls: - if self._security_manager: - try: - quic_conn.tls._request_client_certificate = True - logger.debug( - "request_client_certificate set to True in server TLS" - ) - except Exception as e: - logger.error(f"FAILED to apply request_client_certificate: {e}") # Process events and send response await self._process_quic_events(quic_conn, addr, destination_connection_id) @@ -1061,6 +1068,24 @@ async def _promote_pending_connection( if not getattr(connection, "_background_tasks_started", False): await connection.connect(self._nursery) + # Belt-and-suspenders: reject inbound connections that have no + # peer certificate before we even attempt security verification. + # _verify_peer_identity_with_security already raises for this + # case, but checking here provides an early, explicit rejection + # that is independent of the security manager being configured. + if not connection._is_initiator: + peer_cert = await connection.get_peer_certificate() + if peer_cert is None: + logger.error( + f"Rejecting inbound connection " + f"{destination_connection_id.hex()}: " + f"no peer certificate presented — " + f"remote libp2p identity cannot be verified." + ) + self._stats["connections_rejected"] += 1 + await connection.close() + return + if self._security_manager: try: peer_id = await connection._verify_peer_identity_with_security() @@ -1075,6 +1100,7 @@ async def _promote_pending_connection( f"Security verification failed for " f"{destination_connection_id.hex()}: {e}" ) + self._stats["connections_rejected"] += 1 await connection.close() return diff --git a/newsfragments/1345.bugfix.rst b/newsfragments/1345.bugfix.rst new file mode 100644 index 000000000..a88ab54f6 --- /dev/null +++ b/newsfragments/1345.bugfix.rst @@ -0,0 +1 @@ +Fixed an authentication bypass where inbound QUIC connections were accepted without verifying the peer's libp2p certificate.