Skip to content

Commit 89a9a7f

Browse files
dbkrdbkr
authored
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
1 parent 687d08d commit 89a9a7f

File tree

6 files changed

+150
-27
lines changed

6 files changed

+150
-27
lines changed

‎spec/unit/digest.spec.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
Copyright 2024 The Matrix.org Foundation C.I.C.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
import { encodeUnpaddedBase64Url } from "../../src";
18+
import { sha256 } from "../../src/digest";
19+
20+
describe("sha256", () => {
21+
it("should hash a string", async () => {
22+
const hash = await sha256("test");
23+
expect(encodeUnpaddedBase64Url(hash)).toBe("n4bQgYhMfWWaL-qgxVrQFaO_TxsrC4Is0V1sFbDwCgg");
24+
});
25+
26+
it("should hash a string with emoji", async () => {
27+
const hash = await sha256("test 🍱");
28+
expect(encodeUnpaddedBase64Url(hash)).toBe("X2aDNrrwfq3nCTOl90R9qg9ynxhHnSzsMqtrdYX-SGw");
29+
});
30+
31+
it("throws if webcrypto is not available", async () => {
32+
const oldCrypto = global.crypto;
33+
try {
34+
global.crypto = {} as any;
35+
await expect(sha256("test")).rejects.toThrow();
36+
} finally {
37+
global.crypto = oldCrypto;
38+
}
39+
});
40+
});

‎spec/unit/matrix-client.spec.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,9 @@ describe("MatrixClient", function () {
299299
...(opts || {}),
300300
});
301301
// FIXME: We shouldn't be yanking http like this.
302-
client.http = (["authedRequest", "getContentUri", "request", "uploadContent"] as const).reduce((r, k) => {
302+
client.http = (
303+
["authedRequest", "getContentUri", "request", "uploadContent", "idServerRequest"] as const
304+
).reduce((r, k) => {
303305
r[k] = jest.fn();
304306
return r;
305307
}, {} as MatrixHttpApi<any>);
@@ -3358,4 +3360,45 @@ describe("MatrixClient", function () {
33583360
expect(httpLookups.length).toEqual(0);
33593361
});
33603362
});
3363+
3364+
describe("identityHashedLookup", () => {
3365+
it("should return hashed lookup results", async () => {
3366+
const ID_ACCESS_TOKEN = "hello_id_server_please_let_me_make_a_request";
3367+
3368+
client.http.idServerRequest = jest.fn().mockImplementation((method, path, params) => {
3369+
if (method === "GET" && path === "/hash_details") {
3370+
return { algorithms: ["sha256"], lookup_pepper: "carrot" };
3371+
} else if (method === "POST" && path === "/lookup") {
3372+
return {
3373+
mappings: {
3374+
"WHA-MgrrsZACDI9F8OaVagpiyiV2sjZylGHJteT4OMU": "@bob:homeserver.dummy",
3375+
},
3376+
};
3377+
}
3378+
3379+
throw new Error("Test impl doesn't know about this request");
3380+
});
3381+
3382+
const lookupResult = await client.identityHashedLookup([["[email protected]", "email"]], ID_ACCESS_TOKEN);
3383+
3384+
expect(client.http.idServerRequest).toHaveBeenCalledWith(
3385+
"GET",
3386+
"/hash_details",
3387+
undefined,
3388+
"/_matrix/identity/v2",
3389+
ID_ACCESS_TOKEN,
3390+
);
3391+
3392+
expect(client.http.idServerRequest).toHaveBeenCalledWith(
3393+
"POST",
3394+
"/lookup",
3395+
{ pepper: "carrot", algorithm: "sha256", addresses: ["WHA-MgrrsZACDI9F8OaVagpiyiV2sjZylGHJteT4OMU"] },
3396+
"/_matrix/identity/v2",
3397+
ID_ACCESS_TOKEN,
3398+
);
3399+
3400+
expect(lookupResult).toHaveLength(1);
3401+
expect(lookupResult[0]).toEqual({ address: "[email protected]", mxid: "@bob:homeserver.dummy" });
3402+
});
3403+
});
33613404
});

‎spec/unit/oidc/authorize.spec.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,8 @@ describe("oidc authorization", () => {
8989

9090
describe("generateAuthorizationUrl()", () => {
9191
it("should generate url with correct parameters", async () => {
92-
// test the no crypto case here
93-
// @ts-ignore mocking
94-
globalThis.crypto.subtle = undefined;
95-
9692
const authorizationParams = generateAuthorizationParams({ redirectUri: baseUrl });
93+
authorizationParams.codeVerifier = "test-code-verifier";
9794
const authUrl = new URL(
9895
await generateAuthorizationUrl(authorizationEndpoint, clientId, authorizationParams),
9996
);
@@ -105,6 +102,18 @@ describe("oidc authorization", () => {
105102
expect(authUrl.searchParams.get("scope")).toEqual(authorizationParams.scope);
106103
expect(authUrl.searchParams.get("state")).toEqual(authorizationParams.state);
107104
expect(authUrl.searchParams.get("nonce")).toEqual(authorizationParams.nonce);
105+
expect(authUrl.searchParams.get("code_challenge")).toEqual("0FLIKahrX7kqxncwhV5WD82lu_wi5GA8FsRSLubaOpU");
106+
});
107+
108+
it("should log a warning if crypto is not available", async () => {
109+
// test the no crypto case here
110+
// @ts-ignore mocking
111+
globalThis.crypto.subtle = undefined;
112+
113+
const authorizationParams = generateAuthorizationParams({ redirectUri: baseUrl });
114+
const authUrl = new URL(
115+
await generateAuthorizationUrl(authorizationEndpoint, clientId, authorizationParams),
116+
);
108117

109118
// crypto not available, plain text code_challenge is used
110119
expect(authUrl.searchParams.get("code_challenge")).toEqual(authorizationParams.codeVerifier);

‎src/client.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import { Direction, EventTimeline } from "./models/event-timeline";
4747
import { IActionsObject, PushProcessor } from "./pushprocessor";
4848
import { AutoDiscovery, AutoDiscoveryAction } from "./autodiscovery";
4949
import * as olmlib from "./crypto/olmlib";
50-
import { decodeBase64, encodeBase64 } from "./base64";
50+
import { decodeBase64, encodeBase64, encodeUnpaddedBase64Url } from "./base64";
5151
import { IExportedDevice as IExportedOlmDevice } from "./crypto/OlmDevice";
5252
import { IOlmDevice } from "./crypto/algorithms/megolm";
5353
import { TypedReEmitter } from "./ReEmitter";
@@ -231,6 +231,7 @@ import { KnownMembership, Membership } from "./@types/membership";
231231
import { RoomMessageEventContent, StickerEventContent } from "./@types/events";
232232
import { ImageInfo } from "./@types/media";
233233
import { Capabilities, ServerCapabilities } from "./serverCapabilities";
234+
import { sha256 } from "./digest";
234235

235236
export type Store = IStore;
236237

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

94859486
// When picking an algorithm, we pick the hashed over no hashes
94869487
if (hashes["algorithms"].includes("sha256")) {
9487-
// Abuse the olm hashing
9488-
const olmutil = new global.Olm.Utility();
9489-
params["addresses"] = addressPairs.map((p) => {
9490-
const addr = p[0].toLowerCase(); // lowercase to get consistent hashes
9491-
const med = p[1].toLowerCase();
9492-
const hashed = olmutil
9493-
.sha256(`${addr} ${med} ${params["pepper"]}`)
9494-
.replace(/\+/g, "-")
9495-
.replace(/\//g, "_"); // URL-safe base64
9496-
// Map the hash to a known (case-sensitive) address. We use the case
9497-
// sensitive version because the caller might be expecting that.
9498-
localMapping[hashed] = p[0];
9499-
return hashed;
9500-
});
9488+
params["addresses"] = await Promise.all(
9489+
addressPairs.map(async (p) => {
9490+
const addr = p[0].toLowerCase(); // lowercase to get consistent hashes
9491+
const med = p[1].toLowerCase();
9492+
const hashBuffer = await sha256(`${addr} ${med} ${params["pepper"]}`);
9493+
const hashed = encodeUnpaddedBase64Url(hashBuffer);
9494+
9495+
// Map the hash to a known (case-sensitive) address. We use the case
9496+
// sensitive version because the caller might be expecting that.
9497+
localMapping[hashed] = p[0];
9498+
return hashed;
9499+
}),
9500+
);
95019501
params["algorithm"] = "sha256";
95029502
} else if (hashes["algorithms"].includes("none")) {
95039503
params["addresses"] = addressPairs.map((p) => {

‎src/digest.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
Copyright 2024 The Matrix.org Foundation C.I.C.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
/**
18+
* Computes a SHA-256 hash of a string (after utf-8 encoding) and returns it as an ArrayBuffer.
19+
*
20+
* @param plaintext The string to hash
21+
* @returns An ArrayBuffer containing the SHA-256 hash of the input string
22+
* @throws If the subtle crypto API is not available, for example if the code is running
23+
* in a web page with an insecure context (eg. served over plain HTTP).
24+
*/
25+
export async function sha256(plaintext: string): Promise<ArrayBuffer> {
26+
if (!globalThis.crypto.subtle) {
27+
throw new Error("Crypto.subtle is not available: insecure context?");
28+
}
29+
const utf8 = new TextEncoder().encode(plaintext);
30+
31+
const digest = await globalThis.crypto.subtle.digest("SHA-256", utf8);
32+
33+
return digest;
34+
}

‎src/oidc/authorize.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import {
2727
validateIdToken,
2828
validateStoredUserState,
2929
} from "./validate";
30+
import { sha256 } from "../digest";
31+
import { encodeUnpaddedBase64Url } from "../base64";
3032

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

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

7471
/**

0 commit comments

Comments
 (0)