Is this the best/secure method for encryption in Java?

Question

I am new in security and I'm wondering if I can make my program better, like changing or adding something to make it better ( more secure )
(I have doubt from the program output)

Here is the output:

Encrypted Message: +g@þóv«5Ùû`ž   
keybyte: [B@71e7a66b   
Original string: Message   
Original string (Hex): [B@2ac1fdc4

Here is the code:

public class AES {

  public static void main(String ... args) throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException, IllegalBlockSizeException, BadPaddingException, UnsupportedEncodingException {
    final String Algo="AES";
    String key = "aaaaaaaaaaaaaaaa";
    byte[] keyBytes = key.getBytes(StandardCharsets.UTF_8);

    MessageDigest sha= MessageDigest.getInstance("SHA-1"); 
    keyBytes=sha.digest(keyBytes);
    keyBytes=Arrays.copyOf(keyBytes, 16);

    SecretKeySpec secretKeySpec = new SecretKeySpec(keyBytes, Algo);
    Cipher cipher = Cipher.getInstance(Algo);
    cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec);
    byte[] ciphertext = cipher.doFinal("Message".getBytes());
    System.out.println("Encrypted Message: " +new String(ciphertext));

    cipher.init(Cipher.DECRYPT_MODE, secretKeySpec);
    byte[] original = cipher.doFinal(ciphertext);
    String originalString = new String(original);
    System.out.println("keybyte: "+keyBytes);
    System.out.println("Original string: " + originalString + "\nOriginal string (Hex): " +original);      
  }
}

Make the encryption/decryption more secure ( should I use keyGenerator instead ? ) — Ben193, Mar 29 '17 at 18:51

score 7 · Accepted Answer · edited May 23 '17 at 12:25

No, this code is pretty terrible:

Your key is fixed and a string that is then hashed. This looks to me like your key is actually supposed to be a password. A single hash is not enough to derive a key from a password. You need to use a strong hashing scheme like PBKDF2, bcrypt, scrypt and Argon2. Be sure to use a high cost factor/iteration count. It is common to choose the cost so that a single iteration takes at least 100ms. See more: How to securely hash passwords?
Always use a fully qualified Cipher string. Cipher.getInstance("AES"); may result in different ciphers depending on the default security provider. It most likely results in "AES/ECB/PKCS5Padding", but it doesn't have to be. If it changes, you'll lose compatibility between different JVMs. For reference: Java default Crypto/AES behavior
ECB mode is pretty bad. It's deterministic and therefore not semantically secure. You should at the very least use a randomized mode like CBC or CTR.
Without authentication you have the threat of not detecting (malicious) modifications of your ciphertexts. It is better to authenticate your ciphertexts so that attacks like a padding oracle attack are not possible. This can be done with authenticated modes like GCM or EAX, or with an encrypt-then-MAC scheme through HMAC with a strong hash function such as SHA-256.

Because of these issues you should use a library instead. Try JNCryptor or this library.

score 6 · Answer 2 · answered Mar 29 '17 at 21:21

There are at least two problems with your implementation:

Unless you specify otherwise, you are using AES in ECB mode. ECB mode is not secure regardless of what cipher is under the hood. There are a number of secure modes, but usually people implement CBC mode, which is accomplished by changing your Algo to "AES/CBC/PKCS5Padding" (which really is PKCS7 padding, Java just does not know better). Then, you need to choose an IV via SecureRandom() to encrypt this way. This OWASP example seems to do it right (FYI -- 99% of the implementations you will find on the web have security problems somewhere).
Your key is not a key, instead it is a password that is being turned into a key. You shouldn't be hardcoding this, but I assume you are doing that for proof of concept only. In any case, the problem is using a hash function such as SHA-1 to turn a password into a key is not a good decision because passwords tend to have low entropy and can be brute forced. For this reason, you should be using a function that is dedicated to resisting brute force when turning a password into a key. Such functions include pbkdf2, bcrypt, scrypt, and argon2. For more information, Troy Hunt (a .Net guy) gives a good overview on the problems with using something like SHA-1 (or anything from the SHA2 family for that matter) in this context.

The other thing to keep in mind is that encryption does not generally provide message integrity. What this means is that just because you have encrypted the data does not mean somebody cannot modify it, and your software will still decrypt the modified data without being aware that it is modified. If you need to know that your data was not modified, then you will need to add on something like HMAC, or else you will need to switch to a mode of operation such as GCM.

Finally, AES is an excellent choice for security. But you need to use it in the right mode, you need to implement it right, and you need to understand whether you need more than just encryption.

Thank you very much for all your time and help. That's a lot of new informations for me. I found using Salt with iteration easier for my current level, since I just begun to learn security. I am looking forward to using your tips for the next projects. Thank you very much — Ben193, Mar 30 '17 at 01:11

Is this the best/secure method for encryption in Java?

2 Answers2