Fix hashed ID server lookups with no Olm (#4333)

* Fix hashed ID server lookups with no Olm It used the hash function from Olm (presumably to work cross-platform) but subtle crypto is available on node nowadays so we can just use that. Refactor existing code that did this out to a common function, add tests. * Test the code when crypto is available * Test case of no crypto available * Move digest file to src to get it out of the way of the olm / e2e stuff * Fix import * Fix error string & doc * subtle crypto, not webcrypto * Extract the base64 part * Fix test * Move test file too * Add more doc * Fix imports
2026-01-03 23:22:30 +03:00 · 2024-08-01 11:55:23 +01:00
parent 687d08dc9d
commit 89a9a7fa38
6 changed files with 150 additions and 27 deletions
--- a/spec/unit/digest.spec.ts
+++ b/spec/unit/digest.spec.ts
@@ -0,0 +1,40 @@
+/*
+Copyright 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { encodeUnpaddedBase64Url } from "../../src";
+import { sha256 } from "../../src/digest";
+
+describe("sha256", () => {
+    it("should hash a string", async () => {
+        const hash = await sha256("test");
+        expect(encodeUnpaddedBase64Url(hash)).toBe("n4bQgYhMfWWaL-qgxVrQFaO_TxsrC4Is0V1sFbDwCgg");
+    });
+
+    it("should hash a string with emoji", async () => {
+        const hash = await sha256("test 🍱");
+        expect(encodeUnpaddedBase64Url(hash)).toBe("X2aDNrrwfq3nCTOl90R9qg9ynxhHnSzsMqtrdYX-SGw");
+    });
+
+    it("throws if webcrypto is not available", async () => {
+        const oldCrypto = global.crypto;
+        try {
+            global.crypto = {} as any;
+            await expect(sha256("test")).rejects.toThrow();
+        } finally {
+            global.crypto = oldCrypto;
+        }
+    });
+});
--- a/spec/unit/matrix-client.spec.ts
+++ b/spec/unit/matrix-client.spec.ts
@@ -299,7 +299,9 @@ describe("MatrixClient", function () {
            ...(opts || {}),
        });
        // FIXME: We shouldn't be yanking http like this.
-        client.http = (["authedRequest", "getContentUri", "request", "uploadContent"] as const).reduce((r, k) => {
+        client.http = (
+            ["authedRequest", "getContentUri", "request", "uploadContent", "idServerRequest"] as const
+        ).reduce((r, k) => {
            r[k] = jest.fn();
            return r;
        }, {} as MatrixHttpApi<any>);
@@ -3358,4 +3360,45 @@ describe("MatrixClient", function () {
            expect(httpLookups.length).toEqual(0);
        });
    });
+
+    describe("identityHashedLookup", () => {
+        it("should return hashed lookup results", async () => {
+            const ID_ACCESS_TOKEN = "hello_id_server_please_let_me_make_a_request";
+
+            client.http.idServerRequest = jest.fn().mockImplementation((method, path, params) => {
+                if (method === "GET" && path === "/hash_details") {
+                    return { algorithms: ["sha256"], lookup_pepper: "carrot" };
+                } else if (method === "POST" && path === "/lookup") {
+                    return {
+                        mappings: {
+                            "WHA-MgrrsZACDI9F8OaVagpiyiV2sjZylGHJteT4OMU": "@bob:homeserver.dummy",
+                        },
+                    };
+                }
+
+                throw new Error("Test impl doesn't know about this request");
+            });
+
+            const lookupResult = await client.identityHashedLookup([["bob@email.dummy", "email"]], ID_ACCESS_TOKEN);
+
+            expect(client.http.idServerRequest).toHaveBeenCalledWith(
+                "GET",
+                "/hash_details",
+                undefined,
+                "/_matrix/identity/v2",
+                ID_ACCESS_TOKEN,
+            );
+
+            expect(client.http.idServerRequest).toHaveBeenCalledWith(
+                "POST",
+                "/lookup",
+                { pepper: "carrot", algorithm: "sha256", addresses: ["WHA-MgrrsZACDI9F8OaVagpiyiV2sjZylGHJteT4OMU"] },
+                "/_matrix/identity/v2",
+                ID_ACCESS_TOKEN,
+            );
+
+            expect(lookupResult).toHaveLength(1);
+            expect(lookupResult[0]).toEqual({ address: "bob@email.dummy", mxid: "@bob:homeserver.dummy" });
+        });
+    });
 });
--- a/spec/unit/oidc/authorize.spec.ts
+++ b/spec/unit/oidc/authorize.spec.ts
@@ -89,11 +89,8 @@ describe("oidc authorization", () => {

    describe("generateAuthorizationUrl()", () => {
        it("should generate url with correct parameters", async () => {
-            // test the no crypto case here
-            // @ts-ignore mocking
-            globalThis.crypto.subtle = undefined;
-
            const authorizationParams = generateAuthorizationParams({ redirectUri: baseUrl });
+            authorizationParams.codeVerifier = "test-code-verifier";
            const authUrl = new URL(
                await generateAuthorizationUrl(authorizationEndpoint, clientId, authorizationParams),
            );
@@ -105,6 +102,18 @@ describe("oidc authorization", () => {
            expect(authUrl.searchParams.get("scope")).toEqual(authorizationParams.scope);
            expect(authUrl.searchParams.get("state")).toEqual(authorizationParams.state);
            expect(authUrl.searchParams.get("nonce")).toEqual(authorizationParams.nonce);
+            expect(authUrl.searchParams.get("code_challenge")).toEqual("0FLIKahrX7kqxncwhV5WD82lu_wi5GA8FsRSLubaOpU");
+        });
+
+        it("should log a warning if crypto is not available", async () => {
+            // test the no crypto case here
+            // @ts-ignore mocking
+            globalThis.crypto.subtle = undefined;
+
+            const authorizationParams = generateAuthorizationParams({ redirectUri: baseUrl });
+            const authUrl = new URL(
+                await generateAuthorizationUrl(authorizationEndpoint, clientId, authorizationParams),
+            );

            // crypto not available, plain text code_challenge is used
            expect(authUrl.searchParams.get("code_challenge")).toEqual(authorizationParams.codeVerifier);
--- a/src/client.ts
+++ b/src/client.ts
@@ -47,7 +47,7 @@ import { Direction, EventTimeline } from "./models/event-timeline";
 import { IActionsObject, PushProcessor } from "./pushprocessor";
 import { AutoDiscovery, AutoDiscoveryAction } from "./autodiscovery";
 import * as olmlib from "./crypto/olmlib";
-import { decodeBase64, encodeBase64 } from "./base64";
+import { decodeBase64, encodeBase64, encodeUnpaddedBase64Url } from "./base64";
 import { IExportedDevice as IExportedOlmDevice } from "./crypto/OlmDevice";
 import { IOlmDevice } from "./crypto/algorithms/megolm";
 import { TypedReEmitter } from "./ReEmitter";
@@ -231,6 +231,7 @@ import { KnownMembership, Membership } from "./@types/membership";
 import { RoomMessageEventContent, StickerEventContent } from "./@types/events";
 import { ImageInfo } from "./@types/media";
 import { Capabilities, ServerCapabilities } from "./serverCapabilities";
+import { sha256 } from "./digest";

 export type Store = IStore;

@@ -9484,20 +9485,19 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa

        // When picking an algorithm, we pick the hashed over no hashes
        if (hashes["algorithms"].includes("sha256")) {
-            // Abuse the olm hashing
-            const olmutil = new global.Olm.Utility();
-            params["addresses"] = addressPairs.map((p) => {
-                const addr = p[0].toLowerCase(); // lowercase to get consistent hashes
-                const med = p[1].toLowerCase();
-                const hashed = olmutil
-                    .sha256(`${addr} ${med} ${params["pepper"]}`)
-                    .replace(/\+/g, "-")
-                    .replace(/\//g, "_"); // URL-safe base64
-                // Map the hash to a known (case-sensitive) address. We use the case
-                // sensitive version because the caller might be expecting that.
-                localMapping[hashed] = p[0];
-                return hashed;
-            });
+            params["addresses"] = await Promise.all(
+                addressPairs.map(async (p) => {
+                    const addr = p[0].toLowerCase(); // lowercase to get consistent hashes
+                    const med = p[1].toLowerCase();
+                    const hashBuffer = await sha256(`${addr} ${med} ${params["pepper"]}`);
+                    const hashed = encodeUnpaddedBase64Url(hashBuffer);
+
+                    // Map the hash to a known (case-sensitive) address. We use the case
+                    // sensitive version because the caller might be expecting that.
+                    localMapping[hashed] = p[0];
+                    return hashed;
+                }),
+            );
            params["algorithm"] = "sha256";
        } else if (hashes["algorithms"].includes("none")) {
            params["addresses"] = addressPairs.map((p) => {
--- a/src/digest.ts
+++ b/src/digest.ts
@@ -0,0 +1,34 @@
+/*
+Copyright 2024 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+/**
+ * Computes a SHA-256 hash of a string (after utf-8 encoding) and returns it as an ArrayBuffer.
+ *
+ * @param plaintext The string to hash
+ * @returns An ArrayBuffer containing the SHA-256 hash of the input string
+ * @throws If the subtle crypto API is not available, for example if the code is running
+ *         in a web page with an insecure context (eg. served over plain HTTP).
+ */
+export async function sha256(plaintext: string): Promise<ArrayBuffer> {
+    if (!globalThis.crypto.subtle) {
+        throw new Error("Crypto.subtle is not available: insecure context?");
+    }
+    const utf8 = new TextEncoder().encode(plaintext);
+
+    const digest = await globalThis.crypto.subtle.digest("SHA-256", utf8);
+
+    return digest;
+}
--- a/src/oidc/authorize.ts
+++ b/src/oidc/authorize.ts
@@ -27,6 +27,8 @@ import {
    validateIdToken,
    validateStoredUserState,
 } from "./validate";
+import { sha256 } from "../digest";
+import { encodeUnpaddedBase64Url } from "../base64";

 // reexport for backwards compatibility
 export type { BearerTokenResponse };
@@ -61,14 +63,9 @@ const generateCodeChallenge = async (codeVerifier: string): Promise<string> => {
        logger.warn("A secure context is required to generate code challenge. Using plain text code challenge");
        return codeVerifier;
    }
-    const utf8 = new TextEncoder().encode(codeVerifier);

-    const digest = await globalThis.crypto.subtle.digest("SHA-256", utf8);
-
-    return btoa(String.fromCharCode(...new Uint8Array(digest)))
-        .replace(/=/g, "")
-        .replace(/\+/g, "-")
-        .replace(/\//g, "_");
+    const hashBuffer = await sha256(codeVerifier);
+    return encodeUnpaddedBase64Url(hashBuffer);
 };

 /**