Code review request: 8025123: SNI support in Kerberos cipher suites
Weijun Wang
weijun.wang at oracle.com
Mon Sep 30 14:54:03 UTC 2013
Now that we only check "localhost" and "localhost.localdomain", the
following hack in the SSL.java test is not necessary any more:
92 // Run this after KDC, so our own DNS service can be started
93 try {
94 server =
InetAddress.getLocalHost().getHostName().toLowerCase();
95 } catch (java.net.UnknownHostException e) {
96 server = "localhost";
97 }
Just write
server = "host." + OneKDC.REALM.toLowerCase(Locale.US);
There is another coding style thing in KerberosClientKeyExchangeImpl.java:
+ if ("localhost".equals(serverName) ||
+ "localhost.localdomain".equals(serverName)) {
When a long time is broken to two, the second line should be indented
twice the normal width, i.e.
+ if ("localhost".equals(serverName) ||
+ "localhost.localdomain".equals(serverName)) {
Everything nice is fine.
Thanks
Max
On 9/30/13 10:32 PM, Artem wrote:
> Updated webrev: http://cr.openjdk.java.net/~kshefov/8025123/webrev.01/
> <http://cr.openjdk.java.net/%7Ekshefov/8025123/webrev.01/>
>
> - broke lines exceed 80 characters
> - the code updated accrding to
> http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449
> - updated copy right dates
>
> Artem
>
> On 09/30/2013 06:04 PM, Xuelei Fan wrote:
>> It's a good catch and wonderful fix. Thanks, Artem!
>>
>> The fix looks fine to me, except three very minor coding style comments.
>> 1. we used to have only 80 characters in each line.
>> I think there might some lines exceed 80 characters, would you mind
>> break these lines to keep the style consistent? It is especially
>> friendly to users like me that use vi terminal.
>>
>> 2. KerberosClientKeyExchangeImpl.java
>>
>> + if(localHost != null) serverName = localHost;
>>
>> Personally, I prefer to use (with white space after "if", and "{}" over
>> the statement) (see section 7.4 of [1]).
>>
>> + if (localHost != null) {
>> + serverName = localHost;
>> + }
>>
>> 3. Not necessary, but I normally update the copy right date if it is not
>> up-to-date. Still a very personal preference.
>>
>> Thanks,
>> Xuelei
>>
>> [1]:
>> http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449
>>
>> On 9/30/2013 9:27 PM, Artem wrote:
>>> Hello,
>>>
>>> please review this fix for 8:
>>>
>>> http://cr.openjdk.java.net/~kshefov/8025123/webrev.00/
>>> <http://cr.openjdk.java.net/%7Ekshefov/8025123/webrev.00/>
>>> https://bugs.openjdk.java.net/browse/JDK-8025123
>>>
>>> SNI APIs were introduced in JDK 8, but TLS Kerberos client
>>> implementation does not take into account SNI host name when it requests
>>> TGS.
>>>
>>> For example, there are two HTTPS sites at the same machine:
>>>
>>> https_service_1.test.machine
>>> https_service_2.test.machine
>>>
>>> KDC contains records for both HTTPS services:
>>>
>>> host/https_service_1.test.machine at TEST.REALM
>>> host/https_service_2.test.machine at TEST.REALM
>>>
>>> Client wants to request 'https_service_1.test.machine' service, and it
>>> sets SNI host name 'https_service_1.test.machine' during handshaking.
>>> Currently TLS Kerberos client implementation requests TGS for
>>> 'host/test.machine at TEST.REALM' instead of
>>> 'host/https_service_1.test.machine at TEST.REALM'
>>>
>>> Changes:
>>> - ClientHandshaker uses SNI host name if it is specified.
>>> - If client gets server name extension in server hello then it is
>>> considered as SNI confirmation, so SNI hostname must be used to build
>>> Kerberos service principal name. If there is no SNI confirmation, client
>>> uses SNI first and then fallback to getHostSE().
>>> - KerberosClientKeyExchangeImpl.getServiceTicket() method used to change
>>> a hostname for service principal if loopback address was used. But since
>>> we introduced SNI, using IP address to make the decision does not work
>>> any more. For compatibility reasons, the method checks that "localhost"
>>> or "localhost.localdomain" are passed (they are two known loopback
>>> hostname). If so, it still tries to get the local hostname.
>>> - Added a test case for test/sun/security/krb5/auto/SSL.java
>>>
>>> I have tested this with available reg/jck/sqe tests, no issues found.
>>>
>>> Artem
>
More information about the security-dev
mailing list