From f9c66e100522272543a550736cbe660cad4bfec5 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Wed, 28 Oct 2020 16:22:01 -0400 Subject: Fix sonar security issue in CryptoUtils 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 --- .../onap/policy/common/utils/security/CryptoUtils.java | 10 ++++++---- .../policy/common/utils/coder/PropertyCoderTest.java | 18 ++++++++++++++++-- .../policy/common/utils/security/CryptoUtilsTest.java | 6 +++--- 3 files changed, 25 insertions(+), 9 deletions(-) (limited to 'utils/src') diff --git a/utils/src/main/java/org/onap/policy/common/utils/security/CryptoUtils.java b/utils/src/main/java/org/onap/policy/common/utils/security/CryptoUtils.java index af5b3d49..1a9483a9 100644 --- a/utils/src/main/java/org/onap/policy/common/utils/security/CryptoUtils.java +++ b/utils/src/main/java/org/onap/policy/common/utils/security/CryptoUtils.java @@ -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); diff --git a/utils/src/test/java/org/onap/policy/common/utils/coder/PropertyCoderTest.java b/utils/src/test/java/org/onap/policy/common/utils/coder/PropertyCoderTest.java index 47453dfc..86f8a1b1 100644 --- a/utils/src/test/java/org/onap/policy/common/utils/coder/PropertyCoderTest.java +++ b/utils/src/test/java/org/onap/policy/common/utils/coder/PropertyCoderTest.java @@ -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('\'', '"'); diff --git a/utils/src/test/java/org/onap/policy/common/utils/security/CryptoUtilsTest.java b/utils/src/test/java/org/onap/policy/common/utils/security/CryptoUtilsTest.java index ce9435d8..625fd1f5 100644 --- a/utils/src/test/java/org/onap/policy/common/utils/security/CryptoUtilsTest.java +++ b/utils/src/test/java/org/onap/policy/common/utils/security/CryptoUtilsTest.java @@ -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 +} -- cgit 1.2.3-korg