Bound RT #1 - Generate Session Transport Key (#3487)

* Add KeyManager class * Add STK JWK to BaseAuthRequest * Add STK generation logic to common and browser * Update PublicClientApplication tests to mock out STK generation from Auth Code requests * Undo msal-node-samples changes * Move generateCnf from PopTokenGenerator to KeyManager * Refactor crypto key generation to use different key generation algorithm options for AT and RT PoP * Add missing API from Crypto Interface to msal-node * Fix linter issues * Fix merge conflicts * Refactor Cryptographic constants out of BrowserConstants and CryptoOps * Fix tests after merge * Add feature flag to make RT Binding opt-in * Add error handling to STK generation step * Refactor crypto enum names * Add error handling for crypto key generation * Put KeyManager instance in BaseClient instead of AuthCode and Refresh Clients * Fix import in BaseClient * Extend KeyManager tests * Increase test coverage * Update lib/msal-browser/src/utils/CryptoConstants.ts * Update lib/msal-common/test/client/RefreshTokenClient.spec.ts Co-authored-by: Thomas Norling <thomas.l.norling@gmail.com> * Fix incorrect typing and checks for private key on getPublicKeyThumbprint * Refactor cryptographic constants to have more consistent casing * Fix CryptoOps tests around getPublicKeyThumbprint * Move refreshTokenBinding feature flag to system config * Update browser client config to move refreshTokenBinding flag to system config * Rename KeyManager to CryptoKeyManager for more specificity * Update BrowserAuthError to remove keyId from error message and avoid Pii * Update lib/msal-browser/src/config/Configuration.ts * Refactor CryptoKeyManager into PopTokenGenerator * Change files * Replace crypto key type switch in getPublicKeyThumbprint with a ternary assignment Co-authored-by: Jason Nutter <janutter@microsoft.com> * Update changefiles * Extend CryptoOps logging * Update exception handling and logging in CryptoOps getPublicKeyThumbprint API * Fix failing test in CryptoOps spec Co-authored-by: Thomas Norling <thomas.l.norling@gmail.com> Co-authored-by: Sameera Gajjarapu <sameera.gajjarapu@microsoft.com> Co-authored-by: Jason Nutter <janutter@microsoft.com>
AzureAD · Jan 28, 2022 · a7ec0b8 · a7ec0b8
1 parent ee8153f
commit a7ec0b8
Show file tree

Hide file tree

Showing 44 changed files with 651 additions and 155 deletions.
diff --git a/change/@azure-msal-browser-866a65d5-3485-4066-82bf-ff5d21d1238a.json b/change/@azure-msal-browser-866a65d5-3485-4066-82bf-ff5d21d1238a.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Bound RT #1 - Generate Session Transport Key #3487",
+  "packageName": "@azure/msal-browser",
+  "email": "hemoral@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@azure-msal-common-157327d2-1cbc-46de-8fc8-0ee05d2b4ff6.json b/change/@azure-msal-common-157327d2-1cbc-46de-8fc8-0ee05d2b4ff6.json
@@ -0,0 +1,7 @@
+{
+  "type": "major",
+  "comment": "Bound RT #1 - Generate Session Transport Key #3487",
+  "packageName": "@azure/msal-common",
+  "email": "hemoral@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@azure-msal-node-4c43a4bb-fdda-4770-8636-96722504074a.json b/change/@azure-msal-node-4c43a4bb-fdda-4770-8636-96722504074a.json
@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "Bound RT #1 - Generate Session Transport Key #3487",
+  "packageName": "@azure/msal-node",
+  "email": "hemoral@microsoft.com",
+  "dependentChangeType": "none"
+}
diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts
@@ -66,6 +66,7 @@ export type CacheOptions = {
  * - redirectNavigationTimeout    - Time to wait for redirection to occur before resolving promise
  * - asyncPopups                  - Sets whether popups are opened asynchronously. By default, this flag is set to false. When set to false, blank popups are opened before anything else happens. When set to true, popups are opened when making the network request.
  * - allowRedirectInIframe        - Flag to enable redirect opertaions when the app is rendered in an iframe (to support scenarios such as embedded B2C login).
+ * - refreshTokenBinding        - Boolean that enables refresh token binding (a.k.a refresh token proof-of-possession) for authorization requests
  */
 export type BrowserSystemOptions = SystemOptions & {
     loggerOptions?: LoggerOptions;
@@ -78,6 +79,7 @@ export type BrowserSystemOptions = SystemOptions & {
     redirectNavigationTimeout?: number;
     asyncPopups?: boolean;
     allowRedirectInIframe?: boolean;
+    refreshTokenBinding?: boolean;
 };
 
 /**
@@ -153,7 +155,8 @@ export function buildConfiguration({ auth: userInputAuth, cache: userInputCache,
         navigateFrameWait: isBrowserEnvironment && BrowserUtils.detectIEOrEdge() ? 500 : 0,
         redirectNavigationTimeout: DEFAULT_REDIRECT_TIMEOUT_MS,
         asyncPopups: false,
-        allowRedirectInIframe: false
+        allowRedirectInIframe: false,
+        refreshTokenBinding: false
     };
 
     const overlayedConfig: BrowserConfiguration = {

diff --git a/lib/msal-browser/src/crypto/BrowserCrypto.ts b/lib/msal-browser/src/crypto/BrowserCrypto.ts
@@ -5,27 +5,23 @@
 
 import { BrowserStringUtils } from "../utils/BrowserStringUtils";
 import { BrowserAuthError } from "../error/BrowserAuthError";
-import { KEY_FORMAT_JWK } from "../utils/BrowserConstants";
+import { Algorithms, CryptoKeyFormats } from "../utils/CryptoConstants";
 import { Logger } from "..";
+
 /**
  * See here for more info on RsaHashedKeyGenParams: https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedKeyGenParams
  */
-// RSA KeyGen Algorithm
-const PKCS1_V15_KEYGEN_ALG = "RSASSA-PKCS1-v1_5";
-// SHA-256 hashing algorithm
-const S256_HASH_ALG = "SHA-256";
-// MOD length for PoP tokens
-const MODULUS_LENGTH = 2048;
-// Public Exponent
-const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]);
+export type CryptoKeyOptions = {
+    keyGenAlgorithmOptions: RsaHashedKeyGenParams,
+    keypairUsages: KeyUsage[],
+    privateKeyUsage: KeyUsage[]
+};
 
 /**
  * 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.
  */
 export class BrowserCrypto {
-
-    private _keygenAlgorithmOptions: RsaHashedKeyGenParams;
     private logger: Logger;
 
     constructor(logger: Logger) {
@@ -34,13 +30,6 @@ export class BrowserCrypto {
         if (!(this.hasCryptoAPI())) {
             throw BrowserAuthError.createCryptoNotAvailableError("Browser crypto or msCrypto object not available.");
         }
-
-        this._keygenAlgorithmOptions = {
-            name: PKCS1_V15_KEYGEN_ALG,
-            hash: S256_HASH_ALG,
-            modulusLength: MODULUS_LENGTH,
-            publicExponent: PUBLIC_EXPONENT
-        };
     }
 
     /**
@@ -49,8 +38,7 @@ export class BrowserCrypto {
      */
     async sha256Digest(dataString: string): Promise<ArrayBuffer> {
         const data = BrowserStringUtils.stringToUtf8Arr(dataString);
-
-        return this.hasIECrypto() ? this.getMSCryptoDigest(S256_HASH_ALG, data) : this.getSubtleCryptoDigest(S256_HASH_ALG, data);
+        return this.hasIECrypto() ? this.getMSCryptoDigest(Algorithms.S256_HASH_ALG, data) : this.getSubtleCryptoDigest(Algorithms.S256_HASH_ALG, data);
     }
 
     /**
@@ -70,11 +58,16 @@ export class BrowserCrypto {
      * @param extractable 
      * @param usages 
      */
-    async generateKeyPair(extractable: boolean, usages: Array<KeyUsage>): Promise<CryptoKeyPair> {
+    async generateKeyPair(keyOptions: CryptoKeyOptions, extractable: boolean): Promise<CryptoKeyPair> {
+        const keyGenAlgorithmOptions = keyOptions.keyGenAlgorithmOptions;
         return (
             this.hasIECrypto() ? 
-                this.msCryptoGenerateKey(extractable, usages) 
-                : window.crypto.subtle.generateKey(this._keygenAlgorithmOptions, extractable, usages)
+                this.msCryptoGenerateKey(keyOptions, extractable) 
+                : window.crypto.subtle.generateKey(
+                    keyGenAlgorithmOptions,
+                    extractable,
+                    keyOptions.keypairUsages
+                )
         ) as Promise<CryptoKeyPair>;
     }
 
@@ -84,7 +77,7 @@ export class BrowserCrypto {
      * @param format 
      */
     async exportJwk(key: CryptoKey): Promise<JsonWebKey> {
-        return this.hasIECrypto() ? this.msCryptoExportJwk(key) : window.crypto.subtle.exportKey(KEY_FORMAT_JWK, key);
+        return this.hasIECrypto() ? this.msCryptoExportJwk(key) : window.crypto.subtle.exportKey(CryptoKeyFormats.jwk, key);
     }
 
     /**
@@ -94,24 +87,24 @@ export class BrowserCrypto {
      * @param extractable 
      * @param usages 
      */
-    async importJwk(key: JsonWebKey, extractable: boolean, usages: Array<KeyUsage>): Promise<CryptoKey> {
+    async importJwk(keyOptions: CryptoKeyOptions, key: JsonWebKey, extractable: boolean, usages: Array<KeyUsage>): Promise<CryptoKey> {
         const keyString = BrowserCrypto.getJwkString(key);
         const keyBuffer = BrowserStringUtils.stringToArrayBuffer(keyString);
 
         return this.hasIECrypto() ? 
-            this.msCryptoImportKey(keyBuffer, extractable, usages) 
-            : window.crypto.subtle.importKey(KEY_FORMAT_JWK, key, this._keygenAlgorithmOptions, extractable, usages);
+            this.msCryptoImportKey(keyOptions, keyBuffer, extractable, usages)
+            : window.crypto.subtle.importKey(CryptoKeyFormats.jwk, key, keyOptions.keyGenAlgorithmOptions, extractable, usages);
     }
 
     /**
      * Signs given data with given key
      * @param key 
      * @param data 
      */
-    async sign(key: CryptoKey, data: ArrayBuffer): Promise<ArrayBuffer> {
+    async sign(keyOptions: CryptoKeyOptions, key: CryptoKey, data: ArrayBuffer): Promise<ArrayBuffer> {
         return this.hasIECrypto() ?
-            this.msCryptoSign(key, data)
-            : window.crypto.subtle.sign(this._keygenAlgorithmOptions, key, data);
+            this.msCryptoSign(keyOptions, key, data)
+            : window.crypto.subtle.sign(keyOptions.keyGenAlgorithmOptions, key, data);
     }
 
     /**
@@ -166,9 +159,14 @@ export class BrowserCrypto {
      * @param extractable 
      * @param usages 
      */
-    private async msCryptoGenerateKey(extractable: boolean, usages: Array<KeyUsage>): Promise<CryptoKeyPair> {
+    private async msCryptoGenerateKey(keyOptions: CryptoKeyOptions, extractable: boolean): Promise<CryptoKeyPair> {
         return new Promise((resolve: Function, reject: Function) => {
-            const msGenerateKey = window["msCrypto"].subtle.generateKey(this._keygenAlgorithmOptions, extractable, usages);
+            const msGenerateKey = window["msCrypto"].subtle.generateKey(
+                keyOptions.keyGenAlgorithmOptions,
+                extractable,
+                keyOptions.keypairUsages
+            );
+
             msGenerateKey.addEventListener("complete", (e: { target: { result: CryptoKeyPair | PromiseLike<CryptoKeyPair>; }; }) => {
                 resolve(e.target.result);
             });
@@ -186,7 +184,7 @@ export class BrowserCrypto {
      */
     private async msCryptoExportJwk(key: CryptoKey): Promise<JsonWebKey> {
         return new Promise((resolve: Function, reject: Function) => {
-            const msExportKey = window["msCrypto"].subtle.exportKey(KEY_FORMAT_JWK, key);
+            const msExportKey = window["msCrypto"].subtle.exportKey(CryptoKeyFormats.jwk, key);
             msExportKey.addEventListener("complete", (e: { target: { result: ArrayBuffer; }; }) => {
                 const resultBuffer: ArrayBuffer = e.target.result;
 
@@ -217,9 +215,9 @@ export class BrowserCrypto {
      * @param extractable 
      * @param usages 
      */
-    private async msCryptoImportKey(keyBuffer: ArrayBuffer, extractable: boolean, usages: Array<KeyUsage>): Promise<CryptoKey> {
+    private async msCryptoImportKey(keyOptions: CryptoKeyOptions, keyBuffer: ArrayBuffer, extractable: boolean, usages: Array<KeyUsage>): Promise<CryptoKey> {
         return new Promise((resolve: Function, reject: Function) => {
-            const msImportKey = window["msCrypto"].subtle.importKey(KEY_FORMAT_JWK, keyBuffer, this._keygenAlgorithmOptions, extractable, usages);
+            const msImportKey = window["msCrypto"].subtle.importKey(CryptoKeyFormats.jwk, keyBuffer, keyOptions.keyGenAlgorithmOptions, extractable, usages);
             msImportKey.addEventListener("complete", (e: { target: { result: CryptoKey | PromiseLike<CryptoKey>; }; }) => {
                 resolve(e.target.result);
             });
@@ -235,9 +233,9 @@ export class BrowserCrypto {
      * @param key 
      * @param data 
      */
-    private async msCryptoSign(key: CryptoKey, data: ArrayBuffer): Promise<ArrayBuffer> {
+    private async msCryptoSign(keyOptions: CryptoKeyOptions, key: CryptoKey, data: ArrayBuffer): Promise<ArrayBuffer> {
         return new Promise((resolve: Function, reject: Function) => {
-            const msSign = window["msCrypto"].subtle.sign(this._keygenAlgorithmOptions, key, data);
+            const msSign = window["msCrypto"].subtle.sign(keyOptions, key, data);
             msSign.addEventListener("complete", (e: { target: { result: ArrayBuffer | PromiseLike<ArrayBuffer>; }; }) => {
                 resolve(e.target.result);
             });

diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts
@@ -3,14 +3,14 @@
  * Licensed under the MIT License.
  */
 
-import { ICrypto, Logger, PkceCodes, SignedHttpRequest, SignedHttpRequestParameters } from "@azure/msal-common";
+import { ICrypto, PkceCodes, SignedHttpRequest, SignedHttpRequestParameters, CryptoKeyTypes, Logger } from "@azure/msal-common";
 import { GuidGenerator } from "./GuidGenerator";
 import { Base64Encode } from "../encode/Base64Encode";
 import { Base64Decode } from "../encode/Base64Decode";
 import { PkceGenerator } from "./PkceGenerator";
-import { BrowserCrypto } from "./BrowserCrypto";
+import { BrowserCrypto, CryptoKeyOptions } from "./BrowserCrypto";
 import { BrowserStringUtils } from "../utils/BrowserStringUtils";
-import { KEY_FORMAT_JWK } from "../utils/BrowserConstants";
+import { CryptoKeyFormats, CryptoKeyConfig } from "../utils/CryptoConstants";
 import { BrowserAuthError } from "../error/BrowserAuthError";
 import { AsyncMemoryStorage } from "../cache/AsyncMemoryStorage";
 
@@ -42,7 +42,6 @@ export class CryptoOps implements ICrypto {
     private pkceGenerator: PkceGenerator;
     private logger: Logger;
 
-    private static POP_KEY_USAGES: Array<KeyUsage> = ["sign", "verify"];
     private static EXTRACTABLE: boolean = true;
     private cache: CryptoKeyStore;
 
@@ -91,13 +90,43 @@ export class CryptoOps implements ICrypto {
         return this.pkceGenerator.generateCodes();
     }
 
+    /**
+     * Helper method that wraps a generateKeyPair call in a try/catch block
+     * so errors thrown inside generate key pair can be handled upstream
+     * @param keyOptions
+     */
+    private async generateKeyPairHelper(keyOptions: CryptoKeyOptions): Promise<CryptoKeyPair> {
+        // Attempt to generate Keypair
+        try {
+            return await this.browserCrypto.generateKeyPair(keyOptions, CryptoOps.EXTRACTABLE);
+        } catch (error) {
+            // Throw if key could not be generated
+            const errorMessage = (error instanceof Error) ? error.message : undefined;
+            throw BrowserAuthError.createKeyGenerationFailedError(errorMessage);
+        }
+    }
+
     /**
      * Generates a keypair, stores it and returns a thumbprint
      * @param request
      */
-    async getPublicKeyThumbprint(request: SignedHttpRequestParameters): Promise<string> {
-        // Generate Keypair
-        const keyPair: CryptoKeyPair = await this.browserCrypto.generateKeyPair(CryptoOps.EXTRACTABLE, CryptoOps.POP_KEY_USAGES);
+    async getPublicKeyThumbprint(request: SignedHttpRequestParameters, keyType?: CryptoKeyTypes): Promise<string> {
+        this.logger.verbose(`getPublicKeyThumbprint called to generate a cryptographic keypair of type ${keyType}`);        
+        const keyOptions: CryptoKeyOptions = keyType === CryptoKeyTypes.StkJwk ? CryptoKeyConfig.RefreshTokenBinding : CryptoKeyConfig.AccessTokenBinding;
+
+        // Attempt to generate keypair, helper makes sure to throw if generation fails
+        const keyPair: CryptoKeyPair = await this.generateKeyPairHelper(keyOptions);
+
+        /**
+         * This check should never evaluate to true because the helper above handles key generation
+         * errors, but TypeScript requires that the public and private key values are checked because
+         * the CryptoKeyPair type lists them as optional.
+         */
+        if (!keyPair || !keyPair.publicKey || !keyPair.privateKey) {
+            throw BrowserAuthError.createKeyGenerationFailedError("Either the public or private key component is missing from the generated CryptoKeyPair");
+        }
+
+        this.logger.verbose(`Successfully generated ${keyType} keypair`);
 
         // Generate Thumbprint for Public Key
         const publicKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.publicKey);
@@ -114,8 +143,8 @@ export class CryptoOps implements ICrypto {
         // Generate Thumbprint for Private Key
         const privateKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.privateKey);
         // Re-import private key to make it unextractable
-        const unextractablePrivateKey: CryptoKey = await this.browserCrypto.importJwk(privateKeyJwk, false, ["sign"]);
-
+        const unextractablePrivateKey: CryptoKey = await this.browserCrypto.importJwk(keyOptions, privateKeyJwk, false, keyOptions.privateKeyUsage);
+        this.logger.verbose(`Caching ${keyType} keypair`);
         // Store Keypair data in keystore
         await this.cache.asymmetricKeys.setItem(
             publicJwkHash, 
@@ -126,7 +155,7 @@ export class CryptoOps implements ICrypto {
                 requestUri: request.resourceRequestUri
             }
         );
-
+        
         return publicJwkHash;
     }
 
@@ -158,7 +187,7 @@ export class CryptoOps implements ICrypto {
         const cachedKeyPair = await this.cache.asymmetricKeys.getItem(kid);
 
         if (!cachedKeyPair) {
-            throw BrowserAuthError.createSigningKeyNotFoundInStorageError(kid);
+            throw BrowserAuthError.createSigningKeyNotFoundInStorageError();
         }
 
         // Get public key as JWK
@@ -168,7 +197,7 @@ export class CryptoOps implements ICrypto {
         // Generate header
         const header = {
             alg: publicKeyJwk.alg,
-            type: KEY_FORMAT_JWK
+            type: CryptoKeyFormats.jwk
         };
         const encodedHeader = this.b64Encode.urlEncode(JSON.stringify(header));
 
@@ -183,7 +212,7 @@ export class CryptoOps implements ICrypto {
 
         // Sign token
         const tokenBuffer = BrowserStringUtils.stringToArrayBuffer(tokenString);
-        const signatureBuffer = await this.browserCrypto.sign(cachedKeyPair.privateKey, tokenBuffer);
+        const signatureBuffer = await this.browserCrypto.sign(CryptoKeyConfig.AccessTokenBinding, cachedKeyPair.privateKey, tokenBuffer);
         const encodedSignature = this.b64Encode.urlEncodeArr(new Uint8Array(signatureBuffer));
 
         return `${tokenString}.${encodedSignature}`;
@@ -198,4 +227,27 @@ export class CryptoOps implements ICrypto {
         const hashBytes = new Uint8Array(hashBuffer);
         return this.b64Encode.urlEncodeArr(hashBytes);
     }
+
+    /**
+     * Returns the public key from an asymmetric key pair stored in IndexedDB based on the
+     * public key thumbprint parameter
+     * @param keyThumbprint 
+     * @returns Public Key JWK string
+     */
+    async getAsymmetricPublicKey(keyThumbprint: string): Promise<string> {
+        this.logger.verbose("getAsymmetricPublicKey was called, retrieving requested keypair");
+        const cachedKeyPair = await this.cache.asymmetricKeys.getItem(keyThumbprint);
+
+        if (!cachedKeyPair) {
+            throw BrowserAuthError.createSigningKeyNotFoundInStorageError();
+        }
+
+        this.logger.verbose("Successfully retrieved cached keypair from storage, exporting public key component");
+
+        // Get public key as JWK
+        const publicKeyJwk = await this.browserCrypto.exportJwk(cachedKeyPair.publicKey);
+
+        this.logger.verbose("Successfully exported public key as JSON Web Key, generating JWK string");
+        return BrowserCrypto.getJwkString(publicKeyJwk);
+    }
 }
diff --git a/lib/msal-browser/src/crypto/SignedHttpRequest.ts b/lib/msal-browser/src/crypto/SignedHttpRequest.ts
@@ -4,7 +4,7 @@
  */
 
 import { CryptoOps } from "./CryptoOps";
-import { Logger, LoggerOptions, PopTokenGenerator, SignedHttpRequestParameters } from "@azure/msal-common";
+import { CryptoKeyTypes, Logger, LoggerOptions, PopTokenGenerator, SignedHttpRequestParameters } from "@azure/msal-common";
 import { version, name } from "../packageMetadata";
 
 export type SignedHttpRequestOptions = {
@@ -30,7 +30,7 @@ export class SignedHttpRequest {
      * @returns Public key digest, which should be sent to the token issuer.
      */
     async generatePublicKeyThumbprint(): Promise<string> {
-        const { kid } = await this.popTokenGenerator.generateKid(this.shrParameters);
+        const { kid } = await this.popTokenGenerator.generateKid(this.shrParameters, CryptoKeyTypes.ReqCnf);
 
         return kid;
     }