Fix security issue in SecurityUtil

- Specified mode and padding to address risky algorithm
- Corrected unit test for different exception message
- Moved tests to package

Issue-ID: SDC-3105
Signed-off-by: Neil Derraugh <neil.derraugh@yoppworks.com>
Change-Id: I5773ab555a5468362c775cf99795df4eb8c52136

diff --git a/openecomp-be/backend/openecomp-sdc-security-util/src/main/java/org/openecomp/sdc/securityutil/CipherUtil.java b/openecomp-be/backend/openecomp-sdc-security-util/src/main/java/org/openecomp/sdc/securityutil/CipherUtil.java
index 4f4c18c..71ac615 100644 (file)
--- a/openecomp-be/backend/openecomp-sdc-security-util/src/main/java/org/openecomp/sdc/securityutil/CipherUtil.java
+++ b/openecomp-be/backend/openecomp-sdc-security-util/src/main/java/org/openecomp/sdc/securityutil/CipherUtil.java
@@ -7,9 +7,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -21,43 +21,47 @@
 package org.openecomp.sdc.securityutil;
 
 import java.security.SecureRandom;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
+import java.util.Arrays;
 import javax.crypto.Cipher;
-import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 import org.apache.commons.codec.binary.Base64;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class CipherUtil {
-    private static Logger log = LoggerFactory.getLogger( CipherUtil.class.getName());
+
+    private static Logger log = LoggerFactory.getLogger(CipherUtil.class.getName());
     private static final String ALGORITHM = "AES";
-    private static final String ALGORYTHM_DETAILS = ALGORITHM + "/CBC/PKCS5PADDING";
+    private static final String ALGORITHM_DETAILS = ALGORITHM + "/GCM/NoPadding";
     private static final String CIPHER_PROVIDER = "SunJCE";
-    private static final int BLOCK_SIZE = 128;
-    private static final int BYTE_SIZE = 8;
-    private static final int IV_SIZE = BLOCK_SIZE / BYTE_SIZE;
+
+    public static final int GCM_TAG_LENGTH = 16;
+    public static final int GCM_IV_LENGTH = 12;
+
     private static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
     private static final String ALGORITHM_NAME = "SHA1PRNG";
 
+    private CipherUtil() {}
+
     /**
      * Encrypt the text using the secret key in key.properties file
      *
      * @param value string to encrypt
      * @return The encrypted string
-     * @throws CipherUtilException
-     *             In case of issue with the encryption
+     * @throws CipherUtilException In case of issue with the encryption
      */
     public static String encryptPKC(String value, String base64key) throws CipherUtilException {
         Cipher cipher;
-        byte[] iv = new byte[IV_SIZE];
+        byte[] iv = new byte[GCM_IV_LENGTH];
         byte[] finalByte;
         try {
-            cipher = Cipher.getInstance(ALGORYTHM_DETAILS, CIPHER_PROVIDER);
+            cipher = Cipher.getInstance(ALGORITHM_DETAILS, CIPHER_PROVIDER);
             SecureRandom secureRandom = SecureRandom.getInstance(ALGORITHM_NAME);
             secureRandom.nextBytes(iv);
-            IvParameterSpec ivspec = new IvParameterSpec(iv);
-            cipher.init(Cipher.ENCRYPT_MODE, getSecretKeySpec(base64key), ivspec);
+            GCMParameterSpec spec =
+                new GCMParameterSpec(GCM_TAG_LENGTH * java.lang.Byte.SIZE, iv);
+            cipher.init(Cipher.ENCRYPT_MODE, getSecretKeySpec(base64key), spec);
             finalByte = cipher.doFinal(value.getBytes());
 
         } catch (Exception ex) {
@@ -70,12 +74,10 @@ public class CipherUtil {
     /**
      * Decrypts the text using the secret key in key.properties file.
      *
-     * @param message
-     *            The encrypted string that must be decrypted using the ONAP Portal
-     *            Encryption Key
+     * @param message The encrypted string that must be decrypted using the ONAP Portal Encryption
+     *                Key
      * @return The String decrypted
-     * @throws CipherUtilException
-     *             if any decryption step fails
+     * @throws CipherUtilException if any decryption step fails
      */
 
     public static String decryptPKC(String message, String base64key) throws CipherUtilException {
@@ -83,10 +85,12 @@ public class CipherUtil {
         Cipher cipher;
         byte[] decrypted;
         try {
-            cipher = Cipher.getInstance(ALGORYTHM_DETAILS, CIPHER_PROVIDER);
-            IvParameterSpec ivspec = new IvParameterSpec(subarray(encryptedMessage, 0, IV_SIZE));
-            byte[] realData = subarray(encryptedMessage, IV_SIZE, encryptedMessage.length);
-            cipher.init(Cipher.DECRYPT_MODE, getSecretKeySpec(base64key), ivspec);
+            cipher = Cipher.getInstance(ALGORITHM_DETAILS, CIPHER_PROVIDER);
+            byte[] initVector = Arrays.copyOfRange(encryptedMessage, 0, GCM_IV_LENGTH);
+            GCMParameterSpec spec =
+                new GCMParameterSpec(GCM_TAG_LENGTH * java.lang.Byte.SIZE, initVector);
+            byte[] realData = subarray(encryptedMessage, GCM_IV_LENGTH, encryptedMessage.length);
+            cipher.init(Cipher.DECRYPT_MODE, getSecretKeySpec(base64key), spec);
             decrypted = cipher.doFinal(realData);
 
         } catch (Exception ex) {
@@ -120,7 +124,7 @@ public class CipherUtil {
 
     private static byte[] subarray(byte[] array, int startIndexInclusive, int endIndexExclusive) {
         if (array == null) {
-            return null;
+            return new byte[0];
         } else {
             if (startIndexInclusive < 0) {
                 startIndexInclusive = 0;

diff --git a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/AuthenticationCookieUtilsTest.java b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/AuthenticationCookieUtilsTest.java
similarity index 98%
rename from openecomp-be/backend/openecomp-sdc-security-util/src/test/java/AuthenticationCookieUtilsTest.java
rename to openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/AuthenticationCookieUtilsTest.java
index 4028034..6c1e6cb 100644 (file)
--- a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/AuthenticationCookieUtilsTest.java
+++ b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/AuthenticationCookieUtilsTest.java
@@ -18,6 +18,8 @@
  * ============LICENSE_END=========================================================
  */
 
+package org.openecomp.sdc.securityutil;
+
 import org.junit.Test;
 
 import javax.servlet.http.Cookie;

diff --git a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/CipherUtilTest.java b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/CipherUtilTest.java
similarity index 93%
rename from openecomp-be/backend/openecomp-sdc-security-util/src/test/java/CipherUtilTest.java
rename to openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/CipherUtilTest.java
index 888c888..437974d 100644 (file)
--- a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/CipherUtilTest.java
+++ b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/CipherUtilTest.java
@@ -18,11 +18,11 @@
  * ============LICENSE_END=========================================================
  */
 
+package org.openecomp.sdc.securityutil;
+
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang.RandomStringUtils;
 import org.junit.Test;
-import org.openecomp.sdc.securityutil.CipherUtil;
-import org.openecomp.sdc.securityutil.CipherUtilException;
 
 import static org.junit.Assert.*;
 
@@ -67,7 +67,7 @@ public class CipherUtilTest {
             CipherUtil.decryptPKC(DATA, KEY);
           fail();
        } catch (CipherUtilException ex) {
-          assertTrue(ex.getMessage().contains("Wrong IV length"));
+          assertTrue(ex.getMessage().contains("Input too short"));
         }
     }
 }

diff --git a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/PasswordsTest.java b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/PasswordsTest.java
similarity index 99%
rename from openecomp-be/backend/openecomp-sdc-security-util/src/test/java/PasswordsTest.java
rename to openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/PasswordsTest.java
index d3ba3bb..34a0c52 100644 (file)
--- a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/PasswordsTest.java
+++ b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/PasswordsTest.java
@@ -18,6 +18,8 @@
  * ============LICENSE_END=========================================================
  */
 
+package org.openecomp.sdc.securityutil;
+
 import org.junit.Test;
 import org.openecomp.sdc.securityutil.Passwords;
 

diff --git a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/RepresentationUtilsTest.java b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/RepresentationUtilsTest.java
similarity index 98%
rename from openecomp-be/backend/openecomp-sdc-security-util/src/test/java/RepresentationUtilsTest.java
rename to openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/RepresentationUtilsTest.java
index 5960c82..f7af4a9 100644 (file)
--- a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/RepresentationUtilsTest.java
+++ b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/RepresentationUtilsTest.java
@@ -18,6 +18,8 @@
  * ============LICENSE_END=========================================================
  */
 
+package org.openecomp.sdc.securityutil;
+
 import org.junit.Test;
 
 import java.io.IOException;

diff --git a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/SessionValidationFilterTest.java b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/filters/SessionValidationFilterTest.java
similarity index 99%
rename from openecomp-be/backend/openecomp-sdc-security-util/src/test/java/SessionValidationFilterTest.java
rename to openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/filters/SessionValidationFilterTest.java
index 07151bc..cb11ef9 100644 (file)
--- a/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/SessionValidationFilterTest.java
+++ b/openecomp-be/backend/openecomp-sdc-security-util/src/test/java/org/openecomp/sdc/securityutil/filters/SessionValidationFilterTest.java
@@ -18,6 +18,8 @@
  * ============LICENSE_END=========================================================
  */
 
+package org.openecomp.sdc.securityutil.filters; 
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;