From e180f48407bdf1db63ab07ef253b7be631d49905 Mon Sep 17 00:00:00 2001 From: Cayman Date: Tue, 5 Dec 2023 17:00:18 -0500 Subject: [PATCH 1/3] fix!: remove peerId from secureInbound and secureOutbound --- .../connection-encrypter-plaintext/src/index.ts | 11 +++++++---- .../test/index.spec.ts | 14 ++++++++++---- .../src/connection-encryption/index.ts | 15 +++++++++------ .../interface/src/connection-encrypter/index.ts | 4 ++-- packages/libp2p/src/upgrader.ts | 4 ++-- packages/transport-webtransport/src/index.ts | 10 +++------- 6 files changed, 33 insertions(+), 25 deletions(-) diff --git a/packages/connection-encrypter-plaintext/src/index.ts b/packages/connection-encrypter-plaintext/src/index.ts index 8b286535a0..85f00c5588 100644 --- a/packages/connection-encrypter-plaintext/src/index.ts +++ b/packages/connection-encrypter-plaintext/src/index.ts @@ -31,6 +31,7 @@ import type { Uint8ArrayList } from 'uint8arraylist' const PROTOCOL = '/plaintext/2.0.0' export interface PlaintextComponents { + peerId: PeerId logger: ComponentLogger } @@ -44,20 +45,22 @@ export interface PlaintextInit { class Plaintext implements ConnectionEncrypter { public protocol: string = PROTOCOL + private readonly peerId: PeerId private readonly log: Logger private readonly timeout: number constructor (components: PlaintextComponents, init: PlaintextInit = {}) { + this.peerId = components.peerId this.log = components.logger.forComponent('libp2p:plaintext') this.timeout = init.timeout ?? 1000 } - async secureInbound > = MultiaddrConnection> (localId: PeerId, conn: Stream, remoteId?: PeerId): Promise> { - return this._encrypt(localId, conn, remoteId) + async secureInbound > = MultiaddrConnection> (conn: Stream, remoteId?: PeerId): Promise> { + return this._encrypt(this.peerId, conn, remoteId) } - async secureOutbound > = MultiaddrConnection> (localId: PeerId, conn: Stream, remoteId?: PeerId): Promise> { - return this._encrypt(localId, conn, remoteId) + async secureOutbound > = MultiaddrConnection> (conn: Stream, remoteId?: PeerId): Promise> { + return this._encrypt(this.peerId, conn, remoteId) } /** diff --git a/packages/connection-encrypter-plaintext/test/index.spec.ts b/packages/connection-encrypter-plaintext/test/index.spec.ts index e564e64e87..0f7536d382 100644 --- a/packages/connection-encrypter-plaintext/test/index.spec.ts +++ b/packages/connection-encrypter-plaintext/test/index.spec.ts @@ -19,6 +19,7 @@ describe('plaintext', () => { let remotePeer: PeerId let wrongPeer: PeerId let encrypter: ConnectionEncrypter + let encrypterRemote: ConnectionEncrypter beforeEach(async () => { [localPeer, remotePeer, wrongPeer] = await Promise.all([ @@ -28,6 +29,11 @@ describe('plaintext', () => { ]) encrypter = plaintext()({ + peerId: localPeer, + logger: defaultLogger() + }) + encrypterRemote = plaintext()({ + peerId: remotePeer, logger: defaultLogger() }) }) @@ -46,8 +52,8 @@ describe('plaintext', () => { }) await Promise.all([ - encrypter.secureInbound(remotePeer, inbound), - encrypter.secureOutbound(localPeer, outbound, wrongPeer) + encrypterRemote.secureInbound(inbound), + encrypter.secureOutbound(outbound, wrongPeer) ]).then(() => expect.fail('should have failed'), (err) => { expect(err).to.exist() expect(err).to.have.property('code', UnexpectedPeerError.code) @@ -67,8 +73,8 @@ describe('plaintext', () => { }) await expect(Promise.all([ - encrypter.secureInbound(localPeer, inbound), - encrypter.secureOutbound(remotePeer, outbound, localPeer) + encrypter.secureInbound(inbound), + encrypterRemote.secureOutbound(outbound, localPeer) ])) .to.eventually.be.rejected.with.property('code', InvalidCryptoExchangeError.code) }) diff --git a/packages/interface-compliance-tests/src/connection-encryption/index.ts b/packages/interface-compliance-tests/src/connection-encryption/index.ts index 625cfb6c1d..8261d84b20 100644 --- a/packages/interface-compliance-tests/src/connection-encryption/index.ts +++ b/packages/interface-compliance-tests/src/connection-encryption/index.ts @@ -12,6 +12,7 @@ import type { ConnectionEncrypter, PeerId } from '@libp2p/interface' export default (common: TestSetup): void => { describe('interface-connection-encrypter compliance tests', () => { let crypto: ConnectionEncrypter + let cryptoRemote: ConnectionEncrypter let localPeer: PeerId let remotePeer: PeerId let mitmPeer: PeerId @@ -19,11 +20,13 @@ export default (common: TestSetup): void => { before(async () => { [ crypto, + cryptoRemote, localPeer, remotePeer, mitmPeer ] = await Promise.all([ common.setup(), + common.setup({ peerId: PeerIdFactory.createFromJSON(peers[1]) }), PeerIdFactory.createFromJSON(peers[0]), PeerIdFactory.createFromJSON(peers[1]), PeerIdFactory.createFromJSON(peers[2]) @@ -46,8 +49,8 @@ export default (common: TestSetup): void => { inboundResult, outboundResult ] = await Promise.all([ - crypto.secureInbound(remotePeer, localConn), - crypto.secureOutbound(localPeer, remoteConn, remotePeer) + cryptoRemote.secureInbound(localConn), + crypto.secureOutbound(remoteConn, remotePeer) ]) // Echo server @@ -73,8 +76,8 @@ export default (common: TestSetup): void => { inboundResult, outboundResult ] = await Promise.all([ - crypto.secureInbound(remotePeer, localConn), - crypto.secureOutbound(localPeer, remoteConn, remotePeer) + cryptoRemote.secureInbound(localConn), + crypto.secureOutbound(remoteConn, remotePeer) ]) // Inbound should return the initiator (local) peer @@ -87,8 +90,8 @@ export default (common: TestSetup): void => { const [localConn, remoteConn] = createMaConnPair() await Promise.all([ - crypto.secureInbound(remotePeer, localConn, mitmPeer), - crypto.secureOutbound(localPeer, remoteConn, remotePeer) + cryptoRemote.secureInbound(localConn, mitmPeer), + crypto.secureOutbound(remoteConn, remotePeer) ]).then(() => expect.fail(), (err) => { expect(err).to.exist() expect(err).to.have.property('code', UnexpectedPeerError.code) diff --git a/packages/interface/src/connection-encrypter/index.ts b/packages/interface/src/connection-encrypter/index.ts index 8b2aac4729..40e85919e4 100644 --- a/packages/interface/src/connection-encrypter/index.ts +++ b/packages/interface/src/connection-encrypter/index.ts @@ -15,14 +15,14 @@ export interface ConnectionEncrypter { * pass it for extra verification, otherwise it will be determined during * the handshake. */ - secureOutbound > = MultiaddrConnection> (localPeer: PeerId, connection: Stream, remotePeer?: PeerId): Promise> + secureOutbound > = MultiaddrConnection> (connection: Stream, remotePeer?: PeerId): Promise> /** * Decrypt incoming data. If the remote PeerId is known, * pass it for extra verification, otherwise it will be determined during * the handshake */ - secureInbound > = MultiaddrConnection> (localPeer: PeerId, connection: Stream, remotePeer?: PeerId): Promise> + secureInbound > = MultiaddrConnection> (connection: Stream, remotePeer?: PeerId): Promise> } export interface SecuredConnection { diff --git a/packages/libp2p/src/upgrader.ts b/packages/libp2p/src/upgrader.ts index 0c15295af3..32ce9955aa 100644 --- a/packages/libp2p/src/upgrader.ts +++ b/packages/libp2p/src/upgrader.ts @@ -637,7 +637,7 @@ export class DefaultUpgrader implements Upgrader { connection.log('encrypting inbound connection using', protocol) return { - ...await encrypter.secureInbound(this.components.peerId, stream), + ...await encrypter.secureInbound(stream), protocol } } catch (err: any) { @@ -673,7 +673,7 @@ export class DefaultUpgrader implements Upgrader { connection.log('encrypting outbound connection to %p using %p', remotePeerId) return { - ...await encrypter.secureOutbound(this.components.peerId, stream, remotePeerId), + ...await encrypter.secureOutbound(stream, remotePeerId), protocol } } catch (err: any) { diff --git a/packages/transport-webtransport/src/index.ts b/packages/transport-webtransport/src/index.ts index 7f2446687e..6ede9e916c 100644 --- a/packages/transport-webtransport/src/index.ts +++ b/packages/transport-webtransport/src/index.ts @@ -82,10 +82,6 @@ class WebTransportTransport implements Transport { options?.signal?.throwIfAborted() this.log('dialing %s', ma) - const localPeer = this.components.peerId - if (localPeer === undefined) { - throw new Error('Need a local peerid') - } options = options ?? {} @@ -167,7 +163,7 @@ class WebTransportTransport implements Transport { cleanUpWTSession('remote_close') }) - if (!await this.authenticateWebTransport(wt, localPeer, remotePeer, certhashes)) { + if (!await this.authenticateWebTransport(wt, remotePeer, certhashes)) { throw new Error('Failed to authenticate webtransport') } @@ -213,7 +209,7 @@ class WebTransportTransport implements Transport { } } - async authenticateWebTransport (wt: InstanceType, localPeer: PeerId, remotePeer: PeerId, certhashes: Array>): Promise { + async authenticateWebTransport (wt: InstanceType, remotePeer: PeerId, certhashes: Array>): Promise { const stream = await wt.createBidirectionalStream() const writer = stream.writable.getWriter() const reader = stream.readable.getReader() @@ -246,7 +242,7 @@ class WebTransportTransport implements Transport { const n = noise()(this.components) - const { remoteExtensions } = await n.secureOutbound(localPeer, duplex, remotePeer) + const { remoteExtensions } = await n.secureOutbound(duplex, remotePeer) // We're done with this authentication stream writer.close().catch((err: Error) => { From 3e9b5c5dbe5378169b8cd5e73b4b920b97954fe3 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 9 Aug 2024 15:28:37 +0100 Subject: [PATCH 2/3] chore: fix tests --- .../test/compliance.spec.ts | 4 +++- .../test/index.spec.ts | 5 +++++ packages/connection-encrypter-tls/src/index.ts | 3 ++- packages/connection-encrypter-tls/src/tls.ts | 14 ++++++++------ .../test/compliance.spec.ts | 4 +++- .../connection-encrypter-tls/test/index.spec.ts | 14 ++++++++++---- .../src/connection-encryption/index.ts | 10 +++++++--- .../src/private-to-public/transport.ts | 4 +--- 8 files changed, 39 insertions(+), 19 deletions(-) diff --git a/packages/connection-encrypter-plaintext/test/compliance.spec.ts b/packages/connection-encrypter-plaintext/test/compliance.spec.ts index 9678a0bff8..c1024441fe 100644 --- a/packages/connection-encrypter-plaintext/test/compliance.spec.ts +++ b/packages/connection-encrypter-plaintext/test/compliance.spec.ts @@ -2,12 +2,14 @@ import suite from '@libp2p/interface-compliance-tests/connection-encryption' import { defaultLogger } from '@libp2p/logger' +import { createEd25519PeerId } from '@libp2p/peer-id-factory' import { plaintext } from '../src/index.js' describe('plaintext compliance', () => { suite({ - async setup () { + async setup (opts) { return plaintext()({ + peerId: opts?.peerId ?? await createEd25519PeerId(), logger: defaultLogger() }) }, diff --git a/packages/connection-encrypter-plaintext/test/index.spec.ts b/packages/connection-encrypter-plaintext/test/index.spec.ts index 0f7536d382..016a060aa8 100644 --- a/packages/connection-encrypter-plaintext/test/index.spec.ts +++ b/packages/connection-encrypter-plaintext/test/index.spec.ts @@ -64,6 +64,11 @@ describe('plaintext', () => { const peer = await createRSAPeerId() remotePeer = peerIdFromBytes(peer.toBytes()) + encrypter = plaintext()({ + peerId: remotePeer, + logger: defaultLogger() + }) + const { inbound, outbound } = mockMultiaddrConnPair({ remotePeer, addrs: [ diff --git a/packages/connection-encrypter-tls/src/index.ts b/packages/connection-encrypter-tls/src/index.ts index 745865a406..36d843d8c0 100644 --- a/packages/connection-encrypter-tls/src/index.ts +++ b/packages/connection-encrypter-tls/src/index.ts @@ -19,11 +19,12 @@ */ import { TLS } from './tls.js' -import type { ComponentLogger, ConnectionEncrypter } from '@libp2p/interface' +import type { ComponentLogger, ConnectionEncrypter, PeerId } from '@libp2p/interface' export const PROTOCOL = '/tls/1.0.0' export interface TLSComponents { + peerId: PeerId logger: ComponentLogger } diff --git a/packages/connection-encrypter-tls/src/tls.ts b/packages/connection-encrypter-tls/src/tls.ts index e0facdec2b..9d5b34d913 100644 --- a/packages/connection-encrypter-tls/src/tls.ts +++ b/packages/connection-encrypter-tls/src/tls.ts @@ -30,10 +30,12 @@ import type { Uint8ArrayList } from 'uint8arraylist' export class TLS implements ConnectionEncrypter { public protocol: string = PROTOCOL private readonly log: Logger + private readonly peerId: PeerId private readonly timeout: number constructor (components: TLSComponents, init: TLSInit = {}) { this.log = components.logger.forComponent('libp2p:tls') + this.peerId = components.peerId this.timeout = init.timeout ?? 1000 } @@ -43,20 +45,20 @@ export class TLS implements ConnectionEncrypter { '@libp2p/connection-encryption' ] - async secureInbound > = MultiaddrConnection> (localId: PeerId, conn: Stream, remoteId?: PeerId): Promise> { - return this._encrypt(localId, conn, true, remoteId) + async secureInbound > = MultiaddrConnection> (conn: Stream, remoteId?: PeerId): Promise> { + return this._encrypt(conn, true, remoteId) } - async secureOutbound > = MultiaddrConnection> (localId: PeerId, conn: Stream, remoteId?: PeerId): Promise> { - return this._encrypt(localId, conn, false, remoteId) + async secureOutbound > = MultiaddrConnection> (conn: Stream, remoteId?: PeerId): Promise> { + return this._encrypt(conn, false, remoteId) } /** * Encrypt connection */ - async _encrypt > = MultiaddrConnection> (localId: PeerId, conn: Stream, isServer: boolean, remoteId?: PeerId): Promise> { + async _encrypt > = MultiaddrConnection> (conn: Stream, isServer: boolean, remoteId?: PeerId): Promise> { const opts: TLSSocketOptions = { - ...await generateCertificate(localId), + ...await generateCertificate(this.peerId), isServer, // require TLS 1.3 or later minVersion: 'TLSv1.3', diff --git a/packages/connection-encrypter-tls/test/compliance.spec.ts b/packages/connection-encrypter-tls/test/compliance.spec.ts index 1b606899a9..d926cda042 100644 --- a/packages/connection-encrypter-tls/test/compliance.spec.ts +++ b/packages/connection-encrypter-tls/test/compliance.spec.ts @@ -2,12 +2,14 @@ import suite from '@libp2p/interface-compliance-tests/connection-encryption' import { defaultLogger } from '@libp2p/logger' +import { createEd25519PeerId } from '@libp2p/peer-id-factory' import { tls } from '../src/index.js' describe('tls compliance', () => { suite({ - async setup () { + async setup (opts) { return tls()({ + peerId: opts?.peerId ?? await createEd25519PeerId(), logger: defaultLogger() }) }, diff --git a/packages/connection-encrypter-tls/test/index.spec.ts b/packages/connection-encrypter-tls/test/index.spec.ts index 8be92117bf..7d7a6e543b 100644 --- a/packages/connection-encrypter-tls/test/index.spec.ts +++ b/packages/connection-encrypter-tls/test/index.spec.ts @@ -28,6 +28,7 @@ describe('tls', () => { ]) encrypter = tls()({ + peerId: await createEd25519PeerId(), logger: defaultLogger() }) }) @@ -46,8 +47,8 @@ describe('tls', () => { }) await Promise.all([ - encrypter.secureInbound(remotePeer, inbound), - encrypter.secureOutbound(localPeer, outbound, wrongPeer) + encrypter.secureInbound(inbound, remotePeer), + encrypter.secureOutbound(outbound, wrongPeer) ]).then(() => expect.fail('should have failed'), (err) => { expect(err).to.exist() expect(err).to.have.property('code', UnexpectedPeerError.code) @@ -58,6 +59,11 @@ describe('tls', () => { const peer = await createRSAPeerId() remotePeer = peerIdFromBytes(peer.toBytes()) + encrypter = tls()({ + peerId: remotePeer, + logger: defaultLogger() + }) + const { inbound, outbound } = mockMultiaddrConnPair({ remotePeer, addrs: [ @@ -67,8 +73,8 @@ describe('tls', () => { }) await expect(Promise.all([ - encrypter.secureInbound(localPeer, inbound), - encrypter.secureOutbound(remotePeer, outbound, localPeer) + encrypter.secureInbound(inbound), + encrypter.secureOutbound(outbound, localPeer) ])) .to.eventually.be.rejected.with.property('code', InvalidCryptoExchangeError.code) }) diff --git a/packages/interface-compliance-tests/src/connection-encryption/index.ts b/packages/interface-compliance-tests/src/connection-encryption/index.ts index c66dfac7fa..0f029e6f18 100644 --- a/packages/interface-compliance-tests/src/connection-encryption/index.ts +++ b/packages/interface-compliance-tests/src/connection-encryption/index.ts @@ -10,7 +10,11 @@ import { createMaConnPair } from './utils/index.js' import type { TestSetup } from '../index.js' import type { ConnectionEncrypter, PeerId } from '@libp2p/interface' -export default (common: TestSetup): void => { +export interface ConnectionEncrypterSetupArgs { + peerId: PeerId +} + +export default (common: TestSetup): void => { describe('interface-connection-encrypter compliance tests', () => { let crypto: ConnectionEncrypter let cryptoRemote: ConnectionEncrypter @@ -26,8 +30,8 @@ export default (common: TestSetup): void => { remotePeer, mitmPeer ] = await Promise.all([ - common.setup(), - common.setup({ peerId: PeerIdFactory.createFromJSON(peers[1]) }), + common.setup({ peerId: await PeerIdFactory.createFromJSON(peers[0]) }), + common.setup({ peerId: await PeerIdFactory.createFromJSON(peers[1]) }), PeerIdFactory.createFromJSON(peers[0]), PeerIdFactory.createFromJSON(peers[1]), PeerIdFactory.createFromJSON(peers[2]) diff --git a/packages/transport-webrtc/src/private-to-public/transport.ts b/packages/transport-webrtc/src/private-to-public/transport.ts index 0a9626696f..b5bf39ea87 100644 --- a/packages/transport-webrtc/src/private-to-public/transport.ts +++ b/packages/transport-webrtc/src/private-to-public/transport.ts @@ -190,8 +190,6 @@ export class WebRTCDirectTransport implements Transport { // wait for peerconnection.onopen to fire, or for the datachannel to open const handshakeDataChannel = await dataChannelOpenPromise - const myPeerId = this.components.peerId - // Do noise handshake. // Set the Noise Prologue to libp2p-webrtc-noise: before starting the actual Noise handshake. // is the concatenation of the of the two TLS fingerprints of A and B in their multihash byte representation, sorted in ascending order. @@ -260,7 +258,7 @@ export class WebRTCDirectTransport implements Transport { // For outbound connections, the remote is expected to start the noise handshake. // Therefore, we need to secure an inbound noise connection from the remote. - await connectionEncrypter.secureInbound(myPeerId, wrappedDuplex, theirPeerId) + await connectionEncrypter.secureInbound(wrappedDuplex, theirPeerId) return await options.upgrader.upgradeOutbound(maConn, { skipProtection: true, skipEncryption: true, muxerFactory }) } catch (err) { From d253f50d9daac64370d1d9b4de0d9cf7b0f2a767 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Wed, 14 Aug 2024 13:36:12 +0100 Subject: [PATCH 3/3] chore: fix tests --- packages/libp2p/test/upgrading/upgrader.spec.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/libp2p/test/upgrading/upgrader.spec.ts b/packages/libp2p/test/upgrading/upgrader.spec.ts index 0f4dde9447..47bbee2b0c 100644 --- a/packages/libp2p/test/upgrading/upgrader.spec.ts +++ b/packages/libp2p/test/upgrading/upgrader.spec.ts @@ -174,7 +174,7 @@ describe('Upgrader', () => { }) remoteUpgrader = new DefaultUpgrader(remoteComponents, { connectionEncryption: [ - plaintext()(localComponents) + plaintext()(remoteComponents) ], muxers: [], inboundUpgradeTimeout: 1000 @@ -361,19 +361,19 @@ describe('Upgrader', () => { }) remoteUpgrader = new DefaultUpgrader(remoteComponents, { connectionEncryption: [ - plaintext()(localComponents) + plaintext()(remoteComponents) ], muxers: [ - yamux()(localComponents), - mplex()(localComponents) + yamux()(remoteComponents), + mplex()(remoteComponents) ], inboundUpgradeTimeout: 1000 }) // Wait for the results of each side of the connection const results = await Promise.allSettled([ - localUpgrader.upgradeOutbound(outbound), - remoteUpgrader.upgradeInbound(inbound) + localUpgrader.upgradeOutbound(inbound), + remoteUpgrader.upgradeInbound(outbound) ]) // Ensure both sides fail