From 9090b745e69c380847f97f107d39864d7ace0039 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Thu, 24 Oct 2024 04:04:56 +0200 Subject: fix: primitive 3: validation of non-final url --- packages/backend/src/core/HttpRequestService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index bea5dee6ab..08e9f46b2d 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -129,7 +129,7 @@ export class HttpRequestService { const finalUrl = res.url; // redirects may have been involved const activity = await res.json() as IObject; - assertActivityMatchesUrls(activity, [url, finalUrl]); + assertActivityMatchesUrls(activity, [finalUrl]); return activity; } -- cgit v1.2.3-freya From f36f4b5398561dcf7365729c530f5b1868c6b994 Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 6 Nov 2024 05:31:11 +0900 Subject: fix(backend): check target IP before sending HTTP request --- packages/backend/src/core/DownloadService.ts | 22 ------ packages/backend/src/core/HttpRequestService.ts | 90 ++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 24 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/DownloadService.ts b/packages/backend/src/core/DownloadService.ts index 0e992f05de..05b9e64a37 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'; @@ -70,13 +69,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); @@ -139,18 +131,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/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 08e9f46b2d..6c013eacc0 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -6,6 +6,7 @@ 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'; @@ -25,6 +26,91 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; +@Injectable() +class HttpRequestServiceAgent extends http.Agent { + constructor( + @Inject(DI.config) + private config: Config, + + options?: Object + ) { + super(options); + } + + @bindThis + public createConnection(options: Object, callback?: Function): net.Socket { + const socket = super.createConnection(options, callback) + .on('connect', ()=>{ + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') { + 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() +class HttpsRequestServiceAgent extends https.Agent { + constructor( + @Inject(DI.config) + private config: Config, + + options?: Object + ) { + super(options); + } + + @bindThis + public createConnection(options: Object, callback?: Function): net.Socket { + const socket = super.createConnection(options, callback) + .on('connect', ()=>{ + const address = socket.remoteAddress; + if (process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') { + 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 { /** @@ -57,14 +143,14 @@ export class HttpRequestService { lookup: false, // nativeのdns.lookupにfallbackしない }); - this.http = new http.Agent({ + this.http = new HttpRequestServiceAgent(config, { keepAlive: true, keepAliveMsecs: 30 * 1000, lookup: cache.lookup as unknown as net.LookupFunction, localAddress: config.outgoingAddress, }); - this.https = new https.Agent({ + this.https = new HttpsRequestServiceAgent(config, { keepAlive: true, keepAliveMsecs: 30 * 1000, lookup: cache.lookup as unknown as net.LookupFunction, -- cgit v1.2.3-freya From 7ccccf5545c1292c8de8b24bc426b1f9430528ef Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 6 Nov 2024 06:33:44 +0900 Subject: fix(backend): allow accessing private IP when testing --- packages/backend/src/core/HttpRequestService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 6c013eacc0..8c5e3bdb91 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -42,7 +42,7 @@ class HttpRequestServiceAgent extends http.Agent { const socket = super.createConnection(options, callback) .on('connect', ()=>{ const address = socket.remoteAddress; - if (process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') { + if (process.env.NODE_ENV === 'production') { if (address && ipaddr.isValid(address)) { if (this.isPrivateIp(address)) { socket.destroy(new Error(`Blocked address: ${address}`)); @@ -84,7 +84,7 @@ class HttpsRequestServiceAgent extends https.Agent { const socket = super.createConnection(options, callback) .on('connect', ()=>{ const address = socket.remoteAddress; - if (process.env.NODE_ENV === 'production' || process.env.NODE_ENV === 'test') { + if (process.env.NODE_ENV === 'production') { if (address && ipaddr.isValid(address)) { if (this.isPrivateIp(address)) { socket.destroy(new Error(`Blocked address: ${address}`)); -- cgit v1.2.3-freya From 663c06be00d5b05d6c3e96559b170190805f38ed Mon Sep 17 00:00:00 2001 From: rectcoordsystem <37621004+rectcoordsystem@users.noreply.github.com> Date: Wed, 13 Nov 2024 03:06:22 +0900 Subject: Apply suggestions from code review Co-authored-by: anatawa12 --- packages/backend/src/core/HttpRequestService.ts | 34 ++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 8c5e3bdb91..0c8b5b4ef4 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -26,30 +26,33 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; -@Injectable() +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( - @Inject(DI.config) private config: Config, - - options?: Object + options?: http.AgentOptions, ) { super(options); } @bindThis - public createConnection(options: Object, callback?: Function): net.Socket { + 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}`)); + .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; }; @@ -68,13 +71,10 @@ class HttpRequestServiceAgent extends http.Agent { } } -@Injectable() class HttpsRequestServiceAgent extends https.Agent { constructor( - @Inject(DI.config) private config: Config, - - options?: Object + options?: https.AgentOptions, ) { super(options); } -- cgit v1.2.3-freya From 360d71278a78169c2b2351bbcf518c1e1654b254 Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 13 Nov 2024 03:27:52 +0900 Subject: fix(backend): lint and typecheck --- packages/backend/src/core/HttpRequestService.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 0c8b5b4ef4..0955d1f5bb 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -80,18 +80,18 @@ class HttpsRequestServiceAgent extends https.Agent { } @bindThis - public createConnection(options: Object, callback?: Function): net.Socket { + 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}`)); + .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; }; @@ -110,7 +110,6 @@ class HttpsRequestServiceAgent extends https.Agent { } } - @Injectable() export class HttpRequestService { /** -- cgit v1.2.3-freya From 7b3e3f8e25a09430e250096ee01c0f913840623f Mon Sep 17 00:00:00 2001 From: rectcoordsystem Date: Wed, 13 Nov 2024 13:30:01 +0900 Subject: fix(backend): add isLocalAddressAllowed option to getAgentByUrl and send (HttpRequestService) --- packages/backend/src/core/HttpRequestService.ts | 49 ++++++++++++++++++------- 1 file changed, 36 insertions(+), 13 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 0955d1f5bb..0ad5667049 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -112,6 +112,16 @@ class HttpsRequestServiceAgent extends https.Agent { @Injectable() export class HttpRequestService { + /** + * Get http non-proxy agent (without local address filtering) + */ + private httpNative: http.Agent; + + /** + * Get https non-proxy agent (without local address filtering) + */ + private httpsNative: https.Agent; + /** * Get http non-proxy agent */ @@ -142,19 +152,20 @@ export class HttpRequestService { lookup: false, // nativeのdns.lookupにfallbackしない }); - this.http = new HttpRequestServiceAgent(config, { + const agentOption = { keepAlive: true, keepAliveMsecs: 30 * 1000, lookup: cache.lookup as unknown as net.LookupFunction, localAddress: config.outgoingAddress, - }); + }; - this.https = new HttpsRequestServiceAgent(config, { - keepAlive: true, - keepAliveMsecs: 30 * 1000, - lookup: cache.lookup as unknown as net.LookupFunction, - localAddress: config.outgoingAddress, - }); + this.httpNative = new http.Agent(agentOption); + + this.httpsNative = new https.Agent(agentOption); + + this.http = new HttpRequestServiceAgent(config, agentOption); + + this.https = new HttpsRequestServiceAgent(config, agentOption); const maxSockets = Math.max(256, config.deliverJobConcurrency ?? 128); @@ -189,16 +200,22 @@ export class HttpRequestService { * @param bypassProxy Allways bypass proxy */ @bindThis - public getAgentByUrl(url: URL, bypassProxy = false): http.Agent | https.Agent { + public getAgentByUrl(url: URL, bypassProxy = false, isLocalAddressAllowed = false): http.Agent | https.Agent { if (bypassProxy || (this.config.proxyBypassHosts ?? []).includes(url.hostname)) { + if (isLocalAddressAllowed) { + return url.protocol === 'http:' ? this.httpNative : this.httpsNative; + } return url.protocol === 'http:' ? this.http : this.https; } else { + if (isLocalAddressAllowed && (!this.config.proxy)) { + return url.protocol === 'http:' ? this.httpNative : this.httpsNative; + } return url.protocol === 'http:' ? this.httpAgent : this.httpsAgent; } } @bindThis - public async getActivityJson(url: string): Promise { + public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise { const res = await this.send(url, { method: 'GET', headers: { @@ -206,6 +223,7 @@ export class HttpRequestService { }, timeout: 5000, size: 1024 * 256, + isLocalAddressAllowed: isLocalAddressAllowed, }, { throwErrorWhenResponseNotOk: true, validators: [validateContentTypeSetAsActivityPub], @@ -220,7 +238,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({ @@ -228,19 +246,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(); @@ -255,6 +275,7 @@ export class HttpRequestService { headers?: Record, timeout?: number, size?: number, + isLocalAddressAllowed?: boolean, } = {}, extra: HttpRequestSendOptions = { throwErrorWhenResponseNotOk: true, @@ -268,6 +289,8 @@ export class HttpRequestService { controller.abort(); }, timeout); + const isLocalAddressAllowed = args.isLocalAddressAllowed ?? false; + const res = await fetch(url, { method: args.method ?? 'GET', headers: { @@ -276,7 +299,7 @@ export class HttpRequestService { }, body: args.body, size: args.size ?? 10 * 1024 * 1024, - agent: (url) => this.getAgentByUrl(url), + agent: (url) => this.getAgentByUrl(url, false, isLocalAddressAllowed), signal: controller.signal, }); -- cgit v1.2.3-freya From 23c4aa25714af145098baa7edd74c1d217e51c1a Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 20:24:59 -0500 Subject: Fix style error --- packages/backend/src/core/HttpRequestService.ts | 10 +++++----- packages/backend/src/queue/processors/InboxProcessorService.ts | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 0ad5667049..6dcd0cdff3 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -54,19 +54,19 @@ class HttpRequestServiceAgent extends http.Agent { } }); 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'; } } @@ -98,14 +98,14 @@ class HttpsRequestServiceAgent extends https.Agent { @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/queue/processors/InboxProcessorService.ts b/packages/backend/src/queue/processors/InboxProcessorService.ts index f453d7d1ae..102e835e24 100644 --- a/packages/backend/src/queue/processors/InboxProcessorService.ts +++ b/packages/backend/src/queue/processors/InboxProcessorService.ts @@ -192,8 +192,7 @@ export class InboxProcessorService implements OnApplicationShutdown { if (signerHost !== activityIdHost) { throw new Bull.UnrecoverableError(`skip: signerHost(${signerHost}) !== activity.id host(${activityIdHost}`); } - } - else { + } else { throw new Bull.UnrecoverableError('skip: activity id is not a string'); } -- cgit v1.2.3-freya From 36af07abe28bec670aaebf9f5af5694bb582c29a Mon Sep 17 00:00:00 2001 From: Julia Johannesen Date: Wed, 20 Nov 2024 20:31:22 -0500 Subject: Fix another style error --- packages/backend/src/core/HttpRequestService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 6dcd0cdff3..083153940a 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -93,7 +93,7 @@ class HttpsRequestServiceAgent extends https.Agent { } }); return socket; - }; + } @bindThis private isPrivateIp(ip: string): boolean { -- cgit v1.2.3-freya