sendnrw/misskey
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge commit from fork

Browse Source 
* fix(backend): Fix an issue where the origin of ActivityPub lookup response was not validated correctly.

[GHSA-6w2c-vf6f-xf26](https://github.com/misskey-dev/misskey/security/advisories/GHSA-6w2c-vf6f-xf26)

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* Enhance: Add configuration option to disable all external redirects when responding to an ActivityPub lookup (config.disallowExternalApRedirect)

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* fixup! fix(backend): Fix an issue where the origin of ActivityPub lookup response was not validated correctly.

* docs & one edge case

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* apply suggestions

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* remove stale frontend reference to _responseInvalidIdHostNotMatch

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

* apply suggestions

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

---------

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>
This commit is contained in:
饺子w (Yumechi)
2025-02-23 04:21:34 -06:00
committed by GitHub
parent 7c87dec6ee
commit 25052164c0
15 changed files with 318 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
packages/backend/src/core/HttpRequestService.ts
View File

@@ -16,7 +16,7 @@ import type { Config } from '@/config.js';
import { StatusError } from '@/misc/status-error.js';
import { bindThis } from '@/decorators.js';
import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
import { assertActivityMatchesUrls } from '@/core/activitypub/misc/check-against-url.js';
import { assertActivityMatchesUrls, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
import type { IObject } from '@/core/activitypub/type.js';
import type { Response } from 'node-fetch';
import type { URL } from 'node:url';
@@ -215,7 +215,7 @@ export class HttpRequestService {
}
@bindThis
public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise<IObject> {
public async getActivityJson(url: string, isLocalAddressAllowed = false, allowSoftfail: FetchAllowSoftFailMask = FetchAllowSoftFailMask.Strict): Promise<IObject> {
const res = await this.send(url, {
method: 'GET',
headers: {
@@ -232,7 +232,7 @@ export class HttpRequestService {
const finalUrl = res.url; // redirects may have been involved
const activity = await res.json() as IObject;
assertActivityMatchesUrls(activity, [finalUrl]);
assertActivityMatchesUrls(url, activity, [finalUrl], allowSoftfail);
return activity;
}

8
packages/backend/src/core/activitypub/ApRequestService.ts
View File

@@ -17,7 +17,7 @@ import { LoggerService } from '@/core/LoggerService.js';
import { bindThis } from '@/decorators.js';
import type Logger from '@/logger.js';
import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
import { assertActivityMatchesUrls } from '@/core/activitypub/misc/check-against-url.js';
import { assertActivityMatchesUrls, FetchAllowSoftFailMask as FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
import type { IObject } from './type.js';
type Request = {
@@ -185,7 +185,7 @@ export class ApRequestService {
* @param url URL to fetch
*/
@bindThis
public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<unknown> {
public async signedGet(url: string, user: { id: MiUser['id'] }, allowSoftfail: FetchAllowSoftFailMask = FetchAllowSoftFailMask.Strict, followAlternate?: boolean): Promise<unknown> {
const _followAlternate = followAlternate ?? true;
const keypair = await this.userKeypairService.getUserKeypair(user.id);
@@ -243,7 +243,7 @@ export class ApRequestService {
if (alternate) {
const href = alternate.getAttribute('href');
if (href && this.utilityService.punyHost(url) === this.utilityService.punyHost(href)) {
return await this.signedGet(href, user, false);
return await this.signedGet(href, user, allowSoftfail, false);
}
}
} catch (e) {
@@ -258,7 +258,7 @@ export class ApRequestService {
const finalUrl = res.url; // redirects may have been involved
const activity = await res.json() as IObject;
assertActivityMatchesUrls(activity, [finalUrl]);
assertActivityMatchesUrls(url, activity, [finalUrl], allowSoftfail);
return activity;
}

21
packages/backend/src/core/activitypub/ApResolverService.ts
View File

@@ -21,6 +21,7 @@ import { ApRendererService } from './ApRendererService.js';
import { ApRequestService } from './ApRequestService.js';
import type { IObject, ICollection, IOrderedCollection } from './type.js';
import { IdentifiableError } from '@/misc/identifiable-error.js';
import { FetchAllowSoftFailMask } from './misc/check-against-url.js';
export class Resolver {
private history: Set<string>;
@@ -72,7 +73,7 @@ export class Resolver {
}
@bindThis
public async resolve(value: string | IObject): Promise<IObject> {
public async resolve(value: string | IObject, allowSoftfail: FetchAllowSoftFailMask = FetchAllowSoftFailMask.Strict): Promise<IObject> {
if (typeof value !== 'string') {
return value;
}
@@ -108,8 +109,8 @@ export class Resolver {
}
const object = (this.user
? await this.apRequestService.signedGet(value, this.user) as IObject
: await this.httpRequestService.getActivityJson(value)) as IObject;
? await this.apRequestService.signedGet(value, this.user, allowSoftfail) as IObject
: await this.httpRequestService.getActivityJson(value, undefined, allowSoftfail)) as IObject;
if (
Array.isArray(object['@context']) ?
@@ -118,19 +119,7 @@ export class Resolver {
) {
throw new IdentifiableError('72180409-793c-4973-868e-5a118eb5519b', 'invalid response');
}
// HttpRequestService / ApRequestService have already checked that
// `object.id` or `object.url` matches the URL used to fetch the
// object after redirects; here we double-check that no redirects
// bounced between hosts
if (object.id == null) {
throw new IdentifiableError('ad2dc287-75c1-44c4-839d-3d2e64576675', 'invalid AP object: missing id');
}
if (this.utilityService.punyHost(object.id) !== this.utilityService.punyHost(value)) {
throw new IdentifiableError('fd93c2fa-69a8-440f-880b-bf178e0ec877', `invalid AP object ${value}: id ${object.id} has different host`);
}
return object;
}

134
packages/backend/src/core/activitypub/misc/check-against-url.ts
View File

@@ -4,18 +4,124 @@
*/
import type { IObject } from '../type.js';
export function assertActivityMatchesUrls(activity: IObject, urls: string[]) {
const hosts = urls.map(it => new URL(it).host);
const idOk = activity.id !== undefined && hosts.includes(new URL(activity.id).host);
// technically `activity.url` could be an `ApObject = IObject |
// string | (IObject | string)[]`, but if it's a complicated thing
// and the `activity.id` doesn't match, I think we're fine
// rejecting the activity
const urlOk = typeof(activity.url) === 'string' && hosts.includes(new URL(activity.url).host);
if (!idOk && !urlOk) {
throw new Error(`bad Activity: neither id(${activity?.id}) nor url(${activity?.url}) match location(${urls})`);
}
export enum FetchAllowSoftFailMask {
// Allow no softfail flags
Strict = 0,
// The values in tuple (requestUrl, finalUrl, objectId) are not all identical
//
// This condition is common for user-initiated lookups but should not be allowed in federation loop
//
// Allow variations:
// good example: https://alice.example.com/@user -> https://alice.example.com/user/:userId
// problematic example: https://alice.example.com/redirect?url=https://bad.example.com/ -> https://bad.example.com/ -> https://alice.example.com/somethingElse
NonCanonicalId = 1 << 0,
// Allow the final object to be at most one subdomain deeper than the request URL, similar to SPF relaxed alignment
//
// Currently no code path allows this flag to be set, but is kept in case of future use as some niche deployments do this, and we provide a pre-reviewed mechanism to opt-in.
//
// Allow variations:
// good example: https://example.com/@user -> https://activitypub.example.com/@user { id: 'https://activitypub.example.com/@user' }
// problematic example: https://example.com/@user -> https://untrusted.example.com/@user { id: 'https://untrusted.example.com/@user' }
MisalignedOrigin = 1 << 1,
// The requested URL has a different host than the returned object ID, although the final URL is still consistent with the object ID
//
// This condition is common for user-initiated lookups using an intermediate host but should not be allowed in federation loops
//
// Allow variations:
// good example: https://alice.example.com/@user@bob.example.com -> https://bob.example.com/@user { id: 'https://bob.example.com/@user' }
// problematic example: https://alice.example.com/definitelyAlice -> https://bob.example.com/@somebodyElse { id: 'https://bob.example.com/@somebodyElse' }
CrossOrigin = 1 << 2 | MisalignedOrigin,
// Allow all softfail flags
//
// do not use this flag on released code
Any = ~0
}
/**
* Fuzz match on whether the candidate host has authority over the request host
*
* @param requestHost The host of the requested resources
* @param candidateHost The host of final response
* @returns Whether the candidate host has authority over the request host, or if a soft fail is required for a match
*/
function hostFuzzyMatch(requestHost: string, candidateHost: string): FetchAllowSoftFailMask {
const requestFqdn = requestHost.endsWith('.') ? requestHost : `${requestHost}.`;
const candidateFqdn = candidateHost.endsWith('.') ? candidateHost : `${candidateHost}.`;
if (requestFqdn === candidateFqdn) {
return FetchAllowSoftFailMask.Strict;
}
// allow only one case where candidateHost is a first-level subdomain of requestHost
const requestDnsDepth = requestFqdn.split('.').length;
const candidateDnsDepth = candidateFqdn.split('.').length;
if ((candidateDnsDepth - requestDnsDepth) !== 1) {
return FetchAllowSoftFailMask.CrossOrigin;
}
if (`.${candidateHost}`.endsWith(`.${requestHost}`)) {
return FetchAllowSoftFailMask.MisalignedOrigin;
}
return FetchAllowSoftFailMask.CrossOrigin;
}
// normalize host names by removing www. prefix
function normalizeSynonymousSubdomain(url: URL | string): URL {
const urlParsed = url instanceof URL ? url : new URL(url);
const host = urlParsed.host;
const normalizedHost = host.replace(/^www\./, '');
return new URL(urlParsed.toString().replace(host, normalizedHost));
}
export function assertActivityMatchesUrls(requestUrl: string | URL, activity: IObject, candidateUrls: (string | URL)[], allowSoftfail: FetchAllowSoftFailMask): FetchAllowSoftFailMask {
// must have a unique identifier to verify authority
if (!activity.id) {
throw new Error(`bad Activity: missing id field`);
}
let softfail = 0;
// if the flag is allowed, set the flag on return otherwise throw
const requireSoftfail = (needed: FetchAllowSoftFailMask, message: string) => {
if ((allowSoftfail & needed) !== needed) {
throw new Error(message);
}
softfail |= needed;
}
const requestUrlParsed = normalizeSynonymousSubdomain(requestUrl);
const idParsed = normalizeSynonymousSubdomain(activity.id);
const candidateUrlsParsed = candidateUrls.map(it => normalizeSynonymousSubdomain(it));
const requestUrlSecure = requestUrlParsed.protocol === 'https:';
const finalUrlSecure = candidateUrlsParsed.every(it => it.protocol === 'https:');
if (requestUrlSecure && !finalUrlSecure) {
throw new Error(`bad Activity: id(${activity?.id}) is not allowed to have http:// in the url`);
}
// Compare final URL to the ID
if (!candidateUrlsParsed.some(it => it.href === idParsed.href)) {
requireSoftfail(FetchAllowSoftFailMask.NonCanonicalId, `bad Activity: id(${activity?.id}) does not match response url(${candidateUrlsParsed.map(it => it.toString())})`);
// at lease host need to match exactly (ActivityPub requirement)
if (!candidateUrlsParsed.some(it => idParsed.host === it.host)) {
throw new Error(`bad Activity: id(${activity?.id}) does not match response host(${candidateUrlsParsed.map(it => it.host)})`);
}
}
// Compare request URL to the ID
if (!requestUrlParsed.href.includes(idParsed.href)) {
requireSoftfail(FetchAllowSoftFailMask.NonCanonicalId, `bad Activity: id(${activity?.id}) does not match request url(${requestUrlParsed.toString()})`);
// if cross-origin lookup is allowed, we can accept some variation between the original request URL to the final object ID (but not between the final URL and the object ID)
const hostResult = hostFuzzyMatch(requestUrlParsed.host, idParsed.host);
requireSoftfail(hostResult, `bad Activity: id(${activity?.id}) is valid but is not the same origin as request url(${requestUrlParsed.toString()})`);
}
return softfail;
}