merge: Enforce HTTPS for all federation (!1042)

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

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>

packages/backend/src/core/HttpRequestService.ts

@@ -236,6 +236,8 @@ export class HttpRequestService {
 
 	@bindThis
 	public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise<IObjectWithId> {
+		this.apUtilityService.assertApUrl(url);
+
 		const res = await this.send(url, {
 			method: 'GET',
 			headers: {

packages/backend/src/core/activitypub/ApRequestService.ts

@@ -155,6 +155,8 @@ export class ApRequestService {
 
 	@bindThis
 	public async signedPost(user: { id: MiUser['id'] }, url: string, object: unknown, digest?: string): Promise<void> {
+		this.apUtilityService.assertApUrl(url);
+
 		const body = typeof object === 'string' ? object : JSON.stringify(object);
 
 		const keypair = await this.userKeypairService.getUserKeypair(user.id);
@@ -186,6 +188,8 @@ export class ApRequestService {
 	 */
 	@bindThis
 	public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<IObjectWithId> {
+		this.apUtilityService.assertApUrl(url);
+
 		const _followAlternate = followAlternate ?? true;
 		const keypair = await this.userKeypairService.getUserKeypair(user.id);
 

packages/backend/src/core/activitypub/ApUtilityService.ts

@@ -77,16 +77,48 @@ export class ApUtilityService {
 		return acceptableUrls[0]?.url ?? null;
 	}
 
+	/**
+	 * Verifies that a provided URL is in a format acceptable for federation.
+	 * @throws {IdentifiableError} If URL cannot be parsed
+	 * @throws {IdentifiableError} If URL contains a fragment
+	 * @throws {IdentifiableError} If URL is not HTTPS
+	 */
+	public assertApUrl(url: string | URL): void {
+		// If string, parse and validate
+		if (typeof(url) === 'string') {
+			try {
+				url = new URL(url);
+			} catch {
+				throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: not a valid URL`);
+			}
+		}
+
+		// Hash component breaks federation
+		if (url.hash) {
+			throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: contains a fragment (#)`);
+		}
+
+		// Must be HTTPS
+		if (!this.checkHttps(url)) {
+			throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: unsupported protocol ${url.protocol}`);
+		}
+	}
+
 	/**
 	 * Checks if the URL contains HTTPS.
 	 * Additionally, allows HTTP in non-production environments.
 	 * Based on check-https.ts.
 	 */
-	private checkHttps(url: string): boolean {
+	private checkHttps(url: string | URL): boolean {
 		const isNonProd = this.envService.env.NODE_ENV !== 'production';
 
-		// noinspection HttpUrlsUsage
+		try {
-		return url.startsWith('https://') || (url.startsWith('http://') && isNonProd);
+			const proto = new URL(url).protocol;
+			return proto === 'https:' || (proto === 'http:' && isNonProd);
+		} catch {
+			// Invalid URLs don't "count" as HTTPS
+			return false;
+		}
 	}
 }
 

packages/backend/src/core/activitypub/models/ApNoteService.ts

@@ -95,6 +95,7 @@ export class ApNoteService {
 		actor?: MiRemoteUser,
 		user?: MiRemoteUser,
 	): Error | null {
+		this.apUtilityService.assertApUrl(uri);
 		const expectHost = this.utilityService.extractDbHost(uri);
 		const apType = getApType(object);
 

packages/backend/src/core/activitypub/models/ApPersonService.ts

@@ -153,6 +153,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown {
 	 */
 	@bindThis
 	private validateActor(x: IObject, uri: string): IActor {
+		this.apUtilityService.assertApUrl(uri);
 		const expectHost = this.utilityService.punyHostPSLDomain(uri);
 
 		if (!isActor(x)) {
@@ -167,6 +168,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown {
 			throw new UnrecoverableError(`invalid Actor ${uri} - wrong inbox type`);
 		}
 
+		this.apUtilityService.assertApUrl(x.inbox);
 		const inboxHost = this.utilityService.punyHostPSLDomain(x.inbox);
 		if (inboxHost !== expectHost) {
 			throw new UnrecoverableError(`invalid Actor ${uri} - wrong inbox ${inboxHost}`);
@@ -175,6 +177,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown {
 		const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined);
 		if (sharedInboxObject != null) {
 			const sharedInbox = getApId(sharedInboxObject);
+			this.apUtilityService.assertApUrl(sharedInbox);
 			if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHostPSLDomain(sharedInbox) === expectHost)) {
 				throw new UnrecoverableError(`invalid Actor ${uri} - wrong shared inbox ${sharedInbox}`);
 			}
@@ -185,6 +188,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown {
 			if (xCollection != null) {
 				const collectionUri = getApId(xCollection);
 				if (typeof collectionUri === 'string' && collectionUri.length > 0) {
+					this.apUtilityService.assertApUrl(collectionUri);
 					if (this.utilityService.punyHostPSLDomain(collectionUri) !== expectHost) {
 						throw new UnrecoverableError(`invalid Actor ${uri} - wrong ${collection} ${collectionUri}`);
 					}