Skip to content

Commit a69f8d2

Browse files
committed
fix: remove RTCPeerConnection exhaustion (FILL500)
This partially reverts 750721c (#3098). Not completely though: the `<iframe>` thing is still there, and I have not removed it yet for the sake of being conservative. The most important point of this change is to prepare ourselves for the removal of the `IsolateSandboxedIframes` Chromium feature. It has been enabled by default, and is to be removed soon-ish: https://issues.chromium.org/issues/362246896. The feature makes the RTCPeerConnection exhaustion hack not work anymore, because an attacker can create a sandboxed `<iframe>`, and, as the feature name implies, process-isolated, thus have a separate RTCPeerConnection 500 pool. Earlier we simply un-enabled this Chromium feature in 50bcd88 (#4351), but when it's removed, we will not be able to. We could hold off on upgrading Electron then, but let's not. To answer "Why haven't we done this before?": probably because the 500 hack was the first thing that proved to work, both on Electron and in Android. But in Android apparently one can't control the WebRTC IP handling policy, so this approach is not available on Android. Another thing is, of course, the fact that this speeds up the process of launching an app. And in general this approach seems more thorough to me. It also provides another exfiltration security layer (a dummy proxy) against regular HTTP requests as well, i.e. it guards even against CSP bypass. This uses the same approach that we took in the Tauri version: #4852. On top of that, the 500 limit itself is not something that is set in stone and should be relied on. To check that this still works, follow the instructions in https://github.com/webxdc/webxdc-test/pull/40/files.
1 parent dd42471 commit a69f8d2

File tree

4 files changed

+103
-87
lines changed

4 files changed

+103
-87
lines changed

packages/target-electron/src/deltachat/webxdc.ts

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
import { T } from '@deltachat/jsonrpc-client'
3939
import type * as Jsonrpc from '@deltachat/jsonrpc-client'
4040
import { setContentProtection } from '../content-protection.js'
41+
import { Server } from 'net'
4142

4243
const log = getLogger('main/deltachat/webxdc')
4344

@@ -108,6 +109,62 @@ const DEFAULT_SIZE_MAP: Size = {
108109

109110
export default class DCWebxdc {
110111
constructor(private readonly controller: DeltaChatController) {
112+
let dummyProxy_: { server: Server; url: string } | undefined
113+
const getDummyProxyUrl = async () => {
114+
if (dummyProxy_) {
115+
if (dummyProxy_.server.listening) {
116+
// TODO maybe also close all WebXDC instances
117+
// as soon as we encounter any error?
118+
// This would be more important when/if we get rid of `host-rules`.
119+
throw new Error(
120+
'the dummy proxy is not working anymore, `server.listening` is `false`'
121+
)
122+
}
123+
return dummyProxy_.url
124+
}
125+
126+
const dummyProxy = new Server({}, socket => {
127+
socket.destroy()
128+
})
129+
const listeningP = new Promise((resolve, reject) => {
130+
dummyProxy.once('listening', resolve)
131+
dummyProxy.once('error', reject)
132+
})
133+
dummyProxy.listen({
134+
host: '127.0.0.1',
135+
// Auto-assign port, to avoid a situation
136+
// where a fixed one is occupied.
137+
port: 0,
138+
// We don't really use any connections, but `backlog: 0`
139+
// probably doesn't make sense, so let's set the minimum "sane" value.
140+
backlog: 1,
141+
})
142+
143+
await listeningP
144+
const listenAddress = dummyProxy.address()
145+
if (listenAddress == null) {
146+
throw new Error("'listening' event fired, but address is `null`")
147+
}
148+
if (typeof listenAddress === 'string') {
149+
throw new Error(
150+
'dummy proxy listen address type is string, expected object'
151+
)
152+
}
153+
if (listenAddress.family !== 'IPv4') {
154+
throw new Error(
155+
`dummy proxy listen address family is ${listenAddress.family}, expected "IPv4"`
156+
)
157+
}
158+
159+
const url = `socks5://${listenAddress.address}:${listenAddress.port}`
160+
dummyProxy_ = {
161+
server: dummyProxy,
162+
url: url,
163+
}
164+
log.info('Dummy blackhole proxy listening on', listenAddress)
165+
return url
166+
}
167+
111168
// icon protocol
112169
app.whenReady().then(() => {
113170
protocol.handle('webxdc-icon', async request => {
@@ -129,16 +186,38 @@ export default class DCWebxdc {
129186
})
130187
})
131188

132-
const createSessionIfNotExists = (
189+
const createSessionIfNotExists = async (
133190
accountId: number,
134191
internetAccess: boolean
135-
): string => {
192+
): Promise<string> => {
136193
const partition = partitionKeyFromAccountId(accountId, internetAccess)
137194
if (!existing_sessions.includes(partition)) {
138195
const ses = session.fromPartition(partition, {
139196
cache: false,
140197
})
141198
existing_sessions.push(partition)
199+
200+
// Thanks to the fact that we specify `host-rules`,
201+
// no connection attempt to the proxy should occur at all,
202+
// at least as of now.
203+
// See https://www.chromium.org/developers/design-documents/network-stack/socks-proxy/ :
204+
// > The "EXCLUDE" clause make an exception for "myproxy",
205+
// > because otherwise Chrome would be unable to resolve
206+
// > the address of the SOCKS proxy server itself,
207+
// > and all requests would necessarily fail
208+
// > with PROXY_CONNECTION_FAILED.
209+
//
210+
// However, let's still use our dummy TCP listener, just in case.
211+
await ses.setProxy({
212+
mode: 'fixed_servers',
213+
proxyRules: await getDummyProxyUrl(),
214+
})
215+
await ses.closeAllConnections()
216+
217+
// TODO also consider this. However, this might have observable effects
218+
// on the app (i.e. "offline" status).
219+
// ses.enableNetworkEmulation({ offline: true })
220+
142221
// register appropriate protocols
143222
// see https://www.electronjs.org/docs/latest/api/protocol
144223
ses.protocol.handle('webxdc', (...args) => {
@@ -208,7 +287,7 @@ export default class DCWebxdc {
208287
// TODO intercept / deny network access - CSP should probably be disabled for testing
209288

210289
// used by BrowserWindow
211-
const partition = createSessionIfNotExists(
290+
const partition = await createSessionIfNotExists(
212291
accountId,
213292
webxdcInfo['internetAccess']
214293
)
@@ -232,6 +311,18 @@ export default class DCWebxdc {
232311
alwaysOnTop: main_window?.isAlwaysOnTop(),
233312
show: false,
234313
})
314+
// Settings this should make WebRTC always use the proxy.
315+
// However, since the proxy won't work, this should, in theory,
316+
// effectively disable WebRTC.
317+
//
318+
// However, weirdly, this alone seems to disable WebRTC,
319+
// even without setting a proxy,
320+
// as evident by using Wireshark together with the "Test Webxdc" app
321+
// (but let's not rely on this, let's still use the proxy).
322+
webxdcWindow.webContents.setWebRTCIPHandlingPolicy(
323+
'disable_non_proxied_udp'
324+
)
325+
235326
setContentProtection(webxdcWindow)
236327

237328
// reposition the window to last position (or default)

packages/target-electron/src/index.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ const hostRules = 'MAP * ~NOTFOUND'
2020
rawApp.commandLine.appendSwitch('host-resolver-rules', hostRules)
2121
rawApp.commandLine.appendSwitch('host-rules', hostRules)
2222

23-
rawApp.commandLine.appendSwitch('disable-features', 'IsolateSandboxedIframes')
24-
2523
if (rc['version'] === true || rc['v'] === true) {
2624
// eslint-disable-next-line no-console
2725
console.info(BuildInfo.VERSION)

packages/target-electron/static/webxdc-preload.js

Lines changed: 4 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ class RealtimeListener {
5353
// setLocation was called before all connections were filled
5454
let locationUrl = ''
5555

56-
let connectionsFilled = false
57-
5856
/**
5957
* @type {Parameters<import('@webxdc/types').Webxdc["setUpdateListener"]>[0]|null}
6058
*/
@@ -244,8 +242,6 @@ class RealtimeListener {
244242
},
245243
}
246244

247-
const connections = []
248-
249245
contextBridge.exposeInMainWorld('webxdc_internal', {
250246
setup: (selfAddr, selfName, sendUpdateInterval, sendUpdateMaxSize) => {
251247
if (is_ready) {
@@ -262,64 +258,17 @@ class RealtimeListener {
262258

263259
window.frames[0].window.addEventListener('keydown', keydown_handler)
264260
},
265-
fill_up_connections: async () => {
266-
/** @type {HTMLProgressElement} */
267-
const loadingProgress = document.getElementById('progress')
268-
const numIterations = 50
269-
loadingProgress.max = numIterations
270-
const loadingDiv = document.getElementById('loading')
261+
setInitialIframeSrc: async () => {
271262
const iframe = document.getElementById('frame')
272-
273-
const cert = {
274-
certificates: [
275-
await RTCPeerConnection.generateCertificate({
276-
name: 'ECDSA',
277-
namedCurve: 'P-256',
278-
}),
279-
],
280-
}
281-
282-
try {
283-
for (let i = 0; i < numIterations; i++) {
284-
connections.push(new RTCPeerConnection(cert))
285-
connections.push(new RTCPeerConnection(cert))
286-
connections.push(new RTCPeerConnection(cert))
287-
connections.push(new RTCPeerConnection(cert))
288-
connections.push(new RTCPeerConnection(cert))
289-
connections.push(new RTCPeerConnection(cert))
290-
connections.push(new RTCPeerConnection(cert))
291-
connections.push(new RTCPeerConnection(cert))
292-
connections.push(new RTCPeerConnection(cert))
293-
connections.push(new RTCPeerConnection(cert))
294-
await new Promise(res => setTimeout(res, 0)) // this is to view loading bar, it returns to the ev loop
295-
loadingProgress.value = i + 1
296-
}
297-
try {
298-
connections.push(new RTCPeerConnection(cert))
299-
console.log('could create 501th connection, this should never happen')
300-
ipcRenderer.invoke('webxdc.exit')
301-
} catch (error) {
302-
loadingDiv.remove()
303-
iframe.src = locationUrl !== '' ? locationUrl : 'index.html'
304-
iframe.contentWindow.window.addEventListener(
305-
'keydown',
306-
keydown_handler
307-
)
308-
connectionsFilled = true
309-
}
310-
} catch (error) {
311-
console.log('error loading, should crash/close window', error)
312-
ipcRenderer.invoke('webxdc.exit')
313-
}
263+
iframe.src = locationUrl !== '' ? locationUrl : 'index.html'
264+
iframe.contentWindow.window.addEventListener('keydown', keydown_handler)
314265
},
315266
/**
316267
* called via webContents.executeJavaScript
317268
*/
318269
setLocationUrl(base64EncodedHref) {
319270
locationUrl = Buffer.from(base64EncodedHref, 'base64').toString('utf8')
320-
if (locationUrl && locationUrl !== '' && connectionsFilled) {
321-
// if connectionsFilled is false, the url is loaded after
322-
// the connections are filled
271+
if (locationUrl && locationUrl !== '') {
323272
window.frames[0].window.location = locationUrl
324273
}
325274
},

packages/target-electron/static/webxdc_wrapper.html

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,40 +22,18 @@
2222
top: 0;
2323
width: 100%;
2424
}
25-
#loading {
26-
position: absolute;
27-
background-color: Canvas;
28-
inset: 0;
29-
}
30-
#progress {
31-
width: 100%;
32-
border-radius: 0;
33-
display: block;
34-
height: 5px;
35-
}
36-
#progress::-webkit-progress-bar {
37-
background-color: transparent;
38-
}
39-
#progress::-webkit-progress-value {
40-
background-color: #3D66FF;
41-
}
42-
@media (prefers-color-scheme: dark) {
43-
#progress::-webkit-progress-value {
44-
background-color: #A1C4FF;
45-
}
46-
}
4725
</style>
4826
</head>
4927
<body>
5028
<div class="iframe-container">
5129
<iframe id="frame"></iframe>
52-
<div id="loading">
53-
<progress id="progress" value="0"></progress>
54-
</div>
5530
</div>
5631
<script>
57-
const iframe = document.getElementById('frame')
58-
window.webxdc_internal.fill_up_connections()
32+
// TODO this might get executed before the initial `setLocationUrl()`.
33+
// This can cause the app to load the default URL (index.html)
34+
// initially, but then load the target URL.
35+
// Nothing too bad, but probably should be fixed.
36+
webxdc_internal.setInitialIframeSrc()
5937
</script>
6038
</body>
6139
</html>

0 commit comments

Comments
 (0)