Skip to content

Commit aa20ea2

Browse files
committed
fix(tls,quic): expose libp2p host pubkey as remotePubKey, not the ephemeral cert subject key
Before this change, `SecureChannel.Session.remotePubKey` returned by the TLS and QUIC transports was the ephemeral X.509 cert subject key (created per handshake), not the libp2p host public key embedded in the libp2p cert extension (OID 1.3.6.1.4.1.53594.1.1). This broke the invariant `PeerId.fromPubKey(remotePubKey) == remoteId` that SecIO, Noise, and Plaintext uphold and that downstream consumers rely on (e.g. signed-record verification). Changes: * TLSSecureChannel.kt: introduce `TlsPeerIdentity(peerId, pubKey)` and `verifyAndExtractIdentity(chain)` which derives both fields from the same libp2p host key unmarshalled out of the cert extension. The legacy `verifyAndExtractPeerId` now delegates so existing call sites (CertificatesTest, Libp2pTrustManager) continue to work unchanged. The session-construction site in `buildTlsHandler` is updated to use the new helper, so `remotePubKey` is now the libp2p key, not the ephemeral subject key. * QuicTransport.kt (server inbound): replace the separate `verifyAndExtractPeerId` + `getPublicKeyFromCert` pair with a single `verifyAndExtractIdentity` call. Both `remoteId` and `remotePubKey` now come from the same libp2p host key. * QuicTransport.kt (client dial): delete the `Multihash.Digest.Identity` special case entirely. The libp2p cert extension is the single source of truth for the remote libp2p public key regardless of how the peer id is encoded in the multiaddr (identity-digest vs sha-256). Add a sanity check that the libp2p key extracted from the cert hashes to the dial-target peer id — defence-in-depth on top of `Libp2pTrustManager`. Remove now-unused imports (`Multihash`, `unmarshalPublicKey`, `getPublicKeyFromCert`). Tests: * TlsSecureChannelTest now extends `CipherSecureChannelTest` (matching the SecIO/Noise pattern), which contributes `verify secure session` asserting `remotePubKey == pubKey<remote>` on both sides. The shared malformed-cipher-bytes test is marked `open` in the base and overridden with `@Disabled` for TLS because OpenSSL surfaces those errors through a different path that is out of scope here. * New `QuicRemotePubKeyIdentityTest` exercises a real QUIC handshake between two hosts and asserts `PeerId.fromPubKey(secureSession.remotePubKey) == secureSession.remoteId` on both the client and server connection.
1 parent 138fe9e commit aa20ea2

5 files changed

Lines changed: 162 additions & 61 deletions

File tree

libp2p/src/main/kotlin/io/libp2p/security/tls/TLSSecureChannel.kt

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,12 @@ fun buildTlsHandler(
150150
NegotiatedStreamMuxer(mux, nextProtocol)
151151
}
152152
.firstOrNull()
153+
val remoteIdentity = verifyAndExtractIdentity(engine.session.peerCertificates)
153154
handshakeComplete.complete(
154155
SecureChannel.Session(
155156
PeerId.fromPubKey(localKey.publicKey()),
156-
verifyAndExtractPeerId(engine.session.peerCertificates),
157-
getPublicKeyFromCert(engine.session.peerCertificates),
157+
remoteIdentity.peerId,
158+
remoteIdentity.pubKey,
158159
selectedMuxer
159160
)
160161
)
@@ -278,7 +279,27 @@ fun getContentVerifier(bcX509Cert: X509CertificateHolder): ContentVerifierProvid
278279
return BcECContentVerifierProviderBuilder(DefaultDigestAlgorithmIdentifierFinder()).build(bcX509Cert)
279280
}
280281

281-
fun verifyAndExtractPeerId(chain: Array<Certificate>): PeerId {
282+
/**
283+
* The libp2p identity that a TLS peer asserts via the libp2p certificate extension.
284+
*
285+
* Both [peerId] and [pubKey] are derived from the SAME unmarshalled libp2p host public key
286+
* embedded in the certificate extension (OID 1.3.6.1.4.1.53594.1.1). This guarantees the
287+
* invariant `PeerId.fromPubKey(pubKey) == peerId`, which downstream consumers
288+
* (e.g. signed-record verification) rely on.
289+
*/
290+
data class TlsPeerIdentity(val peerId: PeerId, val pubKey: PubKey)
291+
292+
/**
293+
* Validate a peer's libp2p TLS certificate (chain of exactly one self-signed cert) and
294+
* return the libp2p identity (peer id + libp2p host public key) carried in its extension.
295+
*
296+
* Validation steps mirror the libp2p TLS specification:
297+
* - extract the libp2p host public key + signature from the extension (OID 1.3.6.1.4.1.53594.1.1);
298+
* - verify the signature is over `certificatePrefix || cert.subjectPublicKeyInfo` using the libp2p key;
299+
* - verify the cert's self-signature using its (ephemeral) subject key;
300+
* - check the cert's validity period.
301+
*/
302+
fun verifyAndExtractIdentity(chain: Array<Certificate>): TlsPeerIdentity {
282303
if (chain.size != 1) {
283304
throw java.lang.IllegalStateException("Cert chain must have exactly 1 element!")
284305
}
@@ -314,9 +335,18 @@ fun verifyAndExtractPeerId(chain: Array<Certificate>): PeerId {
314335
if (bcCert.startDate.date.after(now)) {
315336
throw IllegalStateException("TLS certificate is not valid yet!")
316337
}
317-
return PeerId.fromPubKey(pubKey)
338+
return TlsPeerIdentity(PeerId.fromPubKey(pubKey), pubKey)
318339
}
319340

341+
/**
342+
* Backwards-compatible helper that returns only the [PeerId] portion of [verifyAndExtractIdentity].
343+
*
344+
* Prefer [verifyAndExtractIdentity] at session-construction sites so that the libp2p pubkey
345+
* can be exposed via `SecureChannel.Session.remotePubKey` without re-parsing the certificate.
346+
*/
347+
fun verifyAndExtractPeerId(chain: Array<Certificate>): PeerId =
348+
verifyAndExtractIdentity(chain).peerId
349+
320350
fun getAlgorithmName(oid: String): String {
321351
if ("1.2.840.113549.1.1.1".equals(oid)) {
322352
return "RSA"

libp2p/src/main/kotlin/io/libp2p/transport/quic/QuicTransport.kt

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@ package io.libp2p.transport.quic
22

33
import io.libp2p.core.*
44
import io.libp2p.core.crypto.PrivKey
5-
import io.libp2p.core.crypto.unmarshalPublicKey
65
import io.libp2p.core.multiformats.Multiaddr
76
import io.libp2p.core.multiformats.MultiaddrDns
8-
import io.libp2p.core.multiformats.Multihash
97
import io.libp2p.core.multiformats.Protocol.*
108
import io.libp2p.core.multistream.MultistreamProtocol
119
import io.libp2p.core.multistream.MultistreamProtocolV1
@@ -22,8 +20,7 @@ import io.libp2p.etc.util.netty.nettyInitializer
2220
import io.libp2p.security.tls.Libp2pTrustManager
2321
import io.libp2p.security.tls.buildCert
2422
import io.libp2p.security.tls.getJavaKey
25-
import io.libp2p.security.tls.getPublicKeyFromCert
26-
import io.libp2p.security.tls.verifyAndExtractPeerId
23+
import io.libp2p.security.tls.verifyAndExtractIdentity
2724
import io.libp2p.transport.implementation.ConnectionOverNetty
2825
import io.libp2p.transport.implementation.NettyTransport
2926
import io.netty.bootstrap.Bootstrap
@@ -270,31 +267,30 @@ class QuicTransport(
270267
val peerCerts = it.sslEngine()?.session?.peerCertificates
271268
?: throw Libp2pException("No peer certificates available after QUIC handshake with $addr")
272269

270+
// The libp2p host pubkey is carried in the cert extension and is the only
271+
// source of truth for `SecureChannel.Session.remotePubKey` (regardless of how
272+
// the peer id was encoded on the wire — `identity` digest vs. sha-256). The
273+
// ephemeral cert subject key MUST NOT be exposed here; doing so breaks the
274+
// invariant `PeerId.fromPubKey(remotePubKey) == remoteId` that the rest of the
275+
// libp2p stack relies on. Read it from this connection's verified certificates,
276+
// not the shared trust-manager state (avoids a cross-connection race).
277+
val remoteIdentity = verifyAndExtractIdentity(peerCerts)
273278
val expectedPeerId = addr.getPeerId()
274-
val remotePeerId: PeerId
275-
val remotePubKey: io.libp2p.core.crypto.PubKey
276-
277-
if (expectedPeerId != null) {
278-
// PeerId was pre-validated by trustManager during TLS handshake.
279-
// For inline-key peerIds (identity multihash), extract pubkey from the multihash.
280-
val pubHash = Multihash.of(expectedPeerId.bytes.toByteBuf())
281-
remotePubKey = if (pubHash.desc.digest == Multihash.Digest.Identity) {
282-
unmarshalPublicKey(pubHash.bytes.toByteArray())
283-
} else {
284-
getPublicKeyFromCert(peerCerts)
285-
}
286-
remotePeerId = expectedPeerId
287-
} else {
288-
// No PeerId known upfront — extract from TLS certificate post-handshake.
289-
remotePubKey = getPublicKeyFromCert(peerCerts)
290-
remotePeerId = verifyAndExtractPeerId(peerCerts)
279+
if (expectedPeerId != null && remoteIdentity.peerId != expectedPeerId) {
280+
// Defence-in-depth: Libp2pTrustManager already enforces this when an
281+
// expectedRemotePeer is provided, but assert at the session-construction
282+
// site so a misconfigured trust manager cannot silently leak the wrong
283+
// identity into the connection.
284+
throw Libp2pException(
285+
"Remote peer presented libp2p pubkey for ${remoteIdentity.peerId} but dial target was $expectedPeerId"
286+
)
291287
}
292288

293289
connection.setSecureSession(
294290
SecureChannel.Session(
295291
PeerId.fromPubKey(localKey.publicKey()),
296-
remotePeerId,
297-
remotePubKey,
292+
remoteIdentity.peerId,
293+
remoteIdentity.pubKey,
298294
null
299295
)
300296
)
@@ -415,16 +411,19 @@ class QuicTransport(
415411
val peerCerts = (ctx.channel() as QuicChannel).sslEngine()
416412
?.session?.peerCertificates
417413
if (!peerCerts.isNullOrEmpty()) {
418-
val remotePeerId = verifyAndExtractPeerId(peerCerts)
419-
val remotePublicKey = getPublicKeyFromCert(peerCerts)
414+
// Both remoteId and remotePubKey must come from the same libp2p
415+
// host key carried in the certificate extension — never from the
416+
// ephemeral cert subject key. See SecureChannel.Session contract:
417+
// PeerId.fromPubKey(remotePubKey) == remoteId.
418+
val remoteIdentity = verifyAndExtractIdentity(peerCerts)
420419

421-
logger.info("Handshake completed with remote peer id: {}", remotePeerId)
420+
logger.info("Handshake completed with remote peer id: {}", remoteIdentity.peerId)
422421

423422
connection.setSecureSession(
424423
SecureChannel.Session(
425424
PeerId.fromPubKey(localKey.publicKey()),
426-
remotePeerId,
427-
remotePublicKey,
425+
remoteIdentity.peerId,
426+
remoteIdentity.pubKey,
428427
null
429428
)
430429
)
@@ -533,15 +532,22 @@ class QuicTransport(
533532
val peerCerts = quicChannel.sslEngine()?.session?.peerCertificates
534533
?: throw Libp2pException("No peer certificates in hole-punched connection from $addr")
535534

535+
// remoteId and remotePubKey must both come from the libp2p host key in the cert
536+
// extension (never the ephemeral cert subject key), so PeerId.fromPubKey(remotePubKey)
537+
// == remoteId holds. See SecureChannel.Session contract.
538+
val remoteIdentity = verifyAndExtractIdentity(peerCerts)
536539
val expectedPeerId = addr.getPeerId()
537-
val remotePeerId = expectedPeerId ?: verifyAndExtractPeerId(peerCerts)
538-
val remotePubKey = getPublicKeyFromCert(peerCerts)
540+
if (expectedPeerId != null && remoteIdentity.peerId != expectedPeerId) {
541+
throw Libp2pException(
542+
"Remote peer presented libp2p pubkey for ${remoteIdentity.peerId} but dial target was $expectedPeerId"
543+
)
544+
}
539545

540546
connection.setSecureSession(
541547
SecureChannel.Session(
542548
PeerId.fromPubKey(localKey.publicKey()),
543-
remotePeerId,
544-
remotePubKey,
549+
remoteIdentity.peerId,
550+
remoteIdentity.pubKey,
545551
null
546552
)
547553
)

libp2p/src/test/kotlin/io/libp2p/security/CipherSecureChannelTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ abstract class CipherSecureChannelTest(secureChannelCtor: SecureChannelCtor, mux
6363
}
6464

6565
@Test
66-
fun `test that on malformed message from remote the connection closes and no log noise`() {
66+
open fun `test that on malformed message from remote the connection closes and no log noise`() {
6767
val (privKey1, _) = generateKeyPair(KeyType.ECDSA)
6868
val (privKey2, pubKey2) = generateKeyPair(KeyType.ECDSA)
6969

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,26 @@
11
package io.libp2p.security.tls
22

3-
import io.libp2p.core.PeerId
4-
import io.libp2p.core.crypto.KeyType
5-
import io.libp2p.core.crypto.generateKeyPair
63
import io.libp2p.core.multistream.MultistreamProtocolDebug
74
import io.libp2p.core.mux.StreamMuxerProtocol
85
import io.libp2p.multistream.MultistreamProtocolDebugV1
9-
import io.libp2p.security.InvalidRemotePubKey
10-
import io.libp2p.security.SecureChannelTestBase
11-
import io.libp2p.tools.TestChannel
12-
import org.assertj.core.api.Assertions
6+
import io.libp2p.security.CipherSecureChannelTest
7+
import org.junit.jupiter.api.Disabled
138
import org.junit.jupiter.api.Tag
149
import org.junit.jupiter.api.Test
15-
import java.util.concurrent.TimeUnit
1610

1711
val MultistreamProtocolV1: MultistreamProtocolDebug = MultistreamProtocolDebugV1()
1812

1913
@Tag("secure-channel")
20-
class TlsSecureChannelTest : SecureChannelTestBase(
14+
class TlsSecureChannelTest : CipherSecureChannelTest(
2115
::TlsSecureChannel,
2216
listOf(StreamMuxerProtocol.getYamux().createMuxer(MultistreamProtocolV1, listOf())),
2317
TlsSecureChannel.announce
2418
) {
19+
// The malformed-cipher-bytes test from CipherSecureChannelTest is SecIO/Noise-specific:
20+
// it injects raw bytes post-handshake and expects no WARN-level log noise. TLS via OpenSSL
21+
// surfaces these errors through a different path that is out of scope for this test class.
2522
@Test
26-
fun `incorrect initiator remote PeerId should throw`() {
27-
val (privKey1, _) = generateKeyPair(KeyType.ECDSA)
28-
val (privKey2, _) = generateKeyPair(KeyType.ECDSA)
29-
val (_, wrongPubKey) = generateKeyPair(KeyType.ECDSA)
30-
31-
val protocolSelect1 = makeSelector(privKey1, muxerIds)
32-
val protocolSelect2 = makeSelector(privKey2, muxerIds)
33-
34-
val eCh1 = makeDialChannel("#1", protocolSelect1, PeerId.fromPubKey(wrongPubKey))
35-
val eCh2 = makeListenChannel("#2", protocolSelect2)
36-
37-
TestChannel.interConnect(eCh1, eCh2)
38-
39-
Assertions.assertThatThrownBy { protocolSelect1.selectedFuture.get(10, TimeUnit.SECONDS) }
40-
.hasCauseInstanceOf(InvalidRemotePubKey::class.java)
23+
@Disabled("TLS surfaces malformed-cipher-bytes via OpenSSL — covered separately, not part of the SecureChannel.Session contract under test here")
24+
override fun `test that on malformed message from remote the connection closes and no log noise`() {
4125
}
4226
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package io.libp2p.transport.quic
2+
3+
import io.libp2p.core.Connection
4+
import io.libp2p.core.ConnectionHandler
5+
import io.libp2p.core.PeerId
6+
import io.libp2p.core.crypto.KeyType
7+
import io.libp2p.core.dsl.HostBuilder
8+
import io.libp2p.core.multiformats.Multiaddr
9+
import org.assertj.core.api.Assertions.assertThat
10+
import org.junit.jupiter.api.Test
11+
import java.util.Random
12+
import java.util.concurrent.CompletableFuture
13+
import java.util.concurrent.TimeUnit
14+
15+
/**
16+
* Verifies the SecureChannel.Session invariant `PeerId.fromPubKey(remotePubKey) == remoteId`
17+
* for QUIC connections on both the client and server sides.
18+
*
19+
* Before the fix in this commit, QuicTransport exposed the ephemeral TLS subject public key
20+
* (or, on the client, the in-multihash-encoded peer key only for `identity` digests) rather
21+
* than the libp2p host public key embedded in the certificate extension. That violated the
22+
* contract SecIO/Noise/Plaintext uphold and broke any caller that relies on it
23+
* (e.g. signed-record verification).
24+
*/
25+
class QuicRemotePubKeyIdentityTest {
26+
27+
private fun randomPort(): Int = Random().nextInt(20_000) + 10_000
28+
29+
private class CapturingConnectionHandler : ConnectionHandler {
30+
val connectionFuture = CompletableFuture<Connection>()
31+
override fun handleConnection(conn: Connection) {
32+
connectionFuture.complete(conn)
33+
}
34+
}
35+
36+
@Test
37+
fun `remotePubKey hashes to remoteId on both sides of a QUIC handshake`() {
38+
val listenAddress = "/ip4/127.0.0.1/udp/${randomPort()}/quic-v1"
39+
40+
val clientHandler = CapturingConnectionHandler()
41+
val serverHandler = CapturingConnectionHandler()
42+
43+
val clientHost = HostBuilder()
44+
.keyType(KeyType.ED25519)
45+
.secureTransport(QuicTransport::ECDSA)
46+
.builderModifier { b -> b.connectionHandlers.add(clientHandler) }
47+
.build()
48+
49+
val serverHost = HostBuilder()
50+
.keyType(KeyType.ED25519)
51+
.secureTransport(QuicTransport::ECDSA)
52+
.listen(listenAddress)
53+
.builderModifier { b -> b.connectionHandlers.add(serverHandler) }
54+
.build()
55+
56+
try {
57+
clientHost.start().get(5, TimeUnit.SECONDS)
58+
serverHost.start().get(5, TimeUnit.SECONDS)
59+
60+
clientHost.network.connect(serverHost.peerId, Multiaddr(listenAddress))
61+
.get(10, TimeUnit.SECONDS)
62+
63+
val clientConn = clientHandler.connectionFuture.get(10, TimeUnit.SECONDS)
64+
val serverConn = serverHandler.connectionFuture.get(10, TimeUnit.SECONDS)
65+
66+
val clientSession = clientConn.secureSession()
67+
val serverSession = serverConn.secureSession()
68+
69+
// Server-side: the secure session must expose the libp2p host pubkey of the client.
70+
assertThat(serverSession.remoteId).isEqualTo(clientHost.peerId)
71+
assertThat(PeerId.fromPubKey(serverSession.remotePubKey)).isEqualTo(serverSession.remoteId)
72+
73+
// Client-side: the secure session must expose the libp2p host pubkey of the server.
74+
assertThat(clientSession.remoteId).isEqualTo(serverHost.peerId)
75+
assertThat(PeerId.fromPubKey(clientSession.remotePubKey)).isEqualTo(clientSession.remoteId)
76+
} finally {
77+
clientHost.stop().get(5, TimeUnit.SECONDS)
78+
serverHost.stop().get(5, TimeUnit.SECONDS)
79+
}
80+
}
81+
}

0 commit comments

Comments
 (0)