AES encryption class
public class AESEncryption {


Make the class final unless you design for it to have child classes.

    private Cipher cipher = null;
    private ByteArrayOutputStream outputStream;


Cipher is a stateful object. It should not be used as a field. Imagine sharing an object instance between multiple threads or methods. Try and never assign null (if only because Java will do it itself).

outputStream is only necessary locally. Again, a stateful object, while there is otherwise no message specific state.

            e.printStackTrace();


Never ever do this. Use a // TODO tagging comment and then throw IllegalStateException(e); even configure your IDE to to this by default. Don't keep running with illegal state (and behold, if you do this, you can even make cipher final - which would have been useful if you'd needed it anyway).

        return new String(bytes);


Don't even convert random bytes to a string. Furthermore, if you decode bytes to a string, explicitly identify the encoding. You may lose data if you find invalid bytes (.e.g. anything above 0x7F for UTF-8).

        PBEKeySpec spec = new PBEKeySpec("12345asdqwe".toCharArray(), salt.getBytes("UTF-8"), 65536, 128);


Don't use static password values stuck in source code. Why do I even have to explain this? Note by the way that the reason that the password is a character array is to make it easier to destroy the password when it is not required any more. Beware that Java only uses the lowest 8 bits of each character - better check that you don't accept passwords that contain other values!

The 65537 value is a magic value, and should be constant. But generally you want to use a much higher value nowadays - unless your password is really complex. Making such a value configurable is very important. This however also means that you will have to indicate the value for the ciphertext somehow.



All in all, you are using stringified input / output and you simply throw Exception - which is simply telling your user to be screwed upon any error. Please look at my answer on how to use security exceptions on StackOverflow.

You don't specify any protocol that you are using. The user will have to guess the encodings of what you return. There is no warning of the mode, the inclusion of the IV or that the salt needs to be saved, the iteration count, the character encoding or the base-64 encoding.

You use CBC, which is not an authenticated mode such as GCM.

The salt is random, which means that the key is random. Since you don't include the salt, nobody will ever be able to decrypt. If the salt is saved otherwise, then it will probably get mangled because you forgot to base 64 encode it.

The key derivation is simply copied for the encrypt and decrypt methods. Why wasn't this put into a separate method?

And "mum's the word" apparently - this code seems to encrypt any strings, not just words?

Let's finish with a positive note: you do always perform UTF-8 encoding / decoding on the "word". Base 64 is not wrong either if a text interface is assumed. That's nice!