From 712ba617de3a02cc1912f9731d0814c1dbcff08e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 5 Jul 2024 10:42:06 +0100 Subject: [PATCH] Remove `crypto` shims (#4292) * Inline subtlecrypto shim The presence of this thing just makes code more confusing. * Remove pre-node-20 webcrypto hack Until node 20.0, the webcrypto API lived at `crypto.webCrypto`. It's now available at the same place as in web -- `globalThis.crypto`. See: https://nodejs.org/docs/latest-v20.x/api/webcrypto.html#web-crypto-api * oidc auth test: Clean up mocking THe previous reset code wasn't really resetting the right thing. Let's just re-init `window.crypto` on each test. * Remove `crypto` shim This isn't very useful any more. --- spec/unit/oidc/authorize.spec.ts | 18 ++++--------- src/crypto/aes.ts | 22 ++++++++-------- src/crypto/backup.ts | 3 +-- src/crypto/crypto.ts | 24 ++---------------- src/crypto/key_passphrase.ts | 15 ++++++----- src/crypto/verification/QRCode.ts | 3 +-- src/oidc/authorize.ts | 13 +++++----- src/randomstring.ts | 3 +-- .../MSC3903ECDHv2RendezvousChannel.ts | 25 +++++++++---------- src/rust-crypto/DehydratedDeviceManager.ts | 3 +-- src/rust-crypto/rust-crypto.ts | 3 +-- 11 files changed, 51 insertions(+), 81 deletions(-) diff --git a/spec/unit/oidc/authorize.spec.ts b/spec/unit/oidc/authorize.spec.ts index e9cf7589a37..3fab8e5eef4 100644 --- a/spec/unit/oidc/authorize.spec.ts +++ b/spec/unit/oidc/authorize.spec.ts @@ -26,7 +26,6 @@ import { getRandomValues } from "node:crypto"; import { TextEncoder } from "node:util"; import { Method } from "../../../src"; -import * as crypto from "../../../src/crypto/crypto"; import { logger } from "../../../src/logger"; import { completeAuthorizationCodeGrant, @@ -39,11 +38,6 @@ import { makeDelegatedAuthConfig, mockOpenIdConfiguration } from "../../test-uti jest.mock("jwt-decode"); -const webCrypto = new Crypto(); - -// save for resetting mocks -const realSubtleCrypto = crypto.subtleCrypto; - describe("oidc authorization", () => { const delegatedAuthConfig = makeDelegatedAuthConfig(); const authorizationEndpoint = delegatedAuthConfig.authorizationEndpoint; @@ -62,7 +56,11 @@ describe("oidc authorization", () => { delegatedAuthConfig.metadata.issuer + ".well-known/openid-configuration", mockOpenIdConfiguration(), ); + global.TextEncoder = TextEncoder; + }); + beforeEach(() => { + const webCrypto = new Crypto(); Object.defineProperty(window, "crypto", { value: { getRandomValues, @@ -70,12 +68,6 @@ describe("oidc authorization", () => { subtle: webCrypto.subtle, }, }); - global.TextEncoder = TextEncoder; - }); - - afterEach(() => { - // @ts-ignore reset any ugly mocking we did - crypto.subtleCrypto = realSubtleCrypto; }); it("should generate authorization params", () => { @@ -99,7 +91,7 @@ describe("oidc authorization", () => { it("should generate url with correct parameters", async () => { // test the no crypto case here // @ts-ignore mocking - crypto.subtleCrypto = undefined; + globalThis.crypto.subtle = undefined; const authorizationParams = generateAuthorizationParams({ redirectUri: baseUrl }); const authUrl = new URL( diff --git a/src/crypto/aes.ts b/src/crypto/aes.ts index 12acb694009..e6b6e2a817e 100644 --- a/src/crypto/aes.ts +++ b/src/crypto/aes.ts @@ -15,7 +15,6 @@ limitations under the License. */ import { decodeBase64, encodeBase64 } from "../base64"; -import { subtleCrypto, crypto } from "./crypto"; // salt for HKDF, with 8 bytes of zeros const zeroSalt = new Uint8Array(8); @@ -49,7 +48,7 @@ export async function encryptAES( iv = decodeBase64(ivStr); } else { iv = new Uint8Array(16); - crypto.getRandomValues(iv); + globalThis.crypto.getRandomValues(iv); // clear bit 63 of the IV to stop us hitting the 64-bit counter boundary // (which would mean we wouldn't be able to decrypt on Android). The loss @@ -60,7 +59,7 @@ export async function encryptAES( const [aesKey, hmacKey] = await deriveKeys(key, name); const encodedData = new TextEncoder().encode(data); - const ciphertext = await subtleCrypto.encrypt( + const ciphertext = await globalThis.crypto.subtle.encrypt( { name: "AES-CTR", counter: iv, @@ -70,7 +69,7 @@ export async function encryptAES( encodedData, ); - const hmac = await subtleCrypto.sign({ name: "HMAC" }, hmacKey, ciphertext); + const hmac = await globalThis.crypto.subtle.sign({ name: "HMAC" }, hmacKey, ciphertext); return { iv: encodeBase64(iv), @@ -91,11 +90,11 @@ export async function decryptAES(data: IEncryptedPayload, key: Uint8Array, name: const ciphertext = decodeBase64(data.ciphertext); - if (!(await subtleCrypto.verify({ name: "HMAC" }, hmacKey, decodeBase64(data.mac), ciphertext))) { + if (!(await globalThis.crypto.subtle.verify({ name: "HMAC" }, hmacKey, decodeBase64(data.mac), ciphertext))) { throw new Error(`Error decrypting secret ${name}: bad MAC`); } - const plaintext = await subtleCrypto.decrypt( + const plaintext = await globalThis.crypto.subtle.decrypt( { name: "AES-CTR", counter: decodeBase64(data.iv), @@ -109,8 +108,8 @@ export async function decryptAES(data: IEncryptedPayload, key: Uint8Array, name: } async function deriveKeys(key: Uint8Array, name: string): Promise<[CryptoKey, CryptoKey]> { - const hkdfkey = await subtleCrypto.importKey("raw", key, { name: "HKDF" }, false, ["deriveBits"]); - const keybits = await subtleCrypto.deriveBits( + const hkdfkey = await globalThis.crypto.subtle.importKey("raw", key, { name: "HKDF" }, false, ["deriveBits"]); + const keybits = await globalThis.crypto.subtle.deriveBits( { name: "HKDF", salt: zeroSalt, @@ -126,9 +125,12 @@ async function deriveKeys(key: Uint8Array, name: string): Promise<[CryptoKey, Cr const aesKey = keybits.slice(0, 32); const hmacKey = keybits.slice(32); - const aesProm = subtleCrypto.importKey("raw", aesKey, { name: "AES-CTR" }, false, ["encrypt", "decrypt"]); + const aesProm = globalThis.crypto.subtle.importKey("raw", aesKey, { name: "AES-CTR" }, false, [ + "encrypt", + "decrypt", + ]); - const hmacProm = subtleCrypto.importKey( + const hmacProm = globalThis.crypto.subtle.importKey( "raw", hmacKey, { diff --git a/src/crypto/backup.ts b/src/crypto/backup.ts index d51fa641c26..cbbecbca38e 100644 --- a/src/crypto/backup.ts +++ b/src/crypto/backup.ts @@ -38,7 +38,6 @@ import { } from "./keybackup"; import { UnstableValue } from "../NamespacedValue"; import { CryptoEvent } from "./index"; -import { crypto } from "./crypto"; import { ClientPrefix, HTTPError, MatrixError, Method } from "../http-api"; import { BackupTrustInfo } from "../crypto-api/keybackup"; import { BackupDecryptor } from "../common-crypto/CryptoBackend"; @@ -764,7 +763,7 @@ export class Curve25519 implements BackupAlgorithm { function randomBytes(size: number): Uint8Array { const buf = new Uint8Array(size); - crypto.getRandomValues(buf); + globalThis.crypto.getRandomValues(buf); return buf; } diff --git a/src/crypto/crypto.ts b/src/crypto/crypto.ts index e9df7a84ff1..4aea59de52b 100644 --- a/src/crypto/crypto.ts +++ b/src/crypto/crypto.ts @@ -14,25 +14,5 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { logger } from "../logger"; - -export let crypto = globalThis.crypto; -export let subtleCrypto = crypto?.subtle ?? crypto?.webkitSubtle; // TODO: Stop using webkitSubtle fallback - -/* eslint-disable @typescript-eslint/no-var-requires */ -if (!crypto) { - try { - crypto = require("crypto").webcrypto; - } catch (e) { - logger.error("Failed to load webcrypto", e); - } -} -if (!subtleCrypto) { - subtleCrypto = crypto?.subtle; -} -/* eslint-enable @typescript-eslint/no-var-requires */ - -export function setCrypto(_crypto: Crypto): void { - crypto = _crypto; - subtleCrypto = _crypto.subtle ?? _crypto.webkitSubtle; -} +/** @deprecated this is a no-op and should no longer be called. */ +export function setCrypto(_crypto: Crypto): void {} diff --git a/src/crypto/key_passphrase.ts b/src/crypto/key_passphrase.ts index bdd6b5263da..c56c3ce2e5c 100644 --- a/src/crypto/key_passphrase.ts +++ b/src/crypto/key_passphrase.ts @@ -15,7 +15,6 @@ limitations under the License. */ import { randomString } from "../randomstring"; -import { subtleCrypto } from "./crypto"; const DEFAULT_ITERATIONS = 500000; @@ -62,15 +61,19 @@ export async function deriveKey( iterations: number, numBits = DEFAULT_BITSIZE, ): Promise { - if (!subtleCrypto || !TextEncoder) { + if (!globalThis.crypto.subtle || !TextEncoder) { throw new Error("Password-based backup is not available on this platform"); } - const key = await subtleCrypto.importKey("raw", new TextEncoder().encode(password), { name: "PBKDF2" }, false, [ - "deriveBits", - ]); + const key = await globalThis.crypto.subtle.importKey( + "raw", + new TextEncoder().encode(password), + { name: "PBKDF2" }, + false, + ["deriveBits"], + ); - const keybits = await subtleCrypto.deriveBits( + const keybits = await globalThis.crypto.subtle.deriveBits( { name: "PBKDF2", salt: new TextEncoder().encode(salt), diff --git a/src/crypto/verification/QRCode.ts b/src/crypto/verification/QRCode.ts index 7ccd4986d53..cff87dd21ff 100644 --- a/src/crypto/verification/QRCode.ts +++ b/src/crypto/verification/QRCode.ts @@ -18,7 +18,6 @@ limitations under the License. * QR code key verification. */ -import { crypto } from "../crypto"; import { VerificationBase as Base } from "./Base"; import { newKeyMismatchError, newUserCancelledError } from "./Error"; import { decodeBase64, encodeUnpaddedBase64 } from "../../base64"; @@ -202,7 +201,7 @@ export class QRCodeData { private static generateSharedSecret(): string { const secretBytes = new Uint8Array(11); - crypto.getRandomValues(secretBytes); + globalThis.crypto.getRandomValues(secretBytes); return encodeUnpaddedBase64(secretBytes); } diff --git a/src/oidc/authorize.ts b/src/oidc/authorize.ts index 80a95f881bd..5f437c58b6a 100644 --- a/src/oidc/authorize.ts +++ b/src/oidc/authorize.ts @@ -16,17 +16,16 @@ limitations under the License. import { IdTokenClaims, Log, OidcClient, SigninResponse, SigninState, WebStorageStateStore } from "oidc-client-ts"; -import { subtleCrypto } from "../crypto/crypto"; import { logger } from "../logger"; import { randomString } from "../randomstring"; import { OidcError } from "./error"; import { - validateIdToken, - ValidatedIssuerMetadata, - validateStoredUserState, - UserState, BearerTokenResponse, + UserState, validateBearerTokenResponse, + ValidatedIssuerMetadata, + validateIdToken, + validateStoredUserState, } from "./validate"; // reexport for backwards compatibility @@ -57,14 +56,14 @@ export const generateScope = (deviceId?: string): string => { // https://www.rfc-editor.org/rfc/rfc7636 const generateCodeChallenge = async (codeVerifier: string): Promise => { - if (!subtleCrypto) { + if (!globalThis.crypto.subtle) { // @TODO(kerrya) should this be allowed? configurable? 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 subtleCrypto.digest("SHA-256", utf8); + const digest = await globalThis.crypto.subtle.digest("SHA-256", utf8); return btoa(String.fromCharCode(...new Uint8Array(digest))) .replace(/=/g, "") diff --git a/src/randomstring.ts b/src/randomstring.ts index 36a6e748284..d095a2b9ef1 100644 --- a/src/randomstring.ts +++ b/src/randomstring.ts @@ -16,7 +16,6 @@ limitations under the License. */ import { encodeUnpaddedBase64Url } from "./base64"; -import { crypto } from "./crypto/crypto"; const LOWERCASE = "abcdefghijklmnopqrstuvwxyz"; const UPPERCASE = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; @@ -24,7 +23,7 @@ const DIGITS = "0123456789"; export function secureRandomBase64Url(len: number): string { const key = new Uint8Array(len); - crypto.getRandomValues(key); + globalThis.crypto.getRandomValues(key); return encodeUnpaddedBase64Url(key); } diff --git a/src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts b/src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts index 19bfe1c76c1..42b4f7c9969 100644 --- a/src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts +++ b/src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts @@ -17,16 +17,15 @@ limitations under the License. import { SAS } from "@matrix-org/olm"; import { - RendezvousError, + LegacyRendezvousFailureReason as RendezvousFailureReason, + RendezvousChannel, RendezvousCode, + RendezvousError, RendezvousIntent, - RendezvousChannel, - RendezvousTransportDetails, RendezvousTransport, - LegacyRendezvousFailureReason as RendezvousFailureReason, + RendezvousTransportDetails, } from ".."; -import { encodeUnpaddedBase64, decodeBase64 } from "../../base64"; -import { crypto, subtleCrypto } from "../../crypto/crypto"; +import { decodeBase64, encodeUnpaddedBase64 } from "../../base64"; import { generateDecimalSas } from "../../crypto/verification/SASDecimal"; import { UnstableValue } from "../../NamespacedValue"; @@ -56,11 +55,11 @@ export interface EncryptedPayload { } async function importKey(key: Uint8Array): Promise { - if (!subtleCrypto) { + if (!globalThis.crypto.subtle) { throw new Error("Web Crypto is not available"); } - const imported = subtleCrypto.importKey("raw", key, { name: "AES-GCM" }, false, ["encrypt", "decrypt"]); + const imported = globalThis.crypto.subtle.importKey("raw", key, { name: "AES-GCM" }, false, ["encrypt", "decrypt"]); return imported; } @@ -164,16 +163,16 @@ export class MSC3903ECDHv2RendezvousChannel implements RendezvousChannel { } private async encrypt(data: T): Promise { - if (!subtleCrypto) { + if (!globalThis.crypto.subtle) { throw new Error("Web Crypto is not available"); } const iv = new Uint8Array(32); - crypto.getRandomValues(iv); + globalThis.crypto.getRandomValues(iv); const encodedData = new TextEncoder().encode(JSON.stringify(data)); - const ciphertext = await subtleCrypto.encrypt( + const ciphertext = await globalThis.crypto.subtle.encrypt( { name: "AES-GCM", iv, @@ -208,11 +207,11 @@ export class MSC3903ECDHv2RendezvousChannel implements RendezvousChannel { const ciphertextBytes = decodeBase64(ciphertext); - if (!subtleCrypto) { + if (!globalThis.crypto.subtle) { throw new Error("Web Crypto is not available"); } - const plaintext = await subtleCrypto.decrypt( + const plaintext = await globalThis.crypto.subtle.decrypt( { name: "AES-GCM", iv: decodeBase64(iv), diff --git a/src/rust-crypto/DehydratedDeviceManager.ts b/src/rust-crypto/DehydratedDeviceManager.ts index 56f6052b724..cedac96a0e0 100644 --- a/src/rust-crypto/DehydratedDeviceManager.ts +++ b/src/rust-crypto/DehydratedDeviceManager.ts @@ -21,7 +21,6 @@ import { encodeUri } from "../utils"; import { IHttpOpts, MatrixError, MatrixHttpApi, Method } from "../http-api"; import { IToDeviceEvent } from "../sync-accumulator"; import { ServerSideSecretStorage } from "../secret-storage"; -import { crypto } from "../crypto/crypto"; import { decodeBase64, encodeUnpaddedBase64 } from "../base64"; import { Logger } from "../logger"; @@ -155,7 +154,7 @@ export class DehydratedDeviceManager { */ public async resetKey(): Promise { const key = new Uint8Array(32); - crypto.getRandomValues(key); + globalThis.crypto.getRandomValues(key); await this.secretStorage.store(SECRET_STORAGE_NAME, encodeUnpaddedBase64(key)); this.key = key; } diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index 95d7c2495e5..bb7f4f1245c 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -62,7 +62,6 @@ import { CrossSigningIdentity } from "./CrossSigningIdentity"; import { secretStorageCanAccessSecrets, secretStorageContainsCrossSigningKeys } from "./secret-storage"; import { keyFromPassphrase } from "../crypto/key_passphrase"; import { encodeRecoveryKey } from "../crypto/recoverykey"; -import { crypto } from "../crypto/crypto"; import { isVerificationEvent, RustVerificationRequest, verificationMethodIdentifierToMethod } from "./verification"; import { EventType, MsgType } from "../@types/event"; import { CryptoEvent } from "../crypto"; @@ -891,7 +890,7 @@ export class RustCrypto extends TypedEventEmitter