Instrument Crypto APIs (#6512)

- Adds instrumentation for Crypto APIs, specifically around PKCE
generation
- Also refactors `PkceGenerator` and `BrowserCrypto` classes into pure
functions for better minification and simpler usage

change/@azure-msal-browser-a42f4c95-e6b7-42fd-9f86-3e1eb633116c.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Instrument Crypto APIs #6512",
+  "packageName": "@azure/msal-browser",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@azure-msal-common-d4f5a186-023f-4206-8f33-b15b00183000.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Instrument Crypto APIs #6512",
+  "packageName": "@azure/msal-common",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@azure-msal-node-9cd7fd0e-9f0d-4019-9263-a2dab97bf42f.json

@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "Update tests #6512",
+  "packageName": "@azure/msal-node",
+  "email": "[email protected]",
+  "dependentChangeType": "none"
+}

lib/msal-browser/src/broker/nativeBroker/NativeMessageHandler.ts

@@ -16,7 +16,6 @@ import {
     InProgressPerformanceEvent,
     PerformanceEvents,
     IPerformanceClient,
-    ICrypto,
 } from "@azure/msal-common";
 import {
     NativeExtensionRequest,
@@ -28,6 +27,7 @@ import {
     BrowserAuthErrorCodes,
 } from "../../error/BrowserAuthError";
 import { BrowserConfiguration } from "../../config/Configuration";
+import { createNewGuid } from "../../crypto/BrowserCrypto";
 
 type ResponseResolvers<T> = {
     resolve: (value: T | PromiseLike<T>) => void;
@@ -40,7 +40,6 @@ export class NativeMessageHandler {
     private extensionId: string | undefined;
     private extensionVersion: string | undefined;
     private logger: Logger;
-    private crypto: ICrypto;
     private readonly handshakeTimeoutMs: number;
     private timeoutId: number | undefined;
     private resolvers: Map<string, ResponseResolvers<object>>;
@@ -54,7 +53,6 @@ export class NativeMessageHandler {
         logger: Logger,
         handshakeTimeoutMs: number,
         performanceClient: IPerformanceClient,
-        crypto: ICrypto,
         extensionId?: string
     ) {
         this.logger = logger;
@@ -68,7 +66,6 @@ export class NativeMessageHandler {
         this.handshakeEvent = performanceClient.startMeasurement(
             PerformanceEvents.NativeMessageHandlerHandshake
         );
-        this.crypto = crypto;
     }
 
     /**
@@ -80,7 +77,7 @@ export class NativeMessageHandler {
         const req: NativeExtensionRequest = {
             channel: NativeConstants.CHANNEL_ID,
             extensionId: this.extensionId,
-            responseId: this.crypto.createNewGuid(),
+            responseId: createNewGuid(),
             body: body,
         };
 
@@ -109,16 +106,14 @@ export class NativeMessageHandler {
     static async createProvider(
         logger: Logger,
         handshakeTimeoutMs: number,
-        performanceClient: IPerformanceClient,
-        crypto: ICrypto
+        performanceClient: IPerformanceClient
     ): Promise<NativeMessageHandler> {
         logger.trace("NativeMessageHandler - createProvider called.");
         try {
             const preferredProvider = new NativeMessageHandler(
                 logger,
                 handshakeTimeoutMs,
                 performanceClient,
-                crypto,
                 NativeConstants.PREFERRED_EXTENSION_ID
             );
             await preferredProvider.sendHandshakeRequest();
@@ -128,8 +123,7 @@ export class NativeMessageHandler {
             const backupProvider = new NativeMessageHandler(
                 logger,
                 handshakeTimeoutMs,
-                performanceClient,
-                crypto
+                performanceClient
             );
             await backupProvider.sendHandshakeRequest();
             return backupProvider;
@@ -149,7 +143,7 @@ export class NativeMessageHandler {
         const req: NativeExtensionRequest = {
             channel: NativeConstants.CHANNEL_ID,
             extensionId: this.extensionId,
-            responseId: this.crypto.createNewGuid(),
+            responseId: createNewGuid(),
             body: {
                 method: NativeExtensionMethod.HandshakeRequest,
             },

lib/msal-browser/src/controllers/StandardController.ts

@@ -85,6 +85,7 @@ import { BaseOperatingContext } from "../operatingcontext/BaseOperatingContext";
 import { IController } from "./IController";
 import { AuthenticationResult } from "../response/AuthenticationResult";
 import { ClearCacheRequest } from "../request/ClearCacheRequest";
+import { createNewGuid } from "../crypto/BrowserCrypto";
 
 export class StandardController implements IController {
     // OperatingContext
@@ -289,8 +290,7 @@ export class StandardController implements IController {
                     await NativeMessageHandler.createProvider(
                         this.logger,
                         this.config.system.nativeBrokerHandshakeTimeout,
-                        this.performanceClient,
-                        this.browserCrypto
+                        this.performanceClient
                     );
             } catch (e) {
                 this.logger.verbose(e as string);
@@ -1826,7 +1826,7 @@ export class StandardController implements IController {
         }
 
         if (this.isBrowserEnvironment) {
-            return this.browserCrypto.createNewGuid();
+            return createNewGuid();
         }
 
         /*

lib/msal-browser/src/crypto/BrowserCrypto.ts

@@ -8,9 +8,18 @@ import {
     createBrowserAuthError,
     BrowserAuthErrorCodes,
 } from "../error/BrowserAuthError";
-import { ISubtleCrypto } from "./ISubtleCrypto";
-import { ModernBrowserCrypto } from "./ModernBrowserCrypto";
-import { Logger } from "@azure/msal-common";
+import {
+    IPerformanceClient,
+    Logger,
+    PerformanceEvents,
+} from "@azure/msal-common";
+import { KEY_FORMAT_JWK } from "../utils/BrowserConstants";
+
+/**
+ * This file defines functions used by the browser library to perform cryptography operations such as
+ * hashing and encoding. It also has helper functions to validate the availability of specific APIs.
+ */
+
 /**
  * See here for more info on RsaHashedKeyGenParams: https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedKeyGenParams
  */
@@ -23,113 +32,121 @@ const MODULUS_LENGTH = 2048;
 // Public Exponent
 const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]);
 
+const keygenAlgorithmOptions: RsaHashedKeyGenParams = {
+    name: PKCS1_V15_KEYGEN_ALG,
+    hash: S256_HASH_ALG,
+    modulusLength: MODULUS_LENGTH,
+    publicExponent: PUBLIC_EXPONENT,
+};
+
 /**
- * This class implements functions used by the browser library to perform cryptography operations such as
- * hashing and encoding. It also has helper functions to validate the availability of specific APIs.
+ * Check whether browser crypto is available.
  */
-export class BrowserCrypto {
-    private keygenAlgorithmOptions: RsaHashedKeyGenParams;
-    private subtleCrypto: ISubtleCrypto;
-    private logger: Logger;
-
-    constructor(logger: Logger) {
-        this.logger = logger;
-
-        if (this.hasBrowserCrypto()) {
-            // Use standard modern web crypto if available
-            this.logger.verbose(
-                "BrowserCrypto: modern crypto interface available"
-            );
-            this.subtleCrypto = new ModernBrowserCrypto();
-        } else {
-            this.logger.error("BrowserCrypto: crypto interface is unavailable");
-            throw createBrowserAuthError(
-                BrowserAuthErrorCodes.cryptoNonExistent
-            );
-        }
-
-        this.keygenAlgorithmOptions = {
-            name: PKCS1_V15_KEYGEN_ALG,
-            hash: S256_HASH_ALG,
-            modulusLength: MODULUS_LENGTH,
-            publicExponent: PUBLIC_EXPONENT,
-        };
+export function validateCryptoAvailable(logger: Logger): void {
+    if ("crypto" in window) {
+        logger.verbose("BrowserCrypto: modern crypto interface available");
+    } else {
+        logger.error("BrowserCrypto: crypto interface is unavailable");
+        throw createBrowserAuthError(BrowserAuthErrorCodes.cryptoNonExistent);
     }
+}
 
-    /**
-     * Check whether browser crypto is available.
-     */
-    private hasBrowserCrypto(): boolean {
-        return "crypto" in window;
-    }
+/**
+ * Returns a sha-256 hash of the given dataString as an ArrayBuffer.
+ * @param dataString
+ */
+export async function sha256Digest(
+    dataString: string,
+    performanceClient?: IPerformanceClient,
+    correlationId?: string
+): Promise<ArrayBuffer> {
+    performanceClient?.addQueueMeasurement(
+        PerformanceEvents.Sha256Digest,
+        correlationId
+    );
+    const data = BrowserStringUtils.stringToUtf8Arr(dataString);
+    // MSR Crypto wants object with name property, instead of string
+    return window.crypto.subtle.digest(
+        keygenAlgorithmOptions,
+        data
+    ) as Promise<ArrayBuffer>;
+}
 
-    /**
-     * Returns a sha-256 hash of the given dataString as an ArrayBuffer.
-     * @param dataString
-     */
-    async sha256Digest(dataString: string): Promise<ArrayBuffer> {
-        const data = BrowserStringUtils.stringToUtf8Arr(dataString);
-        // MSR Crypto wants object with name property, instead of string
-        return this.subtleCrypto.digest({ name: S256_HASH_ALG }, data);
-    }
+/**
+ * Populates buffer with cryptographically random values.
+ * @param dataBuffer
+ */
+export function getRandomValues(dataBuffer: Uint8Array): Uint8Array {
+    return window.crypto.getRandomValues(dataBuffer);
+}
 
-    /**
-     * Populates buffer with cryptographically random values.
-     * @param dataBuffer
-     */
-    getRandomValues(dataBuffer: Uint8Array): Uint8Array {
-        return this.subtleCrypto.getRandomValues(dataBuffer);
-    }
+/**
+ * Creates a new random GUID
+ * @returns
+ */
+export function createNewGuid(): string {
+    return window.crypto.randomUUID();
+}
 
-    /**
-     * Generates a keypair based on current keygen algorithm config.
-     * @param extractable
-     * @param usages
-     */
-    async generateKeyPair(
-        extractable: boolean,
-        usages: Array<KeyUsage>
-    ): Promise<CryptoKeyPair> {
-        return this.subtleCrypto.generateKey(
-            this.keygenAlgorithmOptions,
-            extractable,
-            usages
-        );
-    }
+/**
+ * Generates a keypair based on current keygen algorithm config.
+ * @param extractable
+ * @param usages
+ */
+export async function generateKeyPair(
+    extractable: boolean,
+    usages: Array<KeyUsage>
+): Promise<CryptoKeyPair> {
+    return window.crypto.subtle.generateKey(
+        keygenAlgorithmOptions,
+        extractable,
+        usages
+    ) as Promise<CryptoKeyPair>;
+}
 
-    /**
-     * Export key as Json Web Key (JWK)
-     * @param key
-     */
-    async exportJwk(key: CryptoKey): Promise<JsonWebKey> {
-        return this.subtleCrypto.exportKey(key);
-    }
+/**
+ * Export key as Json Web Key (JWK)
+ * @param key
+ */
+export async function exportJwk(key: CryptoKey): Promise<JsonWebKey> {
+    return window.crypto.subtle.exportKey(
+        KEY_FORMAT_JWK,
+        key
+    ) as Promise<JsonWebKey>;
+}
 
-    /**
-     * Imports key as Json Web Key (JWK), can set extractable and usages.
-     * @param key
-     * @param extractable
-     * @param usages
-     */
-    async importJwk(
-        key: JsonWebKey,
-        extractable: boolean,
-        usages: Array<KeyUsage>
-    ): Promise<CryptoKey> {
-        return this.subtleCrypto.importKey(
-            key,
-            this.keygenAlgorithmOptions,
-            extractable,
-            usages
-        );
-    }
+/**
+ * Imports key as Json Web Key (JWK), can set extractable and usages.
+ * @param key
+ * @param extractable
+ * @param usages
+ */
+export async function importJwk(
+    key: JsonWebKey,
+    extractable: boolean,
+    usages: Array<KeyUsage>
+): Promise<CryptoKey> {
+    return window.crypto.subtle.importKey(
+        KEY_FORMAT_JWK,
+        key,
+        keygenAlgorithmOptions,
+        extractable,
+        usages
+    ) as Promise<CryptoKey>;
+}
 
-    /**
-     * Signs given data with given key
-     * @param key
-     * @param data
-     */
-    async sign(key: CryptoKey, data: ArrayBuffer): Promise<ArrayBuffer> {
-        return this.subtleCrypto.sign(this.keygenAlgorithmOptions, key, data);
-    }
+/**
+ * Signs given data with given key
+ * @param key
+ * @param data
+ */
+export async function sign(
+    key: CryptoKey,
+    data: ArrayBuffer
+): Promise<ArrayBuffer> {
+    return window.crypto.subtle.sign(
+        keygenAlgorithmOptions,
+        key,
+        data
+    ) as Promise<ArrayBuffer>;
 }