check stricter algorithm names in Java 14
Description
Environment
Activity

Brent Putman August 24, 2019 at 1:40 AM
This regards "Build 8" notes in: https://jdk.java.net/14/release-notes
Prior to this release, the SunJCE provider incorrectly returned a Cipher instance for the "AES/GCM/NoPadding" transformation when a caller requested "AES/GCM/PKCS5Padding". The SunJCE provider now throws NoSuchAlgorithmException when "AES/GCM/PKCS5Padding" is requested.
Pretty confident that we aren't affected here. First, none of the official algorithms in either XML Encryption or JOSE actually specify use of AES + GCM + PKCS5Padding. No XML Encryption algos even specify PKCS5Padding at all. In JOSE, this padding is only specified for use in their AES + CBC + HMAC algos (the special authenticated encryption algos that they define in that spec). For their AES + GCM ones they only specify NoPadding. I confirmed this in the source for nimbus-jose-jwt.
Second, someone could theoretically define some custom algo URI for AES + GCM + PKCS5Padding. They'd have to jump through hoops with the right Santuario and OpenSAML components for it to even be a candidate for runtime use. But if they did, then in OpenSAML it wouldn't be usable, at least via our usual config machinery, b/c in the AlgorithmRegistry
we actually make the Cipher.getInstance(...)
call to confirm runtime support; that would fail and the algo wouldn't be registered.
And third, even if someone circumvented all that, the places in OpenSAML Encrypter
and Decrypter
which effectively obtain the Cipher
instance all have try/catch over Exception
, so it would absolutely be caught there. (That was in part b/c Bouncy Castle was at one point throwing an unchecked RuntimeException
for some "normal" cases). In any case NoSuchAlgorithmException
is a checked exception, so there aren't going to be any surprises as with an unchecked one.
So I think we're covered on all fronts.
If someone else sees an angle I'm missing, let me know. Otherwise I'm closing this for now.

Brent Putman August 20, 2019 at 11:10 PM
I'll take a look, hopefully tomorrow.
My recollection is that this is probably already handled in at least 2 ways. First is that during library init when we eval the runtime support for a given algorithm URI, we're actually testing getting a corresponding Cipher
instance with the associated algorithm and padding. If it throws, then we flag as unsupported, so then the latter crypto machinery won't even attempt that algo URI.
Second, one of the crypto providers, probably SunJCE, started throwing an unchecked runtime exception under certain conditions of unsupported things, so I think in the Encrypter
and/or Decrypter
we added a catch block to trap that.
I'll confirm details.
Details
Details
Assignee

Reporter

Java 14 has a release note indicating that exceptions will now be thrown by the SunJCE provider for certain combinations of crypto algorithms and padding. These weren't supported before, but the unsupported part was just ignored rather than throwing an exception.
This probably doesn't affect us, but it would probably be worth someone (?) checking to be sure.