Merge pull request #24416 from vespa-engine/vekterli/ecies-with-sha256-aes-cbc

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

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