Code Review Request 8149017 Delayed provider selection broken in RSA client key exchange

[Xuelei Fan]

On 3/11/2016 6:03 PM, Jeroen Cranendonk wrote:
> Hmm, it looks like it should solve the problem I originally reported,
> so that's good :)
Yes, it is a fix for your reported issue.  Thank you!

> If there's a testing Windows JRE/JDK somewhere I can test it here ;)
> 
> It might be worth investigating other calls to
> KeyUtil.isOracleJCEProvider, I think I remember this not being the
> only place where this issue might occur, from when I investigated it.
> (But for us this is the only place that matters :) ).
> 
I made a quick evaluation.  So far, other uses of this method are not
impacted.

> 
> Looking at the fix, I wonder if there might be one (very) edge case
> left, but again not one that would affect us :)
> 
> I'll refer to the numbers in the following code.
> When a non Oracle provider (4: results false) is available and has
> high priority, which supports UNWRAP with the given key, but does not
> support DECRYPT with the given key (Ok, this is a -very- hypothetical
> case.. :) ). The first init (3) will select that provider, which will
> be stored in the cipher instance, and the second init (5) will fail,
> resulting in the whole method to fail, even if another provider/cipher
> (with lower priority) is available which could've handled the request.
> 
Yes.  This is a potential problem.  But as disposing the previous cipher
instance and creating a new instance could be expensive, so I did not
fix this issue this time.  I will make the update if there is a real
complain in practice in the future.

Thanks for the quick response!

Regards,
Xuelei

> TBH the fix for 'JDK-8081297' (issue which is sadly hidden to the
> public ;) ) feels like a bit of a kludge. Based on whether 'the'
> provider is an internal Oracle one or not either UNWRAP or DECRYPT is
> selected/used. But because of delayed provider selection, it's hard to
> say what 'the' provider is to base this choice on :) It feels like the
> choice would have to actually involve multiple providers in some
> cases.
> I'd make a recommendation, but I'm not sure what exactly is fixed by
> JDK-8081297 ;)
> 
> 1>  boolean needFailover = false;
> 2>  Cipher cipher = JsseJce.getCipher(JsseJce.CIPHER_RSA_PKCS1);
>     try {
> 3>      cipher.init(Cipher.UNWRAP_MODE, privateKey, new
> TlsRsaPremasterSecretParameterSpec( maxVersion.v, currentVersion.v),
> generator);
> 4>      needFailover = !KeyUtil.isOracleJCEProvider(
> cipher.getProvider().getName());
>     } catch (InvalidKeyException | UnsupportedOperationException iue) {
>         if (debug != null && Debug.isOn("handshake")) {
>             System.out.println("The Cipher provider " +
> cipher.getProvider().getName() + " may not support
> TlsRsaPremasterSecretParameterSpec");
>         }
>         needFailover = true;
>     }
> 
>     if (needFailover) {
> 5>      cipher.init(Cipher.DECRYPT_MODE, privateKey);
>         boolean failed = false;
>         try {
>             encoded = cipher.doFinal(encrypted);
>         } catch (BadPaddingException bpe) {
>             failed = true;
>         }
>         encoded = KeyUtil.checkTlsPreMasterSecretKey( maxVersion.v,
> currentVersion.v, generator, encoded, failed);
>         preMaster = generatePreMasterSecret( maxVersion.v,
> currentVersion.v, encoded, generator);
>     } else {
>         preMaster = (SecretKey)cipher.unwrap(encrypted,
> "TlsRsaPremasterSecret", Cipher.SECRET_KEY);
>     }
> 
> Cheers!
> Jeroen Cranendonk
> 
> 
> On Thu, Mar 10, 2016 at 4:50 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>> Hi,
>>
>> Please review this update:
>>
>>    http://cr.openjdk.java.net/~xuelei/8149017/webrev.00/
>>
>> The problem is that calling Cipher.getProvider, or any method on Cipher,
>> forces the Cipher instance to skip the delayed provider selection which
>> is built into Cipher.
>>
>> In this update, Cipher.init() was changed to be the first call to an
>> instance of Cipher.
>>
>> Thanks,
>> Xuelei