check stricter algorithm names in Java 14

Description

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.

Environment

None

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.

Answered

Details

Assignee

Reporter

Components

Created August 19, 2019 at 4:04 PM
Updated August 24, 2019 at 1:41 AM
Resolved August 24, 2019 at 1:41 AM