[8u] TLSv1.3 RFR: 8245474: Add TLS_KRB5 cipher suites support according to RFC-2712

[Alexey Bakhtin]

Hi Martin,

Thank you for update.
This version is looks good, I do not see any issues and all test passed.
May be some minor comments :
KrbClientKeyExchange.java - krb5Helper is initialized in constructor and should not be changed later. It could be final.
Also, there are some unused imports and method in Krb5 functionality
( not actually related to your code change) :
- SecretKey and KerberosKey are not used in Krb5ProxyImpl and could be removed from imports. If it is applied, KerberosKey should be also removed in refs.allowed script for Krb5ProxyImpl class,
- SecretKey is not used in Krb5Proxy and could be removed from imports,
- Krb5Helper.isAvailable() method is not used also and could be removed.

Thank you
Alexey

> On 20 Jul 2020, at 23:33, Martin Balao <mbalao at redhat.com> wrote:
> 
> Hi Alexey,
> 
> Thanks for your feedback.
> 
> On 7/17/20 5:32 PM, Alexey Bakhtin wrote:
>> 1. You have updated the list of classes in the sun/security/ssl/krb5 package.
>>    These classes are the bridge to JGSS/KRB5 implementation. The compact
>>     profile 1 does not include JGSS/KRB5 implementation, so build scripts
>>     verifies references to removed packages. Exceptions are described in the
>>     make/data/checkdeps/refs.allowed script. This script should be updated
>>      with new class names. Otherwise it fails during profiles creation.
> 
> Well spotted! Should be fixed in Webrev.03.
> 
>> 2. Three kerberos test failed because of server can not select KRB5 cipher suite.
>>     It happens because of server principal name is not specified (it’s allowed behaviour).
>>     As result implementation does not create possession and corresponding cipher suite
>>     is not selected. I suggest to create possession even if no serverPrincipal returned,
>>     similar to original implementation.
>>     The code could be update like following in the KrbKeyExchange.java:
>> @@ -91,7 +91,6 @@ final class KrbKeyExchange {
>>                             }
>>                             return null;
>>                         }
>> -                        return new KrbServiceCreds(serviceCreds);
>>                     }
>>                 }
>>             } catch (PrivilegedActionException e) {
>> @@ -100,8 +99,9 @@ final class KrbKeyExchange {
>>                     SSLLogger.fine("Attempt to obtain Kerberos key failed: "
>>                             + e.toString());
>>                 }
>> +                return null;
>>             }
>> -            return null;
>> +            return (serviceCreds != null)?new KrbServiceCreds(serviceCreds):null;
>>         }
>>     }
> 
> Hmm.. interesting. You're right: I took the extra license of discarding
> the ciphersuite if serverPrincipal is null.  This does not reflect the
> previous behavior in ServerHandshaker::setupKerberosKeys method
> (ServerHandshaker.java). Should be fixed in Webrev.03.
> 
> Webrev.03:
> http://cr.openjdk.java.net/~mbalao/webrevs/8245474/8245474.webrev.03/
> 
> Look forward to more feedback.
> 
> Thanks,
> Martin.-
>