From 557b86f52eec6f976d1543cb1eeb538c18d860be Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Tue, 30 May 2023 16:57:16 -0700 Subject: [PATCH] Use electron's DNS resolver, prioritizing ipv4 connections --- app/main.ts | 10 ++ ts/test-node/util/dns_test.ts | 67 --------- ts/textsecure/WebAPI.ts | 10 +- ts/textsecure/WebSocket.ts | 5 +- ts/updater/got.ts | 9 +- ts/util/createHTTPSAgent.ts | 51 +++++++ ts/util/dns.ts | 257 +++++++++++++--------------------- 7 files changed, 169 insertions(+), 240 deletions(-) delete mode 100644 ts/test-node/util/dns_test.ts create mode 100644 ts/util/createHTTPSAgent.ts diff --git a/app/main.ts b/app/main.ts index 6af222ea6959..efa5a573ab06 100644 --- a/app/main.ts +++ b/app/main.ts @@ -20,6 +20,7 @@ import { ipcMain as ipc, Menu, nativeTheme, + net, powerSaveBlocker, screen, session, @@ -2618,6 +2619,15 @@ ipc.handle('executeMenuAction', async (_event, action: MenuActionType) => { } }); +ipc.handle( + 'net.resolveHost', + (_event, hostname: string, queryType?: 'A' | 'AAAA') => { + return net.resolveHost(hostname, { + queryType, + }); + } +); + let stickerCreatorWindow: BrowserWindow | undefined; async function showStickerCreatorWindow() { if (stickerCreatorWindow) { diff --git a/ts/test-node/util/dns_test.ts b/ts/test-node/util/dns_test.ts deleted file mode 100644 index db921e098d49..000000000000 --- a/ts/test-node/util/dns_test.ts +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2023 Signal Messenger, LLC -// SPDX-License-Identifier: AGPL-3.0-only - -import { assert } from 'chai'; -import * as sinon from 'sinon'; - -import { DNSCache } from '../../util/dns'; -import { SECOND } from '../../util/durations'; - -const NOW = 1680726906000; - -describe('dns/DNSCache', () => { - let sandbox: sinon.SinonSandbox; - let cache: DNSCache; - beforeEach(() => { - sandbox = sinon.createSandbox(); - cache = new DNSCache(); - }); - - afterEach(() => { - sandbox.restore(); - }); - - it('should cache records and pick a random one', () => { - sandbox.useFakeTimers({ - now: NOW, - }); - - const result = cache.setAndPick('signal.org', 4, [ - { - data: '10.0.0.1', - expiresAt: NOW + SECOND, - }, - { - data: '10.0.0.2', - expiresAt: NOW + SECOND, - }, - ]); - - assert.oneOf(result, ['10.0.0.1', '10.0.0.2']); - }); - - it('should invalidate cache after expiration', () => { - const clock = sandbox.useFakeTimers({ - now: NOW, - }); - - cache.setAndPick('signal.org', 4, [ - { - data: '10.0.0.1', - expiresAt: NOW + SECOND, - }, - { - data: '10.0.0.2', - expiresAt: NOW + 2 * SECOND, - }, - ]); - - assert.oneOf(cache.get('signal.org', 4), ['10.0.0.1', '10.0.0.2']); - - clock.tick(SECOND); - assert.strictEqual(cache.get('signal.org', 4), '10.0.0.2'); - - clock.tick(SECOND); - assert.strictEqual(cache.get('signal.org', 4), undefined); - }); -}); diff --git a/ts/textsecure/WebAPI.ts b/ts/textsecure/WebAPI.ts index 2772818f4135..26a2c3ab3409 100644 --- a/ts/textsecure/WebAPI.ts +++ b/ts/textsecure/WebAPI.ts @@ -9,7 +9,7 @@ import type { Response } from 'node-fetch'; import fetch from 'node-fetch'; import ProxyAgent from 'proxy-agent'; -import { Agent } from 'https'; +import type { Agent } from 'https'; import { escapeRegExp, isNumber, isString, isObject } from 'lodash'; import PQueue from 'p-queue'; import { v4 as getGuid } from 'uuid'; @@ -28,7 +28,7 @@ import { formatAcceptLanguageHeader } from '../util/userLanguages'; import { toWebSafeBase64, fromWebSafeBase64 } from '../util/webSafeBase64'; import { getBasicAuth } from '../util/getBasicAuth'; import { isPnpEnabled } from '../util/isPnpEnabled'; -import { lookupWithFallback } from '../util/dns'; +import { createHTTPSAgent } from '../util/createHTTPSAgent'; import type { SocketStatus } from '../types/SocketStatus'; import { toLogFormat } from '../types/errors'; import { isPackIdValid, redactPackId } from '../types/Stickers'; @@ -243,15 +243,15 @@ async function _promiseAjax( agents[cacheKey] = { agent: proxyUrl ? new ProxyAgent(proxyUrl) - : new Agent({ - lookup: lookupWithFallback, + : createHTTPSAgent({ keepAlive: !options.disableSessionResumption, maxCachedSessions: options.disableSessionResumption ? 0 : undefined, }), timestamp: Date.now(), }; } - const { agent } = agents[cacheKey]; + const agentEntry = agents[cacheKey]; + const agent = agentEntry?.agent ?? null; const fetchOptions = { method: options.type, diff --git a/ts/textsecure/WebSocket.ts b/ts/textsecure/WebSocket.ts index f90ede37fee7..354adfec06c5 100644 --- a/ts/textsecure/WebSocket.ts +++ b/ts/textsecure/WebSocket.ts @@ -10,7 +10,7 @@ import { strictAssert } from '../util/assert'; import { explodePromise } from '../util/explodePromise'; import { getUserAgent } from '../util/getUserAgent'; import * as durations from '../util/durations'; -import { lookupWithFallback } from '../util/dns'; +import { createHTTPSAgent } from '../util/createHTTPSAgent'; import * as log from '../logging/log'; import * as Timers from '../Timers'; import { ConnectTimeoutError, HTTPError } from './Errors'; @@ -55,8 +55,7 @@ export function connect({ const client = new WebSocketClient({ tlsOptions: { ca: certificateAuthority, - agent: proxyAgent, - lookup: lookupWithFallback, + agent: proxyAgent ?? createHTTPSAgent(), }, maxReceivedFrameSize: 0x210000, }); diff --git a/ts/updater/got.ts b/ts/updater/got.ts index faea141818c6..b38a272e0553 100644 --- a/ts/updater/got.ts +++ b/ts/updater/got.ts @@ -4,11 +4,12 @@ import type { StrictOptions as GotOptions } from 'got'; import config from 'config'; import ProxyAgent from 'proxy-agent'; +import { Agent as HTTPAgent } from 'http'; import * as packageJson from '../../package.json'; import { getUserAgent } from '../util/getUserAgent'; import * as durations from '../util/durations'; -import { lookupWithFallback } from '../util/dns'; +import { createHTTPSAgent } from '../util/createHTTPSAgent'; export const GOT_CONNECT_TIMEOUT = durations.MINUTE; export const GOT_LOOKUP_TIMEOUT = durations.MINUTE; @@ -31,14 +32,16 @@ export function getGotOptions(): GotOptions { http: new ProxyAgent(proxyUrl), https: new ProxyAgent(proxyUrl), } - : undefined; + : { + http: new HTTPAgent(), + https: createHTTPSAgent(), + }; return { agent, https: { certificateAuthority, }, - lookup: lookupWithFallback as GotOptions['lookup'], headers: { 'Cache-Control': 'no-cache', 'User-Agent': getUserAgent(packageJson.version), diff --git a/ts/util/createHTTPSAgent.ts b/ts/util/createHTTPSAgent.ts new file mode 100644 index 000000000000..322f8cc3fe59 --- /dev/null +++ b/ts/util/createHTTPSAgent.ts @@ -0,0 +1,51 @@ +// Copyright 2013 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { Agent } from 'https'; +import type { AgentOptions } from 'https'; + +import { SECOND } from './durations'; +import { electronLookup } from './dns'; + +// Due to the Node.js bug, we must manually re-apply `servername` when creating +// an outgoing TLS connection with "Happy Eyeballs" (`autoSelectFamily: true`). +// +// See: https://github.com/nodejs/node/pull/48255 +export function createHTTPSAgent(options: AgentOptions = {}): Agent { + type TLSSocketInternals = { + _init: (...args: Array) => unknown; + setServername: (servername: string) => void; + }; + + const agent = new Agent({ + ...options, + lookup: electronLookup, + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 2 * SECOND, + }); + + const typedAgent = agent as unknown as { + createConnection: ( + connectionOptions: { servername?: string }, + callback: () => unknown + ) => TLSSocketInternals; + }; + + const oldCreateConnection = typedAgent.createConnection; + typedAgent.createConnection = function createConnection( + connectionOptions, + callback + ) { + const socket = oldCreateConnection.call(this, connectionOptions, callback); + const oldInit = socket._init; + socket._init = function _init(...args) { + const result = oldInit.apply(this, args); + if (connectionOptions.servername) { + socket.setServername(connectionOptions.servername); + } + return result; + }; + return socket; + }; + return agent; +} diff --git a/ts/util/dns.ts b/ts/util/dns.ts index 6505b8cd3394..9a8a46f54f0b 100644 --- a/ts/util/dns.ts +++ b/ts/util/dns.ts @@ -1,15 +1,14 @@ // Copyright 2023 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import { lookup as nativeLookup } from 'dns'; -import type { LookupOneOptions } from 'dns'; -import fetch from 'node-fetch'; -import { z } from 'zod'; +import type { LookupOneOptions, LookupAllOptions, LookupAddress } from 'dns'; +import { lookup as nodeLookup } from 'dns'; +import { ipcRenderer, net } from 'electron'; +import type { ResolvedHost } from 'electron'; +import { shuffle } from 'lodash'; -import * as log from '../logging/log'; -import * as Errors from '../types/errors'; import { strictAssert } from './assert'; -import { SECOND } from './durations'; +import { drop } from './drop'; const HOST_ALLOWLIST = new Set([ // Production @@ -33,171 +32,105 @@ const HOST_ALLOWLIST = new Set([ 'sfu.voip.signal.org', ]); -const dohResponseSchema = z.object({ - Status: z.number(), - Answer: z.array( - z.object({ - data: z.string(), - TTL: z.number(), - }) - ), - Comment: z.string().optional(), -}); - -type CacheEntry = Readonly<{ - data: string; - expiresAt: number; -}>; - -export class DNSCache { - private readonly ipv4 = new Map>(); - private readonly ipv6 = new Map>(); - - public get(hostname: string, family: 4 | 6): string | undefined { - const map = this.getMap(family); - - const entries = map.get(hostname); - if (!entries) { - return undefined; - } - - // Cleanup old records - this.cleanup(entries); - if (entries.length === 0) { - map.delete(hostname); - return undefined; - } - - // Pick a random record - return this.pick(entries); - } - - public setAndPick( - hostname: string, - family: 4 | 6, - entries: Array - ): string { - strictAssert(entries.length !== 0, 'should have at least on entry'); - - const map = this.getMap(family); - - // Just overwrite the entries - we shouldn't get here unless it was a cache - // miss. - map.set(hostname, entries); - - return this.pick(entries); - } - - // Private - - private getMap(family: 4 | 6): Map> { - return family === 4 ? this.ipv4 : this.ipv6; - } - - private pick(entries: Array): string { - const index = Math.floor(Math.random() * entries.length); - return entries[index].data; - } - - private cleanup(entries: Array): void { - const now = Date.now(); - for (let i = entries.length - 1; i >= 0; i -= 1) { - const { expiresAt } = entries[i]; - if (expiresAt <= now) { - entries.splice(i, 1); - } - } - } -} - -const cache = new DNSCache(); - -export async function doh(hostname: string, family: 4 | 6): Promise { - const cached = cache.get(hostname, family); - if (cached !== undefined) { - log.info(`dns/doh: using cached value for ${hostname}/IPv${family}`); - return cached; - } - - const url = new URL('https://1.1.1.1/dns-query'); - url.searchParams.append('name', hostname); - url.searchParams.append('type', family === 4 ? 'A' : 'AAAA'); - const res = await fetch(url.toString(), { - headers: { - accept: 'application/dns-json', - 'user-agent': 'Electron', - }, - }); - - if (!res.ok) { - throw new Error( - `DoH request for ${hostname} failed with http status: ${res.status}` - ); - } - - const { - Status: status, - Answer: answer, - Comment: comment, - } = dohResponseSchema.parse(await res.json()); - - if (status !== 0) { - throw new Error(`DoH request for ${hostname} failed: ${status}/${comment}`); - } - - if (answer.length === 0) { - throw new Error(`DoH request for ${hostname} failed: empty answer`); - } - - const now = Date.now(); - return cache.setAndPick( - hostname, - family, - answer.map(({ data, TTL }) => { - return { data, expiresAt: now + TTL * SECOND }; - }) - ); -} - -export function lookupWithFallback( +function lookupAll( hostname: string, - opts: LookupOneOptions, + opts: LookupOneOptions | LookupAllOptions, callback: ( err: NodeJS.ErrnoException | null, - address: string, - family: number + addresses: string | Array, + family?: number ) => void ): void { + if (!HOST_ALLOWLIST.has(hostname)) { + nodeLookup(hostname, opts, callback); + return; + } + // Node.js support various signatures, but we only support one. strictAssert(typeof opts === 'object', 'missing options'); - strictAssert(Boolean(opts.all) !== true, 'options.all is not supported'); strictAssert(typeof callback === 'function', 'missing callback'); - nativeLookup(hostname, opts, async (err, ...nativeArgs) => { - if (!err) { - return callback(err, ...nativeArgs); - } - - if (!HOST_ALLOWLIST.has(hostname)) { - log.error( - `dns/lookup: failed for ${hostname}, ` + - `err: ${Errors.toLogFormat(err)}. not retrying` - ); - return callback(err, ...nativeArgs); - } - - const family = opts.family === 6 ? 6 : 4; - - log.error( - `dns/lookup: failed for ${hostname}, err: ${Errors.toLogFormat(err)}. ` + - `Retrying with DoH (IPv${family})` - ); + async function run() { + let result: ResolvedHost; try { - const answer = await doh(hostname, family); - callback(null, answer, family); - } catch (fallbackErr) { - callback(fallbackErr, '', 0); + let queryType: 'A' | 'AAAA' | undefined; + if (opts.family === 4) { + queryType = 'A'; + } else if (opts.family === 6) { + queryType = 'AAAA'; + } + + if (net) { + // Main process + result = await net.resolveHost(hostname, { + queryType, + }); + } else { + // Renderer + result = await ipcRenderer.invoke( + 'net.resolveHost', + hostname, + queryType + ); + } + const addresses = result.endpoints.map(({ address, family }) => { + let numericFamily = -1; + if (family === 'ipv4') { + numericFamily = 4; + } else if (family === 'ipv6') { + numericFamily = 6; + } + return { + address, + family: numericFamily, + }; + }); + + const v4 = shuffle(addresses.filter(({ family }) => family === 4)); + const v6 = shuffle(addresses.filter(({ family }) => family === 6)); + + // Node.js should interleave v4 and v6 addresses when trying them with + // Happy Eyeballs, but it does not do it yet. + // + // See: https://github.com/nodejs/node/pull/48258 + const interleaved = new Array(); + while (v4.length !== 0 || v6.length !== 0) { + const v4Entry = v4.pop(); + // Prioritize v4 over v6 + if (v4Entry !== undefined) { + interleaved.push(v4Entry); + } + const v6Entry = v6.pop(); + if (v6Entry !== undefined) { + interleaved.push(v6Entry); + } + } + + if (!opts.all) { + const random = interleaved.at( + Math.floor(Math.random() * interleaved.length) + ); + if (random === undefined) { + callback( + new Error(`Hostname: ${hostname} cannot be resolved`), + '', + -1 + ); + return; + } + callback(null, random.address, random.family); + return; + } + + callback(null, interleaved); + } catch (error) { + callback(error, []); } - }); + } + + drop(run()); } + +// Note: `nodeLookup` has a complicated type due to compatibility requirements. +export const electronLookup = lookupAll as typeof nodeLookup;