From 5e2a747685490b31efa778241fccf938bd33722d Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Thu, 18 Apr 2024 09:37:47 -0600 Subject: [PATCH] fix(ext/node): Correctly send ALPN on node TLS connections (#23434) Landing work from #21903, plus fixing a node compat bug. We were always sending the HTTP/2 ALPN on TLS connections which might confuse upstream servers. Changes: - Configure HTTP/2 ALPN when making the TLS connection from the HTTP/2 code - Read the `ALPNProtocols` property from the TLS connection options rather than the deno `alpnProtocols` field - Add tests Prereq for landing Deno.serveHttp on Deno.serve: removing older HTTP servers from the codebase. --- ext/node/polyfills/_tls_wrap.ts | 4 +- ext/node/polyfills/http2.ts | 5 ++- tests/unit_node/http_test.ts | 19 +++------ tests/unit_node/net_test.ts | 1 + tests/unit_node/tls_test.ts | 71 +++++++++++++++++++++++++-------- 5 files changed, 68 insertions(+), 32 deletions(-) diff --git a/ext/node/polyfills/_tls_wrap.ts b/ext/node/polyfills/_tls_wrap.ts index a70dd29f14..ed2bdd0a39 100644 --- a/ext/node/polyfills/_tls_wrap.ts +++ b/ext/node/polyfills/_tls_wrap.ts @@ -96,7 +96,7 @@ export class TLSSocket extends net.Socket { caCerts = [new TextDecoder().decode(caCerts)]; } tlsOptions.caCerts = caCerts; - tlsOptions.alpnProtocols = ["h2", "http/1.1"]; + tlsOptions.alpnProtocols = opts.ALPNProtocols; super({ handle: _wrapHandle(tlsOptions, socket), @@ -114,7 +114,7 @@ export class TLSSocket extends net.Socket { this.secureConnecting = true; this._SNICallback = null; this.servername = null; - this.alpnProtocols = tlsOptions.alpnProtocols; + this.alpnProtocols = tlsOptions.ALPNProtocols; this.authorized = false; this.authorizationError = null; this[kRes] = null; diff --git a/ext/node/polyfills/http2.ts b/ext/node/polyfills/http2.ts index 2856d39381..023b6acd3f 100644 --- a/ext/node/polyfills/http2.ts +++ b/ext/node/polyfills/http2.ts @@ -1677,7 +1677,10 @@ export function connect( case "https:": // TODO(bartlomieju): handle `initializeTLSOptions` here url = `https://${host}${port == 443 ? "" : (":" + port)}`; - socket = tlsConnect(port, host, { manualStart: true }); + socket = tlsConnect(port, host, { + manualStart: true, + ALPNProtocols: ["h2", "http/1.1"], + }); break; default: throw new ERR_HTTP2_UNSUPPORTED_PROTOCOL(protocol); diff --git a/tests/unit_node/http_test.ts b/tests/unit_node/http_test.ts index 049cdbbbc7..6672b97474 100644 --- a/tests/unit_node/http_test.ts +++ b/tests/unit_node/http_test.ts @@ -9,7 +9,6 @@ import { assertSpyCalls, spy } from "@std/testing/mock.ts"; import { gzip } from "node:zlib"; import { Buffer } from "node:buffer"; -import { serve } from "@std/http/server.ts"; import { execCode } from "../unit/test_util.ts"; Deno.test("[node/http listen]", async () => { @@ -338,20 +337,18 @@ Deno.test("[node/http] send request with non-chunked body", async () => { const hostname = "localhost"; const port = 4505; - // NOTE: Instead of node/http.createServer(), serve() in std/http/server.ts is used. - // https://github.com/denoland/deno_std/pull/2755#discussion_r1005592634 const handler = async (req: Request) => { requestHeaders = req.headers; requestBody = await req.text(); return new Response("ok"); }; const abortController = new AbortController(); - const servePromise = serve(handler, { + const servePromise = Deno.serve({ hostname, port, signal: abortController.signal, onListen: undefined, - }); + }, handler).finished; const opts: RequestOptions = { host: hostname, @@ -393,20 +390,18 @@ Deno.test("[node/http] send request with chunked body", async () => { const hostname = "localhost"; const port = 4505; - // NOTE: Instead of node/http.createServer(), serve() in std/http/server.ts is used. - // https://github.com/denoland/deno_std/pull/2755#discussion_r1005592634 const handler = async (req: Request) => { requestHeaders = req.headers; requestBody = await req.text(); return new Response("ok"); }; const abortController = new AbortController(); - const servePromise = serve(handler, { + const servePromise = Deno.serve({ hostname, port, signal: abortController.signal, onListen: undefined, - }); + }, handler).finished; const opts: RequestOptions = { host: hostname, @@ -442,20 +437,18 @@ Deno.test("[node/http] send request with chunked body as default", async () => { const hostname = "localhost"; const port = 4505; - // NOTE: Instead of node/http.createServer(), serve() in std/http/server.ts is used. - // https://github.com/denoland/deno_std/pull/2755#discussion_r1005592634 const handler = async (req: Request) => { requestHeaders = req.headers; requestBody = await req.text(); return new Response("ok"); }; const abortController = new AbortController(); - const servePromise = serve(handler, { + const servePromise = Deno.serve({ hostname, port, signal: abortController.signal, onListen: undefined, - }); + }, handler).finished; const opts: RequestOptions = { host: hostname, diff --git a/tests/unit_node/net_test.ts b/tests/unit_node/net_test.ts index eac4633cbc..e08b24c028 100644 --- a/tests/unit_node/net_test.ts +++ b/tests/unit_node/net_test.ts @@ -55,6 +55,7 @@ Deno.test("[node/net] net.connect().unref() works", async () => { const ctl = new AbortController(); const server = Deno.serve({ signal: ctl.signal, + port: 0, // any available port will do handler: () => new Response("hello"), onListen: async ({ port, hostname }) => { const { stdout, stderr } = await new Deno.Command(Deno.execPath(), { diff --git a/tests/unit_node/tls_test.ts b/tests/unit_node/tls_test.ts index 7b4b35b984..db954f3286 100644 --- a/tests/unit_node/tls_test.ts +++ b/tests/unit_node/tls_test.ts @@ -3,7 +3,6 @@ import { assertEquals, assertInstanceOf } from "@std/assert/mod.ts"; import { delay } from "@std/async/delay.ts"; import { fromFileUrl, join } from "@std/path/mod.ts"; -import { serveTls } from "@std/http/server.ts"; import * as tls from "node:tls"; import * as net from "node:net"; import * as stream from "node:stream"; @@ -13,24 +12,61 @@ const tlsTestdataDir = fromFileUrl( ); const keyFile = join(tlsTestdataDir, "localhost.key"); const certFile = join(tlsTestdataDir, "localhost.crt"); -const key = await Deno.readTextFile(keyFile); -const cert = await Deno.readTextFile(certFile); -const rootCaCert = await Deno.readTextFile(join(tlsTestdataDir, "RootCA.pem")); +const key = Deno.readTextFileSync(keyFile); +const cert = Deno.readTextFileSync(certFile); +const rootCaCert = Deno.readTextFileSync(join(tlsTestdataDir, "RootCA.pem")); + +for ( + const [alpnServer, alpnClient, expected] of [ + [["a", "b"], ["a"], ["a"]], + [["a", "b"], ["b"], ["b"]], + [["a", "b"], ["a", "b"], ["a"]], + [["a", "b"], [], []], + [[], ["a", "b"], []], + ] +) { + Deno.test(`tls.connect sends correct ALPN: '${alpnServer}' + '${alpnClient}' = '${expected}'`, async () => { + const listener = Deno.listenTls({ + port: 0, + key, + cert, + alpnProtocols: alpnServer, + }); + const outgoing = tls.connect({ + host: "localhost", + port: listener.addr.port, + ALPNProtocols: alpnClient, + secureContext: { + ca: rootCaCert, + // deno-lint-ignore no-explicit-any + } as any, + }); + + const conn = await listener.accept(); + const handshake = await conn.handshake(); + assertEquals(handshake.alpnProtocol, expected[0] || null); + conn.close(); + outgoing.destroy(); + listener.close(); + }); +} Deno.test("tls.connect makes tls connection", async () => { const ctl = new AbortController(); - const serve = serveTls(() => new Response("hello"), { - port: 8443, + let port; + const serve = Deno.serve({ + port: 0, key, cert, signal: ctl.signal, - }); + onListen: (listen) => port = listen.port, + }, () => new Response("hello")); await delay(200); const conn = tls.connect({ host: "localhost", - port: 8443, + port, secureContext: { ca: rootCaCert, // deno-lint-ignore no-explicit-any @@ -41,26 +77,29 @@ Host: localhost Connection: close `); - conn.on("data", (chunk) => { - const text = new TextDecoder().decode(chunk); - const bodyText = text.split("\r\n\r\n").at(-1)?.trim(); - assertEquals(bodyText, "hello"); + const chunk = Promise.withResolvers(); + conn.on("data", (received) => { conn.destroy(); ctl.abort(); + chunk.resolve(received); }); - await serve; + await serve.finished; + + const text = new TextDecoder().decode(await chunk.promise); + const bodyText = text.split("\r\n\r\n").at(-1)?.trim(); + assertEquals(bodyText, "hello"); }); // https://github.com/denoland/deno/pull/20120 Deno.test("tls.connect mid-read tcp->tls upgrade", async () => { const ctl = new AbortController(); - const serve = serveTls(() => new Response("hello"), { + const serve = Deno.serve({ port: 8443, key, cert, signal: ctl.signal, - }); + }, () => new Response("hello")); await delay(200); @@ -81,7 +120,7 @@ Deno.test("tls.connect mid-read tcp->tls upgrade", async () => { ctl.abort(); }); - await serve; + await serve.finished; }); Deno.test("tls.createServer creates a TLS server", async () => {