Merge pull request #1341 from sumanjeet0012/tls-fix

acul71 · web-flow · commit 50f3a07ea2c5 · 2026-06-28T22:25:49.000+02:00
fix(tls): reject inbound connections that present no client certificate
diff --git a/docs/tls-support.rst b/docs/tls-support.rst
@@ -158,6 +158,21 @@ Testing TLS Connections
 Security Considerations
 -----------------------
 
+Mutual authentication (inbound)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Inbound TLS connections **must** present a client certificate that carries the
+libp2p X.509 extension so py-libp2p can derive and verify the remote Peer ID.
+The server-side SSL context requests a client certificate (``CERT_OPTIONAL``);
+post-handshake logic enforces the requirement. If the remote peer completes the
+TLS handshake without sending a certificate, the connection is rejected and no
+``SecureSession`` is created.
+
+**AutoTLS exception:** When ``enable_autotls=True``, broker registration may
+use the primitive key-exchange side-channel or a placeholder identity instead
+of a libp2p client certificate. This path is scoped exclusively to the
+AutoTLS bootstrap flow and is not reachable on a standard node.
+
 - Never disable certificate verification in production.
 - Use TLS 1.3 or later.
 - Pin certificates for critical peers.
@@ -181,6 +196,9 @@ Troubleshooting
    * - SSL handshake failure
      - TLS version mismatch or clock skew
      - Enforce TLS 1.3, sync system clock.
+   * - Inbound connection rejected / handshake failure
+     - Remote peer sent no client certificate
+     - Ensure the dialer uses libp2p TLS with an identity certificate; do not connect with a plain TLS client.
    * - Connection refused
      - Port blocked or listener not running
      - Check firewall rules and listener status.
diff --git a/libp2p/security/tls/exceptions.py b/libp2p/security/tls/exceptions.py
@@ -1,6 +1,7 @@
 """TLS security transport exceptions."""
 
 from libp2p.exceptions import BaseLibp2pError
+from libp2p.security.exceptions import HandshakeFailure
 
 
 class TLSError(BaseLibp2pError):
@@ -19,3 +20,15 @@ class MissingLibp2pExtensionError(TLSError):
     """
 
     pass
+
+
+class TLSHandshakeFailure(HandshakeFailure):
+    """
+    Raised when the TLS handshake cannot be completed due to an
+    authentication failure, such as a missing or invalid peer certificate.
+
+    This is a hard failure: the connection must be closed immediately and
+    must not be surfaced as an authenticated SecureSession.
+    """
+
+    pass
diff --git a/libp2p/security/tls/io.py b/libp2p/security/tls/io.py
@@ -442,7 +442,7 @@ async def handshake(self, enable_autotls: bool = False) -> None:
         Perform TLS handshake.
 
         Raises:
-            HandshakeFailure: If handshake fails due to protocol errors
+            TLSHandshakeFailure: If handshake fails due to protocol errors
             RuntimeError: If handshake timeout or connection errors occur
             ssl.SSLError: For SSL-specific errors not related to want read/write
 
diff --git a/libp2p/security/tls/transport.py b/libp2p/security/tls/transport.py
@@ -18,7 +18,10 @@
     generate_certificate,
     verify_certificate_chain,
 )
-from libp2p.security.tls.exceptions import MissingLibp2pExtensionError
+from libp2p.security.tls.exceptions import (
+    MissingLibp2pExtensionError,
+    TLSHandshakeFailure,
+)
 from libp2p.security.tls.io import TLSReadWriter
 import libp2p.utils
 import libp2p.utils.paths
@@ -127,15 +130,20 @@ def create_ssl_context(self, server_side: bool = False) -> ssl.SSLContext:
             ssl.PROTOCOL_TLS_SERVER if server_side else ssl.PROTOCOL_TLS_CLIENT
         )
         ctx.minimum_version = ssl.TLSVersion.TLSv1_3
-        # We do our own verification (like Go's InsecureSkipVerify).
-        # Python's ssl module can't request a client cert without CA verification.
-        # TODO: Implement proper mutual TLS with custom verification
+        # We do our own post-handshake verification of the libp2p extension.
+        # check_hostname is disabled because we use self-signed peer certificates
+        # that are not bound to DNS names.
         ctx.check_hostname = False
 
-        # TODO: Fix this default: no client cert verification
-        # INBOUND connection can't ask for tls cert from remote peers
-        # ctx.verify_mode = ssl.CERT_OPTIONAL if server_side else ssl.CERT_NONE
-        ctx.verify_mode = ssl.CERT_NONE
+        # For server-side (inbound) contexts we use CERT_OPTIONAL so that the
+        # TLS engine *requests* a client certificate from the remote peer.
+        # Without a CA configured Python will not reject an absent cert at the
+        # TLS layer, but the cert IS made available via getpeercert(binary_form=True)
+        # if one is sent.  Our post-handshake logic then enforces the hard
+        # requirement.
+        # For client-side (outbound) contexts CERT_NONE is fine because the
+        # server always sends a certificate in TLS 1.3.
+        ctx.verify_mode = ssl.CERT_OPTIONAL if server_side else ssl.CERT_NONE
 
         # Load our cached self-signed certificate bound to libp2p identity
         import os
@@ -304,55 +312,66 @@ async def secure_inbound(self, conn: IRawConnection) -> ISecureConn:
         # Extract peer information
         peer_cert = tls_reader_writer.get_peer_certificate()
         if not peer_cert:
-            logger.warning("[INBOUND] Server couldn't fetch dialer's certificate")
-
-            # TODO: Python ssl can't request client cert without CA verification.
-            # Use placeholder peer ID - client can still verify server identity.
-            logger.warning("TLS inbound: no peer cert (Python ssl limitation)")
+            # The libp2p TLS spec mandates mutual authentication: every inbound
+            # connection MUST present a certificate that carries the libp2p
+            # extension so we can derive and verify the remote Peer ID.
+            #
+            # AutoTLS bootstrap connections are a legitimate exception: when a
+            # node registers with the ACME broker it uses the primitive
+            # key-exchange side-channel instead of the certificate extension.
+            # That path is kept here but is strictly guarded by enable_autotls.
+            # The normal (non-AutoTLS) path raises a hard TLSHandshakeFailure and
+            # never returns a synthetic placeholder identity.
 
-            # Extaract the keys from primitive key-exchange, if autotls enabled
             if self.enable_autotls:
                 remote_public_key = tls_reader_writer.remote_primitive_pk
                 remote_peer_id = tls_reader_writer.remote_primitive_peerid
                 logger.warning(
-                    "TLS inbound: using peerid obtained from primitive key-exchange"
-                )
-            else:
-                placeholder_keypair = libp2p.generate_new_ed25519_identity()
-                remote_public_key = placeholder_keypair.public_key
-                remote_peer_id = ID.from_pubkey(remote_public_key)
-                logger.error(
-                    "TLS inbound: using peerid obtained from placeholder keypair"
+                    "TLS inbound [autotls]: no peer cert; "
+                    "using peer ID from primitive key-exchange"
                 )
 
-            # This is the case when the autotls is enabled, and we did a self-signed
-            # certificate handshake with AUTO-TLS BROKER, and naturally we didn't do the
-            # primitive peer-identify exchange, so again use a placeholde
-            if self.enable_autotls and remote_peer_id is None:
-                placeholder_keypair = libp2p.generate_new_ed25519_identity()
-                remote_public_key = placeholder_keypair.public_key
-                remote_peer_id = ID.from_pubkey(remote_public_key)
+                # AutoTLS broker registration: primitive exchange also absent.
+                # Use a placeholder exclusively for the broker session.
+                if remote_peer_id is None:
+                    placeholder_keypair = libp2p.generate_new_ed25519_identity()
+                    remote_public_key = placeholder_keypair.public_key
+                    remote_peer_id = ID.from_pubkey(remote_public_key)
+                    logger.warning(
+                        "TLS inbound [autotls broker]: no cert and no primitive "
+                        "exchange; using placeholder identity for broker session only"
+                    )
+
+                if remote_peer_id is None or remote_public_key is None:
+                    raise TLSHandshakeFailure(
+                        "TLS inbound [autotls]: could not determine remote identity "
+                        "from either certificate or primitive key-exchange."
+                    )
 
-            if remote_peer_id is None:
-                raise ValueError(
-                    "remote peer ID must be known before creating SecureSession"
+                session = SecureSession(
+                    local_peer=self.local_peer,
+                    local_private_key=self.libp2p_privkey,
+                    remote_peer=remote_peer_id,
+                    remote_permanent_pubkey=remote_public_key,
+                    is_initiator=False,
+                    conn=tls_reader_writer,
                 )
-
-            if remote_public_key is None:
-                raise ValueError(
-                    "remote public-key must be known before creating SecureSession"
+                logger.debug(
+                    "TLS secure_inbound [autotls]: returning SecureSession "
+                    "with primitive-exchange identity"
                 )
+                return session
 
-            session = SecureSession(
-                local_peer=self.local_peer,
-                local_private_key=self.libp2p_privkey,
-                remote_peer=remote_peer_id,
-                remote_permanent_pubkey=remote_public_key,
-                is_initiator=False,
-                conn=tls_reader_writer,
+            # Normal (non-AutoTLS) path: no certificate → hard failure.
+            # Do NOT create a session or assign a synthetic identity.
+            logger.error(
+                "[INBOUND] Rejecting connection: remote peer sent no TLS certificate. "
+                "Mutual authentication is required by the libp2p TLS spec."
+            )
+            raise TLSHandshakeFailure(
+                "Inbound TLS connection presented no client certificate. "
+                "Mutual authentication is required by the libp2p TLS spec."
             )
-            logger.debug("TLS secure_inbound: returning placeholder SecureSession")
-            return session
 
         # Extract remote public key from certificate
         logger.debug("TLS secure_inbound: extracting public key from certificate")
diff --git a/libp2p/transport/webrtc/_aiortc_helpers.py b/libp2p/transport/webrtc/_aiortc_helpers.py
@@ -19,6 +19,7 @@
 
 from aiortc import (
     RTCConfiguration,
+    RTCIceServer,
     RTCPeerConnection,
 )
 from aiortc.rtcdtlstransport import RTCCertificate
@@ -69,7 +70,10 @@ async def create_peer_connection(
     :param ice_servers: Optional STUN/TURN server URLs.
     :returns: A new peer connection.
     """
-    config = RTCConfiguration(iceServers=list(ice_servers) if ice_servers else [])
+    ice_server_list = (
+        [RTCIceServer(urls=[url]) for url in ice_servers] if ice_servers else []
+    )
+    config = RTCConfiguration(iceServers=ice_server_list)
     pc = RTCPeerConnection(configuration=config)
     # Replace aiortc's auto-generated cert.  Must use the mangled name —
     # aiortc reads only `self.__certificates`, which mangles to this.
diff --git a/newsfragments/1340.bugfix.rst b/newsfragments/1340.bugfix.rst
@@ -0,0 +1 @@
+Fixed TLS inbound authentication bypass where a remote peer could complete the libp2p TLS handshake without presenting a client certificate and still receive a valid ``SecureSession`` with a synthetic placeholder Peer ID. The server-side SSL context now requests a client certificate, and a missing certificate is treated as a hard handshake failure rather than a recoverable warning.
diff --git a/tests/core/security/tls/test_tls.py b/tests/core/security/tls/test_tls.py
@@ -1,8 +1,75 @@
+from unittest.mock import AsyncMock, MagicMock, patch
+
 import pytest
 
 from libp2p import generate_new_rsa_identity
+from libp2p.peer.id import ID
+from libp2p.security.exceptions import HandshakeFailure
+from libp2p.security.tls.exceptions import TLSHandshakeFailure
 from libp2p.security.tls.transport import TLSTransport
-from tests.utils.factories import tls_conn_factory
+from libp2p.transport.exceptions import SecurityUpgradeFailure
+from libp2p.transport.upgrader import TransportUpgrader
+from tests.utils.factories import (
+    TLS_PROTOCOL_ID,
+    default_mplex_muxer_transport_factory,
+    raw_conn_factory,
+    security_options_factory_factory,
+    tls_conn_factory,
+)
+
+
+def test_tls_handshake_failure_extends_central_handshake_failure() -> None:
+    assert issubclass(TLSHandshakeFailure, HandshakeFailure)
+
+
+def _mock_tls_reader_writer_no_cert() -> MagicMock:
+    mock_rw = MagicMock()
+    mock_rw.handshake = AsyncMock()
+    mock_rw.get_peer_certificate.return_value = None
+    return mock_rw
+
+
+@pytest.mark.trio
+async def test_secure_inbound_rejects_missing_client_certificate() -> None:
+    keypair = generate_new_rsa_identity()
+    transport = TLSTransport(keypair, enable_autotls=False)
+    mock_conn = MagicMock()
+    mock_rw = _mock_tls_reader_writer_no_cert()
+
+    with (
+        patch("libp2p.security.tls.transport.TLSReadWriter", return_value=mock_rw),
+        pytest.raises(TLSHandshakeFailure, match="no client certificate"),
+    ):
+        await transport.secure_inbound(mock_conn)
+
+
+@pytest.mark.trio
+async def test_upgrader_rejects_missing_client_certificate(nursery) -> None:
+    keypair = generate_new_rsa_identity()
+    sec_opt = security_options_factory_factory(TLS_PROTOCOL_ID)(keypair)
+    upgrader = TransportUpgrader(sec_opt, default_mplex_muxer_transport_factory())
+    mock_rw = _mock_tls_reader_writer_no_cert()
+
+    with patch("libp2p.security.tls.transport.TLSReadWriter", return_value=mock_rw):
+        async with raw_conn_factory(nursery) as (_local_conn, remote_conn):
+            with pytest.raises(SecurityUpgradeFailure):
+                await upgrader.upgrade_security(remote_conn, False)
+
+
+@pytest.mark.trio
+async def test_secure_inbound_autotls_allows_primitive_exchange_identity() -> None:
+    server_keypair = generate_new_rsa_identity()
+    client_keypair = generate_new_rsa_identity()
+    transport = TLSTransport(server_keypair, enable_autotls=True)
+    mock_conn = MagicMock()
+    mock_rw = _mock_tls_reader_writer_no_cert()
+    mock_rw.remote_primitive_pk = client_keypair.public_key
+    mock_rw.remote_primitive_peerid = ID.from_pubkey(client_keypair.public_key)
+
+    with patch("libp2p.security.tls.transport.TLSReadWriter", return_value=mock_rw):
+        session = await transport.secure_inbound(mock_conn)
+
+    assert session.get_remote_peer() == ID.from_pubkey(client_keypair.public_key)
 
 
 @pytest.mark.trio
diff --git a/tests/core/transport/webrtc/test_multiaddr_utils.py b/tests/core/transport/webrtc/test_multiaddr_utils.py
@@ -23,8 +23,10 @@ def test_valid_ipv4_webrtc_direct(self):
         assert is_webrtc_direct_multiaddr(maddr)
 
     def test_valid_ipv4_with_certhash(self):
+        cert = WebRTCCertificate.generate()
+        certhash = cert.fingerprint_to_multibase()
         maddr = Multiaddr(
-            "/ip4/192.168.1.1/udp/4001/webrtc-direct/certhash/uEiBkEKoo3S"
+            f"/ip4/192.168.1.1/udp/4001/webrtc-direct/certhash/{certhash}"
         )
         assert is_webrtc_direct_multiaddr(maddr)
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Fixed TLS inbound authentication bypass where a remote peer could complete the libp2p TLS handshake without presenting a client certificate and still receive a valid ``SecureSession`` with a synthetic placeholder Peer ID. The server-side SSL context now requests a client certificate, and a missing certificate is treated as a hard handshake failure rather than a recoverable warning.