Enforce SHA-256 and AES-CBC for ECIES key wrapping

For some reason requires passing in and keeping an explicit IV. Not sure
why this is the case, since symmetric keys used in the context of a hybrid
crypto scheme are generally derived via a KDF from the shared secret.
This stuff is practically entirely undocumented... :I

3 files changed, 41 insertions, 23 deletions

diff --git a/security-utils/src/main/java/com/yahoo/security/SealedSharedKey.java b/security-utils/src/main/java/com/yahoo/security/SealedSharedKey.java
index 81cf86a535e..237c4976c7c 100644
--- a/security-utils/src/main/java/com/yahoo/security/SealedSharedKey.java
+++ b/security-utils/src/main/java/com/yahoo/security/SealedSharedKey.java
@@ -15,11 +15,13 @@ import java.util.Base64;
  * This token representation is expected to be used as a convenient serialization
  * form when communicating shared keys.
  */
-public record SealedSharedKey(int keyId, byte[] eciesPayload) {
+public record SealedSharedKey(int keyId, byte[] eciesPayload, byte[] iv) {
 
     /** Current encoding version of opaque sealed key tokens. Must be less than 256. */
     public static final int CURRENT_TOKEN_VERSION = 1;
 
+    private static final int ECIES_AES_IV_LENGTH = SharedKeyGenerator.ECIES_AES_CBC_IV_BITS / 8;
+
     /**
      * Creates an opaque URL-safe string token that contains enough information to losslessly
      * reconstruct the SealedSharedKey instance when passed verbatim to fromTokenString().
@@ -28,9 +30,13 @@ public record SealedSharedKey(int keyId, byte[] eciesPayload) {
         if (keyId >= (1 << 24)) {
             throw new IllegalArgumentException("Key id is too large to be encoded");
         }
+        if (iv.length != ECIES_AES_IV_LENGTH) {
+            throw new IllegalStateException("Expected a %d byte IV, got %d bytes".formatted(ECIES_AES_IV_LENGTH, iv.length));
+        }
 
-        ByteBuffer encoded = ByteBuffer.allocate(4 + eciesPayload.length);
+        ByteBuffer encoded = ByteBuffer.allocate(4 + ECIES_AES_IV_LENGTH + eciesPayload.length);
         encoded.putInt((CURRENT_TOKEN_VERSION << 24) | keyId);
+        encoded.put(iv);
         encoded.put(eciesPayload);
         encoded.flip();
 
@@ -55,11 +61,13 @@ public record SealedSharedKey(int keyId, byte[] eciesPayload) {
             throw new IllegalArgumentException("Token had unexpected version. Expected %d, was %d"
                                                .formatted(CURRENT_TOKEN_VERSION, version));
         }
+        byte[] iv = new byte[ECIES_AES_IV_LENGTH];
+        decoded.get(iv);
         byte[] eciesPayload = new byte[decoded.remaining()];
         decoded.get(eciesPayload);
 
         int keyId = versionAndKeyId & 0xffffff;
-        return new SealedSharedKey(keyId, eciesPayload);
+        return new SealedSharedKey(keyId, eciesPayload, iv);
     }
 
 }
diff --git a/security-utils/src/main/java/com/yahoo/security/SharedKeyGenerator.java b/security-utils/src/main/java/com/yahoo/security/SharedKeyGenerator.java
index 440b17eecfa..07e8243ec09 100644
--- a/security-utils/src/main/java/com/yahoo/security/SharedKeyGenerator.java
+++ b/security-utils/src/main/java/com/yahoo/security/SharedKeyGenerator.java
@@ -2,6 +2,7 @@
 package com.yahoo.security;
 
 import org.bouncycastle.jcajce.provider.util.BadBlockException;
+import org.bouncycastle.jce.spec.IESParameterSpec;
 
 import javax.crypto.BadPaddingException;
 import javax.crypto.Cipher;
@@ -12,7 +13,6 @@ import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.InvalidKeyException;
-import java.security.KeyPair;
 import java.security.NoSuchAlgorithmException;
 import java.security.PrivateKey;
 import java.security.PublicKey;
@@ -37,25 +37,33 @@ import java.security.SecureRandom;
  */
 public class SharedKeyGenerator {
 
+    private static final int    AES_GCM_KEY_BITS      = 256;
     private static final int    AES_GCM_AUTH_TAG_BITS = 128;
     private static final String AES_GCM_ALGO_SPEC     = "AES/GCM/NoPadding";
-    private static final String ECIES_CIPHER_NAME     = "ECIES"; // TODO ensure SHA-256+AES. Needs BC version bump
+    private static final String ECIES_CIPHER_NAME     = "ECIESwithSHA256andAES-CBC";
+    protected static final int  ECIES_AES_CBC_IV_BITS = 128;
+    private static final int    ECIES_HMAC_BITS       = 256;
+    private static final int    ECIES_AES_KEY_BITS    = 256;
     private static final SecureRandom SHARED_CSPRNG   = new SecureRandom();
 
     public static SecretSharedKey generateForReceiverPublicKey(PublicKey receiverPublicKey, int keyId) {
         try {
             var keyGen = KeyGenerator.getInstance("AES");
-            keyGen.init(256, SHARED_CSPRNG);
+            keyGen.init(AES_GCM_KEY_BITS, SHARED_CSPRNG);
             var secretKey = keyGen.generateKey();
 
             var cipher = Cipher.getInstance(ECIES_CIPHER_NAME, BouncyCastleProviderHolder.getInstance());
-            cipher.init(Cipher.ENCRYPT_MODE, receiverPublicKey);
+            byte[] iv = new byte[ECIES_AES_CBC_IV_BITS / 8];
+            SHARED_CSPRNG.nextBytes(iv);
+            var iesParamSpec = new IESParameterSpec(null, null, ECIES_HMAC_BITS, ECIES_AES_KEY_BITS, iv);
+
+            cipher.init(Cipher.ENCRYPT_MODE, receiverPublicKey, iesParamSpec);
             byte[] eciesPayload = cipher.doFinal(secretKey.getEncoded());
 
-            var sealedSharedKey = new SealedSharedKey(keyId, eciesPayload);
+            var sealedSharedKey = new SealedSharedKey(keyId, eciesPayload, iv);
             return new SecretSharedKey(secretKey, sealedSharedKey);
         } catch (NoSuchAlgorithmException | InvalidKeyException | NoSuchPaddingException
-                | IllegalBlockSizeException | BadPaddingException e) {
+                | IllegalBlockSizeException | BadPaddingException | InvalidAlgorithmParameterException e) {
             throw new RuntimeException(e);
         }
     }
@@ -63,7 +71,8 @@ public class SharedKeyGenerator {
     public static SecretSharedKey fromSealedKey(SealedSharedKey sealedKey, PrivateKey receiverPrivateKey) {
         try {
             var cipher = Cipher.getInstance(ECIES_CIPHER_NAME, BouncyCastleProviderHolder.getInstance());
-            cipher.init(Cipher.DECRYPT_MODE, receiverPrivateKey);
+            var iesParamSpec = new IESParameterSpec(null, null, ECIES_HMAC_BITS, ECIES_AES_KEY_BITS, sealedKey.iv());
+            cipher.init(Cipher.DECRYPT_MODE, receiverPrivateKey, iesParamSpec);
             byte[] secretKey = cipher.doFinal(sealedKey.eciesPayload());
 
             return new SecretSharedKey(new SecretKeySpec(secretKey, "AES"), sealedKey);
@@ -71,7 +80,7 @@ public class SharedKeyGenerator {
             throw new IllegalArgumentException("Token integrity check failed; token is either corrupt or was " +
                                                "generated for a different public key");
         } catch (NoSuchAlgorithmException | InvalidKeyException | NoSuchPaddingException
-                | IllegalBlockSizeException | BadPaddingException e) {
+                | IllegalBlockSizeException | BadPaddingException | InvalidAlgorithmParameterException e) {
             throw new RuntimeException(e);
         }
     }
@@ -88,7 +97,7 @@ public class SharedKeyGenerator {
         return new byte[] { 'h', 'e', 'r', 'e', 'B', 'd', 'r', 'a', 'g', 'o', 'n', 's' };
     }
 
-    private static Cipher makeAes256GcmCipher(SecretSharedKey secretSharedKey, int cipherMode) {
+    private static Cipher makeAesGcmCipher(SecretSharedKey secretSharedKey, int cipherMode) {
         try {
             var cipher  = Cipher.getInstance(AES_GCM_ALGO_SPEC);
             var gcmSpec = new GCMParameterSpec(AES_GCM_AUTH_TAG_BITS, fixed96BitIvForSingleUseKey());
@@ -101,20 +110,20 @@ public class SharedKeyGenerator {
     }
 
     /**
-     * Creates an AES-GCM-256 Cipher that can be used to encrypt arbitrary plaintext.
+     * Creates an AES-GCM Cipher that can be used to encrypt arbitrary plaintext.
      *
      * The given secret key MUST NOT be used to encrypt more than one plaintext.
      */
-    public static Cipher makeAes256GcmEncryptionCipher(SecretSharedKey secretSharedKey) {
-        return makeAes256GcmCipher(secretSharedKey, Cipher.ENCRYPT_MODE);
+    public static Cipher makeAesGcmEncryptionCipher(SecretSharedKey secretSharedKey) {
+        return makeAesGcmCipher(secretSharedKey, Cipher.ENCRYPT_MODE);
     }
 
     /**
-     * Creates an AES-GCM-256 Cipher that can be used to decrypt ciphertext that was previously
+     * Creates an AES-GCM Cipher that can be used to decrypt ciphertext that was previously
      * encrypted with the given secret key.
      */
-    public static Cipher makeAes256GcmDecryptionCipher(SecretSharedKey secretSharedKey) {
-        return makeAes256GcmCipher(secretSharedKey, Cipher.DECRYPT_MODE);
+    public static Cipher makeAesGcmDecryptionCipher(SecretSharedKey secretSharedKey) {
+        return makeAesGcmCipher(secretSharedKey, Cipher.DECRYPT_MODE);
     }
 
 }
diff --git a/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java b/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java
index 348f214aa85..26a506015c5 100644
--- a/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java
+++ b/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java
@@ -62,9 +62,10 @@ public class SharedKeyTest {
         var receiverKeyPair = new KeyPair(receiverPublic, receiverPrivate);
 
         // Token generated for the above receiver public key, with the below expected shared secret (in hex)
-        var publicToken = "AQAAAQTAVVthmrVAbMgKt8hBc4xColmDmEeEAyPD-ZcPlRmeId9wBaZTTctwV3pwT3FyV0UtvX_7zrRId" +
-                          "3mNxvaru0tvFucd7mYY73Hi9d3j8qS6pN0bTTb1sw_dKYrR_0BXhEFE_py8uZnNxvV8-wHtBIAVXBk_4Q";
-        var expectedSharedSecret = "b1aded9dc19593baa08fe64f916dcbaf3328ec666d2e0c81b1f6f8af9794187b";
+        var publicToken = "AQAAAUfvuJpugUV3knQXwyP7afgEpDXT4JxaF-x7Ykirty2iwUqJv5UsGx78is5Vu4Mdln_mOVbAUv4dj" +
+                          "da7hvzKYNC3IpSMjFrTQ8ab-bEkMpc5tjss_Z7DaJzY4fUlw31Lhx39BMB5yQX0pVLMdFGp5F-_8z8CE" +
+                          "-7d9lkCDP9hPKiD77besjrBt_mEBadCd4oNONqc6zzhuQj4O5T9k_RC5VRV";
+        var expectedSharedSecret = "64e01295e736cb827e86cf0281385d5a0dcca217ec1b59f6609a06e2e9debf78";
 
         var theirSealed = SealedSharedKey.fromTokenString(publicToken);
         var theirShared = SharedKeyGenerator.fromSealedKey(theirSealed, receiverKeyPair.getPrivate());
@@ -92,7 +93,7 @@ public class SharedKeyTest {
     }
 
     static byte[] streamEncryptString(String data, SecretSharedKey secretSharedKey) throws IOException {
-        var cipher = SharedKeyGenerator.makeAes256GcmEncryptionCipher(secretSharedKey);
+        var cipher = SharedKeyGenerator.makeAesGcmEncryptionCipher(secretSharedKey);
         var outStream = new ByteArrayOutputStream();
         try (var cipherStream = new CipherOutputStream(outStream, cipher)) {
             cipherStream.write(data.getBytes(StandardCharsets.UTF_8));
@@ -102,7 +103,7 @@ public class SharedKeyTest {
     }
 
     static String streamDecryptString(byte[] encrypted, SecretSharedKey secretSharedKey) throws IOException {
-        var cipher   = SharedKeyGenerator.makeAes256GcmDecryptionCipher(secretSharedKey);
+        var cipher   = SharedKeyGenerator.makeAesGcmDecryptionCipher(secretSharedKey);
         var inStream = new ByteArrayInputStream(encrypted);
         var total    = ByteBuffer.allocate(encrypted.length); // Assume decrypted form can't be _longer_
         byte[] tmp   = new byte[8]; // short buf to test chunking