refactor: remove useless hmac check, derive key for cbc and rename some variables

Author: RomainLanz

src/base64.ts

@@ -0,0 +1,33 @@
+/*
+ * @boringnode/encryption
+ *
+ * @license MIT
+ * @copyright Boring Node
+ */
+
+export function base64UrlEncode(
+  data: ArrayBuffer | SharedArrayBuffer | Uint8Array | Buffer
+): string {
+  const buffer = Buffer.from(data as any)
+
+  return buffer.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '')
+}
+
+export function base64UrlDecode(encoded: string): Buffer
+export function base64UrlDecode(encoded: string, encoding: BufferEncoding): string
+export function base64UrlDecode(
+  encoded: string,
+  encoding?: BufferEncoding
+): Buffer | string | null {
+  const padded = encoded
+    .replace(/-/g, '+')
+    .replace(/_/g, '/')
+    .padEnd(Math.ceil(encoded.length / 4) * 4, '=')
+
+  try {
+    const buffer = Buffer.from(padded, 'base64')
+    return encoding ? buffer.toString(encoding) : buffer
+  } catch {
+    return null
+  }
+}

src/drivers/aes_256_cbc.ts

@@ -5,12 +5,13 @@
  * @copyright Boring Node
  */
 
-import { createCipheriv, createDecipheriv, randomBytes } from 'node:crypto'
+import { createCipheriv, createDecipheriv, hkdfSync, randomBytes } from 'node:crypto'
 import { MessageBuilder } from '@poppinss/utils'
 import { BaseDriver } from './base_driver.js'
 import { Hmac } from '../hmac.js'
 import * as errors from '../exceptions.js'
-import type { AES256CBCConfig, EncryptionDriverContract } from '../types/main.js'
+import type { AES256CBCConfig, CypherText, EncryptionDriverContract } from '../types/main.js'
+import { base64UrlDecode, base64UrlEncode } from '../base64.js'
 
 export class AES256CBC extends BaseDriver implements EncryptionDriverContract {
   #config: AES256CBCConfig
@@ -39,39 +40,41 @@ export class AES256CBC extends BaseDriver implements EncryptionDriverContract {
    * You can optionally define a purpose for which the value was encrypted and
    * mentioning a different purpose/no purpose during decrypt will fail.
    */
-  encrypt(payload: any, expiresIn?: string | number, purpose?: string): string {
+  encrypt(payload: any, expiresIn?: string | number, purpose?: string): CypherText {
     /**
      * Using a random string as the iv for generating unpredictable values
      */
     const iv = randomBytes(16)
 
+    const { encryptionKey, authenticationKey } = this.#deriveKey(this.getFirstKey().key, iv)
+
     /**
      * Creating chiper
      */
-    const cipher = createCipheriv('aes-256-cbc', this.getFirstKey().key, iv)
+    const cipher = createCipheriv('aes-256-cbc', encryptionKey, iv)
 
     /**
      * Encoding value to a string so that we can set it on the cipher
      */
-    const encodedValue = new MessageBuilder().build(payload, expiresIn, purpose)
+    const plainText = new MessageBuilder().build(payload, expiresIn, purpose)
 
     /**
      * Set final to the cipher instance and encrypt it
      */
-    const encrypted = Buffer.concat([cipher.update(encodedValue, 'utf-8'), cipher.final()])
+    const cipherText = Buffer.concat([cipher.update(plainText), cipher.final()])
 
     /**
      * Concatenate `encrypted value` and `iv` by urlEncoding them. The concatenation is required
      * to generate the HMAC, so that HMAC checks for integrity of both the `encrypted value`
      * and the `iv`.
      */
-    const result = `${encrypted.toString('hex')}${this.separator}${iv.toString('hex')}`
+    const macPayload = `${base64UrlEncode(cipherText)}${this.separator}${base64UrlEncode(iv)}`
 
     /**
      * Returns the id + result + hmac
      */
-    const hmac = new Hmac(this.getFirstKey().key).generate(result)
-    return this.computeReturns([this.#config.id, result, hmac])
+    const hmac = new Hmac(authenticationKey).generate(macPayload)
+    return this.computeReturns([this.#config.id, macPayload, hmac])
   }
 
   /**
@@ -83,11 +86,11 @@ export class AES256CBC extends BaseDriver implements EncryptionDriverContract {
     }
 
     /**
-     * Make sure the encrypted value is in correct format. ie
-     * [id].[encrypted value].[iv].[hash]
+     * Make sure the encrypted value is in the correct format.
+     * i.e.: [id].[encrypted value].[iv].[mac]
      */
-    const [id, encryptedEncoded, ivEncoded, hash] = value.split(this.separator)
-    if (!id || !encryptedEncoded || !ivEncoded || !hash) {
+    const [id, cipherEncoded, ivEncoded, macEncoded] = value.split(this.separator)
+    if (!id || !cipherEncoded || !ivEncoded || !macEncoded) {
       return null
     }
 
@@ -101,15 +104,15 @@ export class AES256CBC extends BaseDriver implements EncryptionDriverContract {
     /**
      * Make sure we are able to decode the encrypted value
      */
-    const encrypted = Buffer.from(encryptedEncoded, 'hex')
-    if (!encrypted) {
+    const cipherText = base64UrlDecode(cipherEncoded)
+    if (!cipherText) {
       return null
     }
 
     /**
      * Make sure we are able to decode the iv
      */
-    const iv = Buffer.from(ivEncoded, 'hex')
+    const iv = base64UrlDecode(ivEncoded)
     if (!iv) {
       return null
     }
@@ -119,9 +122,11 @@ export class AES256CBC extends BaseDriver implements EncryptionDriverContract {
      * string are not tampered.
      */
     for (const { key } of this.cryptoKeys) {
-      const isValidHmac = new Hmac(key).compare(
-        `${encryptedEncoded}${this.separator}${ivEncoded}`,
-        hash
+      const { encryptionKey, authenticationKey } = this.#deriveKey(key, iv)
+
+      const isValidHmac = new Hmac(authenticationKey).compare(
+        `${cipherEncoded}${this.separator}${ivEncoded}`,
+        macEncoded
       )
 
       if (!isValidHmac) {
@@ -133,12 +138,24 @@ export class AES256CBC extends BaseDriver implements EncryptionDriverContract {
        * to avoid leaking sensitive information
        */
       try {
-        const decipher = createDecipheriv('aes-256-cbc', key, iv)
-        const decrypted = decipher.update(encrypted) + decipher.final('utf8')
-        return new MessageBuilder().verify(decrypted, purpose)
+        const decipher = createDecipheriv('aes-256-cbc', encryptionKey, iv)
+        const plainTextBuffer = Buffer.concat([decipher.update(cipherText), decipher.final()])
+        return new MessageBuilder().verify(plainTextBuffer.toString('utf-8'), purpose)
       } catch {}
     }
 
     return null
   }
+
+  #deriveKey(masterKey: Buffer, iv: Buffer) {
+    const info = Buffer.from(this.#config.id)
+    const rawDerivedKey = hkdfSync('sha256', masterKey, iv, info, 64)
+
+    const derivedKey = Buffer.isBuffer(rawDerivedKey) ? rawDerivedKey : Buffer.from(rawDerivedKey)
+
+    return {
+      encryptionKey: derivedKey.subarray(0, 32),
+      authenticationKey: derivedKey.subarray(32),
+    }
+  }
 }

src/drivers/aes_256_gcm.ts

@@ -8,9 +8,9 @@
 import { createCipheriv, createDecipheriv, randomBytes } from 'node:crypto'
 import { MessageBuilder } from '@poppinss/utils'
 import { BaseDriver } from './base_driver.js'
-import { Hmac } from '../hmac.js'
 import * as errors from '../exceptions.js'
-import type { AES256GCMConfig, EncryptionDriverContract } from '../types/main.js'
+import { base64UrlDecode, base64UrlEncode } from '../base64.js'
+import type { AES256GCMConfig, CypherText, EncryptionDriverContract } from '../types/main.js'
 
 export class AES256GCM extends BaseDriver implements EncryptionDriverContract {
   #config: AES256GCMConfig
@@ -39,11 +39,11 @@ export class AES256GCM extends BaseDriver implements EncryptionDriverContract {
    * You can optionally define a purpose for which the value was encrypted and
    * mentioning a different purpose/no purpose during decrypt will fail.
    */
-  encrypt(payload: any, expiresIn?: string | number, purpose?: string): string {
+  encrypt(payload: any, expiresIn?: string | number, purpose?: string): CypherText {
     /**
      * Using a random string as the iv for generating unpredictable values
      */
-    const iv = randomBytes(16)
+    const iv = randomBytes(12)
 
     /**
      * Creating chiper
@@ -57,27 +57,21 @@ export class AES256GCM extends BaseDriver implements EncryptionDriverContract {
     /**
      * Encoding value to a string so that we can set it on the cipher
      */
-    const encodedValue = new MessageBuilder().build(payload, expiresIn)
+    const plainText = new MessageBuilder().build(payload, expiresIn)
 
     /**
      * Set final to the cipher instance and encrypt it
      */
-    const encrypted = Buffer.concat([cipher.update(encodedValue, 'utf-8'), cipher.final()])
+    const cipherText = Buffer.concat([cipher.update(plainText), cipher.final()])
 
-    /**
-     * Concatenate `encrypted value` and `iv` by urlEncoding them. The concatenation is required
-     * to generate the HMAC, so that HMAC checks for integrity of both the `encrypted value`
-     * and the `iv`.
-     */
-    const result = `${encrypted.toString('hex')}${this.separator}${iv.toString('hex')}`
-
-    const nounce = cipher.getAuthTag().toString('hex')
+    const tag = cipher.getAuthTag()
 
-    /**
-     * Returns the id + result + nounce + hmac
-     */
-    const hmac = new Hmac(this.getFirstKey().key).generate(result)
-    return this.computeReturns([this.#config.id, result, nounce, hmac])
+    return this.computeReturns([
+      this.#config.id,
+      base64UrlEncode(cipherText),
+      base64UrlEncode(iv),
+      base64UrlEncode(tag),
+    ])
   }
 
   /**
@@ -89,11 +83,11 @@ export class AES256GCM extends BaseDriver implements EncryptionDriverContract {
     }
 
     /**
-     * Make sure the encrypted value is in correct format. ie
-     * [id].[encrypted value].[iv].[nounce].[hash]
+     * Make sure the encrypted value is in the correct format.
+     * i.e.: [id].[encrypted value].[iv].[tag]
      */
-    const [id, encryptedEncoded, ivEncoded, nounceEncoded, hash] = value.split(this.separator)
-    if (!id || !encryptedEncoded || !ivEncoded || !nounceEncoded || !hash) {
+    const [id, cipherEncoded, ivEncoded, tagEncoded] = value.split(this.separator)
+    if (!id || !cipherEncoded || !ivEncoded || !tagEncoded) {
       return null
     }
 
@@ -107,62 +101,43 @@ export class AES256GCM extends BaseDriver implements EncryptionDriverContract {
     /**
      * Make sure we are able to decode the encrypted value
      */
-    const encrypted = Buffer.from(encryptedEncoded, 'hex')
-    if (!encrypted) {
+    const cipherText = base64UrlDecode(cipherEncoded)
+    if (!cipherText) {
       return null
     }
 
     /**
      * Make sure we are able to decode the iv
      */
-    const iv = Buffer.from(ivEncoded, 'hex')
+    const iv = base64UrlDecode(ivEncoded)
     if (!iv) {
       return null
     }
 
     /**
-     * Make sure we are able to decode the nounce
+     * Make sure we are able to decode the tag
      */
-    const nounce = Buffer.from(nounceEncoded, 'hex')
-    if (!nounce) {
+    const tag = base64UrlDecode(tagEncoded)
+    if (!tag) {
       return null
     }
 
-    /**
-     * Make sure the hash is correct, it means the first 2 parts of the
-     * string are not tampered.
-     */
     for (const { key } of this.cryptoKeys) {
-      const isValidHmac = new Hmac(key).compare(
-        `${encryptedEncoded}${this.separator}${ivEncoded}`,
-        hash
-      )
-
-      if (!isValidHmac) {
-        continue
-      }
-
       /**
        * The Decipher can raise exceptions with malformed input, so we wrap it
        * to avoid leaking sensitive information
        */
       try {
         const decipher = createDecipheriv('aes-256-gcm', key, iv)
 
-        /**
-         * Set the purpose to decipher
-         */
         if (purpose) {
           decipher.setAAD(Buffer.from(purpose), { plaintextLength: Buffer.byteLength(purpose) })
         }
 
-        /**
-         * Set the nounce
-         */
-        decipher.setAuthTag(nounce)
+        decipher.setAuthTag(tag)
 
-        const decrypted = decipher.update(encrypted) + decipher.final('utf8')
-        return new MessageBuilder().verify(decrypted)
+        const plain = Buffer.concat([decipher.update(cipherText), decipher.final()])
+        return new MessageBuilder().verify(plain)
       } catch {}
     }
 

src/drivers/base_driver.ts

@@ -8,7 +8,7 @@
 import { createHash } from 'node:crypto'
 import * as errors from '../exceptions.js'
 import { MessageVerifier } from '../message_verifier.js'
-import type { BaseConfig } from '../types/main.js'
+import type { BaseConfig, CypherText } from '../types/main.js'
 
 export abstract class BaseDriver {
   /**
@@ -50,7 +50,7 @@ export abstract class BaseDriver {
   }
 
   computeReturns(values: string[]) {
-    return values.join(this.separator)
+    return values.join(this.separator) as CypherText
   }
 
   getFirstKey() {
@@ -72,7 +72,7 @@ export abstract class BaseDriver {
    * You can optionally define a purpose for which the value was encrypted and
    * mentioning a different purpose/no purpose during decrypt will fail.
    */
-  abstract encrypt(payload: any, expiresIn?: string | number, purpose?: string): string
+  abstract encrypt(payload: any, expiresIn?: string | number, purpose?: string): CypherText
 
   /**
    * Decrypt value and verify it against a purpose