RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v10]

[Daniel Fuchs]

On Wed, 26 Jan 2022 16:02:09 GMT, Michael McMahon <michaelm at 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 ^M from test

Looks mostly good. Some doubts about catching just any exception indiscriminately though.

test/jdk/sun/security/krb5/auto/HttpsCB.java line 120:

> 118: 
> 119:         boolean expected1 = Boolean.parseBoolean(args[0]);
> 120:         boolean expected2 = Boolean.parseBoolean(args[1]);

It might be better for future maintainers and readability if these two variables could have better names, and possibly a comment to explain their purpose. AFAIU it's the expected result of running with/without CBT - where `true` means that the operation should succeed and `false` that it's expected to fail with some exception...

test/jdk/sun/security/krb5/auto/HttpsCB.java line 201:

> 199:             return reader.readLine().equals(CONTENT);
> 200:         } catch (Exception e) {
> 201:             return false;

Should we log that we have received the excepted exception here? Shouldn't the catch clause only list the exceptions that we are expecting?

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

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