RFR 8014628: Support AES Encryption with HMAC-SHA2 for Kerberos 5

[Weijun Wang]

> On Jan 10, 2018, at 12:22 AM, Sean Mullan <sean.mullan at oracle.com> wrote:
> 
> On 12/19/17 8:39 PM, Weijun Wang wrote:
>>> On Dec 20, 2017, at 5:02 AM, Sean Mullan <sean.mullan at oracle.com> wrote:
>>> 
>>> On 12/19/17 10:52 AM, Weijun Wang wrote:
>>>>> * AesSha2DkCrypto.java
>>>>> 
>>>>> - why does stringToKey(char[] password, String salt, byte[] s2kparams) swallow exceptions but stringToKey(char[] secret, byte[] salt, byte[] params) does not?
>>>> I simply copy the behavior of the same methods for other etypes. Looks like the later is always private and called by the former. The former is called by EncryptionKey::acquireSecretKey and this method was designed to accept a null value instead of handle an exception.
>>> 
>>> Ok, I suggest logging the exception if debug is enabled.
>> The debug flag inside AesDkCrypto and AesSha2DkCrypto are hardcoded to false and not customizable by any system property (and turning it on shows a lot of sensitive things). I can reuse the Krb5.DEBUG flag (controlled by -Dsun.security.krb5.debug=true) but it has never been used inside this level.
>> Also, the only reason for exception I can see is that a user managed to pack a non-positive iteration count into the s2kparams. This should be quite rare.
> 
> Is that a programming error or something that could be from data sent over the network? If only the former, I would say that those exceptions should not be suppressed.

It could be from data sent over the network, this method is called by the client when user uses a username/password pair to login to a KDC. The KDC's response contains the s2kparams value (https://tools.ietf.org/html/rfc4120#section-5.2.7.5) to instruct the user how to generate a key from its password.

> 
> The code can also throw GeneralSecurityException but those are also always suppressed because of the catch block. Is that the right behavior?
> 

Not a right behavior but should be harmless here. In my understanding, in the case of PBE, as long the passphrase, salt, iteration count etc are legal, there will be no further problem in generating a key, choosing a cipher, and do the encryption work, unless there is a programming error.

I think the original designer (of other etypes) meant to let stringToKey(char,String,byte[]) returning a null when there is an error, and all callers of this method will deal with null instead of an exception. If not programmed carefully, this might turn a GeneralSecurityException to a NullPointerException.

--Max

> --Sean