On Wed, 26 Jan 2022 16:02:09 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 ^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