Integrate MediaProxy to bridge authenticated Matrix media (MSC3916)

changelog.d/1805.feature

@@ -0,0 +1 @@
+Use MediaProxy to serve authenticated Matrix media.

config.sample.yaml

@@ -2,15 +2,8 @@
 # are *REQUIRED*.
 # Unless otherwise specified, these keys CANNOT be hot-reloaded.
 homeserver:
-  # The URL to the home server for client-server API calls, also used to form the
-  # media URLs as displayed in bridged IRC channels:
+  # The URL to the home server for client-server API calls
   url: "http://localhost:8008"
-  #
-  # The URL of the homeserver hosting media files. This is only used to transform
-  # mxc URIs to http URIs when bridging m.room.[file|image] events. Optional. By
-  # default, this is the homeserver URL, specified above.
-  # This key CAN be hot-reloaded.
-  # media_url: "http://media.repo:8008"
 
   # Drop Matrix messages which are older than this number of seconds, according to
   # the event's origin_server_ts.
@@ -607,6 +600,18 @@ ircService:
     # Ignore users mentioned in a io.element.functional_members state event when checking admin room membership
     ignoreFunctionalMembersInAdminRooms: false
 
+  # Config for the media proxy, required to serve publically accessible URLs to authenticated Matrix media
+  mediaProxy:
+    # To generate a .jwk file:
+    # $ node src/generate-signing-key.js > signingkey.jwk
+    signingKeyPath: "signingkey.jwk"
+    # How long should the generated URLs be valid for
+    ttlSeconds: 3600
+    # The port for the media proxy to listen on
+    bindPort: 11111
+    # The publically accessible URL to the media proxy
+    publicUrl: "https://irc.bridge/media"
+
   # Maximum number of montly active users, beyond which the bridge gets blocked (both ways)
   # RMAUlimit: 100
 

config.schema.yml

@@ -44,8 +44,6 @@ properties:
         properties:
             url:
                 type: "string"
-            media_url:
-                type: "string"
             domain:
                 type: "string"
             dropMatrixMessagesAfterSecs:
@@ -173,6 +171,18 @@ properties:
                         type: "integer"
                     ignoreFunctionalMembersInAdminRooms:
                         type: "boolean"
+            mediaProxy:
+                type: "object"
+                properties:
+                    signingKeyPath:
+                        type: "string"
+                    ttlSeconds:
+                        type: "integer"
+                    bindPort:
+                        type: "integer"
+                    publicUrl:
+                        type: "string"
+                required: ["signingKeyPath", "ttlSeconds", "bindPort", "publicUrl"]
             ircHandler:
                 type: "object"
                 properties:
@@ -485,4 +495,4 @@ properties:
                                 kickReason:
                                     type: "string"
                             required: ["regex"]
-        required: ["servers"]
+        required: ["servers", "mediaProxy"]

package.json

@@ -41,7 +41,7 @@
     "he": "^1.2.0",
     "ioredis": "^5.3.1",
     "logform": "^2.4.2",
-    "matrix-appservice-bridge": "^10.1.0",
+    "matrix-appservice-bridge": "^10.2.0",
     "matrix-bot-sdk": "npm:@vector-im/matrix-bot-sdk@^0.7.0-element.1",
     "matrix-org-irc": "^3.0.0",
     "nopt": "^7.2.0",

spec/integ/matrix-to-irc.spec.js

@@ -632,10 +632,10 @@ describe("Matrix-to-IRC message bridging", function() {
             expect(client.addr).toEqual(roomMapping.server);
             expect(channel).toEqual(roomMapping.channel);
             // don't be too brittle when checking this, but I expect to see the
-            // start of the first line and the mxc fragment
+            // start of the first line and the media proxy url
             expect(text.indexOf(tBody[0])).toEqual(0);
             expect(text.indexOf(tBody[1])).not.toEqual(0);
-            expect(text.indexOf('deadbeefcafe')).not.toEqual(-1);
+            expect(text.includes(config.ircService.mediaProxy.publicUrl)).toEqual(true);
             done();
         });
 
@@ -667,9 +667,9 @@ describe("Matrix-to-IRC message bridging", function() {
                 expect(client.addr).toEqual(roomMapping.server);
                 expect(channel).toEqual(roomMapping.channel);
                 // don't be too brittle when checking this, but I expect to see the
-                // code type and the mxc fragment.
+                // code type and the media proxy url
                 expect(text.indexOf('javascript')).not.toEqual(-1);
-                expect(text.indexOf('deadbeefcafe')).not.toEqual(-1);
+                expect(text.includes(config.ircService.mediaProxy.publicUrl)).toEqual(true);
                 done();
             });
 
@@ -695,10 +695,10 @@ describe("Matrix-to-IRC message bridging", function() {
             expect(client.addr).toEqual(roomMapping.server);
             expect(channel).toEqual(roomMapping.channel);
             // don't be too brittle when checking this, but I expect to see the
-            // start of the first line and the mxc fragment
+            // start of the first line and the media proxy URL
             expect(text.indexOf(tBody[0])).toEqual(0);
             expect(text.indexOf(tBody[1])).not.toEqual(0);
-            expect(text.indexOf('deadbeefcafe')).not.toEqual(-1);
+            expect(text.includes(config.ircService.mediaProxy.publicUrl)).toEqual(true);
             done();
         });
 
@@ -716,17 +716,12 @@ describe("Matrix-to-IRC message bridging", function() {
     it("should bridge matrix images as IRC action with a URL", function(done) {
         const tBody = "the_image.jpg";
         const tMxcSegment = "/somecontentid";
-        const tHsUrl = "https://some.home.server.goeshere/";
 
         env.ircMock._whenClient(roomMapping.server, testUser.nick, "action", (client, channel, text) => {
             expect(client.nick).toEqual(testUser.nick);
             expect(client.addr).toEqual(roomMapping.server);
             expect(channel).toEqual(roomMapping.channel);
-            // don't be too brittle when checking this, but I expect to see the
-            // filename (body) and the http url.
-            expect(text.indexOf(tBody)).not.toEqual(-1);
-            expect(text.indexOf(tHsUrl)).not.toEqual(-1);
-            expect(text.indexOf(tMxcSegment)).not.toEqual(-1);
+            expect(text.includes(config.ircService.mediaProxy.publicUrl)).toEqual(true);
             done();
         });
 
@@ -745,17 +740,12 @@ describe("Matrix-to-IRC message bridging", function() {
     it("should bridge matrix files as IRC action with a URL", function(done) {
         const tBody = "a_file.apk";
         const tMxcSegment = "/somecontentid";
-        const tHsUrl = "https://some.home.server.goeshere/";
 
         env.ircMock._whenClient(roomMapping.server, testUser.nick, "action", (client, channel, text) => {
             expect(client.nick).toEqual(testUser.nick);
             expect(client.addr).toEqual(roomMapping.server);
             expect(channel).toEqual(roomMapping.channel);
-            // don't be too brittle when checking this, but I expect to see the
-            // filename (body) and the http url.
-            expect(text.indexOf(tBody)).not.toEqual(-1);
-            expect(text.indexOf(tHsUrl)).not.toEqual(-1);
-            expect(text.indexOf(tMxcSegment)).not.toEqual(-1);
+            expect(text.includes(config.ircService.mediaProxy.publicUrl)).toEqual(true);
             done();
         });
 
@@ -1087,21 +1077,13 @@ describe("Matrix-to-IRC message bridging with media URL and drop time", function
     it("should bridge matrix files as IRC action with a configured media URL", function(done) {
         let tBody = "a_file.apk";
         let tMxcSegment = "/somecontentid";
-        let tMediaUrl = mediaUrl;
-        let tHsUrl = "http://somedomain.com";
-        const sdk = env.clientMock._client(config._botUserId);
 
         env.ircMock._whenClient(roomMapping.server, testUser.nick, "action",
         function(client, channel, text) {
             expect(client.nick).toEqual(testUser.nick);
             expect(client.addr).toEqual(roomMapping.server);
             expect(channel).toEqual(roomMapping.channel);
-            // don't be too brittle when checking this, but I expect to see the
-            // filename (body) and the http url.
-            expect(text.indexOf(tBody)).not.toEqual(-1, "File name not present");
-            expect(text.indexOf(tHsUrl)).toEqual(-1, "HS URL present instead of media URL");
-            expect(text.indexOf(tMediaUrl)).not.toEqual(-1, "No media URL");
-            expect(text.indexOf(tMxcSegment)).not.toEqual(-1, "No Mxc segment");
+            expect(text.includes(config.ircService.mediaProxy.publicUrl)).toEqual(true);
             done();
         });
 

spec/support/signingkey.jwk

@@ -0,0 +1,10 @@
+{
+    "key_ops": [
+        "sign",
+        "verify"
+    ],
+    "ext": true,
+    "kty": "oct",
+    "k": "1bWvvQPL7rVtRYMmesyR5z5LDDMZCktGgdiDwTtuYZARd_OIDtxUpRZP8zh46uPy06CaPpJQrCl84BzSKpSzMSsbYcvYM1Zn25VsEzH_OYJP2B0if-mAblVDMNoXva2ZobKXMz0dXrxeE0I-fwrORBKOcuZzveqH4VYGaeFPDkk",
+    "alg": "HS512"
+}

spec/util/e2e-test.ts

@@ -242,6 +242,12 @@ export class IrcBridgeE2ETest {
                     ...MatrixHandlerDefaults,
                     shortReplyTresholdSeconds: opts.shortReplyTresholdSeconds ?? 0,
                 },
+                "mediaProxy": {
+                    "signingKeyPath": "./spec/support/signingkey.jwk",
+                    "ttlSeconds": 5,
+                    "bindPort": 0,
+                    "publicUrl": "https://irc.bridge/media"
+                },
                 servers: {
                     localhost: {
                         ...IrcServer.DEFAULT_CONFIG,

spec/util/test-config.json

@@ -46,6 +46,12 @@
         "matrixHandler": {
             "ignoreFunctionalMembersInAdminRooms": true
         },
+        "mediaProxy": {
+            "signingKeyPath": "./spec/support/signingkey.jwk",
+            "ttlSeconds": 5,
+            "bindPort": 0,
+            "publicUrl": "https://irc.bridge/media"
+        },
         "servers": {
             "irc.example": {
                 "port": 6667,

src/bridge/IrcBridge.ts

@@ -1,5 +1,6 @@
 import Bluebird from "bluebird";
 import extend from "extend";
+import * as fs from "fs";
 import * as promiseutil from "../promiseutil";
 import { IrcHandler, MatrixMembership } from "./IrcHandler";
 import { MatrixHandler, MatrixEventInvite, OnMemberEventData, MatrixEventKick } from "./MatrixHandler";
@@ -41,6 +42,7 @@ import {
     UserActivityTracker,
     UserActivityTrackerConfig,
     WeakStateEvent,
+    MediaProxy,
 } from "matrix-appservice-bridge";
 import { IrcAction } from "../models/IrcAction";
 import { DataStore } from "../datastore/DataStore";
@@ -56,6 +58,7 @@ import { TestingOptions } from "../config/TestOpts";
 import { MatrixBanSync } from "./MatrixBanSync";
 import { configure } from "../logging";
 import { IrcPoolClient } from "../pool-service/IrcPoolClient";
+import { webcrypto } from "node:crypto";
 
 const log = getLogger("IrcBridge");
 const DEFAULT_PORT = 8090;
@@ -88,6 +91,7 @@ export class IrcBridge {
     public activityTracker: ActivityTracker|null = null;
     public readonly roomConfigs: RoomConfig;
     public readonly matrixBanSyncer?: MatrixBanSync;
+    private _mediaProxy?: MediaProxy;
     private clientPool!: ClientPool; // This gets defined in the `run` function
     private ircServers: IrcServer[] = [];
     private memberListSyncers: {[domain: string]: MemberListSyncer} = {};
@@ -255,9 +259,10 @@ export class IrcBridge {
             log.info(`Adjusted dropMatrixMessagesAfterSecs to ${newConfig.homeserver.dropMatrixMessagesAfterSecs}`);
         }
 
-        if (oldConfig.homeserver.media_url !== newConfig.homeserver.media_url) {
-            oldConfig.homeserver.media_url = newConfig.homeserver.media_url;
-            log.info(`Adjusted media_url to ${newConfig.homeserver.media_url}`);
+        if (JSON.stringify(oldConfig.ircService.mediaProxy) !== JSON.stringify(newConfig.ircService.mediaProxy)) {
+            await this._mediaProxy?.close();
+            this._mediaProxy = await this.initialiseMediaProxy();
+            log.info(`Media proxy reinitialized`);
         }
 
         await this.setupStateSyncer(newConfig);
@@ -308,6 +313,25 @@ export class IrcBridge {
         await this.clientPool.checkForBannedConnectedUsers();
     }
 
+    private async initialiseMediaProxy(): Promise<MediaProxy> {
+        const config = this.config.ircService.mediaProxy;
+        const jwk = JSON.parse(fs.readFileSync(config.signingKeyPath, "utf8").toString());
+        const signingKey = await webcrypto.subtle.importKey('jwk', jwk, {
+            name: 'HMAC',
+            hash: 'SHA-512',
+        }, true, ['sign', 'verify']);
+        const publicUrl = new URL(config.publicUrl);
+
+        const mediaProxy = new MediaProxy({
+            publicUrl,
+            signingKey,
+            ttl: config.ttlSeconds * 1000
+        }, this.bridge.getIntent().matrixClient);
+        await mediaProxy.start(config.bindPort);
+
+        return mediaProxy;
+    }
+
     private initialiseMetrics(bindPort: number) {
         const zeroAge = new AgeCounters();
         const registry = new Registry();
@@ -523,6 +547,13 @@ export class IrcBridge {
         return `@${this.registration.getSenderLocalpart()}:${this.domain}`;
     }
 
+    public get mediaProxy(): MediaProxy {
+        if (!this._mediaProxy) {
+            throw new Error(`Bridge not yet initialized`);
+        }
+        return this._mediaProxy;
+    }
+
     public getStore() {
         return this.dataStore;
     }
@@ -654,6 +685,8 @@ export class IrcBridge {
 
         await this.dataStore.removeConfigMappings();
 
+        this._mediaProxy = await this.initialiseMediaProxy();
+
         if (this.activityTracker) {
             log.info("Restoring last active times from DB");
             const users = await this.dataStore.getLastSeenTimeForUsers();
@@ -958,6 +991,16 @@ export class IrcBridge {
     public async kill(reason?: string) {
         log.info("Killing bridge");
         this.bridgeState = "killed";
+        if (this._mediaProxy) {
+            log.info("Killing media proxy");
+            // We don't care about the outcome since we don't really care
+            // about the state of the bridge after a kill().
+            // Awaiting this tripped up the BOTS-70 unit test for reasons I couldn't figure out,
+            // and I spent a not-worth-it amount of time on it,
+            // so this is me giving up and techdebting it.
+            // Please let me know, future programmer, if you do fix this properly. -- tadzik
+            void this._mediaProxy.close().catch(err => log.warn(`Failed to close MediaProxy: ${err}`));
+        }
         log.info("Killing all clients");
         if (!this.config.connectionPool?.persistConnectionsOnShutdown) {
             this.clientPool.killAllClients(reason);