From 31f38550e3fb0ed7312e52b896985477b22f01c3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 20 Oct 2023 16:00:55 +0100 Subject: [PATCH] Refactor & make base64 functions browser-safe We had two identical sets of base64 functions in the js-sdk, both using Buffer which isn't really available in the browser unless you're using an old webpack (ie. what element-web uses). This PR: * Takes the crypto base64 file and moves it out of crypto (because we use base64 for much more than just crypto) * Makes them work in a browser without the Buffer global * Removes the other base64 functions * Changes everything to use the new common ones * Adds a comment explaining why the function is kinda ugly and how soul destroyingly awful the JS ecosystem is. * Runs the tests with both impls * Changes the test to not just test the decoder against the encoder * Adds explicit support & tests for (decoding) base64Url (I'll add an encode method later, no need for that to go in this PR too). --- spec/unit/{crypto => }/base64.spec.ts | 40 ++++++++-- spec/unit/crypto/secrets.spec.ts | 9 ++- .../verification/secret_request.spec.ts | 2 +- spec/unit/rendezvous/ecdhv2.spec.ts | 2 +- spec/unit/rendezvous/rendezvous.spec.ts | 2 +- src/base64.ts | 79 +++++++++++++++++++ src/client.ts | 2 +- src/common-crypto/base64.ts | 46 ----------- src/crypto/CrossSigning.ts | 3 +- src/crypto/aes.ts | 2 +- src/crypto/dehydration.ts | 2 +- src/crypto/index.ts | 21 ++--- src/crypto/olmlib.ts | 27 ------- src/crypto/verification/QRCode.ts | 2 +- .../MSC3903ECDHv2RendezvousChannel.ts | 2 +- src/rust-crypto/rust-crypto.ts | 2 +- 16 files changed, 140 insertions(+), 103 deletions(-) rename spec/unit/{crypto => }/base64.spec.ts (60%) create mode 100644 src/base64.ts delete mode 100644 src/common-crypto/base64.ts diff --git a/spec/unit/crypto/base64.spec.ts b/spec/unit/base64.spec.ts similarity index 60% rename from spec/unit/crypto/base64.spec.ts rename to spec/unit/base64.spec.ts index 0fac4b4548c..fd75f1212d1 100644 --- a/spec/unit/crypto/base64.spec.ts +++ b/spec/unit/base64.spec.ts @@ -15,16 +15,44 @@ limitations under the License. */ import { TextEncoder, TextDecoder } from "util"; +import NodeBuffer from "node:buffer"; -import { decodeBase64, encodeBase64, encodeUnpaddedBase64 } from "../../../src/common-crypto/base64"; +import { decodeBase64, encodeBase64, encodeUnpaddedBase64 } from "../../src/base64"; + +describe.each(["browser", "node"])("Crypto Base64 encoding (%s)", (env) => { + let origBuffer = Buffer; + + beforeAll(() => { + if (env === "browser") { + origBuffer = Buffer; + // @ts-ignore + // eslint-disable-next-line no-global-assign + Buffer = undefined; + + global.atob = NodeBuffer.atob; + global.btoa = NodeBuffer.btoa; + } + }); + + afterAll(() => { + // eslint-disable-next-line no-global-assign + Buffer = origBuffer; + // @ts-ignore + global.atob = undefined; + // @ts-ignore + global.btoa = undefined; + }); -describe("Crypto Base64 encoding", () => { it("Should decode properly encoded data", async () => { - const toEncode = "encoding hello world"; - const encoded = encodeBase64(new TextEncoder().encode(toEncode)); - const decoded = new TextDecoder().decode(decodeBase64(encoded)); + const decoded = new TextDecoder().decode(decodeBase64("ZW5jb2RpbmcgaGVsbG8gd29ybGQ=")); + + expect(decoded).toStrictEqual("encoding hello world"); + }); + + it("Should decode URL-safe base64", async () => { + const decoded = new TextDecoder().decode(decodeBase64("Pz8_Pz8=")); - expect(decoded).toStrictEqual(toEncode); + expect(decoded).toStrictEqual("?????"); }); it("Encode unpadded should not have padding", async () => { diff --git a/spec/unit/crypto/secrets.spec.ts b/spec/unit/crypto/secrets.spec.ts index ff9129468ce..907280d17eb 100644 --- a/spec/unit/crypto/secrets.spec.ts +++ b/spec/unit/crypto/secrets.spec.ts @@ -28,6 +28,7 @@ import { DeviceInfo } from "../../../src/crypto/deviceinfo"; import { ISignatures } from "../../../src/@types/signed"; import { ICurve25519AuthData } from "../../../src/crypto/keybackup"; import { SecretStorageKeyDescription, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage"; +import { decodeBase64 } from "../../../src/base64"; async function makeTestClient( userInfo: { userId: string; deviceId: string }, @@ -275,13 +276,13 @@ describe("Secrets", function () { describe("bootstrap", function () { // keys used in some of the tests - const XSK = new Uint8Array(olmlib.decodeBase64("3lo2YdJugHjfE+Or7KJ47NuKbhE7AAGLgQ/dc19913Q=")); + const XSK = new Uint8Array(decodeBase64("3lo2YdJugHjfE+Or7KJ47NuKbhE7AAGLgQ/dc19913Q=")); const XSPubKey = "DRb8pFVJyEJ9OWvXeUoM0jq/C2Wt+NxzBZVuk2nRb+0"; - const USK = new Uint8Array(olmlib.decodeBase64("lKWi3hJGUie5xxHgySoz8PHFnZv6wvNaud/p2shN9VU=")); + const USK = new Uint8Array(decodeBase64("lKWi3hJGUie5xxHgySoz8PHFnZv6wvNaud/p2shN9VU=")); const USPubKey = "CUpoiTtHiyXpUmd+3ohb7JVxAlUaOG1NYs9Jlx8soQU"; - const SSK = new Uint8Array(olmlib.decodeBase64("1R6JVlXX99UcfUZzKuCDGQgJTw8ur1/ofgPD8pp+96M=")); + const SSK = new Uint8Array(decodeBase64("1R6JVlXX99UcfUZzKuCDGQgJTw8ur1/ofgPD8pp+96M=")); const SSPubKey = "0DfNsRDzEvkCLA0gD3m7VAGJ5VClhjEsewI35xq873Q"; - const SSSSKey = new Uint8Array(olmlib.decodeBase64("XrmITOOdBhw6yY5Bh7trb/bgp1FRdIGyCUxxMP873R0=")); + const SSSSKey = new Uint8Array(decodeBase64("XrmITOOdBhw6yY5Bh7trb/bgp1FRdIGyCUxxMP873R0=")); it("bootstraps when no storage or cross-signing keys locally", async function () { const key = new Uint8Array(16); diff --git a/spec/unit/crypto/verification/secret_request.spec.ts b/spec/unit/crypto/verification/secret_request.spec.ts index ff8b15cc629..06ff0393ef1 100644 --- a/spec/unit/crypto/verification/secret_request.spec.ts +++ b/spec/unit/crypto/verification/secret_request.spec.ts @@ -16,7 +16,7 @@ limitations under the License. import "../../../olm-loader"; import { MatrixClient, MatrixEvent } from "../../../../src/matrix"; -import { encodeBase64 } from "../../../../src/crypto/olmlib"; +import { encodeBase64 } from "../../../../src/base64"; import "../../../../src/crypto"; // import this to cycle-break import { CrossSigningInfo } from "../../../../src/crypto/CrossSigning"; import { VerificationRequest } from "../../../../src/crypto/verification/request/VerificationRequest"; diff --git a/spec/unit/rendezvous/ecdhv2.spec.ts b/spec/unit/rendezvous/ecdhv2.spec.ts index 2e88b345054..caadfbf6e9c 100644 --- a/spec/unit/rendezvous/ecdhv2.spec.ts +++ b/spec/unit/rendezvous/ecdhv2.spec.ts @@ -17,7 +17,7 @@ limitations under the License. import "../../olm-loader"; import { RendezvousFailureReason, RendezvousIntent } from "../../../src/rendezvous"; import { MSC3903ECDHPayload, MSC3903ECDHv2RendezvousChannel } from "../../../src/rendezvous/channels"; -import { decodeBase64 } from "../../../src/crypto/olmlib"; +import { decodeBase64 } from "../../../src/base64"; import { DummyTransport } from "./DummyTransport"; function makeTransport(name: string) { diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index 33ca7090341..eb8a72c26ae 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -29,7 +29,7 @@ import { MSC3886SimpleHttpRendezvousTransportDetails, } from "../../../src/rendezvous/transports"; import { DummyTransport } from "./DummyTransport"; -import { decodeBase64 } from "../../../src/crypto/olmlib"; +import { decodeBase64 } from "../../../src/base64"; import { logger } from "../../../src/logger"; import { DeviceInfo } from "../../../src/crypto/deviceinfo"; diff --git a/src/base64.ts b/src/base64.ts new file mode 100644 index 00000000000..5a4c5c87a06 --- /dev/null +++ b/src/base64.ts @@ -0,0 +1,79 @@ +/* +Copyright 2023 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. +*/ + +/** + * Base64 encoding and decoding utilities + */ + +/** + * Encode a typed array of uint8 as base64. + * @param uint8Array - The data to encode. + * @returns The base64. + */ +export function encodeBase64(uint8Array: ArrayBuffer | Uint8Array): string { + // A brief note on the state of base64 encoding in Javascript. + // As of 2023, there is still no common native impl between both browsers and + // node. Older Webpack provides an impl for Buffer and there is a polyfill class + // for it. There are also plenty of pure js impls, eg. base64-js which has 2336 + // dependents at current count. Using this would probably be fine although it's + // a little under-docced and run by an individual. The node impl works fine, + // the browser impl works but predates Uint8Array and so only uses strings. + // Right now, switching between native (or polyfilled) impls like this feels + // like the least bad option, but... *shrugs*. + if (typeof Buffer === "function") { + return Buffer.from(uint8Array).toString("base64"); + } else if (typeof btoa === "function" && uint8Array instanceof Uint8Array) { + // ArrayBuffer is a node concept so the param should always be a Uint8Array on + // the browser. We need to check because ArrayBuffers don't have reduce. + return btoa(uint8Array.reduce((acc, current) => acc + String.fromCharCode(current), "")); + } else { + throw new Error("No base64 impl found!"); + } +} + +/** + * Encode a typed array of uint8 as unpadded base64. + * @param uint8Array - The data to encode. + * @returns The unpadded base64. + */ +export function encodeUnpaddedBase64(uint8Array: ArrayBuffer | Uint8Array): string { + return encodeBase64(uint8Array).replace(/={1,2}$/, ""); +} + +/** + * Decode a base64 string to a typed array of uint8. + * @param base64 - The base64 to decode. + * @returns The decoded data. + */ +export function decodeBase64(base64: string): Uint8Array { + // See encodeBase64 for a short treatise on base64 en/decoding in JS + if (typeof Buffer === "function") { + return Buffer.from(base64, "base64"); + } else if (typeof atob === "function") { + const itFunc = function* (): Generator { + const decoded = atob( + // built-in atob doesn't support base64url: convert so we support either + base64.replace("-", "+").replace("_", "/"), + ); + for (let i = 0; i < decoded.length; ++i) { + yield decoded.charCodeAt(i); + } + }; + return Uint8Array.from(itFunc()); + } else { + throw new Error("No base64 impl found!"); + } +} diff --git a/src/client.ts b/src/client.ts index 138ebe0d289..2d151bdf527 100644 --- 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 "./crypto/olmlib"; +import { decodeBase64, encodeBase64 } from "./base64"; import { IExportedDevice as IExportedOlmDevice } from "./crypto/OlmDevice"; import { IOlmDevice } from "./crypto/algorithms/megolm"; import { TypedReEmitter } from "./ReEmitter"; diff --git a/src/common-crypto/base64.ts b/src/common-crypto/base64.ts deleted file mode 100644 index ca0476d9e1c..00000000000 --- a/src/common-crypto/base64.ts +++ /dev/null @@ -1,46 +0,0 @@ -/* -Copyright 2023 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. -*/ - -/** - * Base64 encoding and decoding utility for crypo. - */ - -/** - * Encode a typed array of uint8 as base64. - * @param uint8Array - The data to encode. - * @returns The base64. - */ -export function encodeBase64(uint8Array: ArrayBuffer | Uint8Array): string { - return Buffer.from(uint8Array).toString("base64"); -} - -/** - * Encode a typed array of uint8 as unpadded base64. - * @param uint8Array - The data to encode. - * @returns The unpadded base64. - */ -export function encodeUnpaddedBase64(uint8Array: ArrayBuffer | Uint8Array): string { - return encodeBase64(uint8Array).replace(/={1,2}$/, ""); -} - -/** - * Decode a base64 string to a typed array of uint8. - * @param base64 - The base64 to decode. - * @returns The decoded data. - */ -export function decodeBase64(base64: string): Uint8Array { - return Buffer.from(base64, "base64"); -} diff --git a/src/crypto/CrossSigning.ts b/src/crypto/CrossSigning.ts index bafd9f4065a..1bc3d9e8e91 100644 --- a/src/crypto/CrossSigning.ts +++ b/src/crypto/CrossSigning.ts @@ -19,7 +19,7 @@ limitations under the License. */ import type { PkSigning } from "@matrix-org/olm"; -import { decodeBase64, encodeBase64, IObject, pkSign, pkVerify } from "./olmlib"; +import { IObject, pkSign, pkVerify } from "./olmlib"; import { logger } from "../logger"; import { IndexedDBCryptoStore } from "../crypto/store/indexeddb-crypto-store"; import { decryptAES, encryptAES } from "./aes"; @@ -31,6 +31,7 @@ import { ISignatures } from "../@types/signed"; import { CryptoStore, SecretStorePrivateKeys } from "./store/base"; import { ServerSideSecretStorage, SecretStorageKeyDescription } from "../secret-storage"; import { DeviceVerificationStatus, UserVerificationStatus as UserTrustLevel } from "../crypto-api"; +import { decodeBase64, encodeBase64 } from "../base64"; // backwards-compatibility re-exports export { UserTrustLevel }; diff --git a/src/crypto/aes.ts b/src/crypto/aes.ts index 48470af6261..389e2ff6ac3 100644 --- a/src/crypto/aes.ts +++ b/src/crypto/aes.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { decodeBase64, encodeBase64 } from "./olmlib"; +import { decodeBase64, encodeBase64 } from "../base64"; import { subtleCrypto, crypto, TextEncoder } from "./crypto"; // salt for HKDF, with 8 bytes of zeros diff --git a/src/crypto/dehydration.ts b/src/crypto/dehydration.ts index 373b236b256..a24e65e4537 100644 --- a/src/crypto/dehydration.ts +++ b/src/crypto/dehydration.ts @@ -17,7 +17,7 @@ limitations under the License. import anotherjson from "another-json"; import type { IDeviceKeys, IOneTimeKey } from "../@types/crypto"; -import { decodeBase64, encodeBase64 } from "./olmlib"; +import { decodeBase64, encodeBase64 } from "../base64"; import { IndexedDBCryptoStore } from "../crypto/store/indexeddb-crypto-store"; import { decryptAES, encryptAES } from "./aes"; import { logger } from "../logger"; diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 89f69fa6dea..a2a6469feda 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -102,6 +102,7 @@ import { import { Device, DeviceMap } from "../models/device"; import { deviceInfoToDevice } from "./device-converter"; import { ClientPrefix, MatrixError, Method } from "../http-api"; +import { decodeBase64, encodeBase64 } from "../base64"; /* re-exports for backwards compatibility */ export type { @@ -500,7 +501,7 @@ export class Crypto extends TypedEventEmitter { this.cryptoStore.storeSecretStorePrivateKey(txn, "m.megolm_backup.v1", encryptedKey); }); @@ -4196,7 +4197,7 @@ export function fixBackupKey(key?: string): string | null { return null; } const fixedKey = Uint8Array.from(key.split(","), (x) => parseInt(x)); - return olmlib.encodeBase64(fixedKey); + return encodeBase64(fixedKey); } /** diff --git a/src/crypto/olmlib.ts b/src/crypto/olmlib.ts index 68b5aa6f0d7..7396ef47988 100644 --- a/src/crypto/olmlib.ts +++ b/src/crypto/olmlib.ts @@ -537,30 +537,3 @@ export function isOlmEncrypted(event: MatrixEvent): boolean { } return true; } - -/** - * Encode a typed array of uint8 as base64. - * @param uint8Array - The data to encode. - * @returns The base64. - */ -export function encodeBase64(uint8Array: ArrayBuffer | Uint8Array): string { - return Buffer.from(uint8Array).toString("base64"); -} - -/** - * Encode a typed array of uint8 as unpadded base64. - * @param uint8Array - The data to encode. - * @returns The unpadded base64. - */ -export function encodeUnpaddedBase64(uint8Array: ArrayBuffer | Uint8Array): string { - return encodeBase64(uint8Array).replace(/=+$/g, ""); -} - -/** - * Decode a base64 string to a typed array of uint8. - * @param base64 - The base64 to decode. - * @returns The decoded data. - */ -export function decodeBase64(base64: string): Uint8Array { - return Buffer.from(base64, "base64"); -} diff --git a/src/crypto/verification/QRCode.ts b/src/crypto/verification/QRCode.ts index f8a2999f407..b4e43252eaf 100644 --- a/src/crypto/verification/QRCode.ts +++ b/src/crypto/verification/QRCode.ts @@ -21,7 +21,7 @@ limitations under the License. import { crypto } from "../crypto"; import { VerificationBase as Base } from "./Base"; import { newKeyMismatchError, newUserCancelledError } from "./Error"; -import { decodeBase64, encodeUnpaddedBase64 } from "../olmlib"; +import { decodeBase64, encodeUnpaddedBase64 } from "../../base64"; import { logger } from "../../logger"; import { VerificationRequest } from "./request/VerificationRequest"; import { MatrixClient } from "../../client"; diff --git a/src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts b/src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts index be60ee5c9aa..872c6ea2c78 100644 --- a/src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts +++ b/src/rendezvous/channels/MSC3903ECDHv2RendezvousChannel.ts @@ -25,7 +25,7 @@ import { RendezvousTransport, RendezvousFailureReason, } from ".."; -import { encodeUnpaddedBase64, decodeBase64 } from "../../crypto/olmlib"; +import { encodeUnpaddedBase64, decodeBase64 } from "../../base64"; import { crypto, subtleCrypto, TextEncoder } from "../../crypto/crypto"; import { generateDecimalSas } from "../../crypto/verification/SASDecimal"; import { UnstableValue } from "../../NamespacedValue"; diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts index e0238cc81d6..cc7dc3bfda8 100644 --- a/src/rust-crypto/rust-crypto.ts +++ b/src/rust-crypto/rust-crypto.ts @@ -70,7 +70,7 @@ import { TypedReEmitter } from "../ReEmitter"; import { randomString } from "../randomstring"; import { ClientStoppedError } from "../errors"; import { ISignatures } from "../@types/signed"; -import { encodeBase64 } from "../common-crypto/base64"; +import { encodeBase64 } from "../base64"; import { DecryptionError } from "../crypto/algorithms"; const ALL_VERIFICATION_METHODS = ["m.sas.v1", "m.qr_code.scan.v1", "m.qr_code.show.v1", "m.reciprocate.v1"];