Merge pull request #375 from signalapp/jrose/remove-hkdf-versions

Remove support for HKDF "versions"

java/java/src/main/java/org/signal/client/internal/Native.java

@@ -149,7 +149,7 @@ private Native() {}
   public static native long GroupSessionBuilder_CreateSenderKeyDistributionMessage(long sender, UUID distributionId, SenderKeyStore store, Object ctx);
   public static native void GroupSessionBuilder_ProcessSenderKeyDistributionMessage(long sender, long senderKeyDistributionMessage, SenderKeyStore store, Object ctx);
 
-  public static native byte[] HKDF_DeriveSecrets(int outputLength, int version, byte[] ikm, byte[] label, byte[] salt);
+  public static native byte[] HKDF_DeriveSecrets(int outputLength, byte[] ikm, byte[] label, byte[] salt);
 
   public static native void HsmEnclaveClient_CompleteHandshake(long cli, byte[] handshakeReceived);
   public static native void HsmEnclaveClient_Destroy(long handle);

java/java/src/main/java/org/whispersystems/libsignal/kdf/HKDF.java

@@ -9,24 +9,11 @@
 import org.signal.client.internal.Native;
 
 public abstract class HKDF {
-  private static final int HASH_OUTPUT_SIZE  = 32;
-
-  public static HKDF createFor(int messageVersion) {
-    switch (messageVersion) {
-      case 2:  return new HKDFv2();
-      case 3:  return new HKDFv3();
-      default: throw new AssertionError("Unknown version: " + messageVersion);
-    }
-  }
-
-  public byte[] deriveSecrets(byte[] inputKeyMaterial, byte[] info, int outputLength) {
-    return Native.HKDF_DeriveSecrets(outputLength, getVersion(), inputKeyMaterial, info, null);
+  public static byte[] deriveSecrets(byte[] inputKeyMaterial, byte[] info, int outputLength) {
+    return Native.HKDF_DeriveSecrets(outputLength, inputKeyMaterial, info, null);
   }
 
-  public byte[] deriveSecrets(byte[] inputKeyMaterial, byte[] salt, byte[] info, int outputLength) {
-    return Native.HKDF_DeriveSecrets(outputLength, getVersion(), inputKeyMaterial, info, salt);
+  public static byte[] deriveSecrets(byte[] inputKeyMaterial, byte[] salt, byte[] info, int outputLength) {
+    return Native.HKDF_DeriveSecrets(outputLength, inputKeyMaterial, info, salt);
   }
-
-  protected abstract int getVersion();
-
 }

java/java/src/main/java/org/whispersystems/libsignal/kdf/HKDFv3.java

@@ -5,9 +5,9 @@
  */
 package org.whispersystems.libsignal.kdf;
 
+/**
+ * @deprecated Use the static methods of {@link HKDF} instead.
+ */
+@Deprecated
 public class HKDFv3 extends HKDF {
-  @Override
-  protected int getVersion() {
-    return 3;
-  }
 }

java/tests/src/test/java/org/whispersystems/libsignal/kdf/HKDFTest.java

@@ -27,7 +27,7 @@ public void testVectorV3() {
                   (byte) 0x08, (byte) 0xd5, (byte) 0xb8, (byte) 0x87, (byte) 0x18,
                   (byte) 0x58, (byte) 0x65};
 
-    byte[] actualOutput = HKDF.createFor(3).deriveSecrets(ikm, salt, info, 42);
+    byte[] actualOutput = HKDF.deriveSecrets(ikm, salt, info, 42);
 
     assertTrue(Arrays.equals(okm, actualOutput));
   }
@@ -102,36 +102,7 @@ public void testVectorLongV3() {
                    (byte) 0xd5, (byte) 0xc1, (byte) 0xf3, (byte) 0x43, (byte) 0x4f,
                    (byte) 0x1d, (byte) 0x87};
 
-    byte[] actualOutput = HKDF.createFor(3).deriveSecrets(ikm, salt, info, 82);
-    assertTrue(Arrays.equals(okm, actualOutput));
-  }
-
-  public void testVectorV2() {
-    byte[] ikm = {0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,
-                  0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,
-                  0x0b, 0x0b};
-
-    byte[] salt = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09,
-                   0x0a, 0x0b, 0x0c};
-
-    byte[] info = {(byte)0xf0, (byte)0xf1, (byte)0xf2, (byte)0xf3, (byte)0xf4,
-                   (byte)0xf5, (byte)0xf6, (byte)0xf7, (byte)0xf8, (byte)0xf9};
-
-    byte[] okm = {(byte)0x6e, (byte)0xc2, (byte)0x55, (byte)0x6d, (byte)0x5d,
-                  (byte)0x7b, (byte)0x1d, (byte)0x81, (byte)0xde, (byte)0xe4,
-                  (byte)0x22, (byte)0x2a, (byte)0xd7, (byte)0x48, (byte)0x36,
-                  (byte)0x95, (byte)0xdd, (byte)0xc9, (byte)0x8f, (byte)0x4f,
-                  (byte)0x5f, (byte)0xab, (byte)0xc0, (byte)0xe0, (byte)0x20,
-                  (byte)0x5d, (byte)0xc2, (byte)0xef, (byte)0x87, (byte)0x52,
-                  (byte)0xd4, (byte)0x1e, (byte)0x04, (byte)0xe2, (byte)0xe2,
-                  (byte)0x11, (byte)0x01, (byte)0xc6, (byte)0x8f, (byte)0xf0,
-                  (byte)0x93, (byte)0x94, (byte)0xb8, (byte)0xad, (byte)0x0b,
-                  (byte)0xdc, (byte)0xb9, (byte)0x60, (byte)0x9c, (byte)0xd4,
-                  (byte)0xee, (byte)0x82, (byte)0xac, (byte)0x13, (byte)0x19,
-                  (byte)0x9b, (byte)0x4a, (byte)0xa9, (byte)0xfd, (byte)0xa8,
-                  (byte)0x99, (byte)0xda, (byte)0xeb, (byte)0xec};
-
-    byte[] actualOutput = HKDF.createFor(2).deriveSecrets(ikm, salt, info, 64);
+    byte[] actualOutput = HKDF.deriveSecrets(ikm, salt, info, 82);
     assertTrue(Arrays.equals(okm, actualOutput));
   }
 
@@ -153,8 +124,8 @@ public void testNullInfo() {
       (byte) 0x08, (byte) 0xd5, (byte) 0xb8, (byte) 0x87, (byte) 0x18,
       (byte) 0x58, (byte) 0x65};
 
-    byte[] outputWithNull = HKDF.createFor(3).deriveSecrets(ikm, salt, null, 42);
-    byte[] outputWithEmpty = HKDF.createFor(3).deriveSecrets(ikm, salt, new byte[] {}, 42);
+    byte[] outputWithNull = HKDF.deriveSecrets(ikm, salt, null, 42);
+    byte[] outputWithEmpty = HKDF.deriveSecrets(ikm, salt, new byte[] {}, 42);
 
     assertTrue(Arrays.equals(outputWithNull, outputWithEmpty));
   }

node/Native.d.ts

@@ -61,7 +61,7 @@ export function Fingerprint_New(iterations: number, version: number, localIdenti
 export function Fingerprint_ScannableEncoding(obj: Wrapper<Fingerprint>): Buffer;
 export function GroupCipher_DecryptMessage(sender: Wrapper<ProtocolAddress>, message: Buffer, store: SenderKeyStore, ctx: null): Promise<Buffer>;
 export function GroupCipher_EncryptMessage(sender: Wrapper<ProtocolAddress>, distributionId: Uuid, message: Buffer, store: SenderKeyStore, ctx: null): Promise<CiphertextMessage>;
-export function HKDF_DeriveSecrets(outputLength: number, version: number, ikm: Buffer, label: Buffer | null, salt: Buffer | null): Buffer;
+export function HKDF_DeriveSecrets(outputLength: number, ikm: Buffer, label: Buffer | null, salt: Buffer | null): Buffer;
 export function IdentityKeyPair_Serialize(publicKey: Wrapper<PublicKey>, privateKey: Wrapper<PrivateKey>): Buffer;
 export function PlaintextContent_Deserialize(buffer: Buffer): PlaintextContent;
 export function PlaintextContent_FromDecryptionErrorMessage(m: Wrapper<DecryptionErrorMessage>): PlaintextContent;

node/index.ts

@@ -45,14 +45,14 @@ export const enum ContentHint {
 export type Uuid = string;
 
 export class HKDF {
-  private readonly version: number;
-
-  private constructor(version: number) {
-    this.version = version;
-  }
-
+  /**
+   * @deprecated Use the top-level 'hkdf' function for standard HKDF behavior
+   */
   static new(version: number): HKDF {
-    return new HKDF(version);
+    if (version != 3) {
+      throw new Error('HKDF versions other than 3 are no longer supported');
+    }
+    return new HKDF();
   }
 
   deriveSecrets(
@@ -61,16 +61,19 @@ export class HKDF {
     label: Buffer,
     salt: Buffer | null
   ): Buffer {
-    return NativeImpl.HKDF_DeriveSecrets(
-      outputLength,
-      this.version,
-      keyMaterial,
-      label,
-      salt
-    );
+    return hkdf(outputLength, keyMaterial, label, salt);
   }
 }
 
+export function hkdf(
+  outputLength: number,
+  keyMaterial: Buffer,
+  label: Buffer,
+  salt: Buffer | null
+): Buffer {
+  return NativeImpl.HKDF_DeriveSecrets(outputLength, keyMaterial, label, salt);
+}
+
 export class ScannableFingerprint {
   private readonly scannable: Buffer;
 

node/test/PublicAPITest.ts

@@ -177,29 +177,27 @@ class InMemorySenderKeyStore extends SignalClient.SenderKeyStore {
 
 describe('SignalClient', () => {
   it('HKDF test vector', () => {
-    const hkdf = SignalClient.HKDF.new(3);
-
     const secret = Buffer.from(
       '0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B0B',
       'hex'
     );
     const empty = Buffer.from('', 'hex');
 
     assert.deepEqual(
-      hkdf.deriveSecrets(42, secret, empty, empty).toString('hex'),
+      SignalClient.hkdf(42, secret, empty, empty).toString('hex'),
       '8da4e775a563c18f715f802a063c5a31b8a11f5c5ee1879ec3454e5f3c738d2d9d201395faa4b61a96c8'
     );
 
     assert.deepEqual(
-      hkdf.deriveSecrets(42, secret, empty, null).toString('hex'),
+      SignalClient.hkdf(42, secret, empty, null).toString('hex'),
       '8da4e775a563c18f715f802a063c5a31b8a11f5c5ee1879ec3454e5f3c738d2d9d201395faa4b61a96c8'
     );
 
     const salt = Buffer.from('000102030405060708090A0B0C', 'hex');
     const label = Buffer.from('F0F1F2F3F4F5F6F7F8F9', 'hex');
 
     assert.deepEqual(
-      hkdf.deriveSecrets(42, secret, label, salt).toString('hex'),
+      SignalClient.hkdf(42, secret, label, salt).toString('hex'),
       '3cb25f25faacd57a90434f64d0362f2a2d2d0a90cf1a5a4c5db02d56ecc4c5bf34007208d5b887185865'
     );
   });

rust/bridge/shared/Cargo.toml

@@ -17,13 +17,15 @@ device-transfer = { path = "../../device-transfer" }
 hsm-enclave = { path = "../../hsm-enclave" }
 libsignal-bridge-macros = { path = "macros" }
 aes-gcm-siv = "0.10.1"
+async-trait = "0.1.41"
 futures-util = "0.3.7"
+hkdf = "0.11"
 log = "0.4"
 paste = "1.0"
 rand = "0.7.3"
-static_assertions = "1.1"
 scopeguard = "1.0"
-async-trait = "0.1.41"
+sha2 = "0.9"
+static_assertions = "1.1"
 uuid = "0.8"
 
 libc = { version = "0.2", optional = true }

rust/bridge/shared/src/protocol.rs

@@ -42,32 +42,28 @@ bridge_handle!(SealedSenderDecryptionResult, ffi = false, jni = false);
 fn HKDF_DeriveSecrets<E: Env>(
     env: E,
     output_length: u32,
-    version: u32,
     ikm: &[u8],
     label: Option<&[u8]>,
     salt: Option<&[u8]>,
 ) -> Result<E::Buffer> {
-    let kdf = HKDF::new(version)?;
     let label = label.unwrap_or(&[]);
-    let buffer = match salt {
-        Some(salt) => kdf.derive_salted_secrets(ikm, salt, label, output_length as usize)?,
-        None => kdf.derive_secrets(ikm, label, output_length as usize)?,
-    };
-    Ok(env.buffer(buffer.into_vec()))
+    let mut buffer = vec![0; output_length as usize];
+    hkdf::Hkdf::<sha2::Sha256>::new(salt, ikm)
+        .expand(label, &mut buffer)
+        .map_err(|_| {
+            SignalProtocolError::InvalidArgument(format!("output too long ({})", output_length))
+        })?;
+    Ok(env.buffer(buffer))
 }
 
 // Alternate implementation to fill an existing buffer.
 #[bridge_fn_void(jni = false, node = false)]
-fn HKDF_Derive(
-    output: &mut [u8],
-    version: u32,
-    ikm: &[u8],
-    label: &[u8],
-    salt: &[u8],
-) -> Result<()> {
-    let kdf = HKDF::new(version)?;
-    let kdf_output = kdf.derive_salted_secrets(ikm, salt, label, output.len())?;
-    output.copy_from_slice(&kdf_output);
+fn HKDF_Derive(output: &mut [u8], ikm: &[u8], label: &[u8], salt: &[u8]) -> Result<()> {
+    hkdf::Hkdf::<sha2::Sha256>::new(Some(salt), ikm)
+        .expand(label, output)
+        .map_err(|_| {
+            SignalProtocolError::InvalidArgument(format!("output too long ({})", output.len()))
+        })?;
     Ok(())
 }
 

rust/crypto/Cargo.toml

@@ -14,7 +14,7 @@ aes = { version = "0.7.4", features = ["ctr"] }
 subtle = "2.3"
 generic-array = "0.14"
 ghash = "0.4.2"
-hmac = "0.9.0"
+hmac = "0.11"
 sha-1 = "0.9"
 sha2 = "0.9"
 

rust/crypto/src/hash.rs

@@ -19,10 +19,10 @@ impl CryptographicMac {
     pub fn new(algo: &str, key: &[u8]) -> Result<Self> {
         match algo {
             "HMACSha1" | "HmacSha1" => Ok(Self::HmacSha1(
-                Hmac::<Sha1>::new_varkey(key).expect("HMAC accepts any key length"),
+                Hmac::<Sha1>::new_from_slice(key).expect("HMAC accepts any key length"),
             )),
             "HMACSha256" | "HmacSha256" => Ok(Self::HmacSha256(
-                Hmac::<Sha256>::new_varkey(key).expect("HMAC accepts any key length"),
+                Hmac::<Sha256>::new_from_slice(key).expect("HMAC accepts any key length"),
             )),
             _ => Err(Error::UnknownAlgorithm("MAC", algo.to_string())),
         }

rust/poksho/Cargo.toml

@@ -13,7 +13,7 @@ license = "AGPL-3.0-only"
 [dependencies]
 curve25519-dalek = { version = "3.0", features = ["serde"] }
 sha2 = "0.9"
-hmac = "0.9.0"
+hmac = "0.11"
 
 [dev-dependencies]
 hex = "0.4"