Fix sonar security issue in CryptoUtils 74/114374/3
authorJim Hahn <jrh3@att.com>
Wed, 28 Oct 2020 20:22:01 +0000 (16:22 -0400)
committerJim Hahn <jrh3@att.com>
Thu, 29 Oct 2020 12:17:47 +0000 (08:17 -0400)
Sonar reports that CryptoUtils is using AES with CBC, which is known
to be insecure.  Switched to "AES/GCM/NoPadding".
Note: values in any property files using encryption or the "enc:"
prefix will have to be re-encrypted.

Issue-ID: POLICY-2801
Change-Id: I41f00d4f3ee67a00b92135150120d1faa621655a
Signed-off-by: Jim Hahn <jrh3@att.com>
utils/src/main/java/org/onap/policy/common/utils/security/CryptoUtils.java
utils/src/test/java/org/onap/policy/common/utils/coder/PropertyCoderTest.java
utils/src/test/java/org/onap/policy/common/utils/security/CryptoUtilsTest.java

index af5b3d4..1a9483a 100644 (file)
@@ -23,7 +23,7 @@ package org.onap.policy.common.utils.security;
 import java.nio.charset.StandardCharsets;
 import java.util.Random;
 import javax.crypto.Cipher;
-import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 import javax.xml.bind.DatatypeConverter;
 import org.apache.commons.lang3.ArrayUtils;
@@ -44,7 +44,9 @@ public class CryptoUtils implements CryptoCoder {
     /**
      * Detailed definition of encryption algorithm.
      */
-    private static final String ALGORITHM_DETAILS = ALGORITHM + "/CBC/PKCS5PADDING";
+    private static final String ALGORITHM_DETAILS = ALGORITHM + "/GCM/NoPadding";
+
+    private static final int TAG_SIZE_IN_BITS = 128;
 
     private static final int IV_BLOCK_SIZE_IN_BITS = 128;
 
@@ -120,7 +122,7 @@ public class CryptoUtils implements CryptoCoder {
             Cipher cipher = Cipher.getInstance(ALGORITHM_DETAILS);
             byte[] iv = new byte[IV_BLOCK_SIZE_IN_BYTES];
             RANDOM.nextBytes(iv);
-            IvParameterSpec ivspec = new IvParameterSpec(iv);
+            GCMParameterSpec ivspec = new GCMParameterSpec(TAG_SIZE_IN_BITS, iv);
             cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivspec);
 
             return "enc:" + DatatypeConverter.printBase64Binary(
@@ -175,7 +177,7 @@ public class CryptoUtils implements CryptoCoder {
             byte[] encryptedValue = DatatypeConverter.parseBase64Binary(pureValue);
 
             Cipher cipher = Cipher.getInstance(ALGORITHM_DETAILS);
-            IvParameterSpec ivspec = new IvParameterSpec(
+            GCMParameterSpec ivspec = new GCMParameterSpec(TAG_SIZE_IN_BITS,
                     ArrayUtils.subarray(encryptedValue, 0, IV_BLOCK_SIZE_IN_BYTES));
             byte[] realData = ArrayUtils.subarray(encryptedValue, IV_BLOCK_SIZE_IN_BYTES, encryptedValue.length);
 
index 47453df..86f8a1b 100644 (file)
@@ -33,15 +33,29 @@ import org.junit.Test;
 public class PropertyCoderTest {
     private PropertyCoder propertyCoder = null;
     private static final String AES_ENCRYPTION_KEY = "aes_encryption_key";
+
+    /*
+     * Note: to generate the encrypted values, invoke CryptoUtils passing both the value
+     * to be encrypted and the secret key.
+     *
+     * The secret key should typically be 32 characters long, resulting in a 256-bit
+     * key, and is placed in "aes_encryption_key".
+     *
+     * For "xacml.pdp.rest.password", the encrypted value was generated via:
+     *  java org.onap.policy.common.utils.security.CryptoUtils enc alpha abcdefghijklmnopqrstuvwxyzabcdef
+     *
+     * For "pass", the encrypted value was generated via:
+     *  java org.onap.policy.common.utils.security.CryptoUtils enc hello abcdefghijklmnopqrstuvwxyzabcdef
+     */
     private static final String json =
             ("{'aes_encryption_key':'abcdefghijklmnopqrstuvwxyzabcdef'"
-            + ",'xacml.pdp.rest.password':'enc:YZ8EqzsxIOzIuK416SWAdrv+0cKKkqsQt/NYH9+uxwI='"
+            + ",'xacml.pdp.rest.password':'enc:FSfOhDygtmnX3gkMSfTFMoBFW+AG5k6goNj2KZgQmeF0DqgcMg=='"
             + ",'xacml.pdp.rest.user':'testpdp'"
             + ",'xacml.pdp.rest.client.user':'policy'"
             + ",'xacml.pdp.rest.client.password':'policy'"
             + ",'xacml.pdp.rest.environment':'TEST'"
             + ",'servers':[{'name':'server1','port':'10',"
-            + "'pass':'enc:KXIY94KcAapOAAeFbtjQL4kBPB4k+NJfwdP+GpG3LWQ='}"
+            + "'pass':'enc:08Fj6tLhmWjkZkf52O2A2ZNT8PpL80yEOEKXlbV/gnm0lkR9OA=='}"
             + ",{'name':'server2','port':'20','pass':'plaintext'}]"
             + "}").replace('\'', '"');
 
index ce9435d..625fd1f 100644 (file)
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP
  * ================================================================================
- * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2019-2020 AT&T Intellectual Property. All rights reserved.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -37,7 +37,7 @@ public class CryptoUtilsTest {
     private static Logger logger = LoggerFactory.getLogger(CryptoUtilsTest.class);
     private static final String PASS = "HelloWorld";
     private static final String SECRET_KEY = "MTIzNDU2Nzg5MDEyMzQ1Ng==";
-    private static final String ENCRYPTED_PASS = "enc:hcI2XVX+cxPz/6rlbebkWpCFF6WPbBtT7iJRr2VHUkA=";
+    private static final String ENCRYPTED_PASS = "enc:Z6QzirpPyDpwmIcNbE3U2iq6g/ubJBEdzssoigxGGChlQtdWOLD8y00O";
     private static final String DECRYPTED_MSG = "encrypted value: {}  decrypted value : {}";
     private static final String ENCRYPTED_MSG = "original value : {}  encrypted value: {}";
 
@@ -120,4 +120,4 @@ public class CryptoUtilsTest {
         String decryptedAgain = CryptoUtils.decrypt(decryptedValue, SECRET_KEY);
         assertEquals(decryptedValue, decryptedAgain);
     }
-}
\ No newline at end of file
+}