<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Updated webrev:
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<a href="http://cr.openjdk.java.net/%7Ekshefov/8025123/webrev.01/">http://cr.openjdk.java.net/~kshefov/8025123/webrev.01/</a><br>
<br>
- broke lines exceed 80 characters<br>
- the code updated accrding to
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<a
href="http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449">http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449</a><br>
- updated copy right dates<br>
<br>
Artem<br>
<br>
On 09/30/2013 06:04 PM, Xuelei Fan wrote:<br>
</div>
<blockquote cite="mid:524984D4.1000403@oracle.com" type="cite">
<pre wrap="">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]:
<a class="moz-txt-link-freetext" href="http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449">http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449</a>
On 9/30/2013 9:27 PM, Artem wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hello,
please review this fix for 8:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kshefov/8025123/webrev.00/">http://cr.openjdk.java.net/~kshefov/8025123/webrev.00/</a>
<a class="moz-txt-link-rfc2396E" href="http://cr.openjdk.java.net/%7Ekshefov/8025123/webrev.00/"><http://cr.openjdk.java.net/%7Ekshefov/8025123/webrev.00/></a>
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8025123">https://bugs.openjdk.java.net/browse/JDK-8025123</a>
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:
<a class="moz-txt-link-abbreviated" href="mailto:host/https_service_1.test.machine@TEST.REALM">host/https_service_1.test.machine@TEST.REALM</a>
<a class="moz-txt-link-abbreviated" href="mailto:host/https_service_2.test.machine@TEST.REALM">host/https_service_2.test.machine@TEST.REALM</a>
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
'<a class="moz-txt-link-abbreviated" href="mailto:host/test.machine@TEST.REALM">host/test.machine@TEST.REALM</a>' instead of
'<a class="moz-txt-link-abbreviated" href="mailto:host/https_service_1.test.machine@TEST.REALM">host/https_service_1.test.machine@TEST.REALM</a>'
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
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>