This, pretty basic, piece of code is quite common when handling encryption / decryption in Java.
final Cipher cipher = Cipher.getInstance("AES/CBC/NoPadding");
cipher.init(Cipher.ENCRYPT_MODE, key, iv);
cipher.doFinal(*something*);
These three lines alone, potentially throw six exceptions and I'm not sure what's the cleanest (in terms of code readability) way to handle them. A try with six catch clauses really looks like a smell to me.
Are there micropatterns or best practices, I am obviously missing, when working with such objects?
EDIT
Sorry, I think I didn't explain myself very well. My question is not really about avoiding a try\catch clause, but if there is a common way to handle similar situations.
The cryptographic exceptions are
NoSuchPaddingException, NoSuchAlgorithmException
InvalidAlgorithmParameterException, InvalidKeyException,
BadPaddingException, IllegalBlockSizeException
You indicated the following exceptions:
NoSuchPaddingException, NoSuchAlgorithmException
InvalidAlgorithmParameterException, InvalidKeyException,
BadPaddingException, IllegalBlockSizeException
Now all of these are GeneralSecurityException
's, so it would be easy to catch them all. But looking at the use case, you probably don't want to do that.
If you look at the cause of the exceptions then you will find that any of these exceptions - except for the last two - are only thrown when generating an implementation of an algorithm or a key. I think it is reasonable that once you have tested your application that these values remain more or less static. Hence it would be logical to throw - for instance - an IllegalStateException
. IllegalStateException
is a runtime exception which you are not required to throw (in the method signature) or catch. Of course, you should include the security exception as being the cause of the exception.
Now the last two exceptions, BadPaddingException
and IllegalBlockSizeException
are different. They depend on the actual ciphertext, so they are dependent on the input of the algorithm. Now normally you should always verify the integrity of the input before you feed it into your Cipher
instance, initiated for decryption, for instance by first validating a HMAC checksum). So in that sense you could still get away with a runtime exception. If you don't perform a separate check for integrity then you should do should not convert to a RuntimeException
. Instead you could either let the user handle the exception, or re-throw it as a use case specific exception.
If you handle the BadPaddingException
by (re-)throwing it then should understand about plaintext oracle attacks such as padding oracle attacks. For padding oracle attacks in CBC mode: if an adversary can try and let you decrypt ciphertext multiple times and receive an indication that decryption failed (or not) then they can retrieve the plaintext of the message without breaking the cipher. For this reason an authenticated mode such as GCM mode should be preferred in situations that can handle the 16 additional bytes for the authentication tag.
It is probably best to use separate try
/catch
blocks for the construction and initialization of the Cipher
and the decryption itself. You could also catch the exceptions BadPaddingException
and IllegalBlockSizeException
before handling the GeneralSecurityException
. Starting with Java 7 you may use multi-catch statements as well (e.g. catch(final BadPaddingException | IllegalBlockSizeException e)
).
Finally some notes:
BadPaddingException
and IllegalBlockSizeException
may be thrown by Cipher
because of because the data was not completely received, or because of an attacker messing with the data;BadPaddingException
may also be thrown if the key is incorrect.If you are willing to lose some specificty, all of the Crypto exceptions extend GeneralSecurityException, you can just catch that instead.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With