Skip to content

Commit

Permalink
Remove crypto shims (#4292)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
richvdh authored Jul 5, 2024
1 parent 957329b commit 712ba61
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 81 deletions.
18 changes: 5 additions & 13 deletions spec/unit/oidc/authorize.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -62,20 +56,18 @@ 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,
randomUUID: jest.fn().mockReturnValue("not-random-uuid"),
subtle: webCrypto.subtle,
},
});
global.TextEncoder = TextEncoder;
});

afterEach(() => {
// @ts-ignore reset any ugly mocking we did
crypto.subtleCrypto = realSubtleCrypto;
});

it("should generate authorization params", () => {
Expand All @@ -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(
Expand Down
22 changes: 12 additions & 10 deletions src/crypto/aes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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,
Expand All @@ -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,
{
Expand Down
3 changes: 1 addition & 2 deletions src/crypto/backup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
}

Expand Down
24 changes: 2 additions & 22 deletions src/crypto/crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
15 changes: 9 additions & 6 deletions src/crypto/key_passphrase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ limitations under the License.
*/

import { randomString } from "../randomstring";
import { subtleCrypto } from "./crypto";

const DEFAULT_ITERATIONS = 500000;

Expand Down Expand Up @@ -62,15 +61,19 @@ export async function deriveKey(
iterations: number,
numBits = DEFAULT_BITSIZE,
): Promise<Uint8Array> {
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),
Expand Down
3 changes: 1 addition & 2 deletions src/crypto/verification/QRCode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
}

Expand Down
13 changes: 6 additions & 7 deletions src/oidc/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -57,14 +56,14 @@ export const generateScope = (deviceId?: string): string => {

// https://www.rfc-editor.org/rfc/rfc7636
const generateCodeChallenge = async (codeVerifier: string): Promise<string> => {
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, "")
Expand Down
3 changes: 1 addition & 2 deletions src/randomstring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,14 @@ limitations under the License.
*/

import { encodeUnpaddedBase64Url } from "./base64";
import { crypto } from "./crypto/crypto";

const LOWERCASE = "abcdefghijklmnopqrstuvwxyz";
const UPPERCASE = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
const DIGITS = "0123456789";

export function secureRandomBase64Url(len: number): string {
const key = new Uint8Array(len);
crypto.getRandomValues(key);
globalThis.crypto.getRandomValues(key);

return encodeUnpaddedBase64Url(key);
}
Expand Down
25 changes: 12 additions & 13 deletions src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -56,11 +55,11 @@ export interface EncryptedPayload {
}

async function importKey(key: Uint8Array): Promise<CryptoKey> {
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;
}
Expand Down Expand Up @@ -164,16 +163,16 @@ export class MSC3903ECDHv2RendezvousChannel<T> implements RendezvousChannel<T> {
}

private async encrypt(data: T): Promise<MSC3903ECDHPayload> {
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,
Expand Down Expand Up @@ -208,11 +207,11 @@ export class MSC3903ECDHv2RendezvousChannel<T> implements RendezvousChannel<T> {

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),
Expand Down
3 changes: 1 addition & 2 deletions src/rust-crypto/DehydratedDeviceManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -155,7 +154,7 @@ export class DehydratedDeviceManager {
*/
public async resetKey(): Promise<void> {
const key = new Uint8Array(32);
crypto.getRandomValues(key);
globalThis.crypto.getRandomValues(key);
await this.secretStorage.store(SECRET_STORAGE_NAME, encodeUnpaddedBase64(key));
this.key = key;
}
Expand Down
3 changes: 1 addition & 2 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -891,7 +890,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
} else {
// Using the navigator crypto API to generate the private key
const key = new Uint8Array(32);
crypto.getRandomValues(key);
globalThis.crypto.getRandomValues(key);
return {
privateKey: key,
encodedPrivateKey: encodeRecoveryKey(key),
Expand Down

0 comments on commit 712ba61

Please sign in to comment.