summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author饺子w (Yumechi) <35571479+eternal-flame-AD@users.noreply.github.com>2025-02-23 04:21:34 -0600
committerGitHub <noreply@github.com>2025-02-23 19:21:34 +0900
commit25052164c0971497f9177f88446b110e8ca91ce2 (patch)
tree6df09b4f006d9322706d16f650c3e40b7dd7ceba
parentignore js-built (#15523) (diff)
downloadsharkey-25052164c0971497f9177f88446b110e8ca91ce2.tar.gz
sharkey-25052164c0971497f9177f88446b110e8ca91ce2.tar.bz2
sharkey-25052164c0971497f9177f88446b110e8ca91ce2.zip
Merge commit from fork
* 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>
-rw-r--r--.config/cypress-devcontainer.yml5
-rw-r--r--.config/docker_example.yml5
-rw-r--r--.config/example.yml5
-rw-r--r--CHANGELOG.md1
-rw-r--r--locales/index.d.ts8
-rw-r--r--locales/ja-JP.yml4
-rw-r--r--packages/backend/src/config.ts3
-rw-r--r--packages/backend/src/core/HttpRequestService.ts6
-rw-r--r--packages/backend/src/core/activitypub/ApRequestService.ts8
-rw-r--r--packages/backend/src/core/activitypub/ApResolverService.ts21
-rw-r--r--packages/backend/src/core/activitypub/misc/check-against-url.ts126
-rw-r--r--packages/backend/src/server/ServerService.ts37
-rw-r--r--packages/backend/src/server/api/endpoints/ap/show.ts12
-rw-r--r--packages/backend/test/unit/ap-request.ts125
-rw-r--r--packages/frontend/src/scripts/lookup.ts4
15 files changed, 314 insertions, 56 deletions
diff --git a/.config/cypress-devcontainer.yml b/.config/cypress-devcontainer.yml
index 3907615f73..e75e32a17a 100644
--- a/.config/cypress-devcontainer.yml
+++ b/.config/cypress-devcontainer.yml
@@ -220,5 +220,10 @@ allowedPrivateNetworks: [
'127.0.0.1/32'
]
+# Disable automatic redirect for ActivityPub object lookup. (default: false)
+# This is a strong defense against potential impersonation attacks if the viewer instance has inadequate validation.
+# However it will make it impossible for other instances to lookup third-party user and notes through your URL.
+#disallowExternalApRedirect: true
+
# Upload or download file size limits (bytes)
#maxFileSize: 262144000
diff --git a/.config/docker_example.yml b/.config/docker_example.yml
index ad9ae4fd9a..1ffed00cc7 100644
--- a/.config/docker_example.yml
+++ b/.config/docker_example.yml
@@ -235,6 +235,11 @@ signToActivityPubGet: true
# '127.0.0.1/32'
#]
+# Disable automatic redirect for ActivityPub object lookup. (default: false)
+# This is a strong defense against potential impersonation attacks if the viewer instance has inadequate validation.
+# However it will make it impossible for other instances to lookup third-party user and notes through your URL.
+#disallowExternalApRedirect: true
+
# Upload or download file size limits (bytes)
#maxFileSize: 262144000
diff --git a/.config/example.yml b/.config/example.yml
index 349c2e9730..71427c84bc 100644
--- a/.config/example.yml
+++ b/.config/example.yml
@@ -334,6 +334,11 @@ signToActivityPubGet: true
# '127.0.0.1/32'
#]
+# Disable automatic redirect for ActivityPub object lookup. (default: false)
+# This is a strong defense against potential impersonation attacks if the viewer instance has inadequate validation.
+# However it will make it impossible for other instances to lookup third-party user and notes through your URL.
+#disallowExternalApRedirect: true
+
# Upload or download file size limits (bytes)
#maxFileSize: 262144000
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 57100aaf3b..d29e3db0d4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,7 @@
- Fix: CWの注釈が100文字を超えている場合、ノート投稿ボタンを非アクティブに
### Server
+- Enhance: 成り済まし対策として、ActivityPub照会された時にリモートのリダイレクトを拒否できるように (config.disallowExternalApRedirect)
- Fix: `following/invalidate`でフォロワーを解除しようとしているユーザーの情報を返すように
- Fix: オブジェクトストレージの設定でPrefixを設定していなかった場合nullまたは空文字になる問題を修正
- Fix: pgroongaでの検索時にはじめのキーワードのみが検索に使用される問題を修正
diff --git a/locales/index.d.ts b/locales/index.d.ts
index c7996b2ca9..0f71263c96 100644
--- a/locales/index.d.ts
+++ b/locales/index.d.ts
@@ -10896,13 +10896,7 @@ export interface Locale extends ILocale {
*/
"title": string;
/**
- * このサーバーと通信することはできましたが、得られたデータが不正なものでした。
- */
- "description": string;
- };
- "_responseInvalidIdHostNotMatch": {
- /**
- * 入力されたURIのドメインと最終的に得られたURIのドメインとが異なります。第三者のサーバーを介してリモートのコンテンツを照会している場合は、発信元のサーバーで取得できるURIを使用して照会し直してください。
+ * このサーバーと通信することはできましたが、得られたデータが不正なものでした。第三者のサーバーを介してリモートのコンテンツを照会している場合は、発信元のサーバーで取得できるURIを使用して照会し直してください。
*/
"description": string;
};
diff --git a/locales/ja-JP.yml b/locales/ja-JP.yml
index 1aed7c21ae..8c803f1ebe 100644
--- a/locales/ja-JP.yml
+++ b/locales/ja-JP.yml
@@ -2911,9 +2911,7 @@ _remoteLookupErrors:
description: "このサーバーとの通信に失敗しました。相手サーバーがダウンしている可能性があります。また、不正なURIや存在しないURIを入力していないか確認してください。"
_responseInvalid:
title: "レスポンスが不正です"
- description: "このサーバーと通信することはできましたが、得られたデータが不正なものでした。"
- _responseInvalidIdHostNotMatch:
- description: "入力されたURIのドメインと最終的に得られたURIのドメインとが異なります。第三者のサーバーを介してリモートのコンテンツを照会している場合は、発信元のサーバーで取得できるURIを使用して照会し直してください。"
+ description: "このサーバーと通信することはできましたが、得られたデータが不正なものでした。第三者のサーバーを介してリモートのコンテンツを照会している場合は、発信元のサーバーで取得できるURIを使用して照会し直してください。"
_noSuchObject:
title: "見つかりません"
description: "要求されたリソースは見つかりませんでした。URIをもう一度お確かめください。"
diff --git a/packages/backend/src/config.ts b/packages/backend/src/config.ts
index d5fd2ba558..32ea700748 100644
--- a/packages/backend/src/config.ts
+++ b/packages/backend/src/config.ts
@@ -73,6 +73,7 @@ type Source = {
proxyBypassHosts?: string[];
allowedPrivateNetworks?: string[];
+ disallowExternalApRedirect?: boolean;
maxFileSize?: number;
@@ -149,6 +150,7 @@ export type Config = {
proxySmtp: string | undefined;
proxyBypassHosts: string[] | undefined;
allowedPrivateNetworks: string[] | undefined;
+ disallowExternalApRedirect: boolean;
maxFileSize: number;
clusterLimit: number | undefined;
id: string;
@@ -287,6 +289,7 @@ export function loadConfig(): Config {
proxySmtp: config.proxySmtp,
proxyBypassHosts: config.proxyBypassHosts,
allowedPrivateNetworks: config.allowedPrivateNetworks,
+ disallowExternalApRedirect: config.disallowExternalApRedirect ?? false,
maxFileSize: config.maxFileSize ?? 262144000,
clusterLimit: config.clusterLimit,
outgoingAddress: config.outgoingAddress,
diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 083153940a..8085bbf961 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -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;
}
diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts
index 8c3b7295e4..6c29cce325 100644
--- a/packages/backend/src/core/activitypub/ApRequestService.ts
+++ b/packages/backend/src/core/activitypub/ApRequestService.ts
@@ -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;
}
diff --git a/packages/backend/src/core/activitypub/ApResolverService.ts b/packages/backend/src/core/activitypub/ApResolverService.ts
index 52cc569140..fb963294cb 100644
--- a/packages/backend/src/core/activitypub/ApResolverService.ts
+++ b/packages/backend/src/core/activitypub/ApResolverService.ts
@@ -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;
}
diff --git a/packages/backend/src/core/activitypub/misc/check-against-url.ts b/packages/backend/src/core/activitypub/misc/check-against-url.ts
index d679bd8180..30b9b67598 100644
--- a/packages/backend/src/core/activitypub/misc/check-against-url.ts
+++ b/packages/backend/src/core/activitypub/misc/check-against-url.ts
@@ -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);
+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}.`;
- const idOk = activity.id !== undefined && hosts.includes(new URL(activity.id).host);
+ if (requestFqdn === candidateFqdn) {
+ return FetchAllowSoftFailMask.Strict;
+ }
- // 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);
+ // allow only one case where candidateHost is a first-level subdomain of requestHost
+ const requestDnsDepth = requestFqdn.split('.').length;
+ const candidateDnsDepth = candidateFqdn.split('.').length;
- if (!idOk && !urlOk) {
- throw new Error(`bad Activity: neither id(${activity?.id}) nor url(${activity?.url}) match location(${urls})`);
+ 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;
+} \ No newline at end of file
diff --git a/packages/backend/src/server/ServerService.ts b/packages/backend/src/server/ServerService.ts
index fd2bd3267d..b899053287 100644
--- a/packages/backend/src/server/ServerService.ts
+++ b/packages/backend/src/server/ServerService.ts
@@ -103,6 +103,43 @@ export class ServerService implements OnApplicationShutdown {
serve: false,
});
+ // if the requester looks like to be performing an ActivityPub object lookup, reject all external redirects
+ //
+ // this will break lookup that involve copying a URL from a third-party server, like trying to lookup http://charlie.example.com/@alice@alice.com
+ //
+ // this is not required by standard but protect us from peers that did not validate final URL.
+ if (this.config.disallowExternalApRedirect) {
+ const maybeApLookupRegex = /application\/activity\+json|application\/ld\+json.+activitystreams/i;
+ fastify.addHook('onSend', (request, reply, _, done) => {
+ const location = reply.getHeader('location');
+ if (reply.statusCode < 300 || reply.statusCode >= 400 || typeof location !== 'string') {
+ done();
+ return;
+ }
+
+ if (!maybeApLookupRegex.test(request.headers.accept ?? '')) {
+ done();
+ return;
+ }
+
+ const effectiveLocation = process.env.NODE_ENV === 'production' ? location : location.replace(/^http:\/\//, 'https://');
+ if (effectiveLocation.startsWith(`https://${this.config.host}/`)) {
+ done();
+ return;
+ }
+
+ reply.status(406);
+ reply.removeHeader('location');
+ reply.header('content-type', 'text/plain; charset=utf-8');
+ reply.header('link', `<${encodeURI(location)}>; rel="canonical"`);
+ done(null, [
+ "Refusing to relay remote ActivityPub object lookup.",
+ "",
+ `Please remove 'application/activity+json' and 'application/ld+json' from the Accept header or fetch using the authoritative URL at ${location}.`,
+ ].join('\n'));
+ });
+ }
+
fastify.register(this.apiServerService.createServer, { prefix: '/api' });
fastify.register(this.openApiServerService.createServer);
fastify.register(this.fileServerService.createServer);
diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts
index 5c2e82da88..4afed7dc5c 100644
--- a/packages/backend/src/server/api/endpoints/ap/show.ts
+++ b/packages/backend/src/server/api/endpoints/ap/show.ts
@@ -20,6 +20,7 @@ import { UtilityService } from '@/core/UtilityService.js';
import { bindThis } from '@/decorators.js';
import { ApiError } from '../../error.js';
import { IdentifiableError } from '@/misc/identifiable-error.js';
+import { FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
export const meta = {
tags: ['federation'],
@@ -53,11 +54,6 @@ export const meta = {
code: 'RESPONSE_INVALID',
id: '70193c39-54f3-4813-82f0-70a680f7495b',
},
- responseInvalidIdHostNotMatch: {
- message: 'Requested URI and response URI host does not match.',
- code: 'RESPONSE_INVALID_ID_HOST_NOT_MATCH',
- id: 'a2c9c61a-cb72-43ab-a964-3ca5fddb410a',
- },
noSuchObject: {
message: 'No such object.',
code: 'NO_SUCH_OBJECT',
@@ -153,7 +149,8 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
// リモートから一旦オブジェクトフェッチ
const resolver = this.apResolverService.createResolver();
- const object = await resolver.resolve(uri).catch((err) => {
+ // allow ap/show exclusively to lookup URLs that are cross-origin or non-canonical (like https://alice.example.com/@bob@bob.example.com -> https://bob.example.com/@bob)
+ const object = await resolver.resolve(uri, FetchAllowSoftFailMask.CrossOrigin | FetchAllowSoftFailMask.NonCanonicalId).catch((err) => {
if (err instanceof IdentifiableError) {
switch (err.id) {
// resolve
@@ -165,10 +162,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
case '09d79f9e-64f1-4316-9cfa-e75c4d091574':
throw new ApiError(meta.errors.federationNotAllowed);
case '72180409-793c-4973-868e-5a118eb5519b':
- case 'ad2dc287-75c1-44c4-839d-3d2e64576675':
throw new ApiError(meta.errors.responseInvalid);
- case 'fd93c2fa-69a8-440f-880b-bf178e0ec877':
- throw new ApiError(meta.errors.responseInvalidIdHostNotMatch);
// resolveLocal
case '02b40cd0-fa92-4b0c-acc9-fb2ada952ab8':
diff --git a/packages/backend/test/unit/ap-request.ts b/packages/backend/test/unit/ap-request.ts
index d3d39240dc..0426de8e19 100644
--- a/packages/backend/test/unit/ap-request.ts
+++ b/packages/backend/test/unit/ap-request.ts
@@ -8,6 +8,8 @@ import httpSignature from '@peertube/http-signature';
import { genRsaKeyPair } from '@/misc/gen-key-pair.js';
import { ApRequestCreator } from '@/core/activitypub/ApRequestService.js';
+import { assertActivityMatchesUrls, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
+import { IObject } from '@/core/activitypub/type.js';
export const buildParsedSignature = (signingString: string, signature: string, algorithm: string) => {
return {
@@ -24,6 +26,10 @@ export const buildParsedSignature = (signingString: string, signature: string, a
};
};
+function cartesianProduct<T, U>(a: T[], b: U[]): [T, U][] {
+ return a.flatMap(a => b.map(b => [a, b] as [T, U]));
+}
+
describe('ap-request', () => {
test('createSignedPost with verify', async () => {
const keypair = await genRsaKeyPair();
@@ -58,4 +64,123 @@ describe('ap-request', () => {
const result = httpSignature.verifySignature(parsed, keypair.publicKey);
assert.deepStrictEqual(result, true);
});
+
+ test('rejects non matching domain', () => {
+ assert.doesNotThrow(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/abc',
+ { id: 'https://alice.example.com/abc' } as IObject,
+ [
+ 'https://alice.example.com/abc',
+ ],
+ FetchAllowSoftFailMask.Strict,
+ ), 'validation should pass base case');
+ assert.throws(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/abc',
+ { id: 'https://bob.example.com/abc' } as IObject,
+ [
+ 'https://alice.example.com/abc',
+ ],
+ FetchAllowSoftFailMask.Any,
+ ), 'validation should fail no matter what if the response URL is inconsistent with the object ID');
+
+ // fix issues like threads
+ // https://github.com/misskey-dev/misskey/issues/15039
+ const withOrWithoutWWW = [
+ 'https://alice.example.com/abc',
+ 'https://www.alice.example.com/abc',
+ ];
+
+ cartesianProduct(
+ cartesianProduct(
+ withOrWithoutWWW,
+ withOrWithoutWWW,
+ ),
+ withOrWithoutWWW,
+ ).forEach(([[a, b], c]) => {
+ assert.doesNotThrow(() => assertActivityMatchesUrls(
+ a,
+ { id: b } as IObject,
+ [
+ c,
+ ],
+ FetchAllowSoftFailMask.Strict,
+ ), 'validation should pass with or without www. subdomain');
+ });
+ });
+
+ test('cross origin lookup', () => {
+ assert.doesNotThrow(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/abc',
+ { id: 'https://bob.example.com/abc' } as IObject,
+ [
+ 'https://bob.example.com/abc',
+ ],
+ FetchAllowSoftFailMask.CrossOrigin | FetchAllowSoftFailMask.NonCanonicalId,
+ ), 'validation should pass if the response is otherwise consistent and cross-origin is allowed');
+ assert.throws(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/abc',
+ { id: 'https://bob.example.com/abc' } as IObject,
+ [
+ 'https://bob.example.com/abc',
+ ],
+ FetchAllowSoftFailMask.Strict,
+ ), 'validation should fail if the response is otherwise consistent and cross-origin is not allowed');
+ });
+
+ test('rejects non-canonical ID', () => {
+ assert.throws(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/@alice',
+ { id: 'https://alice.example.com/users/alice' } as IObject,
+ [
+ 'https://alice.example.com/users/alice'
+ ],
+ FetchAllowSoftFailMask.Strict,
+ ), 'throws if the response ID did not exactly match the expected ID');
+ assert.doesNotThrow(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/@alice',
+ { id: 'https://alice.example.com/users/alice' } as IObject,
+ [
+ 'https://alice.example.com/users/alice',
+ ],
+ FetchAllowSoftFailMask.NonCanonicalId,
+ ), 'does not throw if non-canonical ID is allowed');
+ });
+
+ test('origin relaxed alignment', () => {
+ assert.doesNotThrow(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/abc',
+ { id: 'https://ap.alice.example.com/abc' } as IObject,
+ [
+ 'https://ap.alice.example.com/abc',
+ ],
+ FetchAllowSoftFailMask.MisalignedOrigin | FetchAllowSoftFailMask.NonCanonicalId,
+ ), 'validation should pass if response is a subdomain of the expected origin');
+ assert.throws(() => assertActivityMatchesUrls(
+ 'https://alice.multi-tenant.example.com/abc',
+ { id: 'https://alice.multi-tenant.example.com/abc' } as IObject,
+ [
+ 'https://bob.multi-tenant.example.com/abc',
+ ],
+ FetchAllowSoftFailMask.MisalignedOrigin | FetchAllowSoftFailMask.NonCanonicalId,
+ ), 'validation should fail if response is a disjoint domain of the expected origin');
+ assert.throws(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/abc',
+ { id: 'https://ap.alice.example.com/abc' } as IObject,
+ [
+ 'https://ap.alice.example.com/abc',
+ ],
+ FetchAllowSoftFailMask.Strict,
+ ), 'throws if relaxed origin is forbidden');
+ });
+
+ test('resist HTTP downgrade', () => {
+ assert.throws(() => assertActivityMatchesUrls(
+ 'https://alice.example.com/abc',
+ { id: 'https://alice.example.com/abc' } as IObject,
+ [
+ 'http://alice.example.com/abc',
+ ],
+ FetchAllowSoftFailMask.Strict,
+ ), 'throws if HTTP downgrade is detected');
+ });
});
diff --git a/packages/frontend/src/scripts/lookup.ts b/packages/frontend/src/scripts/lookup.ts
index ddcbfe1a8d..8ee2a4b99c 100644
--- a/packages/frontend/src/scripts/lookup.ts
+++ b/packages/frontend/src/scripts/lookup.ts
@@ -54,10 +54,6 @@ export async function lookup(router?: Router) {
title = i18n.ts._remoteLookupErrors._responseInvalid.title;
text = i18n.ts._remoteLookupErrors._responseInvalid.description;
break;
- case 'a2c9c61a-cb72-43ab-a964-3ca5fddb410a':
- title = i18n.ts._remoteLookupErrors._responseInvalid.title;
- text = i18n.ts._remoteLookupErrors._responseInvalidIdHostNotMatch.description;
- break;
case 'dc94d745-1262-4e63-a17d-fecaa57efc82':
title = i18n.ts._remoteLookupErrors._noSuchObject.title;
text = i18n.ts._remoteLookupErrors._noSuchObject.description;