Skip to content

Commit 947d304

Browse files
saul-jbtabcat
andauthored
fix(pnet): bound xsalsa20 wasm memory per message and per connection (#3585)
* Handle large messages in pnet. * fix(pnet): release the xsalsa20 ciphers when the connection ends Each cipher holds a slot in a process-global wasm pool that is only returned by finalize, which pnet never called, so every connection leaked one per direction until the pool was exhausted and all pnet traffic began to fail. --------- Co-authored-by: tabcat <tabcat00@proton.me>
1 parent 0967e75 commit 947d304

2 files changed

Lines changed: 658 additions & 21 deletions

File tree

packages/pnet/src/crypto.ts

Lines changed: 141 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,30 @@ import { toString as uint8ArrayToString } from 'uint8arrays/to-string'
44
import xsalsa20 from 'xsalsa20'
55
import * as Errors from './errors.ts'
66
import { KEY_LENGTH } from './key-generator.ts'
7-
import type { AbortOptions, MultiaddrConnection } from '@libp2p/interface'
7+
import type { AbortOptions, MultiaddrConnection, StreamMessageEvent } from '@libp2p/interface'
88
import type { MessageStreamInit, SendResult } from '@libp2p/utils'
99
import type { Uint8ArrayList } from 'uint8arraylist'
1010

11+
const XOR_CHUNK_SIZE = 65536
12+
13+
/**
14+
* The xsalsa20 wasm backend copies each input into linear memory, which starts
15+
* at 640KiB and is capped at 62.5MiB. Growing it detaches every other cipher's
16+
* view of it, so keep chunks under the initial size rather than the cap. The
17+
* cipher resumes where it left off, so the result matches a single update.
18+
*/
19+
function xorInChunks (xor: xsalsa20.Xor, input: Uint8Array): Uint8Array {
20+
const output = new Uint8Array(input.byteLength)
21+
22+
for (let start = 0; start < input.byteLength; start += XOR_CHUNK_SIZE) {
23+
const end = Math.min(start + XOR_CHUNK_SIZE, input.byteLength)
24+
25+
xor.update(input.subarray(start, end), output.subarray(start, end))
26+
}
27+
28+
return output
29+
}
30+
1131
export interface BoxMessageStreamInit extends MessageStreamInit {
1232
maConn: MultiaddrConnection
1333
localNonce: Uint8Array
@@ -19,6 +39,9 @@ export class BoxMessageStream extends AbstractMultiaddrConnection {
1939
private maConn: MultiaddrConnection
2040
private inboundXor: xsalsa20.Xor
2141
private outboundXor: xsalsa20.Xor
42+
private inboundReleased: boolean
43+
private outboundReleased: boolean
44+
private readonly onInboundMessage: (evt: StreamMessageEvent) => void
2245

2346
constructor (init: BoxMessageStreamInit) {
2447
super({
@@ -29,17 +52,31 @@ export class BoxMessageStream extends AbstractMultiaddrConnection {
2952

3053
this.inboundXor = xsalsa20(init.remoteNonce, init.psk)
3154
this.outboundXor = xsalsa20(init.localNonce, init.psk)
55+
this.inboundReleased = false
56+
this.outboundReleased = false
3257
this.maConn = init.maConn
3358

34-
this.maConn.addEventListener('message', (evt) => {
59+
this.onInboundMessage = (evt) => {
3560
const data = evt.data
3661

62+
// a peer can send data after closing and nothing upstream rejects it
63+
if (this.inboundReleased) {
64+
this.log('discarding %d bytes received after the readable end ended', data.byteLength)
65+
return
66+
}
67+
3768
try {
3869
if (data instanceof Uint8Array) {
39-
this.onData(this.inboundXor.update(data))
70+
this.onData(xorInChunks(this.inboundXor, data))
4071
} else {
4172
for (const buf of data) {
42-
this.onData(this.inboundXor.update(buf))
73+
// onData dispatches synchronously, so the consumer can abort and
74+
// release the cipher part way through the message
75+
if (this.inboundReleased) {
76+
break
77+
}
78+
79+
this.onData(xorInChunks(this.inboundXor, buf))
4380
}
4481
}
4582
} catch (err: any) {
@@ -48,24 +85,116 @@ export class BoxMessageStream extends AbstractMultiaddrConnection {
4885
this.log.error('error decrypting inbound data - %e', err)
4986
this.abort(err)
5087
}
51-
})
88+
}
89+
90+
this.maConn.addEventListener('message', this.onInboundMessage)
5291

5392
// resume sending when the underlying connection can accept more data
5493
this.maConn.addEventListener('drain', () => {
5594
this.onMuxerDrain()
5695
})
5796

97+
this.maConn.addEventListener('end', () => {
98+
this.releaseInbound()
99+
})
100+
58101
this.maConn.addEventListener('close', (evt) => {
59102
if (evt.error != null) {
60103
if (evt.local) {
61104
this.abort(evt.error)
62105
} else {
63106
this.onRemoteReset()
64107
}
108+
109+
// an errored connection never delivers what it still holds, so waiting
110+
// for 'end' would keep the cipher forever
111+
this.releaseInbound()
65112
} else {
113+
// the two ends have to close at different moments. the write side
114+
// first, because the drain dispatches to the consumer and a reply
115+
// would otherwise reach a released cipher and abort a graceful close.
116+
// the read side after, because closing it while our buffer is still
117+
// empty makes onData discard everything the drain produces
118+
this.writeStatus = 'closed'
119+
this.drainMaConn()
66120
this.onTransportClosed()
67121
}
122+
123+
// last, the branches above close the write side first and that is what
124+
// stops sendData reaching a released cipher
125+
this.releaseOutbound()
68126
})
127+
128+
// the nonce handshake reads from the connection before this stream exists,
129+
// so it can already have ended or closed. bytes unwrapped back onto it are
130+
// dispatched in a microtask, so run after them
131+
if (this.maConn.readableEnded || this.maConn.status !== 'open') {
132+
queueMicrotask(() => {
133+
if (this.maConn.readableEnded) {
134+
this.releaseInbound()
135+
}
136+
137+
if (this.maConn.status !== 'open' && this.maConn.status !== 'closing') {
138+
this.onTransportClosed()
139+
this.releaseOutbound()
140+
}
141+
})
142+
}
143+
}
144+
145+
/**
146+
* A connection can close while still holding bytes it received. Decrypt them
147+
* into our own read buffer first, so closing does not discard them and the
148+
* application can still read them
149+
*/
150+
private drainMaConn (): void {
151+
if (this.maConn.readableEnded) {
152+
return
153+
}
154+
155+
try {
156+
this.maConn.resume()
157+
} catch (err: any) {
158+
this.log.error('could not drain the connection before closing - %e', err)
159+
}
160+
}
161+
162+
/**
163+
* Return the inbound cipher's slot in the shared wasm memory to the pool and
164+
* zero the key material it holds. If that fails the slot leaks and the key
165+
* stays resident.
166+
*
167+
* 'end' waits for the read buffer to drain, so a connection left paused with
168+
* unread bytes never emits it and keeps its slot until the process exits
169+
*/
170+
private releaseInbound (): void {
171+
if (this.inboundReleased) {
172+
return
173+
}
174+
175+
this.inboundReleased = true
176+
177+
try {
178+
this.inboundXor.finalize()
179+
} catch (err: any) {
180+
// xsalsa20 caches a view of the shared wasm memory that any other cipher
181+
// growing it detaches, and finalize does not refresh it
182+
this.log.error('could not release the inbound cipher, its wasm slot and key leak - %e', err)
183+
}
184+
}
185+
186+
private releaseOutbound (): void {
187+
if (this.outboundReleased) {
188+
return
189+
}
190+
191+
this.outboundReleased = true
192+
193+
try {
194+
this.outboundXor.finalize()
195+
} catch (err: any) {
196+
this.log.error('could not release the outbound cipher, its wasm slot and key leak - %e', err)
197+
}
69198
}
70199

71200
async sendClose (options?: AbortOptions): Promise<void> {
@@ -75,7 +204,7 @@ export class BoxMessageStream extends AbstractMultiaddrConnection {
75204
sendData (data: Uint8ArrayList): SendResult {
76205
return {
77206
sentBytes: data.byteLength,
78-
canSendMore: this.maConn.send(this.outboundXor.update(data.subarray()))
207+
canSendMore: this.maConn.send(xorInChunks(this.outboundXor, data.subarray()))
79208
}
80209
}
81210

@@ -88,6 +217,12 @@ export class BoxMessageStream extends AbstractMultiaddrConnection {
88217
}
89218

90219
sendResume (): void {
220+
// the connection may have finished delivering everything it had, in which
221+
// case resuming it throws
222+
if (this.maConn.readableEnded) {
223+
return
224+
}
225+
91226
this.maConn.resume()
92227
}
93228
}

0 commit comments

Comments
 (0)