From 468c7be3774c565ffe531d164ff7864135cf5a57 Mon Sep 17 00:00:00 2001 From: Neil Derraugh Date: Mon, 27 Apr 2020 21:41:58 -0400 Subject: Fix security issues in SecurityUtil - Removed hard coded key - Specified mode and padding to address risky algorithm Issue-ID: SDC-2975 Signed-off-by: Neil Derraugh Change-Id: I3360c0ace0ae49571294b8e8c160f0415c578d36 --- .../java/org/onap/sdc/security/SecurityUtil.java | 166 ++++++++++++--------- 1 file changed, 96 insertions(+), 70 deletions(-) diff --git a/security-util-lib/src/main/java/org/onap/sdc/security/SecurityUtil.java b/security-util-lib/src/main/java/org/onap/sdc/security/SecurityUtil.java index 349e893..eb67813 100644 --- a/security-util-lib/src/main/java/org/onap/sdc/security/SecurityUtil.java +++ b/security-util-lib/src/main/java/org/onap/sdc/security/SecurityUtil.java @@ -24,48 +24,51 @@ import static java.nio.charset.StandardCharsets.UTF_8; import fj.data.Either; import java.io.UnsupportedEncodingException; +import java.security.InvalidAlgorithmParameterException; import java.security.InvalidKeyException; import java.security.Key; import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.util.Arrays; import java.util.Base64; import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; +import javax.crypto.KeyGenerator; import javax.crypto.NoSuchPaddingException; -import javax.crypto.spec.SecretKeySpec; +import javax.crypto.SecretKey; +import javax.crypto.ShortBufferException; +import javax.crypto.spec.GCMParameterSpec; import org.onap.sdc.security.logging.enums.EcompLoggerErrorCode; import org.onap.sdc.security.logging.wrappers.Logger; public class SecurityUtil { private static final Logger LOG = Logger.getLogger(SecurityUtil.class); - private static final byte[] KEY = - new byte[] {-64, 5, -32, -117, -44, 8, -39, 1, -9, 36, -46, -81, 62, -15, -63, -75}; + public static final SecurityUtil INSTANCE = new SecurityUtil(); public static final String ALGORITHM = "AES"; public static final String CHARSET = UTF_8.name(); - private final static Key secKey = generateKey(KEY, ALGORITHM); + public static final int GCM_TAG_LENGTH = 16; + public static final int GCM_IV_LENGTH = 12; - /** - * cmd commands >$PROGRAM_NAME decrypt "$ENCRYPTED_MSG" - * >$PROGRAM_NAME encrypt "message" - **/ + private static final Key secKey = generateKey(ALGORITHM); private SecurityUtil() { } - - public static Key generateKey(final byte[] KEY, String algorithm) { + public static SecretKey generateKey(String algorithm) { try { - return new SecretKeySpec(KEY, algorithm); - } catch (Exception e) { - LOG.warn(EcompLoggerErrorCode.PERMISSION_ERROR, "cannot generate key for {}, message : {} .", ALGORITHM, e.getMessage()); - return null; + KeyGenerator kgen = KeyGenerator.getInstance(algorithm); + kgen.init(128); + return kgen.generateKey(); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException(e.toString()); } } - //obfuscates key prefix -> ********** + // obfuscates key prefix -> ********** public String obfuscateKey(String sensitiveData) { if (sensitiveData == null) { @@ -79,76 +82,96 @@ public class SecurityUtil { return builder.toString(); } - + //@formatter:off /** - * @param strDataToEncrypt - plain string to encrypt - * Encrypt the Data + * @param strDataToEncrypt - plain string to encrypt Encrypt the Data * a. Declare / Initialize the Data. Here the data is of type String * b. Convert the Input Text to Bytes * c. Encrypt the bytes using doFinal method */ - public Either encrypt(String strDataToEncrypt) { - if (strDataToEncrypt != null) { - try { - LOG.debug("Encrypt key -> {}", secKey); - Cipher aesCipherForEncryption = Cipher.getInstance( - "AES"); // Must specify the mode explicitly as most JCE providers default to ECB mode!! - aesCipherForEncryption.init(Cipher.ENCRYPT_MODE, secKey); - byte[] byteDataToEncrypt = strDataToEncrypt.getBytes(); - byte[] byteCipherText = aesCipherForEncryption.doFinal(byteDataToEncrypt); - String strCipherText = new String(Base64.getMimeEncoder().encode(byteCipherText), CHARSET); - LOG.debug("Cipher Text generated using AES is {}", strCipherText); - return Either.left(strCipherText); - } catch (NoSuchAlgorithmException | UnsupportedEncodingException e) { - LOG.warn(EcompLoggerErrorCode.PERMISSION_ERROR, - "cannot encrypt data unknown algorithm or missing encoding for {}", secKey.getAlgorithm()); - } catch (InvalidKeyException e) { - LOG.warn(EcompLoggerErrorCode.PERMISSION_ERROR, "invalid key recieved - > {} | {}", - new String(Base64.getDecoder().decode(secKey.getEncoded())), e.getMessage()); - } catch (IllegalBlockSizeException | BadPaddingException | NoSuchPaddingException e) { - LOG.warn(EcompLoggerErrorCode.PERMISSION_ERROR, - "bad algorithm definition (Illegal Block Size or padding), please review you algorithm block&padding", - e.getMessage()); - } + //@formatter:on + public static Either encrypt(String strDataToEncrypt) { + try { + byte[] ciphertext = null; + Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding"); + byte[] initVector = new byte[GCM_IV_LENGTH]; + new SecureRandom().nextBytes(initVector); + GCMParameterSpec spec = + new GCMParameterSpec(GCM_TAG_LENGTH * java.lang.Byte.SIZE, initVector); + cipher.init(Cipher.ENCRYPT_MODE, secKey, spec); + byte[] encoded = strDataToEncrypt.getBytes(java.nio.charset.StandardCharsets.UTF_8); + ciphertext = Arrays.copyOf(initVector, initVector.length + cipher.getOutputSize(encoded.length)); + // Perform encryption + cipher.doFinal(encoded, 0, encoded.length, ciphertext, initVector.length); + String strCipherText = new String(Base64.getMimeEncoder().encode(ciphertext), CHARSET); + return Either.left(strCipherText); + } catch (NoSuchAlgorithmException | UnsupportedEncodingException | InvalidAlgorithmParameterException e) { + LOG.warn( + EcompLoggerErrorCode.PERMISSION_ERROR, + "cannot encrypt data unknown algorithm or missing encoding for {}", + secKey.getAlgorithm()); + } catch (InvalidKeyException e) { + LOG.warn( + EcompLoggerErrorCode.PERMISSION_ERROR, + "invalid key recieved - > {} | {}", + new String(Base64.getDecoder().decode(secKey.getEncoded())), + e.getMessage()); + } catch (IllegalBlockSizeException | BadPaddingException | NoSuchPaddingException e) { + LOG.warn( + EcompLoggerErrorCode.PERMISSION_ERROR, + "bad algorithm definition (Illegal Block Size or padding), please review you algorithm block&padding", + e.getMessage()); + } catch (ShortBufferException e) { + LOG.warn( + EcompLoggerErrorCode.PERMISSION_ERROR, + "the given output buffer is too small to hold the result", + e.getMessage()); } return Either.right("Cannot encrypt " + strDataToEncrypt); } + //@formatter:off /** * Decrypt the Data * * @param byteCipherText - should be valid bae64 input in the length of 16bytes * @param isBase64Decoded - is data already base64 encoded&aligned to 16 bytes - * a. Initialize a new instance of Cipher for Decryption (normally don't reuse the same - * object) + * a. Initialize a new instance of Cipher for Decryption (normally don't reuse the same object) * b. Decrypt the cipher bytes using doFinal method */ - public Either decrypt(byte[] byteCipherText, boolean isBase64Decoded) { - if (byteCipherText != null) { - byte[] alignedCipherText = byteCipherText; - try { - if (isBase64Decoded) { - alignedCipherText = Base64.getDecoder().decode(byteCipherText); - } - LOG.debug("Decrypt key -> " + secKey.getEncoded()); - Cipher aesCipherForDecryption = Cipher.getInstance( - "AES"); // Must specify the mode explicitly as most JCE providers default to ECB mode!! - aesCipherForDecryption.init(Cipher.DECRYPT_MODE, secKey); - byte[] byteDecryptedText = aesCipherForDecryption.doFinal(alignedCipherText); - String strDecryptedText = new String(byteDecryptedText); - LOG.debug("Decrypted Text message is: {}", obfuscateKey(strDecryptedText)); - return Either.left(strDecryptedText); - } catch (NoSuchAlgorithmException e) { - LOG.warn(EcompLoggerErrorCode.PERMISSION_ERROR, - "cannot encrypt data unknown algorithm or missing encoding for {}", secKey.getAlgorithm()); - } catch (InvalidKeyException e) { - LOG.warn(EcompLoggerErrorCode.PERMISSION_ERROR, "invalid key recieved - > {} | {}", - new String(Base64.getDecoder().decode(secKey.getEncoded())), e.getMessage()); - } catch (IllegalBlockSizeException | BadPaddingException | NoSuchPaddingException e) { - LOG.warn(EcompLoggerErrorCode.PERMISSION_ERROR, - "bad algorithm definition (Illegal Block Size or padding), please review you algorithm block&padding", - e.getMessage()); + //@formatter:on + public static Either decrypt(byte[] byteCipherText, boolean isBase64Decoded) { + try { + if (isBase64Decoded) { + byteCipherText = Base64.getDecoder().decode(byteCipherText); } + Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding"); + byte[] initVector = Arrays.copyOfRange(byteCipherText, 0, GCM_IV_LENGTH); + GCMParameterSpec spec = + new GCMParameterSpec(GCM_TAG_LENGTH * java.lang.Byte.SIZE, initVector); + cipher.init(Cipher.DECRYPT_MODE, secKey, spec); + byte[] plaintext = + cipher.doFinal(byteCipherText, GCM_IV_LENGTH, byteCipherText.length - GCM_IV_LENGTH); + String strDecryptedText = new String(plaintext); + return Either.left(strDecryptedText); + } catch (NoSuchAlgorithmException | InvalidAlgorithmParameterException e) { + /* None of these exceptions should be possible if precond is met. */ + LOG.warn( + EcompLoggerErrorCode.PERMISSION_ERROR, + "cannot decrypt data, unknown algorithm or missing encoding for {}", + secKey.getAlgorithm()); + } catch (InvalidKeyException e) { + LOG.warn( + EcompLoggerErrorCode.PERMISSION_ERROR, + "invalid key recieved - > {} | {}", + new String(Base64.getDecoder().decode(secKey.getEncoded())), + e.getMessage()); + } catch (IllegalBlockSizeException | BadPaddingException | NoSuchPaddingException e) { + /* these indicate corrupt or malicious ciphertext */ + LOG.warn( + EcompLoggerErrorCode.PERMISSION_ERROR, + "bad algorithm definition (Illegal Block Size or padding), please review you algorithm block&padding", + e.getMessage()); } return Either.right("Decrypt FAILED"); } @@ -157,8 +180,11 @@ public class SecurityUtil { try { return decrypt(byteCipherText.getBytes(CHARSET), true); } catch (UnsupportedEncodingException e) { - LOG.warn(EcompLoggerErrorCode.PERMISSION_ERROR, "Missing encoding for {} | {} ", secKey.getAlgorithm(), - e.getMessage()); + LOG.warn( + EcompLoggerErrorCode.PERMISSION_ERROR, + "Missing encoding for {} | {} ", + secKey.getAlgorithm(), + e.getMessage()); } return Either.right("Decrypt FAILED"); } -- cgit 1.2.3-korg