Normalize default ports and use X-Forwarded-Port in the origin check (#8059)

The origin check compared the host header to the origin's host as plain
strings, so `example.com:443` did not match `https://example.com`, and a
proxy forwarding `$host` (no port) together with X-Forwarded-Port failed
on any non-default port.

Default ports (80 for http, 443 for https) are now treated as no port,
and when X-Forwarded-Host has no port the port from X-Forwarded-Port is
used.  Everything else is still an exact match.  getHost() is unchanged
for the domain proxy.

src/node/http.ts

@@ -381,19 +381,20 @@ export function authenticateOrigin(req: express.Request): void {
     return
   }
 
-  let origin: string
+  let originUrl: URL
   try {
-    origin = new URL(originRaw).host.trim().toLowerCase()
+    originUrl = new URL(originRaw)
   } catch (error) {
     throw new Error(`unable to parse malformed origin "${originRaw}"`)
   }
+  const origin = originUrl.host.trim().toLowerCase()
 
   const trustedOrigins = req.args["trusted-origins"] || []
   if (isTrustedOrigin(origin, trustedOrigins)) {
     return
   }
 
-  const host = getHost(req)
+  const host = getHost(req, true)
   if (typeof host === "undefined") {
     // A missing host likely means the reverse proxy has not been configured to
     // forward the host which means we cannot perform the check.  Emit an error
@@ -403,15 +404,64 @@ export function authenticateOrigin(req: express.Request): void {
     throw new Error("no host headers found")
   }
 
-  if (host !== origin) {
+  if (!hostMatchesOrigin(host, originUrl)) {
     throw new Error(`host "${host}" does not match origin "${origin}"`)
   }
 }
 
+const defaultPorts: { [protocol: string]: string } = {
+  "http:": "80",
+  "https:": "443",
+}
+
+/**
+ * Parse a host like `example.com:8080` or `[::1]:8080`.  A non-special scheme
+ * keeps the port as written instead of dropping default ports.
+ */
+function parseHost(host: string): URL | undefined {
+  try {
+    return new URL(`host://${host}`)
+  } catch (error) {
+    return undefined
+  }
+}
+
+/**
+ * Check whether the host from the request headers matches the origin.
+ *
+ * This is an exact match, except that a default port (80 for http, 443 for
+ * https) is the same as no port, since `example.com:443` and
+ * `https://example.com` are the same origin.
+ */
+export function hostMatchesOrigin(host: string, origin: URL): boolean {
+  const parsed = parseHost(host)
+  if (!parsed) {
+    return false
+  }
+  const port = parsed.port === defaultPorts[origin.protocol] ? "" : parsed.port
+  return parsed.hostname === origin.hostname.toLowerCase() && port === origin.port
+}
+
+/**
+ * Add the port from X-Forwarded-Port to a host that does not have one.  Some
+ * reverse proxies (for example nginx with `$host`) forward the host without
+ * the port and send the port separately.
+ */
+function addForwardedPort(req: express.Request, host: string): string {
+  const port = getFirstHeader(req, "x-forwarded-port")?.split(",")[0].trim()
+  if (!port || !/^\d+$/.test(port) || parseHost(host)?.port !== "") {
+    return host
+  }
+  return `${host}:${port}`
+}
+
 /**
  * Get the host from headers.  It will be trimmed and lowercased.
+ *
+ * If `withForwardedPort` is set and the host comes from X-Forwarded-Host
+ * without a port, the port from X-Forwarded-Port is added.
  */
-export function getHost(req: express.Request): string | undefined {
+export function getHost(req: express.Request, withForwardedPort = false): string | undefined {
   // Honor Forwarded if present.
   const forwardedRaw = getFirstHeader(req, "forwarded")
   if (forwardedRaw) {
@@ -430,7 +480,8 @@ export function getHost(req: express.Request): string | undefined {
   if (xHost) {
     const firstXHost = xHost.split(",")[0]
     if (firstXHost) {
-      return firstXHost.trim().toLowerCase()
+      const host = firstXHost.trim().toLowerCase()
+      return withForwardedPort ? addForwardedPort(req, host) : host
     }
   }
 

test/unit/node/http.test.ts

@@ -68,6 +68,43 @@ describe("http", () => {
         host: "localhost:8081",
         expected: "does not match",
       },
+      {
+        origin: "https://example.com",
+        host: "example.com:443", // Explicit default port.
+      },
+      {
+        origin: "http://example.com",
+        host: "example.com:80",
+      },
+      {
+        origin: "https://example.com",
+        host: "example.com:80", // Default port of the other protocol.
+        expected: "does not match",
+      },
+      {
+        origin: "https://example.com:8080",
+        host: "example.com", // Missing port is not ignored.
+        expected: "does not match",
+      },
+      {
+        origin: "https://example.com",
+        host: "example.com:8443",
+        expected: "does not match",
+      },
+      {
+        origin: "http://example.com:8080",
+        host: "other.com",
+        expected: "does not match",
+      },
+      {
+        origin: "http://[::1]:8080",
+        host: "[::1]:8080",
+      },
+      {
+        origin: "http://[::1]:8080",
+        host: "[::1]:8081",
+        expected: "does not match",
+      },
       {
         origin: "localhost:8080",
         host: "localhost:8080",
@@ -122,6 +159,75 @@ describe("http", () => {
         })
       })
     })
+
+    describe("x-forwarded-port", () => {
+      ;[
+        {
+          origin: "https://example.com:8080",
+          headers: { "x-forwarded-host": "example.com", "x-forwarded-port": "8080" },
+        },
+        {
+          origin: "https://example.com",
+          headers: { "x-forwarded-host": "example.com", "x-forwarded-port": "443" },
+        },
+        {
+          origin: "http://[::1]:8080",
+          headers: { "x-forwarded-host": "[::1]", "x-forwarded-port": "8080" },
+        },
+        {
+          origin: "https://example.com:8080",
+          headers: { "x-forwarded-host": "example.com", "x-forwarded-port": "8080, 443" },
+        },
+        {
+          origin: "https://example.com:8080",
+          headers: { "x-forwarded-host": "example.com", "x-forwarded-port": "8081" },
+          expected: "does not match",
+        },
+        {
+          origin: "https://example.com:8080",
+          headers: { "x-forwarded-host": "example.com:8081", "x-forwarded-port": "8080" },
+          expected: "does not match", // The port in the host wins.
+        },
+        {
+          origin: "https://example.com:8080",
+          headers: { "x-forwarded-host": "example.com", "x-forwarded-port": "foo" },
+          expected: "does not match",
+        },
+        {
+          origin: "https://example.com:8080",
+          headers: { host: "example.com", "x-forwarded-port": "8080" },
+          expected: "does not match", // Only used with X-Forwarded-Host.
+        },
+        {
+          origin: "https://example.com:8080",
+          headers: { forwarded: "host=example.com", "x-forwarded-port": "8080" },
+          expected: "does not match",
+        },
+      ].forEach((test) => {
+        it(`${test.origin} -> ${JSON.stringify(test.headers)}`, () => {
+          const req = getMockReq({
+            originalUrl: "localhost:8080",
+            headers: {
+              origin: test.origin,
+              ...test.headers,
+            },
+            args: {},
+          })
+          if (typeof test.expected === "string") {
+            expect(() => http.authenticateOrigin(req)).toThrow(test.expected)
+          } else {
+            expect(() => http.authenticateOrigin(req)).not.toThrow()
+          }
+        })
+      })
+
+      it("should not add the port to the host for other uses", () => {
+        const req = getMockReq({
+          headers: { "x-forwarded-host": "code-8080.example.com", "x-forwarded-port": "443" },
+        })
+        expect(http.getHost(req)).toBe("code-8080.example.com")
+      })
+    })
   })
 
   describe("constructRedirectPath", () => {