I'm not a security expert, but while checking over our AES implementation for our flagship product, I've noticed some strange things, like the output length having a relation with the input length and no apparent use of an IV.
@Service
public class EncryptionServiceImpl implements EncryptionService {
/** The logger for this class */
private static final Logger LOGGER = new Logger(EncryptionServiceImpl.class);
/** There's one and only one instance of this class */
private volatile static EncryptionServiceImpl INSTANCE;
/** True if EncryptionService is initialized. */
private boolean isInitialized = false;
private Cipher cipherEncrypt;
private Cipher cipherDecrypt;
private String keyHex;
/**
* Constructor is private, use getInstance to get an instance of this class
*/
private EncryptionServiceImpl() {
initialize();
}
/**
* Returns the singleton instance of this class.
*
* @return the singleton instance of this class.
*/
public static EncryptionServiceImpl getInstance() {
if (INSTANCE == null) {
synchronized (EncryptionServiceImpl.class) {
if (INSTANCE == null) {
INSTANCE = new EncryptionServiceImpl();
}
}
}
return INSTANCE;
}
/**
* Initialize EncryptionService.
*/
private synchronized void initialize() {
if (!isInitialized){
// Get key from SystemSettings.
SystemSettingsService systemSettingsService = (SystemSettingsService) ServiceFactory.getInstance().createService(SystemSettingsService.class);
keyHex = systemSettingsService.getScmuuid();
byte[] keyBytes;
// If keyHex is not blank (field "scmuuid" already exists in the database):
if (StringUtils.isNotBlank(keyHex)) {
keyBytes = hexToBytes(keyHex);
SecretKeySpec secretKeySpec = new SecretKeySpec(keyBytes, "AES");
try {
cipherEncrypt = Cipher.getInstance("AES");
cipherDecrypt = Cipher.getInstance("AES");
cipherEncrypt.init(Cipher.ENCRYPT_MODE, secretKeySpec);
cipherDecrypt.init(Cipher.DECRYPT_MODE, secretKeySpec);
} catch (InvalidKeyException e) {
throw new InitializationFailureException(
"Failure to generate a new encryption key.", e);
} catch (NoSuchAlgorithmException e) {
throw new InitializationFailureException(
"Failure to generate a new encryption key.", e);
} catch (NoSuchPaddingException e) {
throw new InitializationFailureException(
"Failure to generate a new encryption key.", e);
}
//EncryptionService is initialized.
isInitialized = true;
} else {
/*
* If keyHex is blank, either we have an SQL exception or the key hasn't
* been generated yet. Trying to use the EncryptionService without proper
* initialization, will throw a FatalException.
* If the key hasn't been generated yet, the next exception will trigger
* the caller to use the generateKey() method in the catch block.
*/
//throw new NoEncryptionkeyException();
}
}
}
/**
* @see shared.bs.encryption.EncryptionService#isInitialized()
*/
public boolean isInitialized() {
return isInitialized;
}
/**
* @see shared.bs.encryption.EncryptionService#decrypt()
*/
public String decrypt(String value) {
if (StringUtils.isBlank(value)){
return null;
}
// NULL values from log files can be interpreted as a String with value "null" (see e.g. bug REDACTED)
if (value != null && value.equalsIgnoreCase("null")) {
return null;
}
if (getCipherDecrypt() == null) {
throw new EncryptionFailureException("Decryption failure. EncryptionService is not properly initialized.");
}
byte[] encryptedBytes = null;
byte[] decryptedBytes = null;
try {
encryptedBytes = hexToBytes(value);
decryptedBytes = cipherDecrypt.doFinal(encryptedBytes);
} catch (NumberFormatException e) {
throw new EncryptionFailureException("Decryption failure.", e);
} catch (IllegalBlockSizeException e) {
throw new EncryptionFailureException("Decryption failure.", e);
} catch (BadPaddingException e) {
throw new EncryptionFailureException("Decryption failure.", e);
}
return new String(decryptedBytes);
}
/**
* @see shared.bs.encryption.EncryptionService#encrypt()
*/
public String encrypt(String value) {
if (StringUtils.isBlank(value)){
return null;
}
if (getCipherEncrypt() == null) {
throw new EncryptionFailureException("Encryption failure. EncryptionService is not properly initialized.");
}
byte[] encrypted = null;
String encHex = null;
try {
encrypted = cipherEncrypt.doFinal(value.getBytes());
encHex = asHex(encrypted);
} catch (IllegalBlockSizeException e) {
throw new EncryptionFailureException("Encryption failure.", e);
} catch (BadPaddingException e) {
throw new EncryptionFailureException("Encryption failure.", e);
} catch (NumberFormatException e) {
throw new EncryptionFailureException("Encryption failure.", e);
}
return encHex;
}
/** convert a byte array to a hex String */
private String asHex(byte buf[]) {
StringBuffer strbuf = new StringBuffer(buf.length * 2);
int i;
for (i = 0; i < buf.length; i++) {
if (((int) buf[i] & 0xff) < 0x10) {
strbuf.append("0");
}
strbuf.append(Long.toString((int) buf[i] & 0xff, 16));
}
return strbuf.toString();
}
/** convert a hex String to a byte array */
private byte[] hexToBytes(String hex) {
byte[] bts = new byte[hex.length() / 2];
for (int i = 0; i < bts.length; i++) {
bts[i] = (byte) Integer.parseInt(hex.substring(2 * i, 2 * i + 2), 16);
}
return bts;
}
/**
* @see shared.bs.encryption.EncryptionService#generateKey()
*/
public synchronized void generateKey(){
if (!isInitialized){
/*
* Make sure the key really doesn't already exist and that an initialization failure
* isn't the result of an earlier SQLException.
* Try again retrieving the key from the database.
*/
SystemSettingsService systemSettingsService = (SystemSettingsService) ServiceFactory.getInstance().createService(SystemSettingsService.class);
String tryKeyHex = systemSettingsService.getScmuuid();
if (StringUtils.isNotBlank(tryKeyHex)) {
// Something came back from the database, we try to initialize again and return silently.
initialize();
return;
}
// Generate a new 128 bit strong AES key.
KeyGenerator kgen;
try {
kgen = KeyGenerator.getInstance("AES");
} catch (NoSuchAlgorithmException e) {
throw new InitializationFailureException(
"Failure to generate a new encryption key.", e);
}
kgen.init(128); // 128 is in standard JCE
SecretKey secretKey = kgen.generateKey();
byte[] keyBytes = secretKey.getEncoded();
keyHex = asHex(keyBytes);
// We have a keyHex, it's time to generate the ciphers:
SecretKeySpec secretKeySpec = new SecretKeySpec(keyBytes, "AES");
try {
cipherEncrypt = Cipher.getInstance("AES");
cipherDecrypt = Cipher.getInstance("AES");
cipherEncrypt.init(Cipher.ENCRYPT_MODE, secretKeySpec);
cipherDecrypt.init(Cipher.DECRYPT_MODE, secretKeySpec);
} catch (InvalidKeyException e) {
throw new InitializationFailureException(
"Failure to generate a new encryption key.", e);
} catch (NoSuchAlgorithmException e) {
throw new InitializationFailureException(
"Failure to generate a new encryption key.", e);
} catch (NoSuchPaddingException e) {
throw new InitializationFailureException(
"Failure to generate a new encryption key.", e);
}
/*
* Persist keyHex (field scmuuid in SystemSettings) and encrypt all existing non-encrypted
* passwords and secure build/deploy-parameters in the database with this new key.
* removed for brevity in this example: each of these is an extra method call.
*/
}//End: if (!isInitialized)
}
I got the impression that this has room for improvement, since information about the plaintext is leaking. Please note that because of legacy code throughout the entire project (mainly database field lengths), all our ciphertext output has to be shorter than 255 characters. In effect, this means that the output has to be 224 bytes long.
And yes, I know that we're encrypting passwords. These are not user passwords. Those are handled through external systems like Active Directory and LDAP. These encrypted passwords are used to authenticate to external 3rd party systems where implementing a token-based authentication scheme either is not possible, is not feasible, or has been tried without success.