From 62e290eb9e7769abab3b45fe74c7d48635f3bb4f Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Mon, 5 Jun 2023 12:55:09 -0700 Subject: [PATCH] Implement simplified Happy Eyeballs --- ts/util/createHTTPSAgent.ts | 247 ++++++++++++++++++++++++++++++------ ts/util/dns.ts | 27 +--- 2 files changed, 210 insertions(+), 64 deletions(-) diff --git a/ts/util/createHTTPSAgent.ts b/ts/util/createHTTPSAgent.ts index 322f8cc3fe59..b628f0a8522f 100644 --- a/ts/util/createHTTPSAgent.ts +++ b/ts/util/createHTTPSAgent.ts @@ -1,51 +1,218 @@ // Copyright 2013 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import { Agent } from 'https'; -import type { AgentOptions } from 'https'; +import { Agent as HTTPSAgent } from 'https'; +import type { AgentOptions, RequestOptions } from 'https'; +import type { LookupAddress } from 'dns'; +import net from 'net'; +import tls from 'tls'; +import { callbackify, promisify } from 'util'; +import { shuffle } from 'lodash'; +import pTimeout from 'p-timeout'; +import * as log from '../logging/log'; +import { electronLookup as electronLookupWithCb } from './dns'; +import { strictAssert } from './assert'; +import { parseIntOrThrow } from './parseIntOrThrow'; +import { sleep } from './sleep'; import { SECOND } from './durations'; -import { electronLookup } from './dns'; +import { dropNull } from './dropNull'; -// 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; - }; +// See https://www.rfc-editor.org/rfc/rfc8305#section-8 +const DELAY_MS = 250; - const agent = new Agent({ - ...options, - lookup: electronLookup, - autoSelectFamily: true, - autoSelectFamilyAttemptTimeout: 2 * SECOND, +// Warning threshold +const CONNECT_THRESHOLD_MS = SECOND; + +const CONNECT_TIMEOUT_MS = 10 * SECOND; + +const electronLookup = promisify(electronLookupWithCb); + +export class Agent extends HTTPSAgent { + constructor(options: AgentOptions = {}) { + super({ + ...options, + lookup: electronLookup, + }); + } + + public createConnection = callbackify( + async (options: RequestOptions): Promise => { + const { host = options.hostname, port: portString } = options; + strictAssert(host, 'Agent.createConnection: Missing options.host'); + strictAssert(portString, 'Agent.createConnection: Missing options.port'); + + const port = parseIntOrThrow( + portString, + 'Agent.createConnection: options.port is not an integer' + ); + + const addresses = await electronLookup(host, { all: true }); + const firstAddr = addresses.find( + ({ family }) => family === 4 || family === 6 + ); + if (!firstAddr) { + throw new Error(`Agent.createConnection: failed to resolve ${host}`); + } + + const v4 = shuffle(addresses.filter(({ family }) => family === 4)); + const v6 = shuffle(addresses.filter(({ family }) => family === 6)); + + // Interleave addresses for Happy Eyeballs, but keep the first address + // type from the DNS response first in the list. + const interleaved = new Array(); + while (v4.length !== 0 || v6.length !== 0) { + const v4Entry = v4.pop(); + const v6Entry = v6.pop(); + + if (firstAddr.family === 4) { + if (v4Entry !== undefined) { + interleaved.push(v4Entry); + } + if (v6Entry !== undefined) { + interleaved.push(v6Entry); + } + } else { + if (v6Entry !== undefined) { + interleaved.push(v6Entry); + } + if (v4Entry !== undefined) { + interleaved.push(v4Entry); + } + } + } + + const start = Date.now(); + + const { socket, address, v4Attempts, v6Attempts } = await happyEyeballs( + interleaved, + port + ); + + const duration = Date.now() - start; + const logLine = + `createHTTPSAgent.createConnection(${host}): connected to ` + + `IPv${address.family} addr after ${duration}ms ` + + `(v4_attempts=${v4Attempts} v6_attempts=${v6Attempts})`; + + if (v4Attempts + v6Attempts > 1 || duration > CONNECT_THRESHOLD_MS) { + log.warn(logLine); + } else { + log.info(logLine); + } + + return tls.connect({ + socket, + ca: options.ca, + host: dropNull(options.host), + servername: options.servername, + }); + } + ); +} + +export type HappyEyeballsResult = Readonly<{ + socket: net.Socket; + address: LookupAddress; + v4Attempts: number; + v6Attempts: number; +}>; + +export async function happyEyeballs( + addrs: ReadonlyArray, + port = 443 +): Promise { + const abortControllers = addrs.map(() => new AbortController()); + + let v4Attempts = 0; + let v6Attempts = 0; + + const results = await Promise.allSettled( + addrs.map(async (addr, index) => { + const abortController = abortControllers[index]; + if (index !== 0) { + await sleep(index * DELAY_MS, abortController.signal); + } + + if (addr.family === 4) { + v4Attempts += 1; + } else { + v6Attempts += 1; + } + + const socket = await connect({ + address: addr.address, + port, + abortSignal: abortController.signal, + }); + + // Abort other connection attempts + for (const otherController of abortControllers) { + if (otherController !== abortController) { + otherController.abort(); + } + } + return { socket, abortController, index }; + }) + ); + + const fulfilled = results.find(({ status }) => status === 'fulfilled'); + if (fulfilled) { + strictAssert( + fulfilled.status === 'fulfilled', + 'Fulfilled promise was not fulfilled' + ); + const { socket, index } = fulfilled.value; + + return { + socket, + address: addrs[index], + v4Attempts, + v6Attempts, + }; + } + + strictAssert( + results[0].status === 'rejected', + 'No fulfilled promises, but no rejected either' + ); + + // Abort all connection attempts for consistency + for (const controller of abortControllers) { + controller.abort(); + } + throw results[0].reason; +} + +type DelayedConnectOptionsType = Readonly<{ + port: number; + address: string; + abortSignal?: AbortSignal; + timeout?: number; +}>; + +async function connect({ + port, + address, + abortSignal, + timeout = CONNECT_TIMEOUT_MS, +}: DelayedConnectOptionsType): Promise { + const socket = new net.Socket({ + signal: abortSignal, }); - const typedAgent = agent as unknown as { - createConnection: ( - connectionOptions: { servername?: string }, - callback: () => unknown - ) => TLSSocketInternals; - }; + return pTimeout( + new Promise((resolve, reject) => { + socket.once('connect', () => resolve(socket)); + socket.once('error', err => reject(err)); - 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; + socket.connect(port, address); + }), + timeout, + 'createHTTPSAgent.connect: connection timed out' + ); +} + +export function createHTTPSAgent(options: AgentOptions = {}): Agent { + return new Agent(options); } diff --git a/ts/util/dns.ts b/ts/util/dns.ts index 9a8a46f54f0b..22a5e90aa8c2 100644 --- a/ts/util/dns.ts +++ b/ts/util/dns.ts @@ -5,7 +5,6 @@ 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 { strictAssert } from './assert'; import { drop } from './drop'; @@ -87,29 +86,9 @@ function lookupAll( }; }); - 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) + const random = addresses.at( + Math.floor(Math.random() * addresses.length) ); if (random === undefined) { callback( @@ -123,7 +102,7 @@ function lookupAll( return; } - callback(null, interleaved); + callback(null, addresses); } catch (error) { callback(error, []); }