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