diff options
| author | dakkar <dakkar@thenautilus.net> | 2025-04-21 14:44:19 +0100 |
|---|---|---|
| committer | dakkar <dakkar@thenautilus.net> | 2025-04-21 16:44:13 +0100 |
| commit | 58c0ac6c8986194d735071f17c008850c28b2064 (patch) | |
| tree | 094ca3984768d899fc30a907fe1e7cac0ac0a927 /packages/backend/src/server/ActivityPubServerService.ts | |
| parent | merge: Verify links in remote accounts. (!964) (diff) | |
| download | sharkey-58c0ac6c8986194d735071f17c008850c28b2064.tar.gz sharkey-58c0ac6c8986194d735071f17c008850c28b2064.tar.bz2 sharkey-58c0ac6c8986194d735071f17c008850c28b2064.zip | |
check signatures with and without query - fix #1036
@Oneric explained:
> Spec says query params must be included in the signature; Mastodon
> being Mastodon used to always exclude it though and for
> compatibility everyone followed this. At some point GtS decided to
> follow spec instead which caused interop issues, but succeeded in
> getting Mastodon (and others like *oma) to accept incoming requests
> with (and also still without) query params though outgoing requests
> remaing query-param-free. Some still only accept query-param-less
> requests though and GtS uses a retry mechanism to resend any request
> failing with 401 with an query-parama-less signature once. (Also
> see:
> https://docs.gotosocial.org/en/latest/federation/http_signatures/ )
>
> So for incoming requests both versions need to be checked. For
> outgoing requests, unless you want to jump through retry hoops like
> GtS, omitting query-params is the safer bet for now (presumably this
> will only change if Mastodon ever decides to send out requests
> signed with query params)
Diffstat (limited to 'packages/backend/src/server/ActivityPubServerService.ts')
| -rw-r--r-- | packages/backend/src/server/ActivityPubServerService.ts | 30 |
1 files changed, 27 insertions, 3 deletions
diff --git a/packages/backend/src/server/ActivityPubServerService.ts b/packages/backend/src/server/ActivityPubServerService.ts index ea534af458..f40fbe8245 100644 --- a/packages/backend/src/server/ActivityPubServerService.ts +++ b/packages/backend/src/server/ActivityPubServerService.ts @@ -177,7 +177,7 @@ export class ActivityPubServerService { this is also inspired by FireFish's `checkFetch` */ - let signature; + let signature: httpSignature.IParsedSignature; try { signature = httpSignature.parseRequest(request.raw, { @@ -230,14 +230,38 @@ export class ActivityPubServerService { return `${logPrefix} signer is suspended: refuse`; } - let httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem); + // some fedi implementations include the query (`?foo=bar`) in the + // signature, some don't, so we have to handle both cases + function verifyWithOrWithoutQuery() { + const httpSignatureValidated = httpSignature.verifySignature(signature, authUser!.key!.keyPem); + if (httpSignatureValidated) return true; + + const requestUrl = new URL(`http://whatever${request.raw.url}`); + if (! requestUrl.search) return false; + + // verification failed, the request URL contained a query, let's try without + const semiRawRequest = request.raw; + semiRawRequest.url = requestUrl.pathname; + + // no need for try/catch, if the original request parsed, this + // one will, too + const signatureWithoutQuery = httpSignature.parseRequest(semiRawRequest, { + headers: ['(request-target)', 'host', 'date'], + authorizationHeaderName: 'signature', + }); + + return httpSignature.verifySignature(signatureWithoutQuery, authUser!.key!.keyPem); + } + + console.warn('starting verification'); + let httpSignatureValidated = verifyWithOrWithoutQuery(); // maybe they changed their key? refetch it // TODO rate-limit this using lastFetchedAt if (!httpSignatureValidated) { authUser.key = await this.apDbResolverService.refetchPublicKeyForApId(authUser.user); if (authUser.key != null) { - httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem); + httpSignatureValidated = verifyWithOrWithoutQuery(); } } |