puniko/FoundKey
1
0
Fork 0
forked from FoundKeyGang/FoundKey
Code Issues Pull requests Projects Releases Packages Wiki Activity

server: refactor remote host check to validateActor

Browse source 
Instead of checking that an actor is not from the local host separately,
it seems like a good idea to do it in the central place that is supposed
to validate an actor.
This commit is contained in:
Johann150 2023-01-07 14:48:35 +01:00
parent 8f21275dd5
commit 8772181b6f
Signed by untrusted user: Johann150
GPG key ID: 9EE6577A2A06F8F1
1 changed files with 12 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
packages/backend/src/remote/activitypub/models/person.ts
View file

@ -1,4 +1,5 @@
import promiseLimit from 'promise-limit'; import promiseLimit from 'promise-limit';
import { Not, IsNull } from 'typeorm';
import config from '@/config/index.js'; import config from '@/config/index.js';
import { registerOrFetchInstanceDoc } from '@/services/register-or-fetch-instance-doc.js'; import { registerOrFetchInstanceDoc } from '@/services/register-or-fetch-instance-doc.js';
@ -54,6 +55,12 @@ function validateActor(x: IObject): IActor {
throw new Error('invalid Actor: wrong id'); throw new Error('invalid Actor: wrong id');
} }
// This check is security critical.
// Without this check, an entry could be inserted into UserPublickey for a local user.
if (extractDbHost(uri) === extractDbHost(config.url)) {
throw new StatusError('cannot resolve local user', 400, 'cannot resolve local user');
}
if (!(typeof x.inbox === 'string' && x.inbox.length > 0)) { if (!(typeof x.inbox === 'string' && x.inbox.length > 0)) {
throw new Error('invalid Actor: wrong inbox'); throw new Error('invalid Actor: wrong inbox');
} }
@ -83,9 +90,10 @@ function validateActor(x: IObject): IActor {
throw new Error('invalid Actor: publicKey.id is not a string'); throw new Error('invalid Actor: publicKey.id is not a string');
} }
const expectHost = extractDbHost(uri);
const publicKeyIdHost = extractDbHost(x.publicKey.id); const publicKeyIdHost = extractDbHost(x.publicKey.id);
if (publicKeyIdHost !== expectHost) { // This is a security critical check to not insert or change an entry of
// UserPublickey to point to a local key id.
if (extractDbHost(uri) !== extractDbHost(x.publicKey.id) {
throw new Error('invalid Actor: publicKey.id has different host'); throw new Error('invalid Actor: publicKey.id has different host');
} }
} }
@ -104,7 +112,7 @@ export async function fetchPerson(uri: string): Promise<CacheableUser | null> {
const cached = uriPersonCache.get(uri); const cached = uriPersonCache.get(uri);
if (cached) return cached; if (cached) return cached;
// URIがこのサーバーを指しているならデータベースからフェッチ // If the URI points to this server, fetch from database.
if (uri.startsWith(config.url + '/')) { if (uri.startsWith(config.url + '/')) {
const id = uri.split('/').pop(); const id = uri.split('/').pop();
const u = await Users.findOneBy({ id }); const u = await Users.findOneBy({ id });
@ -128,10 +136,6 @@ export async function fetchPerson(uri: string): Promise<CacheableUser | null> {
* Personを作成します。 * Personを作成します。
*/ */
export async function createPerson(value: string | IObject, resolver: Resolver): Promise<User> { export async function createPerson(value: string | IObject, resolver: Resolver): Promise<User> {
if (getApId(value).startsWith(config.url)) {
throw new StatusError('cannot resolve local user', 400, 'cannot resolve local user');
}
const object = await resolver.resolve(value) as any; const object = await resolver.resolve(value) as any;
const person = validateActor(object); const person = validateActor(object);
@ -275,13 +279,8 @@ export async function createPerson(value: string | IObject, resolver: Resolver):
export async function updatePerson(value: IObject | string, resolver: Resolver): Promise<void> { export async function updatePerson(value: IObject | string, resolver: Resolver): Promise<void> {
const uri = getApId(value); const uri = getApId(value);
// skip local URIs
if (uri.startsWith(config.url)) {
return;
}
// do we already know this user? // do we already know this user?
const exist = await Users.findOneBy({ uri }) as IRemoteUser; const exist = await Users.findOneBy({ uri, host: Not(IsNull()) }) as IRemoteUser;
if (exist == null) { if (exist == null) {
return; return;