merge: check signatures with and without query - fix #1036 (!966)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/966

Closes #1036

Approved-by: Hazelnoot <acomputerdog@gmail.com>
Approved-by: Marie <github@yuugi.dev>

packages/backend/src/core/UtilityService.ts

@@ -106,13 +106,22 @@ export class UtilityService {
 
 	@bindThis
 	public toPuny(host: string): string {
-		return domainToASCII(host.toLowerCase());
+		// domainToASCII will return an empty string if we give it a
+		// string like `name:123`, but `host` may well be in that form
+		// (e.g. when testing locally, you'll get `localhost:3000`); split
+		// the port off, and add it back later
+		const hostParts = host.toLowerCase().match(/^(.+?)(:.+)?$/);
+		if (!hostParts) return '';
+		const hostname = hostParts[1];
+		const port = hostParts[2] ?? '';
+
+		return domainToASCII(hostname) + port;
 	}
 
 	@bindThis
 	public toPunyNullable(host: string | null | undefined): string | null {
 		if (host == null) return null;
-		return domainToASCII(host.toLowerCase());
+		return this.toPuny(host);
 	}
 
 	@bindThis

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,37 @@ 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);
+		}
+
+		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();
 			}
 		}
 

packages/backend/test/unit/UtilityService.ts

@@ -22,6 +22,12 @@ describe('UtilityService', () => {
 		test('japanese', () => {
 			assert.equal(utilityService.punyHost('http://www.新聞.com'), 'www.xn--efvv70d.com');
 		});
+		test('simple, with port', () => {
+			assert.equal(utilityService.punyHost('http://www.foo.com:3000'), 'www.foo.com:3000');
+		});
+		test('japanese, with port', () => {
+			assert.equal(utilityService.punyHost('http://www.新聞.com:3000'), 'www.xn--efvv70d.com:3000');
+		});
 	});
 
 	describe('punyHostPSLDomain', () => {
@@ -31,6 +37,12 @@ describe('UtilityService', () => {
 		test('japanese', () => {
 			assert.equal(utilityService.punyHostPSLDomain('http://www.新聞.com'), 'xn--efvv70d.com');
 		});
+		test('simple, with port', () => {
+			assert.equal(utilityService.punyHostPSLDomain('http://www.foo.com:3000'), 'foo.com:3000');
+		});
+		test('japanese, with port', () => {
+			assert.equal(utilityService.punyHostPSLDomain('http://www.新聞.com:3000'), 'xn--efvv70d.com:3000');
+		});
 		test('lower', () => {
 			assert.equal(utilityService.punyHostPSLDomain('http://foo.github.io'), 'foo.github.io');
 			assert.equal(utilityService.punyHostPSLDomain('http://foo.bar.github.io'), 'bar.github.io');
@@ -40,4 +52,13 @@ describe('UtilityService', () => {
 			assert.equal(utilityService.punyHostPSLDomain('http://foo.bar.masto.host'), 'bar.masto.host');
 		});
 	});
+
+	describe('toPuny', () => {
+		test('without port ', () => {
+			assert.equal(utilityService.toPuny('www.foo.com'), 'www.foo.com');
+		});
+		test('with port ', () => {
+			assert.equal(utilityService.toPuny('www.foo.com:3000'), 'www.foo.com:3000');
+		});
+	});
 });