Cipher Encryption is noticeably slow

Question

We are incorporating some encryption into our URL generator and during testing I noticed that the following step makes the program hang for quite some time.

cipher.init(Cipher.ENCRYPT_MODE, key, new IvParameterSpec(IV.getBytes("UTF-8")));

Here is part of the function in which it is called, once it reaches that line it will simply hang and can take more than 2 minutes to finally pass. Wondering if anyone knows the cause or a solution.

public static String encrypt(String toEncrypt) throws Exception 
  {

        SecretKeySpec key = new SecretKeySpec(encryptionKey.getBytes("UTF-8"), "AES");

        Security.addProvider(new BouncyCastleProvider());
        Cipher cipher = Cipher.getInstance("AES/CBC/PKCS7Padding"); 

        cipher.init(Cipher.ENCRYPT_MODE, key, new IvParameterSpec(IV.getBytes("UTF-8")));

        byte[] encrypted = cipher.doFinal(toEncrypt.getBytes());
        byte[] encryptedValue = Base64.encodeBase64(encrypted);

        return new String(encryptedValue);
  }

Thanks,

Answer 1

Security.addProvider(new BouncyCastleProvider());

So for every string to be encrypted, you create a new security provider. Would you start a new web server for every web request? Would you buy a new computer when you want edit a file?

I'm not claiming that exactly this line is the culprit, but doing all the initialization every time just can't be right.

Drop static (*), use a singleton, ideally from Guice or alike, initialize it once. Your encryption should look like

Fixed according to the comments by @apangin and @IlmariKaronen.

static {
    // This really should be done just once.
    // Moreover, you most probably don't need it.
    Security.addProvider(new BouncyCastleProvider());
}

Encryptor() {
    SecretKeySpec key = new SecretKeySpec(encryptionKey.getBytes("UTF-8"), "AES");
    Cipher cipher = Cipher.getInstance("AES/CBC/PKCS7Padding"); 
    cipher.init(Cipher.ENCRYPT_MODE, key, new IvParameterSpec(IVBytes));
}

public synchronized String encrypt(String toEncrypt) throws Exception {
    byte[] encrypted = cipher.doFinal(toEncrypt.getBytes("UTF-8"));
    byte[] encryptedValue = Base64.encodeBase64(encrypted);
    return new String(encryptedValue);
}

As encryption itself is very fast and you're encrypting just short strings, using synchronized should work well. Alternatively, go for a thread-local cipher or alike.

Note that reusing the IV makes it insecure. But that's another story.

(*) That's unrelated to performance, but a good thing.