Skip to content

Commit 15f3e5a

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 dcf913f commit 15f3e5a

File tree

5 files changed

+101
-84
lines changed

5 files changed

+101
-84
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
- update `@deltachat/stdio-rpc-server` and `deltachat/jsonrpc-client` to `2.12.0`
1010

1111
### Fixed
12+
- open WebXDC apps faster: remove the initial progress bar (`RTCPeerConnection` exhaustion hack, a.k.a. FILL500)
1213
- fix app picker sometimes incorrectly showing "Offline"
1314
- allow using the app picker even when offline, as long as the app store data is cached
1415
- if adding an app from the app picker fails, show an error dialog

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
import { T } from '@deltachat/jsonrpc-client'
4040
import type * as Jsonrpc from '@deltachat/jsonrpc-client'
4141
import { setContentProtection } from '../content-protection.js'
42+
import { Server } from 'net'
4243

4344
const __dirname = dirname(fileURLToPath(import.meta.url))
4445

@@ -103,6 +104,62 @@ const DEFAULT_SIZE_MAP: Size = {
103104

104105
export default class DCWebxdc {
105106
constructor(private readonly controller: DeltaChatController) {
107+
let dummyProxy_: { server: Server; url: string } | undefined
108+
const getDummyProxyUrl = async () => {
109+
if (dummyProxy_) {
110+
if (dummyProxy_.server.listening) {
111+
// TODO maybe also close all WebXDC instances
112+
// as soon as we encounter any error?
113+
// This would be more important when/if we get rid of `host-rules`.
114+
throw new Error(
115+
'the dummy proxy is not working anymore, `server.listening` is `false`'
116+
)
117+
}
118+
return dummyProxy_.url
119+
}
120+
121+
const dummyProxy = new Server({}, socket => {
122+
socket.destroy()
123+
})
124+
const listeningP = new Promise((resolve, reject) => {
125+
dummyProxy.once('listening', resolve)
126+
dummyProxy.once('error', reject)
127+
})
128+
dummyProxy.listen({
129+
host: '127.0.0.1',
130+
// Auto-assign port, to avoid a situation
131+
// where a fixed one is occupied.
132+
port: 0,
133+
// We don't really use any connections, but `backlog: 0`
134+
// probably doesn't make sense, so let's set the minimum "sane" value.
135+
backlog: 1,
136+
})
137+
138+
await listeningP
139+
const listenAddress = dummyProxy.address()
140+
if (listenAddress == null) {
141+
throw new Error("'listening' event fired, but address is `null`")
142+
}
143+
if (typeof listenAddress === 'string') {
144+
throw new Error(
145+
'dummy proxy listen address type is string, expected object'
146+
)
147+
}
148+
if (listenAddress.family !== 'IPv4') {
149+
throw new Error(
150+
`dummy proxy listen address family is ${listenAddress.family}, expected "IPv4"`
151+
)
152+
}
153+
154+
const url = `socks5://${listenAddress.address}:${listenAddress.port}`
155+
dummyProxy_ = {
156+
server: dummyProxy,
157+
url: url,
158+
}
159+
log.info('Dummy blackhole proxy listening on', listenAddress)
160+
return url
161+
}
162+
106163
// icon protocol
107164
app.whenReady().then(() => {
108165
protocol.handle('webxdc-icon', async request => {
@@ -179,6 +236,27 @@ export default class DCWebxdc {
179236
ses.protocol.handle('webxdc', (...args) =>
180237
webxdcProtocolHandler(this.rpc, ...args)
181238
)
239+
240+
// Thanks to the fact that we specify `host-rules`,
241+
// no connection attempt to the proxy should occur at all,
242+
// at least as of now.
243+
// See https://www.chromium.org/developers/design-documents/network-stack/socks-proxy/ :
244+
// > The "EXCLUDE" clause make an exception for "myproxy",
245+
// > because otherwise Chrome would be unable to resolve
246+
// > the address of the SOCKS proxy server itself,
247+
// > and all requests would necessarily fail
248+
// > with PROXY_CONNECTION_FAILED.
249+
//
250+
// However, let's still use our dummy TCP listener, just in case.
251+
await ses.setProxy({
252+
mode: 'fixed_servers',
253+
proxyRules: await getDummyProxyUrl(),
254+
})
255+
await ses.closeAllConnections()
256+
257+
// TODO also consider this. However, this might have observable effects
258+
// on the app (i.e. "offline" status).
259+
// ses.enableNetworkEmulation({ offline: true })
182260
}
183261

184262
const app_icon = icon_blob && nativeImage?.createFromBuffer(icon_blob)
@@ -200,6 +278,19 @@ export default class DCWebxdc {
200278
alwaysOnTop: main_window?.isAlwaysOnTop(),
201279
show: false,
202280
})
281+
282+
// Settings this should make WebRTC always use the proxy.
283+
// However, since the proxy won't work, this should, in theory,
284+
// effectively disable WebRTC.
285+
//
286+
// However, weirdly, this alone seems to disable WebRTC,
287+
// even without setting a proxy,
288+
// as evident by using Wireshark together with the "Test Webxdc" app
289+
// (but let's not rely on this).
290+
webxdcWindow.webContents.setWebRTCIPHandlingPolicy(
291+
'disable_non_proxied_udp'
292+
)
293+
203294
setContentProtection(webxdcWindow)
204295

205296
// 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
@@ -21,8 +21,6 @@ const hostRules = 'MAP * ~NOTFOUND, EXCLUDE *.openstreetmap.org'
2121
rawApp.commandLine.appendSwitch('host-resolver-rules', hostRules)
2222
rawApp.commandLine.appendSwitch('host-rules', hostRules)
2323

24-
rawApp.commandLine.appendSwitch('disable-features', 'IsolateSandboxedIframes')
25-
2624
if (rc['version'] === true || rc['v'] === true) {
2725
/* ignore-console-log */
2826
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)