Fix remotePubKey in Noise secure Session (libp2p#364)

* Fix Noise negotiator to put the right remote PubKey into SecureChannel.Session * Add test * Increase the RelayTest test timeout. It weirdly may take up to 10 sec. Sometimes it's almost instant
Peergos · May 15, 2024 · 838d7da · 838d7da
1 parent 9fd7741
commit 838d7da
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 6 deletions.
diff --git a/libp2p/src/main/kotlin/io/libp2p/security/noise/NoiseXXSecureChannel.kt b/libp2p/src/main/kotlin/io/libp2p/security/noise/NoiseXXSecureChannel.kt
@@ -97,7 +97,8 @@ class NoiseIoHandshake(
     private var sentNoiseKeyPayload = false
     private var instancePayload: ByteArray? = null
     private var activated = false
-    private var remotePeerId: PeerId? = null
+    private var remotePubKey: PubKey? = null
+    private val remotePeerId: PeerId? get() = remotePubKey?.let { PeerId.fromPubKey(it) }
     private var expectedRemotePeerId: PeerId? = null
 
     init {
@@ -139,7 +140,7 @@ class NoiseIoHandshake(
         // the remote public key has been provided by the XX protocol
         val derivedRemotePublicKey = handshakeState.remotePublicKey
         if (derivedRemotePublicKey.hasPublicKey()) {
-            remotePeerId = verifyPayload(ctx, instancePayload!!, derivedRemotePublicKey)
+            remotePubKey = verifyPayload(ctx, instancePayload!!, derivedRemotePublicKey)
             if (role == Role.INIT && expectedRemotePeerId != remotePeerId) {
                 throw InvalidRemotePubKey()
             }
@@ -248,7 +249,7 @@ class NoiseIoHandshake(
         ctx: ChannelHandlerContext,
         payload: ByteArray,
         remotePublicKeyState: DHState
-    ): PeerId {
+    ): PubKey {
         log.debug("Verifying noise static key payload")
 
         val (pubKeyFromMessage, signatureFromMessage) = unpackKeyAndSignature(payload)
@@ -264,7 +265,7 @@ class NoiseIoHandshake(
             handshakeFailed(ctx, InvalidRemotePubKey())
         }
 
-        return PeerId.fromPubKey(pubKeyFromMessage)
+        return pubKeyFromMessage
     } // verifyPayload
 
     private fun unpackKeyAndSignature(payload: ByteArray): Pair<PubKey, ByteArray> {
@@ -287,7 +288,7 @@ class NoiseIoHandshake(
         val secureSession = NoiseSecureChannelSession(
             PeerId.fromPubKey(localKey.publicKey()),
             remotePeerId!!,
-            localKey.publicKey(),
+            remotePubKey!!,
             aliceSplit,
             bobSplit
         )

diff --git a/libp2p/src/test/java/io/libp2p/core/RelayTestJava.java b/libp2p/src/test/java/io/libp2p/core/RelayTestJava.java
@@ -211,7 +211,7 @@ void relayStreamsAreLimited() throws Exception {
     System.out.println("Blob controller created");
 
     Assertions.assertThrows(
-        ExecutionException.class, () -> blobCtr.blob().get(5, TimeUnit.SECONDS));
+        ExecutionException.class, () -> blobCtr.blob().get(30, TimeUnit.SECONDS));
 
     clientHost.stop().get(5, TimeUnit.SECONDS);
     System.out.println("Client stopped");

diff --git a/libp2p/src/test/kotlin/io/libp2p/security/CipherSecureChannelTest.kt b/libp2p/src/test/kotlin/io/libp2p/security/CipherSecureChannelTest.kt
@@ -16,6 +16,33 @@ import java.util.concurrent.TimeUnit.SECONDS
 abstract class CipherSecureChannelTest(secureChannelCtor: SecureChannelCtor, muxers: List<StreamMuxer>, announce: String) :
     SecureChannelTestBase(secureChannelCtor, muxers, announce) {
 
+    @Test
+    fun `verify secure session`() {
+        val (privKey1, pubKey1) = generateKeyPair(KeyType.ECDSA)
+        val (privKey2, pubKey2) = generateKeyPair(KeyType.ECDSA)
+
+        val protocolSelect1 = makeSelector(privKey1, muxerIds)
+        val protocolSelect2 = makeSelector(privKey2, muxerIds)
+
+        val eCh1 = makeDialChannel("#1", protocolSelect1, PeerId.fromPubKey(pubKey2))
+        val eCh2 = makeListenChannel("#2", protocolSelect2)
+
+        logger.debug("Connecting channels...")
+        val connection = TestChannel.interConnect(eCh1, eCh2)
+
+        val secSession1 = protocolSelect1.selectedFuture.join()
+        assertThat(secSession1.localId).isEqualTo(PeerId.fromPubKey(pubKey1))
+        assertThat(secSession1.remoteId).isEqualTo(PeerId.fromPubKey(pubKey2))
+        assertThat(secSession1.remotePubKey).isEqualTo(pubKey2)
+
+        val secSession2 = protocolSelect2.selectedFuture.join()
+        assertThat(secSession2.localId).isEqualTo(PeerId.fromPubKey(pubKey2))
+        assertThat(secSession2.remoteId).isEqualTo(PeerId.fromPubKey(pubKey1))
+        assertThat(secSession2.remotePubKey).isEqualTo(pubKey1)
+
+        logger.debug("Connection made: $connection")
+    }
+
     @Test
     fun `incorrect initiator remote PeerId should throw`() {
         val (privKey1, _) = generateKeyPair(KeyType.ECDSA)