diff --git a/packages/fetch-node/package.json b/packages/fetch-node/package.json index 89c77cf7f8f..96d69b2486c 100644 --- a/packages/fetch-node/package.json +++ b/packages/fetch-node/package.json @@ -26,12 +26,10 @@ "dependencies": { "@atproto/fetch": "workspace:*", "@atproto/transformer": "workspace:*", - "http-errors": "^2.0.0", "ipaddr.js": "^2.1.0", "tslib": "^2.6.2" }, "devDependencies": { - "@types/http-errors": "^2.0.4", "typescript": "^5.3.3" }, "scripts": { diff --git a/packages/fetch-node/src/index.ts b/packages/fetch-node/src/index.ts index 3d1acba67f7..e8beb920c88 100644 --- a/packages/fetch-node/src/index.ts +++ b/packages/fetch-node/src/index.ts @@ -1 +1,2 @@ -export * from './fetch-wrap.js' +export * from './safe.js' +export * from './ssrf.js' diff --git a/packages/fetch-node/src/fetch-wrap.ts b/packages/fetch-node/src/safe.ts similarity index 60% rename from packages/fetch-node/src/fetch-wrap.ts rename to packages/fetch-node/src/safe.ts index 5b74296a562..3bbea231672 100644 --- a/packages/fetch-node/src/fetch-wrap.ts +++ b/packages/fetch-node/src/safe.ts @@ -6,11 +6,16 @@ import { } from '@atproto/fetch' import { compose } from '@atproto/transformer' -import { ssrfSafeHostname } from './ssrf.js' +import { ssrfFetchWrap } from './ssrf.js' export type SafeFetchWrapOptions = NonNullable< Parameters[0] > + +/** + * Wrap a fetch function with safety checks so that it can be safely used + * with user provided input (URL). + */ export const safeFetchWrap = ({ fetch = globalThis.fetch as Fetch, responseMaxSize = 512 * 1024, // 512kB @@ -44,7 +49,7 @@ export const safeFetchWrap = ({ * input, we need to make sure that the request is not vulnerable to SSRF * attacks. */ - ssrfProtection ? ssrfSafeFetchWrap({ fetch }) : fetch, + ssrfProtection ? ssrfFetchWrap({ fetch }) : fetch, /** * Since we will be fetching user owned data, we need to make sure that an @@ -52,33 +57,3 @@ export const safeFetchWrap = ({ */ fetchMaxSizeProcessor(responseMaxSize), ) - -export type SsrfSafeFetchWrapOptions = NonNullable< - Parameters[0] -> -export const ssrfSafeFetchWrap = ({ - fetch = globalThis.fetch as Fetch, -} = {}): Fetch => { - const ssrfSafeFetch: Fetch = async (request) => { - const { hostname } = new URL(request.url) - - // Make sure the hostname is a valid IP address - const ip = await ssrfSafeHostname(hostname) - if (ip) { - // Normally we would replace the hostname with the IP address and set the - // Host header to the original hostname. However, since we are using - // fetch() we can't set the Host header. - } - - if (request.redirect === 'follow') { - // TODO: actually implement by calling ssrfSafeFetch recursively - throw new Error( - 'Request redirect must be "error" or "manual" when SSRF is enabled', - ) - } - - return fetch(request) - } - - return ssrfSafeFetch -} diff --git a/packages/fetch-node/src/ssrf.ts b/packages/fetch-node/src/ssrf.ts index f9202223296..24bf44131ff 100644 --- a/packages/fetch-node/src/ssrf.ts +++ b/packages/fetch-node/src/ssrf.ts @@ -1,23 +1,51 @@ import dns, { promises as dnsPromises } from 'node:dns' -import createError from 'http-errors' +import { Fetch, FetchError } from '@atproto/fetch' import ipaddr from 'ipaddr.js' const { IPv4, IPv6 } = ipaddr -export async function ssrfSafeHostname(hostname: string): Promise { - const ip = await hostnameLookup(hostname).catch((cause) => { - throw cause?.code === 'ENOTFOUND' - ? createError(400, `Invalid hostname ${hostname}`, { cause }) - : createError(500, `Unable resolve DNS for ${hostname}`, { cause }) - }) - if (ip.range() !== 'unicast') { - throw createError(400, `Invalid hostname IP address ${ip}`) +export type SsrfSafeFetchWrapOptions = NonNullable< + Parameters[0] +> +export const ssrfFetchWrap = ({ + fetch = globalThis.fetch as Fetch, +} = {}): Fetch => { + const ssrfSafeFetch: Fetch = async (request) => { + if (request.redirect === 'follow') { + // TODO: actually implement by calling ssrfSafeFetch recursively + throw new Error( + 'Request redirect must be "error" or "manual" when SSRF is enabled', + ) + } + + const { hostname } = new URL(request.url) + + // Make sure the hostname is a unicast IP address + const ip = await hostnameLookup(hostname).catch((cause) => { + throw cause?.code === 'ENOTFOUND' + ? new FetchError(400, `Invalid hostname ${hostname}`, { + request, + cause, + }) + : new FetchError(500, `Unable resolve DNS for ${hostname}`, { + request, + cause, + }) + }) + if (ip.range() !== 'unicast') { + throw new FetchError(400, `Invalid hostname IP address ${ip}`, { + request, + }) + } + + return fetch(request) } - return ip.toString() + + return ssrfSafeFetch } -export async function hostnameLookup( +async function hostnameLookup( hostname: string, ): Promise { if (IPv4.isIPv4(hostname)) { @@ -31,7 +59,7 @@ export async function hostnameLookup( return domainLookup(hostname) } -export async function domainLookup( +async function domainLookup( domain: string, ): Promise { const addr = await dnsPromises.lookup(domain, { diff --git a/packages/fetch/src/fetch-request.ts b/packages/fetch/src/fetch-request.ts index 0f25ab925ba..e50fe1635a3 100644 --- a/packages/fetch/src/fetch-request.ts +++ b/packages/fetch/src/fetch-request.ts @@ -24,7 +24,6 @@ export function forbiddenDomainNameRequestTransform( forbiddenDomainNames: Iterable, ): RequestTranformer { const forbiddenDomainNameSet = new Set(forbiddenDomainNames) - if (forbiddenDomainNameSet.size === 0) return (request) => request return async (request) => { const { hostname } = new URL(request.url) diff --git a/packages/pds/src/config/config.ts b/packages/pds/src/config/config.ts index eb75fc9ddee..a132e9734e0 100644 --- a/packages/pds/src/config/config.ts +++ b/packages/pds/src/config/config.ts @@ -243,8 +243,9 @@ export const envToCfg = (env: ServerEnvironment): ServerConfig => { } const safeFetchCfg: ServerConfig['safeFetch'] = { - allowHttp: false, + allowHttp: env.fetchDisableSafeties ? true : false, forbiddenDomainNames: [ + 'google.com', 'example.com', 'example.org', 'example.net', diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1b8c9ffdecb..f5777f1700d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -513,9 +513,6 @@ importers: '@atproto/transformer': specifier: workspace:* version: link:../transformer - http-errors: - specifier: ^2.0.0 - version: 2.0.0 ipaddr.js: specifier: ^2.1.0 version: 2.1.0 @@ -523,9 +520,6 @@ importers: specifier: ^2.6.2 version: 2.6.2 devDependencies: - '@types/http-errors': - specifier: ^2.0.4 - version: 2.0.4 typescript: specifier: ^5.3.3 version: 5.3.3