On Thu, 20 Jan 2022 10:58:27 GMT, Michael McMahon <micha...@openjdk.org> wrote:

>> Hi,
>> 
>> This change adds Channel Binding Token (CBT) support to HTTPS 
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, 
>> Kerberos) authentication scheme. When enabled, the implementation 
>> preemptively includes a CBT with authentication requests over Kerberos. The 
>> feature is enabled as follows:
>> 
>> A system property "jdk.spnego.cbt" is defined which can have the values 
>> "never" (default), which means the feature is disabled, "always", which 
>> means the CBT is included for all https Negotiate authentications, or it can 
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of 
>> domains/hosts where the feature is enabled, and disabled everywhere else. In 
>> the given example, the CBT would be included in authentication requests for 
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
>> sub-domains.
>> 
>> A test will be added separately to the implementation.
>> 
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   removed sasl module dependency and added SaslException cause

src/java.base/share/classes/java/net/doc-files/net-properties.html line 220:

> 218:  This controls the generation and sending of TLS channel binding tokens 
> (CBT) when Kerberos 
> 219:         or the Negotiate authentication scheme using Kerberos are 
> employed over HTTPS with 
> 220:         {@code HttpURLConnection}. There are three possible settings:</P>

Should it be `{@code HttpsURLConnection}`?
(BTW - can we use {@code } here ? Would be worth checking the generated doc)

src/java.base/share/classes/sun/net/www/http/HttpClient.java line 189:

> 187:         } else {
> 188:             logError("Unexpected value for \"jdk.https.negotiate.cbt\" 
> system property");
> 189:             return s;

Should this return either "always" or "never" instead? It seems that junk 
values will be treated as "always". It would be better to make it clear here.

src/java.base/share/classes/sun/security/util/ChannelBindingException.java line 
31:

> 29:  * Thrown by TlsChannelBinding if an error occurs
> 30:  */
> 31: public class ChannelBindingException extends Exception {

Should this extend `GeneralSecurityException` instead? Or should we just remove 
this class and throw plain `GeneralSecurityException` in `TlsChannelBinding` ?

src/java.naming/share/classes/com/sun/jndi/ldap/sasl/LdapSasl.java line 143:

> 141:                             tlsCB = TlsChannelBinding.create(cert);
> 142:                         } catch (ChannelBindingException e) {
> 143:                             throw new SaslException(e.getMessage());

Why is there a difference compared to line 133?

-------------

PR: https://git.openjdk.java.net/jdk/pull/7065

Reply via email to