From ce08d2c82793d44d50d5b49c7b555cd0480afe2a Mon Sep 17 00:00:00 2001 From: rectcoordsystem <37621004+rectcoordsystem@users.noreply.github.com> Date: Thu, 21 Nov 2024 08:27:09 +0900 Subject: [PATCH] Merge commit from fork * fix(backend): check target IP before sending HTTP request * fix(backend): allow accessing private IP when testing * Apply suggestions from code review Co-authored-by: anatawa12 * fix(backend): lint and typecheck * fix(backend): add isLocalAddressAllowed option to getAgentByUrl and send (HttpRequestService) * fix(backend): allow fetchSummaryFromProxy, trueMail to access local addresses --------- Co-authored-by: anatawa12 Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com> Signed-off-by: eternal-flame-AD --- packages/backend/src/core/DownloadService.ts | 22 ---- packages/backend/src/core/EmailService.ts | 1 + .../backend/src/core/HttpRequestService.ts | 115 ++++++++++++++++-- .../src/server/web/UrlPreviewService.ts | 2 +- 4 files changed, 109 insertions(+), 31 deletions(-) diff --git a/packages/backend/src/core/DownloadService.ts b/packages/backend/src/core/DownloadService.ts index 6266e51496..8921b7e3f2 100644 --- a/packages/backend/src/core/DownloadService.ts +++ b/packages/backend/src/core/DownloadService.ts @@ -6,7 +6,6 @@ import * as fs from 'node:fs'; import * as stream from 'node:stream/promises'; import { Inject, Injectable } from '@nestjs/common'; -import ipaddr from 'ipaddr.js'; import chalk from 'chalk'; import got, * as Got from 'got'; import { parse } from 'content-disposition'; @@ -69,13 +68,6 @@ export class DownloadService { }, enableUnixSockets: false, }).on('response', (res: Got.Response) => { - if ((process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') && !this.config.proxy && res.ip) { - if (this.isPrivateIp(res.ip)) { - this.logger.warn(`Blocked address: ${res.ip}`); - req.destroy(); - } - } - const contentLength = res.headers['content-length']; if (contentLength != null) { const size = Number(contentLength); @@ -138,18 +130,4 @@ export class DownloadService { cleanup(); } } - - @bindThis - private isPrivateIp(ip: string): boolean { - const parsedIp = ipaddr.parse(ip); - - for (const net of this.config.allowedPrivateNetworks ?? []) { - const cidr = ipaddr.parseCIDR(net); - if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { - return false; - } - } - - return parsedIp.range() !== 'unicast'; - } } diff --git a/packages/backend/src/core/EmailService.ts b/packages/backend/src/core/EmailService.ts index 609cb70448..f38e0565ac 100644 --- a/packages/backend/src/core/EmailService.ts +++ b/packages/backend/src/core/EmailService.ts @@ -313,6 +313,7 @@ export class EmailService { Accept: 'application/json', Authorization: truemailAuthKey, }, + isLocalAddressAllowed: true, }); const json = (await res.json()) as { diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 0f39c0536b..d7db1acd8c 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -6,9 +6,10 @@ import * as http from 'node:http'; import * as https from 'node:https'; import * as net from 'node:net'; +import ipaddr from 'ipaddr.js'; import CacheableLookup from 'cacheable-lookup'; import fetch from 'node-fetch'; -import { HttpProxyAgent, HttpsProxyAgent } from 'hpagent'; +import { HttpsProxyAgent } from 'hpagent'; import { Inject, Injectable } from '@nestjs/common'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; @@ -25,8 +26,97 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; +declare module 'node:http' { + interface Agent { + createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket; + } +} + +class HttpRequestServiceAgent extends http.Agent { + constructor( + private config: Config, + options?: http.AgentOptions, + ) { + super(options); + } + + @bindThis + public createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket { + const socket = super.createConnection(options, callback) + .on('connect', () => { + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production') { + if (address && ipaddr.isValid(address)) { + if (this.isPrivateIp(address)) { + socket.destroy(new Error(`Blocked address: ${address}`)); + } + } + } + }); + return socket; + }; + + @bindThis + private isPrivateIp(ip: string): boolean { + const parsedIp = ipaddr.parse(ip); + + for (const net of this.config.allowedPrivateNetworks ?? []) { + const cidr = ipaddr.parseCIDR(net); + if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { + return false; + } + } + + return parsedIp.range() !== 'unicast'; + } +} + +class HttpsRequestServiceAgent extends https.Agent { + constructor( + private config: Config, + options?: https.AgentOptions, + ) { + super(options); + } + + @bindThis + public createConnection(options: net.NetConnectOpts, callback?: (err: unknown, stream: net.Socket) => void): net.Socket { + const socket = super.createConnection(options, callback) + .on('connect', () => { + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production') { + if (address && ipaddr.isValid(address)) { + if (this.isPrivateIp(address)) { + socket.destroy(new Error(`Blocked address: ${address}`)); + } + } + } + }); + return socket; + }; + + @bindThis + private isPrivateIp(ip: string): boolean { + const parsedIp = ipaddr.parse(ip); + + for (const net of this.config.allowedPrivateNetworks ?? []) { + const cidr = ipaddr.parseCIDR(net); + if (cidr[0].kind() === parsedIp.kind() && parsedIp.match(ipaddr.parseCIDR(net))) { + return false; + } + } + + return parsedIp.range() !== 'unicast'; + } +} + @Injectable() export class HttpRequestService { + /** + * Get https non-proxy agent (without local address filtering) + */ + private httpsNative: https.Agent; + /** * Get https non-proxy agent */ @@ -47,13 +137,18 @@ export class HttpRequestService { lookup: false, // nativeのdns.lookupにfallbackしない }); - this.https = new https.Agent({ + const agentOption = { keepAlive: true, keepAliveMsecs: 30 * 1000, lookup: cache.lookup as unknown as net.LookupFunction, localAddress: config.outgoingAddress, - minVersion: 'TLSv1.2', - }); + minVersion: 'TLSv1.2' as const, + }; + + + this.httpsNative = new https.Agent(agentOption); + + this.https = new HttpsRequestServiceAgent(config, agentOption); const maxSockets = Math.max(256, config.deliverJobConcurrency ?? 128); @@ -91,7 +186,7 @@ export class HttpRequestService { } @bindThis - public async getActivityJson(url: string): Promise { + public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: { @@ -99,6 +194,7 @@ export class HttpRequestService { }, timeout: 5000, size: 1024 * 256, + isLocalAddressAllowed: isLocalAddressAllowed, }, { throwErrorWhenResponseNotOk: true, validators: [validateContentTypeSetAsActivityPub], @@ -113,7 +209,7 @@ export class HttpRequestService { } @bindThis - public async getJson(url: string, accept = 'application/json, */*', headers?: Record): Promise { + public async getJson(url: string, accept = 'application/json, */*', headers?: Record, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: Object.assign({ @@ -121,19 +217,21 @@ export class HttpRequestService { }, headers ?? {}), timeout: 5000, size: 1024 * 256, + isLocalAddressAllowed: isLocalAddressAllowed, }); return await res.json() as T; } @bindThis - public async getHtml(url: string, accept = 'text/html, */*', headers?: Record): Promise { + public async getHtml(url: string, accept = 'text/html, */*', headers?: Record, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: Object.assign({ Accept: accept, }, headers ?? {}), timeout: 5000, + isLocalAddressAllowed: isLocalAddressAllowed, }); return await res.text(); @@ -148,6 +246,7 @@ export class HttpRequestService { headers?: Record, timeout?: number, size?: number, + isLocalAddressAllowed?: boolean, } = {}, extra: HttpRequestSendOptions = { throwErrorWhenResponseNotOk: true, @@ -179,7 +278,7 @@ export class HttpRequestService { }, body: args.body, size: args.size ?? 10 * 1024 * 1024, - agent: (url) => this.getAgentByUrl(url), + agent: (url) => this.getAgentByUrl(url, false), signal: controller.signal, }); diff --git a/packages/backend/src/server/web/UrlPreviewService.ts b/packages/backend/src/server/web/UrlPreviewService.ts index 94b4a9fd90..3642c358c0 100644 --- a/packages/backend/src/server/web/UrlPreviewService.ts +++ b/packages/backend/src/server/web/UrlPreviewService.ts @@ -144,6 +144,6 @@ export class UrlPreviewService { contentLengthRequired: meta.urlPreviewRequireContentLength, }); - return this.httpRequestService.getJson(`${proxy}?${queryStr}`); + return this.httpRequestService.getJson(`${proxy}?${queryStr}`, 'application/json, */*', undefined, true); } }