fix(kyc-controller): Fix wrapped/encrypted key and token

packages/kyc-controller/CHANGELOG.md

@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Stop incorrectly prefixing the session client public key onto wrapped `encryptionDataKey` and `ukycCapabilityToken` values from `wrapEncryptionKey`. `data` is now ciphertext+tag only; the server already has the client public key from session creation. ([#10036](https://github.com/MetaMask/core/pull/10036))
 - Clear leftover MoonPay `sessionToken`, `accessToken`, and Check/Auth frame credentials when `initialize` or `createVendorCustomer` switches to another vendor, so `buildCheckFrameUrl` cannot return a MoonPay URL for a consents-path session. ([#9908](https://github.com/MetaMask/core/pull/9908))
 - Rewind the consents path when SumSub fails before completion (thrown step or SDK close without `Completed`), instead of refreshing user status and forcing `phase` to `done`. A terminal UKYC rejection after the SDK completed still finishes as `done` so the decision can be reflected in user status. ([#9908](https://github.com/MetaMask/core/pull/9908))
 - Make `createVendorCustomer` a no-op during in-progress phases (matching `initialize`), so a vendor switch cannot leave Check/Auth frames attached to the wrong vendor. ([#9908](https://github.com/MetaMask/core/pull/9908))

packages/kyc-controller/src/ukyc/wrapEncryptionKey.test.ts

@@ -7,26 +7,24 @@ import { wrapEncryptionKey } from './wrapEncryptionKey.js';
 const DATA_ENCRYPTION_KEY = new Uint8Array(32).fill(7);
 
 /**
- * Reverses {@link wrapEncryptionKey} from the server's perspective: reads the
- * client public key from the first 32 bytes of `data` and opens the NaCl box
- * with the server private key. The client also registers this public key on
- * `createUkycSession`.
+ * Reverses {@link wrapEncryptionKey} from the server's perspective: opens the
+ * NaCl box with the server private key and the client public key previously
+ * registered on `createUkycSession`.
  *
  * @param serverPrivateKey - The server's X25519 private key.
- * @param data - The base64url `clientPublicKey || ciphertext+tag`.
+ * @param clientPublicKey - The client's X25519 public key from session creation.
+ * @param data - The base64url ciphertext+tag.
  * @param nonce - The base64url nonce.
  * @returns The recovered plaintext.
  */
 function unwrap(
   serverPrivateKey: Uint8Array,
+  clientPublicKey: Uint8Array,
   data: string,
   nonce: string,
 ): Uint8Array {
-  const packed = base64UrlToBytes(data);
-  const clientPublicKey = packed.slice(0, box.publicKeyLength);
-  const ciphertext = packed.slice(box.publicKeyLength);
   const recovered = box.open(
-    ciphertext,
+    base64UrlToBytes(data),
     base64UrlToBytes(nonce),
     clientPublicKey,
     serverPrivateKey,
@@ -38,7 +36,7 @@ function unwrap(
 }
 
 describe('UKYC wrapEncryptionKey', () => {
-  it('wraps a key the session server can recover using only the packed data', () => {
+  it('wraps a key the session server can recover with the registered client public key', () => {
     const serverKeyPair = box.keyPair();
     const clientKeyPair = box.keyPair();
 
@@ -48,11 +46,16 @@ describe('UKYC wrapEncryptionKey', () => {
       DATA_ENCRYPTION_KEY,
     );
 
-    const recovered = unwrap(serverKeyPair.secretKey, data, nonce);
+    const recovered = unwrap(
+      serverKeyPair.secretKey,
+      clientKeyPair.publicKey,
+      data,
+      nonce,
+    );
     expect(areUint8ArraysEqual(recovered, DATA_ENCRYPTION_KEY)).toBe(true);
   });
 
-  it('prefixes the client public key onto data so the server can open the box', () => {
+  it('emits ciphertext-only data without embedding the client public key', () => {
     const serverKeyPair = box.keyPair();
     const clientKeyPair = box.keyPair();
 
@@ -63,14 +66,15 @@ describe('UKYC wrapEncryptionKey', () => {
     );
 
     const packed = base64UrlToBytes(data);
-    expect(packed.slice(0, box.publicKeyLength)).toStrictEqual(
+    expect(packed.slice(0, box.publicKeyLength)).not.toStrictEqual(
       clientKeyPair.publicKey,
     );
   });
 
-  it('cannot be opened if data is treated as ciphertext with no embedded public key', () => {
+  it('cannot be opened with the wrong client public key', () => {
     const serverKeyPair = box.keyPair();
     const clientKeyPair = box.keyPair();
+    const otherClientKeyPair = box.keyPair();
 
     const { data, nonce } = wrapEncryptionKey(
       clientKeyPair.secretKey,
@@ -81,7 +85,7 @@ describe('UKYC wrapEncryptionKey', () => {
     const opened = box.open(
       base64UrlToBytes(data),
       base64UrlToBytes(nonce),
-      clientKeyPair.publicKey,
+      otherClientKeyPair.publicKey,
       serverKeyPair.secretKey,
     );
     expect(opened).toBeNull();
@@ -112,7 +116,12 @@ describe('UKYC wrapEncryptionKey', () => {
       tokenBytes,
     );
 
-    const recovered = unwrap(serverKeyPair.secretKey, data, nonce);
+    const recovered = unwrap(
+      serverKeyPair.secretKey,
+      clientKeyPair.publicKey,
+      data,
+      nonce,
+    );
     expect(areUint8ArraysEqual(recovered, tokenBytes)).toBe(true);
   });
 

packages/kyc-controller/src/ukyc/wrapEncryptionKey.ts

@@ -8,16 +8,15 @@ import { base64UrlToBytes, toBase64Url } from '../encoding.js';
  * returned inside an encryption schema from `createUkycSession`.
  *
  * Reuses a session client keypair whose public half is registered on
- * `createUkycSession`. `data` is still `clientPublicKey (32) || ciphertext+tag`
- * so the box can be opened from `{ data, nonce }` alone. Used for both the
+ * `createUkycSession`. `data` is the ciphertext+tag only; the server already
+ * knows the client public key from session creation. Used for both the
  * `data_encryption_key` and the `ukyc_capability_token`.
  */
 
 /**
- * The transmitted portion of a wrapped secret: the sender public key prefixed
- * to the `crypto_box` ciphertext (which includes the 16-byte Poly1305 auth
- * tag), and the nonce, both unpadded base64url-encoded. Matches the KYC API
- * `CapabilityAuthorization` wire shape.
+ * The transmitted portion of a wrapped secret: the `crypto_box` ciphertext
+ * (which includes the 16-byte Poly1305 auth tag) and the nonce, both unpadded
+ * base64url-encoded. Matches the KYC API `CapabilityAuthorization` wire shape.
  */
 export type WrappedEncryptionKeyParts = {
   data: string;
@@ -30,37 +29,30 @@ export type WrappedEncryptionKeyParts = {
  * The box is sealed with NaCl's `crypto_box`, keyed by the X25519 shared secret
  * between our session client private key and the session server public key
  * from an encryption schema (`encryptionDataKey` or `ukycCapabilityToken`)
- * returned by `createUkycSession`. The 32-byte client public key is still
- * prefixed onto `data` so the box is self-describing on the wire.
+ * returned by `createUkycSession`. The client public key is not embedded in
+ * `data`; it was already registered on `createUkycSession`.
  *
  * @param sessionClientPrivateKey - Our session's X25519 private key.
  * @param sessionServerPublicKey - The server's X25519 public key (base64url).
  * @param plaintext - The raw bytes to encrypt (e.g. the `data_encryption_key`
  * or the encoded `ukyc_capability_token`).
- * @returns The base64url `data` (`clientPublicKey || ciphertext+tag`) and
- * `nonce`.
+ * @returns The base64url `data` (ciphertext+tag) and `nonce`.
  */
 export function wrapEncryptionKey(
   sessionClientPrivateKey: Uint8Array,
   sessionServerPublicKey: string,
   plaintext: Uint8Array,
 ): WrappedEncryptionKeyParts {
   const serverPublicKey = base64UrlToBytes(sessionServerPublicKey);
-  const { publicKey: clientPublicKey } = box.keyPair.fromSecretKey(
-    sessionClientPrivateKey,
-  );
   const nonce = randomBytes(box.nonceLength);
   const ciphertext = box(
     plaintext,
     nonce,
     serverPublicKey,
     sessionClientPrivateKey,
   );
-  const data = new Uint8Array(clientPublicKey.length + ciphertext.length);
-  data.set(clientPublicKey, 0);
-  data.set(ciphertext, clientPublicKey.length);
   return {
-    data: toBase64Url(data),
+    data: toBase64Url(ciphertext),
     nonce: toBase64Url(nonce),
   };
 }