Skip to content

Commit b435a21

Browse files
wemeetagainachingbrain
wemeetagain
and
achingbrain
committed
fix!: remove localPeer from secureInbound and secureOutbound (#2304)
Requiring the local peer id in connection encryption appears to be historical cruft. The more current approach would be to have a `ConnectionEncrypter` be specific to a single peer id, passed in during instantiation in its Components. BREAKING CHANGE: removes `localPeer: PeerId` first parameter from `secureInbound` and `secureOutbound` in `ConnectionEncrypter` --------- Co-authored-by: Alex Potsides <alex@achingbrain.net>
1 parent 2988602 commit b435a21

File tree

13 files changed

+77
-50
lines changed

13 files changed

+77
-50
lines changed

‎packages/connection-encrypter-plaintext/src/index.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import type { Uint8ArrayList } from 'uint8arraylist'
3333
const PROTOCOL = '/plaintext/2.0.0'
3434

3535
export interface PlaintextComponents {
36+
peerId: PeerId
3637
logger: ComponentLogger
3738
}
3839

@@ -46,10 +47,12 @@ export interface PlaintextInit {
4647

4748
class Plaintext implements ConnectionEncrypter {
4849
public protocol: string = PROTOCOL
50+
private readonly peerId: PeerId
4951
private readonly log: Logger
5052
private readonly timeout: number
5153

5254
constructor (components: PlaintextComponents, init: PlaintextInit = {}) {
55+
this.peerId = components.peerId
5356
this.log = components.logger.forComponent('libp2p:plaintext')
5457
this.timeout = init.timeout ?? 1000
5558
}
@@ -60,12 +63,12 @@ class Plaintext implements ConnectionEncrypter {
6063
'@libp2p/connection-encryption'
6164
]
6265

63-
async secureInbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (localId: PeerId, conn: Stream, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
64-
return this._encrypt(localId, conn, remoteId)
66+
async secureInbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (conn: Stream, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
67+
return this._encrypt(this.peerId, conn, remoteId)
6568
}
6669

67-
async secureOutbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (localId: PeerId, conn: Stream, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
68-
return this._encrypt(localId, conn, remoteId)
70+
async secureOutbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (conn: Stream, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
71+
return this._encrypt(this.peerId, conn, remoteId)
6972
}
7073

7174
/**

‎packages/connection-encrypter-plaintext/test/compliance.spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22

33
import suite from '@libp2p/interface-compliance-tests/connection-encryption'
44
import { defaultLogger } from '@libp2p/logger'
5+
import { createEd25519PeerId } from '@libp2p/peer-id-factory'
56
import { plaintext } from '../src/index.js'
67

78
describe('plaintext compliance', () => {
89
suite({
9-
async setup () {
10+
async setup (opts) {
1011
return plaintext()({
12+
peerId: opts?.peerId ?? await createEd25519PeerId(),
1113
logger: defaultLogger()
1214
})
1315
},

‎packages/connection-encrypter-plaintext/test/index.spec.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ describe('plaintext', () => {
1919
let remotePeer: PeerId
2020
let wrongPeer: PeerId
2121
let encrypter: ConnectionEncrypter
22+
let encrypterRemote: ConnectionEncrypter
2223

2324
beforeEach(async () => {
2425
[localPeer, remotePeer, wrongPeer] = await Promise.all([
@@ -28,6 +29,11 @@ describe('plaintext', () => {
2829
])
2930

3031
encrypter = plaintext()({
32+
peerId: localPeer,
33+
logger: defaultLogger()
34+
})
35+
encrypterRemote = plaintext()({
36+
peerId: remotePeer,
3137
logger: defaultLogger()
3238
})
3339
})
@@ -46,8 +52,8 @@ describe('plaintext', () => {
4652
})
4753

4854
await Promise.all([
49-
encrypter.secureInbound(remotePeer, inbound),
50-
encrypter.secureOutbound(localPeer, outbound, wrongPeer)
55+
encrypterRemote.secureInbound(inbound),
56+
encrypter.secureOutbound(outbound, wrongPeer)
5157
]).then(() => expect.fail('should have failed'), (err) => {
5258
expect(err).to.exist()
5359
expect(err).to.have.property('code', UnexpectedPeerError.code)
@@ -58,6 +64,11 @@ describe('plaintext', () => {
5864
const peer = await createRSAPeerId()
5965
remotePeer = peerIdFromBytes(peer.toBytes())
6066

67+
encrypter = plaintext()({
68+
peerId: remotePeer,
69+
logger: defaultLogger()
70+
})
71+
6172
const { inbound, outbound } = mockMultiaddrConnPair({
6273
remotePeer,
6374
addrs: [
@@ -67,8 +78,8 @@ describe('plaintext', () => {
6778
})
6879

6980
await expect(Promise.all([
70-
encrypter.secureInbound(localPeer, inbound),
71-
encrypter.secureOutbound(remotePeer, outbound, localPeer)
81+
encrypter.secureInbound(inbound),
82+
encrypterRemote.secureOutbound(outbound, localPeer)
7283
]))
7384
.to.eventually.be.rejected.with.property('code', InvalidCryptoExchangeError.code)
7485
})

‎packages/connection-encrypter-tls/src/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@
1919
*/
2020

2121
import { TLS } from './tls.js'
22-
import type { ComponentLogger, ConnectionEncrypter } from '@libp2p/interface'
22+
import type { ComponentLogger, ConnectionEncrypter, PeerId } from '@libp2p/interface'
2323

2424
export const PROTOCOL = '/tls/1.0.0'
2525

2626
export interface TLSComponents {
27+
peerId: PeerId
2728
logger: ComponentLogger
2829
}
2930

‎packages/connection-encrypter-tls/src/tls.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@ import type { Uint8ArrayList } from 'uint8arraylist'
3030
export class TLS implements ConnectionEncrypter {
3131
public protocol: string = PROTOCOL
3232
private readonly log: Logger
33+
private readonly peerId: PeerId
3334
private readonly timeout: number
3435

3536
constructor (components: TLSComponents, init: TLSInit = {}) {
3637
this.log = components.logger.forComponent('libp2p:tls')
38+
this.peerId = components.peerId
3739
this.timeout = init.timeout ?? 1000
3840
}
3941

@@ -43,20 +45,20 @@ export class TLS implements ConnectionEncrypter {
4345
'@libp2p/connection-encryption'
4446
]
4547

46-
async secureInbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (localId: PeerId, conn: Stream, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
47-
return this._encrypt(localId, conn, true, remoteId)
48+
async secureInbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (conn: Stream, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
49+
return this._encrypt(conn, true, remoteId)
4850
}
4951

50-
async secureOutbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (localId: PeerId, conn: Stream, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
51-
return this._encrypt(localId, conn, false, remoteId)
52+
async secureOutbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (conn: Stream, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
53+
return this._encrypt(conn, false, remoteId)
5254
}
5355

5456
/**
5557
* Encrypt connection
5658
*/
57-
async _encrypt <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (localId: PeerId, conn: Stream, isServer: boolean, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
59+
async _encrypt <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (conn: Stream, isServer: boolean, remoteId?: PeerId): Promise<SecuredConnection<Stream>> {
5860
const opts: TLSSocketOptions = {
59-
...await generateCertificate(localId),
61+
...await generateCertificate(this.peerId),
6062
isServer,
6163
// require TLS 1.3 or later
6264
minVersion: 'TLSv1.3',

‎packages/connection-encrypter-tls/test/compliance.spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22

33
import suite from '@libp2p/interface-compliance-tests/connection-encryption'
44
import { defaultLogger } from '@libp2p/logger'
5+
import { createEd25519PeerId } from '@libp2p/peer-id-factory'
56
import { tls } from '../src/index.js'
67

78
describe('tls compliance', () => {
89
suite({
9-
async setup () {
10+
async setup (opts) {
1011
return tls()({
12+
peerId: opts?.peerId ?? await createEd25519PeerId(),
1113
logger: defaultLogger()
1214
})
1315
},

‎packages/connection-encrypter-tls/test/index.spec.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ describe('tls', () => {
2828
])
2929

3030
encrypter = tls()({
31+
peerId: await createEd25519PeerId(),
3132
logger: defaultLogger()
3233
})
3334
})
@@ -46,8 +47,8 @@ describe('tls', () => {
4647
})
4748

4849
await Promise.all([
49-
encrypter.secureInbound(remotePeer, inbound),
50-
encrypter.secureOutbound(localPeer, outbound, wrongPeer)
50+
encrypter.secureInbound(inbound, remotePeer),
51+
encrypter.secureOutbound(outbound, wrongPeer)
5152
]).then(() => expect.fail('should have failed'), (err) => {
5253
expect(err).to.exist()
5354
expect(err).to.have.property('code', UnexpectedPeerError.code)
@@ -58,6 +59,11 @@ describe('tls', () => {
5859
const peer = await createRSAPeerId()
5960
remotePeer = peerIdFromBytes(peer.toBytes())
6061

62+
encrypter = tls()({
63+
peerId: remotePeer,
64+
logger: defaultLogger()
65+
})
66+
6167
const { inbound, outbound } = mockMultiaddrConnPair({
6268
remotePeer,
6369
addrs: [
@@ -67,8 +73,8 @@ describe('tls', () => {
6773
})
6874

6975
await expect(Promise.all([
70-
encrypter.secureInbound(localPeer, inbound),
71-
encrypter.secureOutbound(remotePeer, outbound, localPeer)
76+
encrypter.secureInbound(inbound),
77+
encrypter.secureOutbound(outbound, localPeer)
7278
]))
7379
.to.eventually.be.rejected.with.property('code', InvalidCryptoExchangeError.code)
7480
})

‎packages/interface-compliance-tests/src/connection-encryption/index.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,28 @@ import { createMaConnPair } from './utils/index.js'
1010
import type { TestSetup } from '../index.js'
1111
import type { ConnectionEncrypter, PeerId } from '@libp2p/interface'
1212

13-
export default (common: TestSetup<ConnectionEncrypter>): void => {
13+
export interface ConnectionEncrypterSetupArgs {
14+
peerId: PeerId
15+
}
16+
17+
export default (common: TestSetup<ConnectionEncrypter, ConnectionEncrypterSetupArgs>): void => {
1418
describe('interface-connection-encrypter compliance tests', () => {
1519
let crypto: ConnectionEncrypter
20+
let cryptoRemote: ConnectionEncrypter
1621
let localPeer: PeerId
1722
let remotePeer: PeerId
1823
let mitmPeer: PeerId
1924

2025
before(async () => {
2126
[
2227
crypto,
28+
cryptoRemote,
2329
localPeer,
2430
remotePeer,
2531
mitmPeer
2632
] = await Promise.all([
27-
common.setup(),
33+
common.setup({ peerId: await PeerIdFactory.createFromJSON(peers[0]) }),
34+
common.setup({ peerId: await PeerIdFactory.createFromJSON(peers[1]) }),
2835
PeerIdFactory.createFromJSON(peers[0]),
2936
PeerIdFactory.createFromJSON(peers[1]),
3037
PeerIdFactory.createFromJSON(peers[2])
@@ -47,8 +54,8 @@ export default (common: TestSetup<ConnectionEncrypter>): void => {
4754
inboundResult,
4855
outboundResult
4956
] = await Promise.all([
50-
crypto.secureInbound(remotePeer, localConn),
51-
crypto.secureOutbound(localPeer, remoteConn, remotePeer)
57+
cryptoRemote.secureInbound(localConn),
58+
crypto.secureOutbound(remoteConn, remotePeer)
5259
])
5360

5461
// Echo server
@@ -77,8 +84,8 @@ export default (common: TestSetup<ConnectionEncrypter>): void => {
7784
inboundResult,
7885
outboundResult
7986
] = await Promise.all([
80-
crypto.secureInbound(remotePeer, localConn),
81-
crypto.secureOutbound(localPeer, remoteConn, remotePeer)
87+
cryptoRemote.secureInbound(localConn),
88+
crypto.secureOutbound(remoteConn, remotePeer)
8289
])
8390

8491
// Inbound should return the initiator (local) peer
@@ -91,8 +98,8 @@ export default (common: TestSetup<ConnectionEncrypter>): void => {
9198
const [localConn, remoteConn] = createMaConnPair()
9299

93100
await Promise.all([
94-
crypto.secureInbound(remotePeer, localConn, mitmPeer),
95-
crypto.secureOutbound(localPeer, remoteConn, remotePeer)
101+
cryptoRemote.secureInbound(localConn, mitmPeer),
102+
crypto.secureOutbound(remoteConn, remotePeer)
96103
]).then(() => expect.fail(), (err) => {
97104
expect(err).to.exist()
98105
expect(err).to.have.property('code', UnexpectedPeerError.code)

‎packages/interface/src/connection-encrypter/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ export interface ConnectionEncrypter<Extension = unknown> {
1515
* pass it for extra verification, otherwise it will be determined during
1616
* the handshake.
1717
*/
18-
secureOutbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (localPeer: PeerId, connection: Stream, remotePeer?: PeerId): Promise<SecuredConnection<Stream, Extension>>
18+
secureOutbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (connection: Stream, remotePeer?: PeerId): Promise<SecuredConnection<Stream, Extension>>
1919

2020
/**
2121
* Decrypt incoming data. If the remote PeerId is known,
2222
* pass it for extra verification, otherwise it will be determined during
2323
* the handshake
2424
*/
25-
secureInbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (localPeer: PeerId, connection: Stream, remotePeer?: PeerId): Promise<SecuredConnection<Stream, Extension>>
25+
secureInbound <Stream extends Duplex<AsyncGenerator<Uint8Array | Uint8ArrayList>> = MultiaddrConnection> (connection: Stream, remotePeer?: PeerId): Promise<SecuredConnection<Stream, Extension>>
2626
}
2727

2828
export interface SecuredConnection<Stream = any, Extension = unknown> {

‎packages/libp2p/src/upgrader.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ export class DefaultUpgrader implements Upgrader {
644644
connection.log('encrypting inbound connection using', protocol)
645645

646646
return {
647-
...await encrypter.secureInbound(this.components.peerId, stream),
647+
...await encrypter.secureInbound(stream),
648648
protocol
649649
}
650650
} catch (err: any) {
@@ -681,7 +681,7 @@ export class DefaultUpgrader implements Upgrader {
681681
connection.log('encrypting outbound connection to %p using %s', remotePeerId, encrypter)
682682

683683
return {
684-
...await encrypter.secureOutbound(this.components.peerId, stream, remotePeerId),
684+
...await encrypter.secureOutbound(stream, remotePeerId),
685685
protocol
686686
}
687687
} catch (err: any) {

0 commit comments

Comments
 (0)