Use GCM mode as default

Use GCM to resolve: https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY JIRA: LIGHTY-193 Signed-off-by: tobias.pobocik <[email protected]>
Tobianas · Oct 17, 2024 · 96074e4 · 96074e4
1 parent 688cae9
commit 96074e4
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 67 deletions.
diff --git a/...rcgnmi-app-module/src/main/java/io/lighty/applications/rcgnmi/module/RcGnmiAppModule.java b/...rcgnmi-app-module/src/main/java/io/lighty/applications/rcgnmi/module/RcGnmiAppModule.java
@@ -39,7 +39,7 @@
 import javax.crypto.NoSuchPaddingException;
 import javax.crypto.SecretKey;
 import javax.crypto.SecretKeyFactory;
-import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.PBEKeySpec;
 import javax.crypto.spec.SecretKeySpec;
 import org.eclipse.jdt.annotation.Nullable;
@@ -190,15 +190,16 @@ private AAAEncryptionServiceImpl createEncryptionService() throws NoSuchPaddingE
                 encrySrvConfig.getEncryptIterationCount(), encrySrvConfig.getEncryptKeyLength());
         final SecretKey key
                 = new SecretKeySpec(keyFactory.generateSecret(keySpec).getEncoded(), encrySrvConfig.getEncryptType());
-        final IvParameterSpec ivParameterSpec = new IvParameterSpec(encryptionKeySalt);
+        final GCMParameterSpec gcmParameterSpec = new GCMParameterSpec(encrySrvConfig.getAuthTagLength(),
+                encryptionKeySalt);
 
         final Cipher encryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
-        encryptCipher.init(Cipher.ENCRYPT_MODE, key, ivParameterSpec);
+        encryptCipher.init(Cipher.ENCRYPT_MODE, key, gcmParameterSpec);
 
         final Cipher decryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
-        decryptCipher.init(Cipher.DECRYPT_MODE, key, ivParameterSpec);
+        decryptCipher.init(Cipher.DECRYPT_MODE, key, gcmParameterSpec);
 
-        return new AAAEncryptionServiceImpl(encryptCipher, decryptCipher);
+        return new AAAEncryptionServiceImpl(gcmParameterSpec, encrySrvConfig.getCipherTransforms(), key);
     }
 
     private AaaEncryptServiceConfig getDefaultAaaEncryptServiceConfig() {
@@ -209,6 +210,6 @@ private AaaEncryptServiceConfig getDefaultAaaEncryptServiceConfig() {
                 .setPasswordLength(12).setEncryptSalt(salt)
                 .setEncryptMethod("PBKDF2WithHmacSHA1").setEncryptType("AES")
                 .setEncryptIterationCount(32768).setEncryptKeyLength(128)
-                .setCipherTransforms("AES/CBC/PKCS5Padding").build();
+                .setAuthTagLength(128).setCipherTransforms("AES/GCM/NoPadding").build();
     }
 }
diff --git a/...on-service/src/main/java/io/lighty/aaa/encrypt/service/impl/AAAEncryptionServiceImpl.java b/...on-service/src/main/java/io/lighty/aaa/encrypt/service/impl/AAAEncryptionServiceImpl.java
@@ -7,9 +7,20 @@
  */
 package io.lighty.aaa.encrypt.service.impl;
 
+import static java.util.Objects.requireNonNull;
+
+import java.nio.ByteBuffer;
+import java.security.GeneralSecurityException;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
 import javax.crypto.BadPaddingException;
 import javax.crypto.Cipher;
 import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.NoSuchPaddingException;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.GCMParameterSpec;
 import org.opendaylight.aaa.encrypt.AAAEncryptionService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -18,44 +29,87 @@ public class AAAEncryptionServiceImpl implements AAAEncryptionService {
 
     private static final Logger LOG = LoggerFactory.getLogger(AAAEncryptionServiceImpl.class);
 
-    private final Cipher encryptCipher;
-    private final Cipher decryptCipher;
+    private final String cipherTransforms;
+    private final SecretKey key;
+    private final byte[] iv;
+    private final int tagLength;
+    final SecureRandom random;
 
-    public AAAEncryptionServiceImpl(Cipher encryptCipher, Cipher decryptCipher) {
-        this.encryptCipher = encryptCipher;
-        this.decryptCipher = decryptCipher;
+    public AAAEncryptionServiceImpl(GCMParameterSpec gcmParameterSpec, String cipherTransforms, SecretKey key) {
+        this.iv = gcmParameterSpec.getIV();
+        this.tagLength = gcmParameterSpec.getTLen();
+        this.cipherTransforms = cipherTransforms;
+        this.key = key;
+        this.random = new SecureRandom();
     }
 
     @Override
-    public byte[] encrypt(byte[] data) {
+    public byte[] encrypt(final byte[] data) {
         if (data != null && data.length != 0) {
+            final Cipher encryptCipher;
+            try {
+                encryptCipher = initCipher(Cipher.ENCRYPT_MODE, iv);
+            } catch (GeneralSecurityException e) {
+                throw new IllegalStateException("Failed to create encrypt cipher.", e);
+            }
+            final byte[] encryptedData;
             try {
-                synchronized (encryptCipher) {
-                    return encryptCipher.doFinal(data);
-                }
-            } catch (IllegalBlockSizeException | BadPaddingException e) {
+                encryptedData = encryptCipher.doFinal(requireNonNull(data));
+                return ByteBuffer.allocate(iv.length + encryptedData.length)
+                    .put(iv)
+                    .put(encryptedData)
+                    .array();
+            } catch (final IllegalBlockSizeException | BadPaddingException e) {
                 LOG.error("Failed to encrypt data.", e);
                 return data;
             }
-        } else {
-            LOG.warn("data is empty or null.");
+        }
+        else {
+            LOG.warn("encrypt data is empty or null.");
             return data;
         }
     }
 
     @Override
-    public byte[] decrypt(byte[] encryptedData) {
-        if (encryptedData != null && encryptedData.length != 0) {
+    public byte[] decrypt(final byte[] encryptedDataWithIv) {
+        if (encryptedDataWithIv != null && encryptedDataWithIv.length != 0) {
+            final var ivLength = iv.length;
+            if (encryptedDataWithIv.length < ivLength) {
+                LOG.error("Invalid encrypted data length.");
+                return encryptedDataWithIv;
+            }
+            final var byteBuffer = ByteBuffer.wrap(encryptedDataWithIv);
+
+            final var localIv = new byte[ivLength];
+            byteBuffer.get(localIv);
+
+            final var encryptedData = new byte[byteBuffer.remaining()];
+            byteBuffer.get(encryptedData);
+
+            final Cipher decryptCipher;
+            try {
+                decryptCipher = initCipher(Cipher.DECRYPT_MODE, iv);
+            } catch (GeneralSecurityException e) {
+                throw new IllegalStateException("Failed to create decrypt cipher.", e);
+            }
             try {
-                return decryptCipher.doFinal(encryptedData);
-            } catch (IllegalBlockSizeException | BadPaddingException e) {
-                LOG.error("Failed to decrypt encoded data", e);
+                return decryptCipher.doFinal(requireNonNull(encryptedData));
+            } catch (final IllegalBlockSizeException | BadPaddingException e) {
+                LOG.error("Failed to decrypt data", e);
                 return encryptedData;
             }
-        } else {
-            LOG.warn("encryptedData is empty or null.");
-            return encryptedData;
         }
+        else {
+            LOG.warn("decrypt data is empty or null.");
+            return encryptedDataWithIv;
+        }
+    }
+
+    private Cipher initCipher(final int mode, final byte[] localIv) throws
+        NoSuchPaddingException, NoSuchAlgorithmException, InvalidAlgorithmParameterException, InvalidKeyException {
+        final var cipher = Cipher.getInstance(cipherTransforms);
+        cipher.init(mode, key, new GCMParameterSpec(tagLength, localIv));
+        return cipher;
     }
 
 }
diff --git a/...aa-aggregator/lighty-aaa/src/main/java/io/lighty/aaa/config/CertificateManagerConfig.java b/...aa-aggregator/lighty-aaa/src/main/java/io/lighty/aaa/config/CertificateManagerConfig.java
@@ -8,19 +8,15 @@
 package io.lighty.aaa.config;
 
 import io.lighty.aaa.encrypt.service.impl.AAAEncryptionServiceImpl;
-import java.security.InvalidAlgorithmParameterException;
-import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.KeySpec;
 import java.util.ArrayList;
 import java.util.Base64;
 import java.util.List;
-import javax.crypto.Cipher;
-import javax.crypto.NoSuchPaddingException;
 import javax.crypto.SecretKey;
 import javax.crypto.SecretKeyFactory;
-import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.PBEKeySpec;
 import javax.crypto.spec.SecretKeySpec;
 import org.opendaylight.aaa.cert.api.ICertificateManager;
@@ -75,7 +71,8 @@ public static ICertificateManager getDefault(final DataBroker bindingDataBroker,
                 .setEncryptType("AES")
                 .setEncryptIterationCount(32768)
                 .setEncryptKeyLength(128)
-                .setCipherTransforms("AES/CBC/PKCS5Padding")
+                .setCipherTransforms("AES/GCM/NoPadding")
+                .setAuthTagLength(128)
                 .build();
 
         final byte[] encryptionKeySalt = Base64.getDecoder().decode(encrySrvConfig.getEncryptSalt());
@@ -86,19 +83,16 @@ public static ICertificateManager getDefault(final DataBroker bindingDataBroker,
                     encrySrvConfig.getEncryptIterationCount(), encrySrvConfig.getEncryptKeyLength());
             SecretKey key = new SecretKeySpec(keyFactory.generateSecret(keySpec).getEncoded(),
                     encrySrvConfig.getEncryptType());
-            IvParameterSpec ivParameterSpec = new IvParameterSpec(encryptionKeySalt);
+            GCMParameterSpec gcmParameterSpec = new GCMParameterSpec(encrySrvConfig.getAuthTagLength(),
+                encryptionKeySalt);
 
-            Cipher encryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
-            encryptCipher.init(Cipher.ENCRYPT_MODE, key, ivParameterSpec);
-
-            Cipher decryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
-            decryptCipher.init(Cipher.DECRYPT_MODE, key, ivParameterSpec);
-            final AAAEncryptionService encryptionSrv = new AAAEncryptionServiceImpl(encryptCipher, decryptCipher);
+            final AAAEncryptionService encryptionSrv = new AAAEncryptionServiceImpl(gcmParameterSpec,
+                encrySrvConfig.getCipherTransforms(), key);
 
             return new CertificateManagerService(rpcProviderService, bindingDataBroker, encryptionSrv,
                     aaaCertServiceConfig);
-        } catch (InvalidAlgorithmParameterException | InvalidKeyException | InvalidKeySpecException
-                 | NoSuchAlgorithmException | NoSuchPaddingException e) {
+        } catch (InvalidKeySpecException
+                 | NoSuchAlgorithmException e) {
             return null;
         }
     }

diff --git a/...odules/lighty-aaa-aggregator/lighty-aaa/src/main/resources/aaa-encrypt-service-config.xml b/...odules/lighty-aaa-aggregator/lighty-aaa/src/main/resources/aaa-encrypt-service-config.xml
@@ -6,5 +6,5 @@
     <encrypt-type>AES</encrypt-type>
     <encrypt-iteration-count>32768</encrypt-iteration-count>
     <encrypt-key-length>128</encrypt-key-length>
-    <cipher-transforms>AES/CBC/PKCS5Padding</cipher-transforms>
+    <cipher-transforms>AES/GCM/NoPadding</cipher-transforms>
 </aaa-encrypt-service-config>
diff --git a/...ghty-gnmi-sb/src/test/java/io/lighty/gnmi/southbound/device/KeystoreGnmiSecurityTest.java b/...ghty-gnmi-sb/src/test/java/io/lighty/gnmi/southbound/device/KeystoreGnmiSecurityTest.java
@@ -50,7 +50,7 @@
 import javax.crypto.NoSuchPaddingException;
 import javax.crypto.SecretKey;
 import javax.crypto.SecretKeyFactory;
-import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.PBEKeySpec;
 import javax.crypto.spec.SecretKeySpec;
 import javax.xml.bind.DatatypeConverter;
@@ -465,22 +465,23 @@ private static AAAEncryptionServiceImpl createEncryptionService() throws NoSuchP
                 encrySrvConfig.getEncryptIterationCount(), encrySrvConfig.getEncryptKeyLength());
         final SecretKey key = new SecretKeySpec(keyFactory.generateSecret(keySpec).getEncoded(),
                 encrySrvConfig.getEncryptType());
-        final IvParameterSpec ivParameterSpec = new IvParameterSpec(encryptionKeySalt);
+        final GCMParameterSpec ivParameterSpec = new GCMParameterSpec(encrySrvConfig.getAuthTagLength(),
+                encryptionKeySalt);
 
         final Cipher encryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
         encryptCipher.init(Cipher.ENCRYPT_MODE, key, ivParameterSpec);
 
         final Cipher decryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
         decryptCipher.init(Cipher.DECRYPT_MODE, key, ivParameterSpec);
 
-        return new AAAEncryptionServiceImpl(encryptCipher, decryptCipher);
+        return new AAAEncryptionServiceImpl(ivParameterSpec, encrySrvConfig.getCipherTransforms(), key);
     }
 
     private static AaaEncryptServiceConfig getDefaultAaaEncryptServiceConfig() {
         return new AaaEncryptServiceConfigBuilder().setEncryptKey("V1S1ED4OMeEh")
                 .setPasswordLength(12).setEncryptSalt("TdtWeHbch/7xP52/rp3Usw==")
                 .setEncryptMethod("PBKDF2WithHmacSHA1").setEncryptType("AES")
                 .setEncryptIterationCount(32768).setEncryptKeyLength(128)
-                .setCipherTransforms("AES/CBC/PKCS5Padding").build();
+                .setAuthTagLength(128).setCipherTransforms("AES/GCM/NoPadding").build();
     }
 }
diff --git a/...nmi-sb/src/test/java/io/lighty/gnmi/southbound/lightymodule/GnmiSouthBoundModuleTest.java b/...nmi-sb/src/test/java/io/lighty/gnmi/southbound/lightymodule/GnmiSouthBoundModuleTest.java
@@ -25,11 +25,10 @@
 import java.util.List;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
-import javax.crypto.Cipher;
 import javax.crypto.NoSuchPaddingException;
 import javax.crypto.SecretKey;
 import javax.crypto.SecretKeyFactory;
-import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.PBEKeySpec;
 import javax.crypto.spec.SecretKeySpec;
 import org.junit.jupiter.api.Assertions;
@@ -79,23 +78,19 @@ private static AAAEncryptionServiceImpl createEncryptionService() throws NoSuchP
                 encrySrvConfig.getEncryptIterationCount(), encrySrvConfig.getEncryptKeyLength());
         final SecretKey key
                 = new SecretKeySpec(keyFactory.generateSecret(keySpec).getEncoded(), encrySrvConfig.getEncryptType());
-        final IvParameterSpec ivParameterSpec = new IvParameterSpec(encryptionKeySalt);
+        final GCMParameterSpec ivParameterSpec = new GCMParameterSpec(encrySrvConfig.getAuthTagLength(),
+                encryptionKeySalt);
 
-        final Cipher encryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
-        encryptCipher.init(Cipher.ENCRYPT_MODE, key, ivParameterSpec);
-
-        final Cipher decryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
-        decryptCipher.init(Cipher.DECRYPT_MODE, key, ivParameterSpec);
-
-        return new AAAEncryptionServiceImpl(encryptCipher, decryptCipher);
+        return new AAAEncryptionServiceImpl(ivParameterSpec, encrySrvConfig.getCipherTransforms(), key);
     }
 
     private static AaaEncryptServiceConfig getDefaultAaaEncryptServiceConfig() {
         return new AaaEncryptServiceConfigBuilder().setEncryptKey("V1S1ED4OMeEh")
                 .setPasswordLength(12).setEncryptSalt("TdtWeHbch/7xP52/rp3Usw==")
                 .setEncryptMethod("PBKDF2WithHmacSHA1").setEncryptType("AES")
                 .setEncryptIterationCount(32768).setEncryptKeyLength(128)
-                .setCipherTransforms("AES/CBC/PKCS5Padding").build();
+                .setAuthTagLength(128)
+                .setCipherTransforms("AES/GCM/NoPadding").build();
     }
 }
 
diff --git a/...hty-gnmi-test/src/test/java/io/lighty/modules/gnmi/test/gnmi/GnmiWithoutRestconfTest.java b/...hty-gnmi-test/src/test/java/io/lighty/modules/gnmi/test/gnmi/GnmiWithoutRestconfTest.java
@@ -48,7 +48,7 @@
 import javax.crypto.NoSuchPaddingException;
 import javax.crypto.SecretKey;
 import javax.crypto.SecretKeyFactory;
-import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.PBEKeySpec;
 import javax.crypto.spec.SecretKeySpec;
 import org.awaitility.Awaitility;
@@ -509,23 +509,24 @@ private static AAAEncryptionServiceImpl createEncryptionService() throws NoSuchP
                 encrySrvConfig.getEncryptIterationCount(), encrySrvConfig.getEncryptKeyLength());
         final SecretKey key
                 = new SecretKeySpec(keyFactory.generateSecret(keySpec).getEncoded(), encrySrvConfig.getEncryptType());
-        final IvParameterSpec ivParameterSpec = new IvParameterSpec(encryptionKeySalt);
+        final GCMParameterSpec ivParameterSpec = new GCMParameterSpec(encrySrvConfig.getAuthTagLength(),
+            encryptionKeySalt);
 
         final Cipher encryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
         encryptCipher.init(Cipher.ENCRYPT_MODE, key, ivParameterSpec);
 
         final Cipher decryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
         decryptCipher.init(Cipher.DECRYPT_MODE, key, ivParameterSpec);
 
-        return new AAAEncryptionServiceImpl(encryptCipher, decryptCipher);
+        return new AAAEncryptionServiceImpl(ivParameterSpec, encrySrvConfig.getCipherTransforms(), key);
     }
 
     private static AaaEncryptServiceConfig getDefaultAaaEncryptServiceConfig() {
         return new AaaEncryptServiceConfigBuilder().setEncryptKey("V1S1ED4OMeEh")
                 .setPasswordLength(12).setEncryptSalt("TdtWeHbch/7xP52/rp3Usw==")
                 .setEncryptMethod("PBKDF2WithHmacSHA1").setEncryptType("AES")
                 .setEncryptIterationCount(32768).setEncryptKeyLength(128)
-                .setCipherTransforms("AES/CBC/PKCS5Padding").build();
+                .setAuthTagLength(128).setCipherTransforms("AES/GCM/NoPadding").build();
     }
 
     private static SimulatedGnmiDevice getUnsecureGnmiDevice(final String host, final int port) {

diff --git a/...f-sb/src/main/java/io/lighty/modules/southbound/netconf/impl/util/NetconfConfigUtils.java b/...f-sb/src/main/java/io/lighty/modules/southbound/netconf/impl/util/NetconfConfigUtils.java
@@ -29,7 +29,7 @@
 import javax.crypto.NoSuchPaddingException;
 import javax.crypto.SecretKey;
 import javax.crypto.SecretKeyFactory;
-import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.PBEKeySpec;
 import javax.crypto.spec.SecretKeySpec;
 import org.opendaylight.aaa.encrypt.AAAEncryptionService;
@@ -157,7 +157,7 @@ public static AaaEncryptServiceConfig getDefaultAaaEncryptServiceConfig() {
                 .setPasswordLength(12).setEncryptSalt(salt)
                 .setEncryptMethod("PBKDF2WithHmacSHA1").setEncryptType("AES")
                 .setEncryptIterationCount(32768).setEncryptKeyLength(128)
-                .setCipherTransforms("AES/CBC/PKCS5Padding").build();
+                .setAuthTagLength(128).setCipherTransforms("AES/GCM/NoPadding").build();
     }
 
     /**
@@ -176,15 +176,16 @@ public static AAAEncryptionService createAAAEncryptionService(AaaEncryptServiceC
                     encrySrvConfig.getEncryptIterationCount(), encrySrvConfig.getEncryptKeyLength());
             SecretKey key = new SecretKeySpec(keyFactory.generateSecret(keySpec).getEncoded(),
                     encrySrvConfig.getEncryptType());
-            IvParameterSpec ivParameterSpec = new IvParameterSpec(encryptionKeySalt);
+            GCMParameterSpec gcmParameterSpec = new GCMParameterSpec(encrySrvConfig.getAuthTagLength(),
+                    encryptionKeySalt);
 
             Cipher encryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
-            encryptCipher.init(Cipher.ENCRYPT_MODE, key, ivParameterSpec);
+            encryptCipher.init(Cipher.ENCRYPT_MODE, key, gcmParameterSpec);
 
             Cipher decryptCipher = Cipher.getInstance(encrySrvConfig.getCipherTransforms());
-            decryptCipher.init(Cipher.DECRYPT_MODE, key, ivParameterSpec);
+            decryptCipher.init(Cipher.DECRYPT_MODE, key, gcmParameterSpec);
 
-            return new AAAEncryptionServiceImpl(encryptCipher, decryptCipher);
+            return new AAAEncryptionServiceImpl(gcmParameterSpec, encrySrvConfig.getCipherTransforms(), key);
 
         } catch (NoSuchAlgorithmException | InvalidKeySpecException | NoSuchPaddingException
                 | InvalidAlgorithmParameterException | InvalidKeyException e) {