Integrated: 8259707: LDAP channel binding does not work with StartTLS extension
On Thu, 14 Jan 2021 19:28:27 GMT, Alexey Bakhtin wrote: > Please review a small patch to enable LDAP TLS Channel Binding with StartTLS > Extension. > Test from the bug report and jtreg javax/naming tests are passed. This pull request has now been integrated. Changeset: 874aef4a Author: Alexey Bakhtin Committer: Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/874aef4a Stats: 74 lines in 2 files changed: 39 ins; 21 del; 14 mod 8259707: LDAP channel binding does not work with StartTLS extension Reviewed-by: mullan, dfuchs, aefimov - PR: https://git.openjdk.java.net/jdk/pull/2085
Re: RFR: 8259707: LDAP channel binding does not work with StartTLS extension [v4]
> Please review a small patch to enable LDAP TLS Channel Binding with StartTLS > Extension. > Test from the bug report and jtreg javax/naming tests are passed. Alexey Bakhtin has updated the pull request incrementally with one additional commit since the last revision: Update copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/2085/files - new: https://git.openjdk.java.net/jdk/pull/2085/files/e3bf8c36..40447456 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2085&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2085&range=02-03 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2085.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2085/head:pull/2085 PR: https://git.openjdk.java.net/jdk/pull/2085
Re: RFR: 8259707: LDAP channel binding does not work with StartTLS extension [v2]
On Thu, 21 Jan 2021 18:19:10 GMT, Aleksei Efimov wrote:
>> Alexey Bakhtin has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> separate tlsHandshakeCompleted for every StartTLS connection
>
> src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java line 1034:
>
>> 1032: }
>> 1033:
>> 1034: private HandshakeListener tlsHandshakeListener;
>
> I believe that `volatile` modifier should be added here. And it could be
> beneficial for future maintainers to have here a comment block with a brief
> description of when `tlsHandshakeListener` is used.
Agree. Added comments and volatile modifier.
> src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java line 1059:
>
>> 1057: private class HandshakeListener implements
>> HandshakeCompletedListener {
>> 1058:
>> 1059: private CompletableFuture
>> tlsHandshakeCompleted =
>
> `tlsHandshakeCompleted` could be made `final`
Thank you. Made it final
-
PR: https://git.openjdk.java.net/jdk/pull/2085
Re: RFR: 8259707: LDAP channel binding does not work with StartTLS extension [v3]
> Please review a small patch to enable LDAP TLS Channel Binding with StartTLS > Extension. > Test from the bug report and jtreg javax/naming tests are passed. Alexey Bakhtin has updated the pull request incrementally with one additional commit since the last revision: Add comments and volatile modifier for tlsHandshakeListener - Changes: - all: https://git.openjdk.java.net/jdk/pull/2085/files - new: https://git.openjdk.java.net/jdk/pull/2085/files/30a29e07..e3bf8c36 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2085&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2085&range=01-02 Stats: 7 lines in 1 file changed: 5 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2085.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2085/head:pull/2085 PR: https://git.openjdk.java.net/jdk/pull/2085
Re: RFR: 8259707: LDAP channel binding does not work with StartTLS extension [v2]
On Wed, 20 Jan 2021 15:54:41 GMT, Daniel Fuchs wrote: >> New ChannelBinding Data will be recreated for every TLS connection and >> provided to SASL Client in the new environment properties set (cloned from >> the original). >> LdapSasl.java lines 133 - 136: >> TlsChannelBinding tlsCB = >> TlsChannelBinding.create(cert); >> envProps = (Hashtable) env.clone(); >> envProps.put(TlsChannelBinding.CHANNEL_BINDING, >> tlsCB.getData()); > >> New ChannelBinding Data will be recreated for every TLS connection and >> provided to SASL Client in the new environment properties set (cloned from >> the original). >> LdapSasl.java lines 133 - 136: >> >> ``` >> TlsChannelBinding tlsCB = >> TlsChannelBinding.create(cert); >> envProps = (Hashtable) env.clone(); > > Hi Alexey, > > Aleksei and I have concern because this code uses a `cert` that is obtained > from a CompletableFuture, and the completable future can be completed only > once. The second time around - you will therefore find the same `cert` that > was set when the first StartTLSResponse was negotiated. This may - or may not > matter - depending on whether the `cert` certificate returned by the server > the second time around should be the same - or not. > Could you test this scenario? > It may be that it's a niche scenario that makes no sense or that we don't > want to support - I'm not sure how STARTTLS is used in the wild. Do you have > any insights on this? > > best regards, > > -- daniel Hi Daniel, Aleksei, You are right, the second StartTLS request works incorrectly because of a single CompletableFuture. I do not know if several StartTLS sessions are used in the wild, but there are no such restrictions in the spec. I have updated the review to create new CompletableFuture for each TLS session. Updated test, suggested by Aleksei is passed. I have verified that the Channel Binding data is created on the base of a new cert object every TLS session. - PR: https://git.openjdk.java.net/jdk/pull/2085
Re: RFR: 8259707: LDAP channel binding does not work with StartTLS extension [v2]
> Please review a small patch to enable LDAP TLS Channel Binding with StartTLS > Extension. > Test from the bug report and jtreg javax/naming tests are passed. Alexey Bakhtin has updated the pull request incrementally with one additional commit since the last revision: separate tlsHandshakeCompleted for every StartTLS connection - Changes: - all: https://git.openjdk.java.net/jdk/pull/2085/files - new: https://git.openjdk.java.net/jdk/pull/2085/files/d2f470e7..30a29e07 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2085&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2085&range=00-01 Stats: 71 lines in 2 files changed: 37 ins; 25 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/2085.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2085/head:pull/2085 PR: https://git.openjdk.java.net/jdk/pull/2085
Re: RFR: 8259707: LDAP channel binding does not work with StartTLS extension
On Wed, 20 Jan 2021 15:08:56 GMT, Aleksei Efimov wrote:
>> That look reasonable to me. But what would happen if at some point after
>> performing some LDAP operations, you called StartTLSResponse::close and then
>> after some more time you tried to again create a StartTLSRequest on the same
>> context? Would that work - or would you be using a possibly obsolete channel
>> binding obtained from the first upgrade?
>
> The change looks valid to me too.
> I believe Daniel question could be illustrated with the following change to
> `CBwithTLS` reproducer attached to the bug report:
> --- CBwithTLS_Original.java 2021-01-20 14:56:09.620844903 +
> +++ CBwithTLS.java2021-01-20 15:01:47.253982227 +
> @@ -45,7 +45,7 @@
> System.out.println(ctxt.getAttributes("", new
> String[]{"defaultNamingContext"}).get("defaultNamingContext").get());
>
> // Switch to TLS
> -
> +for (int i=0; i<2; i++) {
> StartTlsResponse tls = (StartTlsResponse) ctxt.extendedOperation(new
> StartTlsRequest());
> tls.negotiate();
>
> @@ -64,6 +64,9 @@
> lc.login();
>
> Subject.doAs(lc.getSubject(), (PrivilegedAction)
> CBwithTLS::run);
> +lc.logout();
> +tls.close();
> +}
> }
>
> private static Void run() {
New ChannelBinding Data will be recreated for every TLS connection and provided
to SASL Client in the new environment properties set (cloned from the
original).
LdapSasl.java lines 133 - 136:
TlsChannelBinding tlsCB =
TlsChannelBinding.create(cert);
envProps = (Hashtable) env.clone();
envProps.put(TlsChannelBinding.CHANNEL_BINDING,
tlsCB.getData());
-
PR: https://git.openjdk.java.net/jdk/pull/2085
Re: RFR: 8259707: LDAP channel binding does not work with StartTLS extension
On Wed, 20 Jan 2021 14:01:45 GMT, Sean Mullan wrote: >> Please review a small patch to enable LDAP TLS Channel Binding with StartTLS >> Extension. >> Test from the bug report and jtreg javax/naming tests are passed. > > Marked as reviewed by mullan (Reviewer). Sean, Thank you for review - PR: https://git.openjdk.java.net/jdk/pull/2085
Re: RFR: 8259707: LDAP channel binding does not work with StartTLS extension
On Tue, 19 Jan 2021 20:24:21 GMT, Sean Mullan wrote: >> Please review a small patch to enable LDAP TLS Channel Binding with StartTLS >> Extension. >> Test from the bug report and jtreg javax/naming tests are passed. > > Can you add a test for this or is it too hard? There are existing tests for > StartTLS in the security/infra area -- could they be enhanced to test this > case? Unfortunately, I can not find any LDAP StartTLS Extended Operation regression tests. security/infra area contains RevocationChecker tests. They can not be used for this scenario. - PR: https://git.openjdk.java.net/jdk/pull/2085
RFR: 8259707: LDAP channel binding does not work with StartTLS extension
Please review a small patch to enable LDAP TLS Channel Binding with StartTLS Extension. Test from the bug report and jtreg javax/naming tests are passed. - Commit messages: - 8259707: LDAP channel binding does not work with StartTLS extension Changes: https://git.openjdk.java.net/jdk/pull/2085/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2085&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259707 Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2085.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2085/head:pull/2085 PR: https://git.openjdk.java.net/jdk/pull/2085
Integrated: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Mon, 21 Sep 2020 08:19:28 GMT, Alexey Bakhtin wrote: > Hi, > > Plaese review JDK-8245527 fix which implements LDAP Channel Binding support > for Java GSS/Kerberos. > Initial review is available at core-devs: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html > This version removes "tls-unique" CB type from the list of possible channel > binding types. The only supported type is > "tls-server-end-point" > CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 > > Thank you > Alexey This pull request has now been integrated. Changeset: cfa3f749 Author:Alexey Bakhtin URL: https://git.openjdk.java.net/jdk/commit/cfa3f749 Stats: 543 lines in 10 files changed: 531 ins; 0 del; 12 mod 8245527: LDAP Channel Binding support for Java GSS/Kerberos Reviewed-by: dfuchs, aefimov, mullan - PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos [v2]
On Tue, 22 Sep 2020 15:36:24 GMT, Daniel Fuchs wrote:
>> No, It is not used.
>> However, I'd like to leave it as is (it is mentioned in the documentation as
>> unsupported value).
>> Otherwise, TlsChannelBindingType enum will have one element only and should
>> be simplified/removed in all places. In
>> this case, it would be double work to add TlsChannelBindingType enum back in
>> the future if "tls-unique" required. If
>> required I can remove TLS_UNIQUE item, but not remove TlsChannelBindingType
>> enum
>
> I was suggesting to keep TlsChannelBindingType but remove TLS_UNIQUE;
> However, I'm OK to keep things as is: this is an
> internal API. I wonder if it would deserve a comment:
> /**
> * Channel binding on the basis of TLS Finished message
> */
> // TLS_UNIQUE is defined by RFC 5929 but is not supported by the
> current LDAP stack.
> TLS_UNIQUE("tls-unique"),
Thank you. Added suggested comment.
-
PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos [v2]
> Hi, > > Plaese review JDK-8245527 fix which implements LDAP Channel Binding support > for Java GSS/Kerberos. > Initial review is available at core-devs: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html > This version removes "tls-unique" CB type from the list of possible channel > binding types. The only supported type is > "tls-server-end-point" > CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 > > Thank you > Alexey Alexey Bakhtin has updated the pull request incrementally with one additional commit since the last revision: 8245527: version.01 - Changes: - all: https://git.openjdk.java.net/jdk/pull/278/files - new: https://git.openjdk.java.net/jdk/pull/278/files/3f4ae08c..8b135f48 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=278&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=278&range=00-01 Stats: 15 lines in 4 files changed: 7 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/278/head:pull/278 PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos [v2]
On Tue, 22 Sep 2020 15:11:57 GMT, Weijun Wang wrote:
>> Alexey Bakhtin has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8245527: version.01
>
> src/jdk.security.jgss/share/classes/com/sun/security/sasl/gsskerb/GssKrb5Client.java
> line 156:
>
>> 154: if (props != null) {
>> 155: // TLS Channel Binding
>> 156: byte[] tlsCB =
>> (byte[])props.get("jdk.internal.sasl.tlschannelbinding");
>
> You can say the name is defined in another class in another module. If we
> really want to rename it one day we will know
> where it's from.
Thank you. Comment is added
> src/java.security.jgss/share/classes/sun/security/jgss/krb5/InitialToken.java
> line 389:
>
>> 387: int acceptorAddressType = getAddrType(acceptorAddress,
>> 388: (channelBinding instanceof TlsChannelBindingImpl)?
>> 389:
>> CHANNEL_BINDING_AF_UNSPEC:CHANNEL_BINDING_AF_NULL_ADDR);
>
> Normally we put a white space around "?" and ":".
Thank you. Fixed.
> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
> line 82:
>
>> 80: /**
>> 81: * Parse value of "com.sun.jndi.ldap.tls.cbtype" property
>> 82: * @param cbType
>
> Please add a `@return` here, esp, about null.
Added @return with comments
> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
> line 137:
>
>> 135: public TlsChannelBindingType getType() {
>> 136: return cbType;
>> 137: }
>
> Add a new line here.
Fixed
-
PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Tue, 22 Sep 2020 14:47:35 GMT, Daniel Fuchs wrote:
>> Hi,
>>
>> Plaese review JDK-8245527 fix which implements LDAP Channel Binding support
>> for Java GSS/Kerberos.
>> Initial review is available at core-devs:
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html
>> This version removes "tls-unique" CB type from the list of possible channel
>> binding types. The only supported type is
>> "tls-server-end-point"
>> CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311
>>
>> Thank you
>> Alexey
>
> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java
> line 63:
>
>> 61: * Channel binding on the basis of TLS Finished message
>> 62: */
>> 63: TLS_UNIQUE("tls-unique"),
>
> Is that still used? If not maybe it should be removed?
No, It is not used.
However, I'd like to leave it as is (it is mentioned in the documentation as
unsupported value).
Otherwise, TlsChannelBindingType enum will have one element only and should be
simplified/removed in all places. In
this case, it would be double work to add TlsChannelBindingType enum back in
the future if "tls-unique" required. If
required I can remove TLS_UNIQUE item, but not remove TlsChannelBindingType enum
-
PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
On Tue, 22 Sep 2020 14:41:57 GMT, Daniel Fuchs wrote: >> Hi, >> >> Plaese review JDK-8245527 fix which implements LDAP Channel Binding support >> for Java GSS/Kerberos. >> Initial review is available at core-devs: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html >> This version removes "tls-unique" CB type from the list of possible channel >> binding types. The only supported type is >> "tls-server-end-point" >> CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 >> >> Thank you >> Alexey > > src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java line 994: > >> 992: } >> 993: >> 994: private CompletableFuture tlsHandshakeCompleted = > > Should be `final`? Thank you. Agree. It should be final. - PR: https://git.openjdk.java.net/jdk/pull/278
RFR: 8245527: LDAP Channel Binding support for Java GSS/Kerberos
Hi, Plaese review JDK-8245527 fix which implements LDAP Channel Binding support for Java GSS/Kerberos. Initial review is available at core-devs: https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068197.html This version removes "tls-unique" CB type from the list of possible channel binding types. The only supported type is "tls-server-end-point" CSR is also updated : https://bugs.openjdk.java.net/browse/JDK-8247311 Thank you Alexey - Commit messages: - 8245527: LDAP Channel Binding support for Java GSS/Kerberos Changes: https://git.openjdk.java.net/jdk/pull/278/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=278&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8245527 Stats: 536 lines in 10 files changed: 524 ins; 0 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/278/head:pull/278 PR: https://git.openjdk.java.net/jdk/pull/278
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Sorry, That’s my bad. I have reverted @systemProperty to @code. The wording is fixed. Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v17/ Regards Alexey > On 14 Aug 2020, at 17:23, Sean Mullan wrote: > > Sorry you are right! Would be nice to have a more general @property for these > types of properties and security properties, etc but that's a different > change ... > > --Sean > > On 8/14/20 10:18 AM, Daniel Fuchs wrote: >> Hi Sean, >> Wait wait... Are they system properties really? >> Only system properties should be documented with @systemProperty. >> I can't find the place were com.sun.jndi.ldap.connect.timeout or >> com.sun.jndi.ldap.read.timeout is read from System. >> I believe they are just plain environment properties >> that you need to specify in the environment map. >> best regards, >> -- daniel >> On 14/08/2020 14:18, Sean Mullan wrote: >>> On 8/14/20 8:41 AM, Alexey Bakhtin wrote: >>>> Hello Sean, >>>> >>>> Thank you for review. >>>> I’ve changed wording and replaced @code with @systemProperty (tested, it >>>> works for module-info.java) >>> >>> Thanks. Now that you know it works, I think you should change the other >>> properties documented in that file to @systemProperty to be consistent. I >>> think it is fine to do that as part of this fix. >>> >>> --Sean
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
OK Updated for all properties in the module-info.java Webrev at: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v16/ Regards Alexey > On 14 Aug 2020, at 16:18, Sean Mullan wrote: > > On 8/14/20 8:41 AM, Alexey Bakhtin wrote: >> Hello Sean, >> Thank you for review. >> I’ve changed wording and replaced @code with @systemProperty (tested, it >> works for module-info.java) > > Thanks. Now that you know it works, I think you should change the other > properties documented in that file to @systemProperty to be consistent. I > think it is fine to do that as part of this fix. > > --Sean > >> Updated webrev at: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v15/ >> Regards >> Alexey >>> On 14 Aug 2020, at 14:50, Sean Mullan wrote: >>> >>> On the property wording, change "for LDAP connection" to "for an LDAP >>> connection". >>> >>> Also, for the definition of the property, can you use the "@systemProperty" >>> annotation instead of "@code"? Does that work inside the module-info.java >>> file? >>> >>> I added my name as Reviewer. >>> >>> --Sean >>> >>> On 7/30/20 6:14 AM, Daniel Fuchs wrote: >>>> Hi Alexey, >>>> I have added myself as a reviewer to the CSR [1]. >>>> It would be good to get someone from security-dev to do the >>>> same, and then move the CSR state to "Proposed". >>>> best regards, >>>> -- daniel >>>> [1] https://bugs.openjdk.java.net/browse/JDK-8247311 >>>> On 30/07/2020 10:17, Alexey Bakhtin wrote: >>>>> Gentle ping >>>>> >>>>> Regards >>>>> Alexey
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Sean, Thank you for review. I’ve changed wording and replaced @code with @systemProperty (tested, it works for module-info.java) Updated webrev at: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v15/ Regards Alexey > On 14 Aug 2020, at 14:50, Sean Mullan wrote: > > On the property wording, change "for LDAP connection" to "for an LDAP > connection". > > Also, for the definition of the property, can you use the "@systemProperty" > annotation instead of "@code"? Does that work inside the module-info.java > file? > > I added my name as Reviewer. > > --Sean > > On 7/30/20 6:14 AM, Daniel Fuchs wrote: >> Hi Alexey, >> I have added myself as a reviewer to the CSR [1]. >> It would be good to get someone from security-dev to do the >> same, and then move the CSR state to "Proposed". >> best regards, >> -- daniel >> [1] https://bugs.openjdk.java.net/browse/JDK-8247311 >> On 30/07/2020 10:17, Alexey Bakhtin wrote: >>> Gentle ping >>> >>> Regards >>> Alexey
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Gentle ping Regards Alexey > On 15 Jul 2020, at 14:18, Alexey Bakhtin wrote: > > Hello Daniel, > > I’ve updated CSR as you suggested and added kerberos ldap setup commands for > the client host in the JDK-8245527 > > Regards > Alexey
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Daniel,
I’ve updated CSR as you suggested and added kerberos ldap setup commands for
the client host in the JDK-8245527
Regards
Alexey
> On 14 Jul 2020, at 18:28, Daniel Fuchs wrote:
>
> Hi Alexey,
>
> On 10/07/2020 21:37, Alexey Bakhtin wrote:
>> Updated webrev:http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v14/
>
> In what the JNDI part is concerned this looks good to me now.
>
> nit: java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java:
> 138 }catch(NoSuchAlgorithmException | CertificateEncodingException e)
> {
>
> missing spaces around `catch`; No need for a new webrev.
>
> Please make sure to update the CSR, and in particular update
> the specification section with the diffs from
>
> src/java.naming/share/classes/module-info.java
>
> Also I am not sure the links that are currently in the
> specification section are at their place. They may be better
> placed in the Solution section (the solution is to implement
> the client part of the channel binding as described by these
> documents in the default JNDI/LDAP/GSS provider).
>
> Since we don't really have any end-to-end regression test
> (what we have is mostly a smoke test) - it would be good if
> you could describe in more details what you did to test your
> fix against a real server in a JBS comment in JDK-8245527 - so
> that someone (future or current maintainers) could reproduce
> this testing to verify that nothing is broken by future evolutions.
> In particular - if anything specific needs to be
> installed/configured on the test machine (LDAP server? Which?
> Is that all?)
>
>
> best regards,
>
> -- daniel
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Aleksei, Thank you for review. Please see my comments below. Updated webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v14/ Regards Alexey > On 10 Jul 2020, at 19:40, Aleks Efimov wrote: > > Hi Alexey, > > Thank you for removing the dependency on the timeout property and adding > tests for TLS handshake cases. > > Please, find the comments about the latest webrev below: > > Not quite sure why the CF is completed at two places. Probably that’s OK, but > it would be good to know the reason :) HandshakeCompletedListener is called in case of successful handshake only. In case of async handshake failed we close the connection and complete CF exceptionally to release CF.get() > > The ExecutionException could be unpacked instead of using it directly - and > its cause used instead. Thank you. Fixed in Connection.java and LdapCBPropertiesTest.java > > 'getTlsServerCertificate' should return null if 'isTlsConnection()' is false > - rather than waiting forever. We call isTlsConnection() in the LdapSasl before getTlsServerCertificate(). But your are right, we can double check it to prevent possible deadlock in the future, if code changed. Fixed in Connection.java > > java.naming/share/classes/module-info.java: could we try to improve the > formatting of the possible values for the new system property - maybe format > them as a list. Done > > Connection.java:995 - you could use diamond operator here Updated > > Formatting: Connection.java:1027 'catch (‘ Updated > > Could we use the test/jdk/com/sun/jndi/ldap/lib/BaseLdapServer.java from the > test library to implement dummy LDAPS server, I believe it could help to > increase the test verbosity and reduce its code size. Thank you for suggestion. Updated to use BaseLdapServer in the test > > LdapCBPropertiesTest.java:122 - could use no param Hastable constructor Fixed > > I've also run your latest patch through our CI system and it showed no > failures with your latest changes. > > -Aleksei > > On 09/07/2020 20:34, Alexey Bakhtin wrote: >> Hello Sean, Daniel, >> >> Thank you for review >> I’ve added “com.sun.jndi.ldap.tls.cbtype” property into the module-info file >> and updated synchronisation using CompletableFuture. >> Also, I have added new test cases : successful and unsuccessful TLS >> handshake, >> synchronous and asynchronous TLS handshake. >> >> New webrev at : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v13 >> >> Connection property is removed from CSR : >> https://bugs.openjdk.java.net/browse/JDK-8247311 >> >> Regards >> Alexey >> >>> On 8 Jul 2020, at 17:41, Daniel Fuchs wrote: >>> >>> Thanks Sean, Alexey, >>> >>> On 08/07/2020 13:25, Sean Mullan wrote: >>>>> Updated webrev : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v12/ >>>> You will also need to update the CSR to remove the connection timeout >>>> property. >>>> Also, you should document the "com.sun.jndi.ldap.tls.cbtype" property in >>>> the java.naming module-info file as was done for other properties in >>>> Daniel's recent RFR, once he has pushed it [1]. >>> I have pushed the fix: >>> https://hg.openjdk.java.net/jdk/jdk/rev/ed375ae6c779 >>> >>> Alexey, you should now be able to document your new >>> "com.sun.jndi.ldap.tls.cbtype" >>> property in that @implNote as well, and update your CSR >>> consequently. >>> >>>> * src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java >>>> It looks like there is a possibility of deadlock if >>>> getTlsServerCertificate() is called before handshakeCompleted(). In that >>>> case the lock could be obtained first and the thread would end up holding >>>> the lock and waiting forever. >>> I also have a concern with the use of wait/notify in that code. >>> I would suggest prototyping using a CompletableFuture instead >>> (or can new certificates be exchanged later on the same >>> connection?) >>> >>> CompletableFuture would allow you to relay and propagate any >>> exception raised in the callback handler as well, and would >>> avoid running into deadlocks. >>> >>> best regards, >>> >>> -- daniel
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Sean, Daniel, Thank you for review I’ve added “com.sun.jndi.ldap.tls.cbtype” property into the module-info file and updated synchronisation using CompletableFuture. Also, I have added new test cases : successful and unsuccessful TLS handshake, synchronous and asynchronous TLS handshake. New webrev at : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v13 Connection property is removed from CSR : https://bugs.openjdk.java.net/browse/JDK-8247311 Regards Alexey > On 8 Jul 2020, at 17:41, Daniel Fuchs wrote: > > Thanks Sean, Alexey, > > On 08/07/2020 13:25, Sean Mullan wrote: >>> Updated webrev : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v12/ >> You will also need to update the CSR to remove the connection timeout >> property. >> Also, you should document the "com.sun.jndi.ldap.tls.cbtype" property in the >> java.naming module-info file as was done for other properties in Daniel's >> recent RFR, once he has pushed it [1]. > > I have pushed the fix: > https://hg.openjdk.java.net/jdk/jdk/rev/ed375ae6c779 > > Alexey, you should now be able to document your new > "com.sun.jndi.ldap.tls.cbtype" > property in that @implNote as well, and update your CSR > consequently. > >> * src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java >> It looks like there is a possibility of deadlock if >> getTlsServerCertificate() is called before handshakeCompleted(). In that >> case the lock could be obtained first and the thread would end up holding >> the lock and waiting forever. > > I also have a concern with the use of wait/notify in that code. > I would suggest prototyping using a CompletableFuture instead > (or can new certificates be exchanged later on the same > connection?) > > CompletableFuture would allow you to relay and propagate any > exception raised in the callback handler as well, and would > avoid running into deadlocks. > > best regards, > > -- daniel
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Sean,
Thank you for review.
You are right, we can eliminate requirements for connection timeout property.
I’ve added handshakeComletedListener to the LDAP over SSl. In this case we’ll
have no possible performance impact caused by synchronous handshake.
Also, it allows to exclude changes in the LdapCtx class.
Updated webrev : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v12/
Regards
Alexey
> On 7 Jul 2020, at 02:30, Sean Mullan wrote:
>
> Thanks for that extra info.
>
> I think it would be much cleaner to avoid having to set that property and
> instead start the handshake synchronously if the
> "com.sun.jndi.ldap.tls.cbtype" property is set. This way only one property
> needs to be set and you don't need to guess what an acceptable value is for
> the timeout property, which could also cause the connection to be interrupted
> before the TLS handshake is complete if you use too small of a value.
>
> Or better yet, there may be another way to do this with JSSE where you wait
> for the TLS connection to complete. I'll ask my team and get back to you.
>
> --Sean
>
>
> On 7/6/20 6:06 PM, Aleks Efimov wrote:
>> Hi Sean,
>> Alexey answered the same question for me:
>>> I mean “com.sun.jndi.ldap.connect.timeout” property.
>>> The positive value forces to start TLS handshake and wait for it completion
>>> during the connectTimeout milliseconds:
>>> Connection.java
>>>>> if (connectTimeout > 0) {
>>>>> int socketTimeout = sslSocket.getSoTimeout();
>>>>> sslSocket.setSoTimeout(connectTimeout); // reuse full timeout value
>>>>> sslSocket.startHandshake();
>>>>> sslSocket.setSoTimeout(socketTimeout);
>>>>> }
>>> Without this property handshake is started later asynchronously.
>>> As result
>>>>>certs = ssock.getSession().getPeerCertificates();
>>> in the LdapClient.java could return SSLPeerUnverifiedException().
>>> This exception will be wrapped to NamingException and thrown to application.
>>>
>>> This is not usually happens but I saw it on the slow connection
>> The full context of LDAP Connection code that initiates the SSL handshake
>> could be viewed here:
>> https://github.com/openjdk/jdk/blob/master/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#L345
>> -- Aleksei
>> On 06/07/2020 21:11, Sean Mullan wrote:
>>> Hi Alexey,
>>>
>>> This may have been discussed already, but can you explain why the
>>> "com.sun.jndi.ldap.connect.timeout" property needs to be set in order to
>>> use this feature? That property is mostly used in tests to avoid long
>>> socket timeouts, etc.
>>>
>>> Why does that need to be set? What problem are you trying to solve?
>>>
>>> --Sean
>>>
>>>
>>> On 7/3/20 11:31 AM, Alexey Bakhtin wrote:
>>>>
>>>>> I would suggest removing it. At least for the SASL GSS-API mech, it seems
>>>>> the GSSContext object will not be leaked and no one has a chance to call
>>>>> setChannelBinding again on it.
>>>>>
>>>>> There is no spec saying setChannelBinding() can only be called once, so
>>>>> I'd rather we don't enforce that, although you might say there is no need
>>>>> to call it twice.
>>>>
>>>> OK.
>>>> GSSContextImpl class is removed from patch.
>>>>
>>>> Webrev : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v11
>>>>
>>>> Thank you
>>>> Alexey
>>>>
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
> I would suggest removing it. At least for the SASL GSS-API mech, it seems the > GSSContext object will not be leaked and no one has a chance to call > setChannelBinding again on it. > > There is no spec saying setChannelBinding() can only be called once, so I'd > rather we don't enforce that, although you might say there is no need to call > it twice. OK. GSSContextImpl class is removed from patch. Webrev : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v11 Thank you Alexey
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello All, Thank you for review. > 1. If the change in java.security.jgss/share/classes/module-info.java is > unavoidable, can we create a sub-package for this single class so that we > only need to export it? As suggested by Max I’ve moved TlsChannelBindingImpl class into sub-package, so module-info.java exports TlsChannelBindingImpl only. > > 2. Is GSSContextImpl::setChannelBinding really necessary? I don't know if > there are people using null to erase a CB set earlier. I think these changes could be useful to exclude situations when application trying to set Channel Binding with GSSContext::setChannelBinding and “com.sun.jndi.ldap.tls.cbtype” property altogether. I can remove it, if you think it is not necessary. Also, I've fixed Exception text and parseType(String prop) parameter name as suggested by Michael. Unfortunately, I can not completely exclude usage of the literal names because of module import issues. Fixed in the TlsChannelBinding class only. Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v10/ Regards Alexey
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Daniel, Thank you for review. I have updated LdapCBPropertiesTest according to your comments. Updated webrev at http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v9/ Regards Alexey
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Daniel,
Thank you for review.
As you suggested I’ve added static factory methods to create TlsChannelBinding
class and changed connectionTimeout verification to "if (connectTimeout <= 0)"
Also, I’ve added simple regression test to verify Channel Binding parameters.
Please find updated webrev at:
http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v8/
The link to CSR for this feature :
https://bugs.openjdk.java.net/browse/JDK-8247311
Regards
Alexey
> On 12 Jun 2020, at 12:20, Daniel Fuchs wrote:
>
> Hi Alexey,
>
> This is starting to look good.
> Thank you for persisting!
>
> I have a couple of comments - on the LDAP/JNDI side.
>
> There are several places where your new code is supposed to
> trigger the throwing of a NamingException:
>
> LdapSasl.java: lines 76, 85, 140, 168
>
> Please write a test to verify that it does so. Since the
> exceptions are all supposed to be thrown before the actual
> binding happens, there's no need to have an actual LDAP server
> that supports any kind of authentication to test that.
>
> The simple dummy servers that we have in the test base should
> be enough to write such a test.
>
> In addition, there are a couple of places where stray exceptions
> could theoretically be thrown, and where they should be wrapped in
> NamingException (but are not). Maybe it's OK, because this has
> been checked before - but it would be better if we had a test that
> proves that it is so:
>
> For instance: LdapSasl.java
> 82 connectTimeout = Integer.parseInt((String)propVal);
> What if the value of the propVal is "Foo" - or not a String?
> What unexpected exception might be relayed to the calling code?
>
> Still in that same file:
> 84 if (connectTimeout == -1)
>
> should probably be if (connectTimeout <= 0) since
> Connection.java checks for connectTimeout > 0 to determine
> whether to start the initial handshake.
>
> Which makes me wonder if we should find a better way to
> instruct Connection to start the initial handshake...
>
> TlsChannelBinding.java:
>
> It would be much better if static factories methods were used instead of
> public constructors. That would allow you to check all arguments before
> actually constructing your object:
>
> public static TlsChannelBinding create((byte[] finishedMessage) throws
> SaslException {
>throw new UnsupportedOperationException("tls-unique channel binding is not
> supported");
> }
>
> public statuic TlsChannelBinding create(X509Certificate serverCertificate)
> throws SaslException {
> var cbType = TlsChannelBindingType.TLS_SERVER_END_POINT;
> byte[] cbData;
> try {
> // compute cbdata
> ...
>
> return new TlsChannelBinding(cbType, cbData);
> }
>
> private TlsChannelBinding(TlsChannelBindingType cbType, byte[] cbData) {
> this.cbType = cbType;
> this.cbData = cbData;
> }
>
> That's all I have for now.
>
> best regards,
>
> -- daniel
>
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Sean, The link to CSR for this feature : https://bugs.openjdk.java.net/browse/JDK-8247311 Regards Alexey > On 9 Jun 2020, at 19:50, Sean Mullan wrote: > > On 6/9/20 12:40 PM, Xuelei Fan wrote: >> About the prefix, it may follow RFC 5056 (See page 7, section 2.1). >>o Specifications of channel bindings for any secure channels MUST >> provide for a single, canonical octet string encoding of the >> channel bindings. Under this framework, channel bindings MUST >> start with the channel binding unique prefix followed by a colon >> (ASCII 0x3A). > > Thanks! Easy to miss. > > I would recommend adding more comments in the code (ex, in TLSChannelBinding) > pointing to that RFC section, and other RFCs such 5929 for the tls cbtypes. > Also, this RFC (and other specifications such as RFC 5959) should be listed > in the CSR so that we document precisely what encodings and types the JDK > implementation supports and is using. > > --Sean
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Aleks, Thank you very much for review. I’ve fixed missed spaces and removed casting from LdapSasl.java Failure of the SaslMutual test was caused by prop.remove() in the GssKrb5Client This operation is not required any more. GssKrb5Client receives temporary copy of the properties. Fixed Also, I’ve added references to RFC-5929 and RFC-5056 into the TlsChannelBinding class Updated patch is located at: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v7/ Regards Alexey > On 9 Jun 2020, at 15:59, Aleks Efimov wrote: > > Hi Alexey, > > Thank you for incorporating LdapCtx and LdapSasl changes. I've reviewed both > classes and they look good to me, with few minor comments in LdapSasl.java: > missing spaces in the following lines: 78, 152 > With your last changes we can remove explicit cast of 'envProps' on > line 176 > > I've also run your changes through our CI and one test is failing due to the > changes in GssKrb5Client: > The failed test name: sun/security/krb5/auto/SaslMutual.java > > The observed failure: > java.lang.UnsupportedOperationException > at > java.base/java.util.ImmutableCollections.uoe(ImmutableCollections.java:127) > at > java.base/java.util.ImmutableCollections$AbstractImmutableMap.remove(ImmutableCollections.java:910) > at > jdk.security.jgss/com.sun.security.sasl.gsskerb.GssKrb5Client.(GssKrb5Client.java:156) > at > jdk.security.jgss/com.sun.security.sasl.gsskerb.FactoryImpl.createSaslClient(FactoryImpl.java:63) > at > java.security.sasl/javax.security.sasl.Sasl.createSaslClient(Sasl.java:433) > at SaslMutual.main(SaslMutual.java:50) > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native > Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:564) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) > at java.base/java.lang.Thread.run(Thread.java:832) > > > For information about CSR process you can start from this wiki page: > https://wiki.openjdk.java.net/display/csr > > Best Regards, > Aleksei > > On 08/06/2020 22:33, Alexey Bakhtin wrote: >> Hello Sean, >> >> Yes, I think we'll need CSR and release notes as soon as this patch adds new >> property. >> I do not know exact process for it, so I will be grateful if you could >> explain me exact steps. >> >> This patch was developed to address compatibility issue with new LDAP >> authentication over SSL/TLS announced by Microsoft [1]. It is not related to >> RFC 5801. In my opinion “com.sun.jndi.ldap.tls.cbtype” name looks more >> suitable for this property and should allow backport it to early JDK >> versions. >> >> [1] - >> https://support.microsoft.com/en-au/help/4034879/how-to-add-the-ldapenforcechannelbinding-registry-entry >> >> Regards >> Alexey >> >> >>> On 8 Jun 2020, at 22:03, Sean Mullan >>> wrote: >>> >>> (resending to all lists on the review) >>> >>> I'm just catching up a bit on this review. >>> >>> Sorry if this has mentioned before, but are you planning to write a CSR and >>> release note? I think this is needed for the com.sun.jndi.ldap.tls.cbtype >>> property. I'm also wondering if this property should be documented in the >>> javadocs, and why it is not a standard property (i.e. >>> "java.naming.ldap.tls.cbtype"). >>> >>> I was also wondering what relation this has to the "G2" standard SASL >>> mechanisms defined in RFC 5801 [1], and whether that is something we should >>> be using to negotiate this channel binding, and if not, why not. Or if this >>> is something that is implementation-specific and will only work with >>> Microsoft LDAP technology, in which case, we might want to make that more >>> explicit, perhaps by including "microsoft" or something like that in the >>> property name. >>> >>> Thanks, >>> Sean >>> >>> [1] >>> https://tools.ietf.org/html/rfc5801 >>> >>> >>> On 6/8/20 9:07 AM, Aleks Efimov wrote: >>> >>>> Hi Alexey, >>>> I've looked through LdapCtx/LdapClient/SaslBind changes: >>>> Do we need to check if CHANNEL_BINDING is set explicitly for all >>>> connection ty
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Sean,
Thank you for the link. I’ll follow it to create CSR
I could not find any clear document or specification for this Channel Binding
format.
The only document I found that describes this format is the following:
https://docs.microsoft.com/en-us/archive/blogs/openspecification/ntlm-and-channel-binding-hash-aka-extended-protection-for-authentication
So, it is hard to say - is it a standard or Microsoft implementation specific
Regards
Alexey
> On 9 Jun 2020, at 18:35, Sean Mullan wrote:
>
> On 6/8/20 5:33 PM, Alexey Bakhtin wrote:
>> Hello Sean,
>> Yes, I think we'll need CSR and release notes as soon as this patch adds new
>> property.
>> I do not know exact process for it, so I will be grateful if you could
>> explain me exact steps.
>
> The CSR process is documented at
> https://wiki.openjdk.java.net/display/csr/Main. It should be fairly
> self-explanatory but let me know if you have questions.
>
> For the release note, we can tackle that later once the CSR is approved now I
> have tagged the issue with the "release-note=yes" label so we don't forget it.
>
>> This patch was developed to address compatibility issue with new LDAP
>> authentication over SSL/TLS announced by Microsoft [1]. It is not related to
>> RFC 5801. In my opinion “com.sun.jndi.ldap.tls.cbtype” name looks more
>> suitable for this property and should allow backport it to early JDK
>> versions.
>
> Good point about backporting.
>
> What RFC or specification defines the format you are using for the channel
> binding in TlsChannelBinding.java, specifically where the type prefix is
> encoded as "tls-server-end-point:" followed by the binding data? I have
> looked through various RFCs but I can't find exactly where this format is
> defined, so I am wondering if this is a standard encoding or not.
>
> Thanks,
> Sean
>
>> [1] -
>> https://support.microsoft.com/en-au/help/4034879/how-to-add-the-ldapenforcechannelbinding-registry-entry
>> Regards
>> Alexey
>>> On 8 Jun 2020, at 22:03, Sean Mullan wrote:
>>>
>>> (resending to all lists on the review)
>>>
>>> I'm just catching up a bit on this review.
>>>
>>> Sorry if this has mentioned before, but are you planning to write a CSR and
>>> release note? I think this is needed for the com.sun.jndi.ldap.tls.cbtype
>>> property. I'm also wondering if this property should be documented in the
>>> javadocs, and why it is not a standard property (i.e.
>>> "java.naming.ldap.tls.cbtype").
>>>
>>> I was also wondering what relation this has to the "G2" standard SASL
>>> mechanisms defined in RFC 5801 [1], and whether that is something we should
>>> be using to negotiate this channel binding, and if not, why not. Or if this
>>> is something that is implementation-specific and will only work with
>>> Microsoft LDAP technology, in which case, we might want to make that more
>>> explicit, perhaps by including "microsoft" or something like that in the
>>> property name.
>>>
>>> Thanks,
>>> Sean
>>>
>>> [1] https://tools.ietf.org/html/rfc5801
>>>
>>> On 6/8/20 9:07 AM, Aleks Efimov wrote:
>>>> Hi Alexey,
>>>> I've looked through LdapCtx/LdapClient/SaslBind changes:
>>>> Do we need to check if CHANNEL_BINDING is set explicitly for all
>>>> connection types? Maybe we can move the check inside 'if (conn.sock
>>>> instanceof SSLSocket) {' block.
>>>> Also, instead of setting CHANNEL_BINDING in context environment and then
>>>> removing it in finally block, it would be better to clone the environment,
>>>> put calculated CHANNEL_BINDING into it, and pass the cloned one to
>>>> Sasl.createSaslClient.
>>>> Another suggestion about the code that verifies if both properties are set
>>>> before connection is started:
>>>> As you've already mentioned the new code in LdapCtx is only needed for
>>>> checking if timeout is set. Could we try to remove LdapCtx::cbType field
>>>> and all related methods from LdapCtx (this class is already
>>>> over-complicated and hard to read) and replace it with some static method
>>>> in LdapSasl? It will help to localize all changes to LdapSasl except for
>>>> one line in LdapCtx.
>>>> I mean something like this:
>>>> Replace
>>>> +
>>>> +// verify LDAP channel binding property
>&g
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Sean,
Yes, I think we'll need CSR and release notes as soon as this patch adds new
property.
I do not know exact process for it, so I will be grateful if you could explain
me exact steps.
This patch was developed to address compatibility issue with new LDAP
authentication over SSL/TLS announced by Microsoft [1]. It is not related to
RFC 5801. In my opinion “com.sun.jndi.ldap.tls.cbtype” name looks more suitable
for this property and should allow backport it to early JDK versions.
[1] -
https://support.microsoft.com/en-au/help/4034879/how-to-add-the-ldapenforcechannelbinding-registry-entry
Regards
Alexey
> On 8 Jun 2020, at 22:03, Sean Mullan wrote:
>
> (resending to all lists on the review)
>
> I'm just catching up a bit on this review.
>
> Sorry if this has mentioned before, but are you planning to write a CSR and
> release note? I think this is needed for the com.sun.jndi.ldap.tls.cbtype
> property. I'm also wondering if this property should be documented in the
> javadocs, and why it is not a standard property (i.e.
> "java.naming.ldap.tls.cbtype").
>
> I was also wondering what relation this has to the "G2" standard SASL
> mechanisms defined in RFC 5801 [1], and whether that is something we should
> be using to negotiate this channel binding, and if not, why not. Or if this
> is something that is implementation-specific and will only work with
> Microsoft LDAP technology, in which case, we might want to make that more
> explicit, perhaps by including "microsoft" or something like that in the
> property name.
>
> Thanks,
> Sean
>
> [1] https://tools.ietf.org/html/rfc5801
>
> On 6/8/20 9:07 AM, Aleks Efimov wrote:
>> Hi Alexey,
>> I've looked through LdapCtx/LdapClient/SaslBind changes:
>> Do we need to check if CHANNEL_BINDING is set explicitly for all connection
>> types? Maybe we can move the check inside 'if (conn.sock instanceof
>> SSLSocket) {' block.
>> Also, instead of setting CHANNEL_BINDING in context environment and then
>> removing it in finally block, it would be better to clone the environment,
>> put calculated CHANNEL_BINDING into it, and pass the cloned one to
>> Sasl.createSaslClient.
>> Another suggestion about the code that verifies if both properties are set
>> before connection is started:
>> As you've already mentioned the new code in LdapCtx is only needed for
>> checking if timeout is set. Could we try to remove LdapCtx::cbType field and
>> all related methods from LdapCtx (this class is already over-complicated and
>> hard to read) and replace it with some static method in LdapSasl? It will
>> help to localize all changes to LdapSasl except for one line in LdapCtx.
>> I mean something like this:
>> Replace
>> +
>> +// verify LDAP channel binding property
>> +if (cbType != null && connectTimeout == -1)
>> +throw new
>> NamingException(TlsChannelBinding.CHANNEL_BINDING_TYPE +
>> +" property requires " +
>> +CONNECT_TIMEOUT +
>> +" property is set.");
>> With
>> +
>> LdapSasl.checkCbParameters((String)envprops.get(TlsChannelBinding.CHANNEL_BINDING_TYPE),
>> connectTimeout);
>> And add something like that to LdapSasl (or maybe pass the full env here):
>> + public static void checkCbParameters(String cbTypePropertyValue, int
>> connectTimeout) throws NamingException {
>> + TlsChannelBindingType cbType =
>> TlsChannelBinding.parseType(cbTypePropertyValue);
>> + // verify LDAP channel binding property
>> + if (cbType != null && connectTimeout == -1) {
>> + throw new NamingException(TlsChannelBinding.CHANNEL_BINDING_TYPE +
>> + " property requires com.sun.jndi.ldap.connect.timeout" +
>> + " property is set.");
>> + }
>> + }
>> Other LdapCtx/LdapClient/SaslBind changes look fine to me.
>> With Kind Regards,
>> Aleksei
>> On 06/06/2020 20:45, Alexey Bakhtin wrote:
>>> Hello Max, Daniel,
>>>
>>> Thank you for review.
>>>
>>> Please review new version of the patch :
>>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v5/
>>>
>>> In this version:
>>> - TlsChannelBinding class is moved into the com.sun.jndi.ldap.sasl package
>>> - SSL Ceritificate related code is moved from LdapClient into the
>>> LdapSasl.saslBind method
>>> - verification and removal of interna
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Max, Daniel,
Thank you for review and suggestions.
Could you please check the updated webrev:
http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v6/
This version contains the following changes:
1. Updated license for new files
2. Comments about default address type usage in the InitialToken.java and
GSSLibStub.c
3. internal property is renamed to “jdk.internal.sasl.tlschannelbinding"
4. I did not create test but provided detailed reproducer in the bug description
5. Property verification is moved from LdapCTX into LdapSasl as suggested by
Aleks
6. Use clone of enviroment properties as suggested by Aleks
I did not move internal property check under 'if (conn.sock instanceof
SSLSocket) {' block.
It was already discussed in [1] :
"If not TLS, this property value be kept there and visible inside the SASL
mech."
[1] -
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-June/067042.html
Thank you
Alexey
> On 7 Jun 2020, at 04:47, Weijun Wang wrote:
>
> Some comments:
>
> 1. I just noticed your 2 new files are using the plain GPU license, it should
> be GPL + Classpath. Read another source file for an example.
>
> 2. Please add some comments in InitialToken.java on what happens to the
> default type.
>
> 3. I still think "com.sun.security.sasl.tlschannelbinding" sounds like
> sun.com will support it.
>
> 4. If an automatic test is not feasible, please provide some instructions so
> our SQE can see if we can setup some internal tests. Then we can add
> noreg-hard with some justification to the test and go on.
>
> I cannot comment on LdapCtx.java, but the others look fine to me.
>
> Thanks,
> Max
>
>> On Jun 7, 2020, at 3:45 AM, Alexey Bakhtin wrote:
>>
>> Hello Max, Daniel,
>>
>> Thank you for review.
>>
>> Please review new version of the patch :
>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v5/
>>
>> In this version:
>> - TlsChannelBinding class is moved into the com.sun.jndi.ldap.sasl package
>> - SSL Ceritificate related code is moved from LdapClient into the
>> LdapSasl.saslBind method
>> - verification and removal of internal property is also moved to
>> LdapSasl.saslBind method
>> - verification of connectTimeout property is moved to LdapCtx.connect. I’ve
>> found that connectionTimeout could be assigned later then cbType
>>
>> The test for this issue is not ready yet. I did not find any suitable test
>> case that can be reused.
>>
>> Thank you
>> Alexey
>>
>>> On 6 Jun 2020, at 09:44, Weijun Wang wrote:
>>>
>>>
>>>
>>>> On Jun 6, 2020, at 2:41 PM, Weijun Wang wrote:
>>>>
>>>>
>>>>
>>>>> On Jun 5, 2020, at 11:03 PM, Alexey Bakhtin wrote:
>>>>>
>>>>> Hello Max,
>>>>>
>>>>> Thank you a lot for review.
>>>>>
>>>>> Could you check the new version of the patch :
>>>>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v4/
>>>>>
>>>>> I’ve made the following changes:
>>>>> - TlsChannelBinding class is moved to java.naming module.
>>>>> java.security.sasl module is not affected any more
>>>>> - I pass tlsCB.getData() directly to the SASL mechanism as you suggested
>>>>> - I’ve made some guards to prevent application from using
>>>>> "com.sun.security.sasl.tlschannelbinding” property directly:
>>>>> - LdapClient verifies if internal property is not set
>>>>
>>>> 245 // Prepare TLS Channel Binding data
>>>> 246 if (conn.sock instanceof SSLSocket) {
>>>> 247 // Internal property cannot be set explicitly
>>>> 248 if (env.get(TlsChannelBinding.CHANNEL_BINDING)
>>>> != null) {
>>>> 249 throw new
>>>> NamingException(TlsChannelBinding.CHANNEL_BINDING +
>>>> 250 " property cannot be set
>>>> explicitly");
>>>> 251 }
>>>>
>>>> If not TLS, this property value be kept there and visible inside the SASL
>>>> mech.
>>>>
>>>>> - GssKrb5Client uses props.remove() to read and remove internal property
>>>
>>> Maybe you can remove the value in LdapClient.java, in case the mech used is
>>> not GSSAPI. You can remove it in a finally block after line 303.
>>>
>>> --Max
>>>
>>>>
>>>> Traditionally, we use "com.sun..." name which is a JDK supported name
>>>> (although not at Java SE level), you might want to use a name which is
>>>> even more internal.
>>>>
>>>>
>>>> Thanks,
>>>> Max
>>>>
>>>> p.s. I see that NTLM also supports ChannelBinding. I'll see if I can
>>>> improve the NTLM SASL mech to support it.
>>
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Max, Daniel,
Thank you for review.
Please review new version of the patch :
http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v5/
In this version:
- TlsChannelBinding class is moved into the com.sun.jndi.ldap.sasl package
- SSL Ceritificate related code is moved from LdapClient into the
LdapSasl.saslBind method
- verification and removal of internal property is also moved to
LdapSasl.saslBind method
- verification of connectTimeout property is moved to LdapCtx.connect. I’ve
found that connectionTimeout could be assigned later then cbType
The test for this issue is not ready yet. I did not find any suitable test case
that can be reused.
Thank you
Alexey
> On 6 Jun 2020, at 09:44, Weijun Wang wrote:
>
>
>
>> On Jun 6, 2020, at 2:41 PM, Weijun Wang wrote:
>>
>>
>>
>>> On Jun 5, 2020, at 11:03 PM, Alexey Bakhtin wrote:
>>>
>>> Hello Max,
>>>
>>> Thank you a lot for review.
>>>
>>> Could you check the new version of the patch :
>>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v4/
>>>
>>> I’ve made the following changes:
>>> - TlsChannelBinding class is moved to java.naming module.
>>> java.security.sasl module is not affected any more
>>> - I pass tlsCB.getData() directly to the SASL mechanism as you suggested
>>> - I’ve made some guards to prevent application from using
>>> "com.sun.security.sasl.tlschannelbinding” property directly:
>>> - LdapClient verifies if internal property is not set
>>
>> 245 // Prepare TLS Channel Binding data
>> 246 if (conn.sock instanceof SSLSocket) {
>> 247 // Internal property cannot be set explicitly
>> 248 if (env.get(TlsChannelBinding.CHANNEL_BINDING)
>> != null) {
>> 249 throw new
>> NamingException(TlsChannelBinding.CHANNEL_BINDING +
>> 250 " property cannot be set
>> explicitly");
>> 251 }
>>
>> If not TLS, this property value be kept there and visible inside the SASL
>> mech.
>>
>>> - GssKrb5Client uses props.remove() to read and remove internal property
>
> Maybe you can remove the value in LdapClient.java, in case the mech used is
> not GSSAPI. You can remove it in a finally block after line 303.
>
> --Max
>
>>
>> Traditionally, we use "com.sun..." name which is a JDK supported name
>> (although not at Java SE level), you might want to use a name which is even
>> more internal.
>>
>>
>> Thanks,
>> Max
>>
>> p.s. I see that NTLM also supports ChannelBinding. I'll see if I can improve
>> the NTLM SASL mech to support it.
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hi Daniel,
Thank you for review
Yes, I can move TlsChannelBinding class into the com.sun.jndi.ldap.sasl package
and LdapClient related changes into the LdapSasl.saslBind method.
Also, you are right with exceptions. I will rename them to the NamingException.
However, I’d like to parse TLS Channel Binding property in the LdapCtx class.
The reason is “com.sun.jndi.ldap.connect.timeout” property. This property
should be set together with TLS Channel Binding. So, I’d like to verify if both
properties are set before connection is started. The best place for it is
LdapCtx.initEnv()
Is it acceptable ?
Thank you
Alexey
> On 5 Jun 2020, at 18:17, Daniel Fuchs wrote:
>
> Hi Alexey,
>
> Could we move the new code from LdapClient.java and LdapCtxt.java
> into the com.sun.jndi.ldap.sasl package, and possibly delay
> its execution until the com.sun.jndi.ldap.sasl.LdapSasl.saslBind
> method is called?
>
> The new TlsChannelBinding class should also be preferably moved
> to com.sun.jndi.ldap.sasl since it's apparently only useful
> with SASL.
>
> Also:
>
> 2573 if
> (cbTypeProp.equals(TlsChannelBindingType.TLS_UNIQUE.getName())) {
> 2574 throw new UnsupportedOperationException( "Channel
> binding type " +
> 2575 TlsChannelBindingType.TLS_UNIQUE.getName() +
> 2576 " is not supported");
> 2577 } else if
> (cbTypeProp.equals(TlsChannelBindingType.TLS_SERVER_END_POINT.getName())) {
> 2578 if (connectTimeout == -1)
> 2579 throw new IllegalStateException(CHANNEL_BINDING_TYPE
> + " property requires " +
> 2580 CONNECT_TIMEOUT + " property is set.");
> 2581 cbType = TlsChannelBindingType.TLS_SERVER_END_POINT;
> 2582 } else {
> 2583 throw new IllegalArgumentException("Illegal value for " +
> 2584 CHANNEL_BINDING_TYPE + " property.");
> 2585 }
>
> is not correct - as IllegalArgumentException, IllegalStateException and
> UnsupportedOperationException will percolate up to the calling code, and
> are not documented at the public API level.
> These should really be NamingException.
>
> best regards,
>
> -- daniel
>
>
>
> On 05/06/2020 16:03, Alexey Bakhtin wrote:
>> Hello Max,
>> Thank you a lot for review.
>> Could you check the new version of the patch :
>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v4/
>> I’ve made the following changes:
>> - TlsChannelBinding class is moved to java.naming module.
>> java.security.sasl module is not affected any more
>> - I pass tlsCB.getData() directly to the SASL mechanism as you suggested
>> - I’ve made some guards to prevent application from using
>> "com.sun.security.sasl.tlschannelbinding” property directly:
>> - LdapClient verifies if internal property is not set
>> - GssKrb5Client uses props.remove() to read and remove internal property
>> Regards
>> Alexey
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Max, Thank you a lot for review. Could you check the new version of the patch : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v4/ I’ve made the following changes: - TlsChannelBinding class is moved to java.naming module. java.security.sasl module is not affected any more - I pass tlsCB.getData() directly to the SASL mechanism as you suggested - I’ve made some guards to prevent application from using "com.sun.security.sasl.tlschannelbinding” property directly: - LdapClient verifies if internal property is not set - GssKrb5Client uses props.remove() to read and remove internal property Regards Alexey > On 5 Jun 2020, at 06:41, Weijun Wang wrote: > > Hi Alexey, > > It's so unfortunate that different addressType must be used. I'm OK with the > new TlsChannelBindingImpl class. > > One thing I'm not comfortable is the > java.security.sasl/share/classes/module-info.java change. We've struggled > hard to avoid these kind of secret tunnels. Is it possible to pass the > tlsCB.getData() directly to the SASL mechanism? The property name is clear > enough to avoid any misuse. > > Another question: can an application user set the > "com.sun.security.sasl.tlschannelbinding" property? and can they read it > after it's set internally? I cannot think of a good attack now but at least > it seems they have no need to access that property value. If we keep it > internal then we also have the chance to modify the approach later. > > Thanks, > Max > > >> On Jun 5, 2020, at 2:15 AM, Alexey Bakhtin wrote: >> >> Hello, >> >> Could you please review new version of the patch: >> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v3/ >> >> I’ve moved all logic for creating TLS Channel Binding data out of >> GssKrb5Client.java file. >> All data is prepared inside TlsChannelBinding class. >> Internal property name is renamed to >> “com.sun.security.sasl.tlschannelbinding”. >> TlsChannelBinding object is still used to pass channel binding data from >> LdapClient to GssKrb5Client. >> I do not change it to byte[] because of TlsChannelBinding class is still >> used for internal property name. >> Also, I’ve updated TlsChannelBinding class to select SHA-256 hash algorithm >> if it can not be derived >> from the certificate signature name. >> >> Regards >> Alexey >> >> >>> On 26 May 2020, at 17:50, Weijun Wang wrote: >>> >>> I have a question on GssKrb5Client.java: >>> >>> Do you think it's a good idea to let the SASL mechanism understand what TLS >>> binding is? Instead of passing the whole TlsChannelBinding object through a >>> SASL property, is it possible to only pass the actual cbData? After all, >>> the name "com.sun.security.sasl.channelbinding" suggests it's just a >>> general ChannelBinding which is independent with any application level info. >>> >>> From my reading of the code change, it looks like this cbData can be >>> calculated on the LDAP side. >>> >>> Thanks, >>> Max >>> >>>> On May 21, 2020, at 3:35 PM, Alexey Bakhtin wrote: >>>> >>>> Hello, >>>> >>>> Could you please review the following patch: >>>> >>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8245527 >>>> Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v0/ >>>> >>>> The Windows LDAP server with LdapEnforceChannelBinding=2 uses the >>>> tls-server-end-point channel binding >>>> (based on the TLS server certificate). The channel binding data is >>>> calculated as following : >>>>• The client calculates a hash of the TLS server certificate. >>>>The hash algorithm is selected on the base of the certificate >>>> signature algorithm. >>>>Also, the client should use SHA-256 algorithm, in case of the >>>> certificate signature algorithm is SHA1 or MD5 based >>>>• The channel binding information is the same as defined in rfc4121 [1] >>>> with small corrections: >>>>• initiator and acceptor addresses should be set to NULL >>>>• initiator and acceptor address types should be zero. >>>> It contradicts to the “Using Channel Bindings in GSS-API” >>>> document [2] that say that >>>> the address type should be set to GSS_C_AF_NULLADDR=0xFF, >>>> instead of GSS_C_AF_UNSPEC=0x00.
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello, Could you please review new version of the patch: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v3/ I’ve moved all logic for creating TLS Channel Binding data out of GssKrb5Client.java file. All data is prepared inside TlsChannelBinding class. Internal property name is renamed to “com.sun.security.sasl.tlschannelbinding”. TlsChannelBinding object is still used to pass channel binding data from LdapClient to GssKrb5Client. I do not change it to byte[] because of TlsChannelBinding class is still used for internal property name. Also, I’ve updated TlsChannelBinding class to select SHA-256 hash algorithm if it can not be derived from the certificate signature name. Regards Alexey > On 26 May 2020, at 17:50, Weijun Wang wrote: > > I have a question on GssKrb5Client.java: > > Do you think it's a good idea to let the SASL mechanism understand what TLS > binding is? Instead of passing the whole TlsChannelBinding object through a > SASL property, is it possible to only pass the actual cbData? After all, the > name "com.sun.security.sasl.channelbinding" suggests it's just a general > ChannelBinding which is independent with any application level info. > > From my reading of the code change, it looks like this cbData can be > calculated on the LDAP side. > > Thanks, > Max > >> On May 21, 2020, at 3:35 PM, Alexey Bakhtin wrote: >> >> Hello, >> >> Could you please review the following patch: >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8245527 >> Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v0/ >> >> The Windows LDAP server with LdapEnforceChannelBinding=2 uses the >> tls-server-end-point channel binding >> (based on the TLS server certificate). The channel binding data is >> calculated as following : >> • The client calculates a hash of the TLS server certificate. >> The hash algorithm is selected on the base of the certificate >> signature algorithm. >> Also, the client should use SHA-256 algorithm, in case of the >> certificate signature algorithm is SHA1 or MD5 based >> • The channel binding information is the same as defined in rfc4121 [1] >> with small corrections: >> • initiator and acceptor addresses should be set to NULL >> • initiator and acceptor address types should be zero. >> It contradicts to the “Using Channel Bindings in GSS-API” >> document [2] that say that >> the address type should be set to GSS_C_AF_NULLADDR=0xFF, >> instead of GSS_C_AF_UNSPEC=0x00. >> >> This patch introduces changes in the LDAP, SASL and JGSS modules >> to generate channel binding data automatically if requested by an >> application. >> This patch reuses existing org.ietf.jgss.ChannelBinding class implementation >> but changes >> initial unspecified address type from CHANNEL_BINDING_AF_NULL_ADDR to >> CHANNEL_BINDING_AF_UNSPEC. >> The patch introduces new environment property “com.sun.jndi.ldap.tls.cbtype” >> that indicates >> Channel Binding type that should be used in the LDAPS connection over >> JGSS/Kerberos. >> Right now "tls-server-end-point" Channel Binding type is supported only. >> The client extracts server certificate from the underlying TLS connection >> and creates >> Channel Binding data for JGSS/Kerberos authentication if application >> indicates >> com.sun.jndi.ldap.tls.cbtype=tls-server-end-point property. >> Client application should also specify existing >> "com.sun.jndi.ldap.connect.timeout” property >> to force and wait TLS handshake completion before JGSS/Kerberos >> authentication data is generated. >> >> [1] - https://tools.ietf.org/html/rfc4121#section-4.1.1.2 >> >> [2] - >> https://docs.oracle.com/cd/E19120-01/open.solaris/819-2145/overview-52/index.html >> >> Initial discussion of this issue is available at security-dev list: >> https://mail.openjdk.java.net/pipermail/security-dev/2019-December/021052.html >> https://mail.openjdk.java.net/pipermail/security-dev/2020-January/021140.html >> https://mail.openjdk.java.net/pipermail/security-dev/2020-February/021278.html >> https://mail.openjdk.java.net/pipermail/security-dev/2020-May/021864.html
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Aleks,
I mean “com.sun.jndi.ldap.connect.timeout” property.
The positive value forces to start TLS handshake and wait for it completion
during the connectTimeout milliseconds:
Connection.java
>> if (connectTimeout > 0) {
>> int socketTimeout = sslSocket.getSoTimeout();
>> sslSocket.setSoTimeout(connectTimeout); // reuse full timeout value
>> sslSocket.startHandshake();
>> sslSocket.setSoTimeout(socketTimeout);
>> }
Without this property handshake is started later asynchronously.
As result
>>certs = ssock.getSession().getPeerCertificates();
in the LdapClient.java could return SSLPeerUnverifiedException().
This exception will be wrapped to NamingException and thrown to application.
This is not usually happens but I saw it on the slow connection
Alexey
> On 27 May 2020, at 16:13, Aleks Efimov wrote:
>
> Hi Alexey,
>
> I have question about timeouts:
> LdapCtx has 2 timeout properties: connectTimeout and readTimeout.
> First one is for controlling the Socket::connect timeout
> (Connection::createSocket), another - for reading out the replies
> (Connection::readReply).
> Both of them have default values set to '-1' which means that the LDAP stack
> would not set any timeouts for connect and/or reading operations.
>
> Could you, please, share the failures you're observing with no connect
> timeout set?
>
> Best,
> Aleksei
>
> On 27/05/2020 11:48, Alexey Bakhtin wrote:
>> Hello Bernd,
>>
>>> On 27 May 2020, at 13:12, Bernd Eckenfels wrote:
>>>
>>> LdapCtxt:
>>> 2568 /**
>>> 2569 * Sets the read timeout value
>>> 2570 */
>>> 2571 private void setChannelBindingType(String cbTypeProp) {
>>
>> Thank you. This is misprint. Should be “Sets the channel binding type”
>> About timeout - Yes. It is required. Otherwise, LdapClient does not wait for
>> TLS handshake completion and we can not get TLS server certificate before
>> Channel Binding data is populated.
>> Actually, we can force to wait for handshake completion but what timeout
>> should be set in this case.
>> As mentioned by Danial, information about new property and timeout should be
>> documented in the release notes.
>> For the TlsChannelBindingType.TLS_SERVER_END_POINT_COMPAT type name, I do
>> not think it is good approach. If you think different servers could accept
>> different address types for the same Channel Binding type, It would be
>> better to introduce separate boolean compatibility property like
>> “com.sun.security.sasl.addrtype.compat”. In this case it would be applied
>> not only for tls-server-end-point but for any provided Channel Bindings
>>
>>
>>> Not sure if that javadoc is the right one? And I also wonder if enforcing
>>> the timeout is needed, and if yes if it should be documented why. Was not
>>> obvious to me,
>>>
>>> what about having two type names
>>> (TlsChannelBindingType.TLS_SERVER_END_POINT and
>>> TlsChannelBindingType.TLS_SERVER_END_POINT_COMPAT?)
>>>
>>> This could be configured as a SASL property and it would add the benefit
>>> that you don't need the instance specific if in the gssstub native code if
>>> you instead have two different types values?
>>>
>>> Gruss
>>> Bernd
>>>
>>> Von: security-dev im Auftrag von
>>> Alexey Bakhtin
>>> Gesendet: Mittwoch, Mai 27, 2020 11:43 AM
>>> An: Valerie Peng
>>> Cc: security-...@openjdk.java.net; core-libs-dev@openjdk.java.net; Thomas
>>> Maslen
>>> Betreff: Re: RFR: 8245527: LDAP Cnannel Binding support for Java
>>> GSS/Kerberos
>>>
>>> Hello Valerie, Unfortunately, Windows LDAP server with
>>> LdapEnforceChannelBinding=2 does not accept GSS_C_AF_NULLADDR address type.
>>> This is exact reason of these changes. I ve tried to fix inconsistency of
>>> address type value in the latest webrev:
>>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v2/
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hi Max, You are right, It is possible that algorithm name is not confirm With format. As soon as RFC5929 does not specify this situation I would suggest to use “SHA-256” hash instead of throwing SaslException exception. Regards Alexey > On 27 May 2020, at 13:25, Weijun Wang wrote: > > > >> On May 21, 2020, at 3:35 PM, Alexey Bakhtin wrote: >> >> The hash algorithm is selected on the base of the certificate >> signature algorithm. >> Also, the client should use SHA-256 algorithm, in case of the >> certificate signature algorithm is SHA1 or MD5 > > According to https://www.rfc-editor.org/rfc/rfc5929#section-4.1, this is the > right approach. I'm just curious if you have seen newer signature algorithms > like RSASSA-PSS and EdDSA used in reality, since the latest TLS spec already > defined ciphersuites around them. > > Thanks, > Max
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Bernd,
> On 27 May 2020, at 13:12, Bernd Eckenfels wrote:
>
> LdapCtxt:
> 2568 /**
> 2569 * Sets the read timeout value
> 2570 */
> 2571 private void setChannelBindingType(String cbTypeProp) {
Thank you. This is misprint. Should be “Sets the channel binding type”
About timeout - Yes. It is required. Otherwise, LdapClient does not wait for
TLS handshake completion and we can not get TLS server certificate before
Channel Binding data is populated.
Actually, we can force to wait for handshake completion but what timeout should
be set in this case.
As mentioned by Danial, information about new property and timeout should be
documented in the release notes.
For the TlsChannelBindingType.TLS_SERVER_END_POINT_COMPAT type name, I do not
think it is good approach. If you think different servers could accept
different address types for the same Channel Binding type, It would be better
to introduce separate boolean compatibility property like
“com.sun.security.sasl.addrtype.compat”. In this case it would be applied not
only for tls-server-end-point but for any provided Channel Bindings
>
> Not sure if that javadoc is the right one? And I also wonder if enforcing the
> timeout is needed, and if yes if it should be documented why. Was not obvious
> to me,
>
> what about having two type names (TlsChannelBindingType.TLS_SERVER_END_POINT
> and TlsChannelBindingType.TLS_SERVER_END_POINT_COMPAT?)
>
> This could be configured as a SASL property and it would add the benefit that
> you don't need the instance specific if in the gssstub native code if you
> instead have two different types values?
>
> Gruss
> Bernd
>
> Von: security-dev im Auftrag von
> Alexey Bakhtin
> Gesendet: Mittwoch, Mai 27, 2020 11:43 AM
> An: Valerie Peng
> Cc: security-...@openjdk.java.net; core-libs-dev@openjdk.java.net; Thomas
> Maslen
> Betreff: Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
>
> Hello Valerie, Unfortunately, Windows LDAP server with
> LdapEnforceChannelBinding=2 does not accept GSS_C_AF_NULLADDR address type.
> This is exact reason of these changes. I ve tried to fix inconsistency of
> address type value in the latest webrev:
> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v2/
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Valerie,
Unfortunately, Windows LDAP server with LdapEnforceChannelBinding=2 does not
accept GSS_C_AF_NULLADDR address type.
This is exact reason of these changes.
I’ve tried to fix inconsistency of address type value in the latest webrev:
http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v2/
This fix allows to set GSS_C_AF_UNSPEC address type for the
tls-server-end-point channel binding only
Regards
Alexey
> On 26 May 2020, at 23:37, Valerie Peng wrote:
>
> I am also concerned about the changes in GSSLibStub.c about the default value
> being GSS_C_AF_UNSPEC instead of GSS_C_AF_NULLADDR (line 194-195).
>
> Can you try and see if Window works with GSS_C_AF_NULLADDR? If yes, I'd
> prefer to not changing the default value for addresstype for the same reason
> which Michael has already stated.
>
> Thanks,
> Valerie
>
>
> On 5/25/2020 8:33 AM, Alexey Bakhtin wrote:
>> Hello Michael, Thomas,
>>
>> Thank you a lot for review and suggestions.
>> I’ve fixed most of the issues except of fundamental one
>> I need more time to evaluate suggested usage of UnspecEmptyInetAddress
>> subtype.
>>
>> Updated webrev is available at:
>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v1/
>>
>>
>> Also, please see my comments below.
>>
>> Regards
>> Alexey
>>
>>
>>> On 24 May 2020, at 02:38, Michael Osipov <1983-01...@gmx.net>
>>> wrote:
>>>
>>> Am 2020-05-21 um 09:35 schrieb Alexey Bakhtin:
>>>
>>>> Hello,
>>>>
>>>> Could you please review the following patch:
>>>>
>>>> JBS:
>>>> https://bugs.openjdk.java.net/browse/JDK-8245527
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v0/
>>> Let's go through your changes statically:
>>>
>>> * The JIRA issue title has a typo
>>>
>> Thank you. Fixed in Jira
>>
>>> * The word "cannot" is incorrectly spelled throughout all exception messages
>>>
>> Fixed from “can not” to “cannot"
>>
>>>> +if
>>>> (cbTypeProp.equals(TlsChannelBindingType.TLS_UNIQUE.getName())) {
>>>> +throw new UnsupportedOperationException("LdapCtx: " +
>>>> +TlsChannelBindingType.TLS_UNIQUE.getName() + "
>>>> type is not supported");
>>>>
>>>
>>> "LdapCtx: " is redundant because the stacktrace will contain the class
>>> name already. A better message would be: "Channel binding type '%s' is
>>> not supported". Not just the plain value.
>>>
>> Exception message is corrected
>>
>>>> +} else if
>>>> (cbTypeProp.equals(TlsChannelBindingType.TLS_SERVER_END_POINT.getName())) {
>>>> +if (connectTimeout == -1)
>>>> +throw new
>>>> IllegalArgumentException(CHANNEL_BINDING_TYPE + " property requires " +
>>>> +CONNECT_TIMEOUT + " property is set.");
>>>>
>>> * Same here with the message
>>>
>> Not sure, What’s wrong with the message ?
>>
>>> * The IAE is wrong because passed value is correct, but leads to an
>>> invalid state because connection timeout is -1. You need an
>>> IllegalStateException here.
>>>
>> Thank you. You are right again. Changed to IllegalStateException
>>
>>> Stupid question: how can one create a GSS security context when the TLS
>>> security context has not been established yet?
>>>
>> This logic already existed here. It could be a reason for it and I don’t
>> want change it without strong purpose.
>> The only changes here is to prevent double setting of channel binding data.
>>
>>
>>>> ---
>>>> old/src/java.security.jgss/share/classes/sun/security/jgss/GSSContextImpl.java
>>>> 2020-05-18 19:39:46.0 +0300
>>>> +++
>>>> new/src/java.security.jgss/share/classes/sun/security/jgss/GSSContextImpl.java
>>>> 2020-05-18 19:39:46.0 +0300
>>>> @@ -531,9 +531,12 @@
>>>> public void setChannelBinding(ChannelBinding channelBindings)
>>>> throws GSSException {
>>>>
>>>> -if (mechCtxt == null)
>>>> +if (mechCtxt == null) {
>>>> +if (this.channelBindings != null) {
>>>>
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Max, Thank you review. If I understand correctly tls-server-end-point channel binding data is a hash of server certificate. Different SASL protocols could send cbData differently, with different prefix format. This is a reason I create TLSChannelBinding class and calculate hash from the LdapClient and add “tls-server-end-point:” prefix in the JGSS/Kerberos. Alexey > On 26 May 2020, at 17:50, Weijun Wang wrote: > > I have a question on GssKrb5Client.java: > > Do you think it's a good idea to let the SASL mechanism understand what TLS > binding is? Instead of passing the whole TlsChannelBinding object through a > SASL property, is it possible to only pass the actual cbData? After all, the > name "com.sun.security.sasl.channelbinding" suggests it's just a general > ChannelBinding which is independent with any application level info. > > From my reading of the code change, it looks like this cbData can be > calculated on the LDAP side. > > Thanks, > Max > >> On May 21, 2020, at 3:35 PM, Alexey Bakhtin wrote: >> >> Hello, >> >> Could you please review the following patch: >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8245527 >> Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v0/ >> >> The Windows LDAP server with LdapEnforceChannelBinding=2 uses the >> tls-server-end-point channel binding >> (based on the TLS server certificate). The channel binding data is >> calculated as following : >> • The client calculates a hash of the TLS server certificate. >> The hash algorithm is selected on the base of the certificate >> signature algorithm. >> Also, the client should use SHA-256 algorithm, in case of the >> certificate signature algorithm is SHA1 or MD5 based >> • The channel binding information is the same as defined in rfc4121 [1] >> with small corrections: >> • initiator and acceptor addresses should be set to NULL >> • initiator and acceptor address types should be zero. >> It contradicts to the “Using Channel Bindings in GSS-API” >> document [2] that say that >> the address type should be set to GSS_C_AF_NULLADDR=0xFF, >> instead of GSS_C_AF_UNSPEC=0x00. >> >> This patch introduces changes in the LDAP, SASL and JGSS modules >> to generate channel binding data automatically if requested by an >> application. >> This patch reuses existing org.ietf.jgss.ChannelBinding class implementation >> but changes >> initial unspecified address type from CHANNEL_BINDING_AF_NULL_ADDR to >> CHANNEL_BINDING_AF_UNSPEC. >> The patch introduces new environment property “com.sun.jndi.ldap.tls.cbtype” >> that indicates >> Channel Binding type that should be used in the LDAPS connection over >> JGSS/Kerberos. >> Right now "tls-server-end-point" Channel Binding type is supported only. >> The client extracts server certificate from the underlying TLS connection >> and creates >> Channel Binding data for JGSS/Kerberos authentication if application >> indicates >> com.sun.jndi.ldap.tls.cbtype=tls-server-end-point property. >> Client application should also specify existing >> "com.sun.jndi.ldap.connect.timeout” property >> to force and wait TLS handshake completion before JGSS/Kerberos >> authentication data is generated. >> >> [1] - https://tools.ietf.org/html/rfc4121#section-4.1.1.2 >> >> [2] - >> https://docs.oracle.com/cd/E19120-01/open.solaris/819-2145/overview-52/index.html >> >> Initial discussion of this issue is available at security-dev list: >> https://mail.openjdk.java.net/pipermail/security-dev/2019-December/021052.html >> https://mail.openjdk.java.net/pipermail/security-dev/2020-January/021140.html >> https://mail.openjdk.java.net/pipermail/security-dev/2020-February/021278.html >> https://mail.openjdk.java.net/pipermail/security-dev/2020-May/021864.html
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Aleks, Daniel, Thank you for your comments. I don’t like that the code is split into several modules too. The reason of doing it is I can not get TLS server certificate from the JGSS/Kerberos code. The only place Where I can fetch it is LdapClient. If I understand your idea correctly, I can extract TLS server certificate in the LdapClient and put it into internal environment property as byte array without mention about Channel Binding. It will be done for every LDAPS connection. In case of “com.sun.security.sasl.channelbinding=tls-server-end-point” property specified, GssKrb5Client extracts certificate from the internal environment property and use it to create TLS Channel Binding. In this case almost all Channel Binding code could be moved to GssKrb5Client. LdapClient still changed but not mention about Channel Binding. Changes in the LdapCtx could be omitted. Regards Alexey > On 26 May 2020, at 17:55, Aleks Efimov wrote: > > Hi Alexey, > > I agree with all Daniel's comments. > > Few thoughts about moving the implementation down to SASL layers: > Will it be possible to make this new code specific only for JGSS/Kerberos > authentication mechanism? > Maybe investigate moving all the new code to GssKrb5Client SASL client > implementation (GssKrb5Client class, "GSSAPI" authentication mechanism name): > - That may require to store the server certificate extracted from SSLSocket > into new context environment property > - The code that processes and checks the String value of channel binding type > value could also be moved to GssKrb5Client or to TlsChannelBinding > - Add TlsChannelBinding factory method that creates an object from the server > certificate and the string value of the environment property could also be > useful here > > All of that will allow us to keep the fix specific to "JGSS/Kerberos" area > and will keep LdapCtx/LdapClient code changes minimal and clear of > "JGSS/Kerberos" details > > Best Regards, > Aleksei > > On 26/05/2020 15:14, Daniel Fuchs wrote: >> Hi Alexey, >> >> This is not a review. A few high level comments however: >> >> For that kind of change that introduce a new environment >> property you will need to file a CSR, and probably provide >> some release notes as well. >> >> Your changes seem to trigger new IllegalStateException and >> UnsupportedOperationExceptions which are undocumented. >> I believe they should be replaced by NamingException which >> is documented at the public API level. >> >> Also it would be good to have a test that validates that >> the proposed changes works as expected. >> >> I will not comment on the security libs changes - I'm clearly >> out of my depth there. It's a bit distasteful that the >> LdapCtxt/LdapClient have to have knowledge of channel binding >> and extract the certificates from the SSLSocket to pass them to >> the lower layer. Ideally this should rather be handled by the >> SASL layers? >> >> best regards, >> >> -- daniel >> >> >> On 25/05/2020 16:33, Alexey Bakhtin wrote: >>> Updated webrev is available >>> at:http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v1/ >>
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hi Michael, Thomas, Unfortunately we can not fix address type issue with the UnspecEmptyInetAddress subclass. We can not create subclass of InetAddress without changing public API. We can try similar approach but create subclass of ChannelBinding class instead. It is not so elegant like UnspecEmptyInetAddress approach but it could help to distinguish between TLS channel binding and Channel Bindings set by application. Later we can remove this workaround In case we prove that UNSPEC type should be used in all types of Channel Bindings. Updated webrev : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v2/ Regards Alexey > On 26 May 2020, at 06:04, Thomas Maslen wrote: > > On Mon, May 25, 2020 at 3:15 AM Michael Osipov <1983-01...@gmx.net> wrote: > [...] > So I read the discussions. RFC 2744 shall not be changed, people > admitted that the spec of SASL GS2 mechs is wrong and should be changed, > but this has not happened yet. It remained at UNSPEC all the years. > > So we have several issues here: > * GSS-API C bindings and SASL requests are two distinct RFCs which > require/mandate differnt things. > * The change in JGSS in unrelated to this patch because GSS-API knows > nothing about SASL and its fauly spec. > > Since we are doing LDAP over SASL here and RFC 5801 requires to be > UNSPEC (0) the SASL TlsChannelBinding class must take that into account. > Unfortunately, JGSS implies with the args of the ChannelBinding the type > fo the adress. So will change my opinion a bit: > > No property for AD/non-AD is necessary, but handling of UNSPEC is > required. JGSS shall remain at NULLADDR. The subtype > UnspecEmptyInetAddress should be at least evaluated. > > Michael > > No. This isn't just about RFC 5801. As Alexey Bakhtin observed, this also > applies to channel bindings for HTTP Negotiate Authentication (loosely aka > "SPNEGO"), not only for NTLM (which probably isn't at issue here) but also > for Kerberos -- that's where I first encountered this, working on a > proprietary Java Kerberos implementation. > > More generally, if you want channel bindings to interoperate in the GSSAPI > Kerberos Mechanism for any protocol -- SASL GS2, HTTP Negotiate > Authentication, or anything else -- ignore the fact that RFC 2744 specifies > 255 for the "no address" case and do what everyone actually does: use zero. > > Here is a test from MIT Kerberos that (implicitly) uses zero: > https://github.com/krb5/krb5/blob/master/src/tests/gssapi/t_bindings.c > > And here is one from Heimdal: > https://github.com/heimdal/heimdal/blob/5057d04f6a47f05f1ed7c617458722104d4c17dc/lib/gssapi/test_context.c
Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello Michael, Thomas,
Thank you a lot for review and suggestions.
I’ve fixed most of the issues except of fundamental one
I need more time to evaluate suggested usage of UnspecEmptyInetAddress subtype.
Updated webrev is available at:
http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v1/
Also, please see my comments below.
Regards
Alexey
> On 24 May 2020, at 02:38, Michael Osipov <1983-01...@gmx.net> wrote:
>
> Am 2020-05-21 um 09:35 schrieb Alexey Bakhtin:
>> Hello,
>>
>> Could you please review the following patch:
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8245527
>> Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v0/
>
> Let's go through your changes statically:
>
> * The JIRA issue title has a typo
Thank you. Fixed in Jira
> * The word "cannot" is incorrectly spelled throughout all exception messages
Fixed from “can not” to “cannot"
>
>> +if
>> (cbTypeProp.equals(TlsChannelBindingType.TLS_UNIQUE.getName())) {
>> +throw new UnsupportedOperationException("LdapCtx: " +
>> +TlsChannelBindingType.TLS_UNIQUE.getName() + " type
>> is not supported");
>
>
> "LdapCtx: " is redundant because the stacktrace will contain the class
> name already. A better message would be: "Channel binding type '%s' is
> not supported". Not just the plain value.
Exception message is corrected
>
>> +} else if
>> (cbTypeProp.equals(TlsChannelBindingType.TLS_SERVER_END_POINT.getName())) {
>> +if (connectTimeout == -1)
>> +throw new IllegalArgumentException(CHANNEL_BINDING_TYPE
>> + " property requires " +
>> +CONNECT_TIMEOUT + " property is set.");
>
> * Same here with the message
Not sure, What’s wrong with the message ?
> * The IAE is wrong because passed value is correct, but leads to an
> invalid state because connection timeout is -1. You need an
> IllegalStateException here.
Thank you. You are right again. Changed to IllegalStateException
>
> Stupid question: how can one create a GSS security context when the TLS
> security context has not been established yet?
This logic already existed here. It could be a reason for it and I don’t want
change it without strong purpose.
The only changes here is to prevent double setting of channel binding data.
>
>> ---
>> old/src/java.security.jgss/share/classes/sun/security/jgss/GSSContextImpl.java
>>2020-05-18 19:39:46.0 +0300
>> +++
>> new/src/java.security.jgss/share/classes/sun/security/jgss/GSSContextImpl.java
>>2020-05-18 19:39:46.0 +0300
>> @@ -531,9 +531,12 @@
>> public void setChannelBinding(ChannelBinding channelBindings)
>> throws GSSException {
>>
>> -if (mechCtxt == null)
>> +if (mechCtxt == null) {
>> +if (this.channelBindings != null) {
>> +throw new GSSException(GSSException.BAD_BINDINGS);
>> +}
>> this.channelBindings = channelBindings;
>> -
>> +}
>> }
>
> I don't understand the purpose of this hunk. Is this safeguard to set
> bindings only once?
>
>> private static final int CHANNEL_BINDING_AF_INET = 2;
>> private static final int CHANNEL_BINDING_AF_INET6 = 24;
>> private static final int CHANNEL_BINDING_AF_NULL_ADDR = 255;
>> +private static final int CHANNEL_BINDING_AF_UNSPEC = 0;
>
> This should sort from 0 to 255 and not at the end.
OK. Moved to the top.
>
>> private int getAddrType(InetAddress addr) {
>> -int addressType = CHANNEL_BINDING_AF_NULL_ADDR;
>> +int addressType = CHANNEL_BINDING_AF_UNSPEC;
>
>> // initialize addrtype in CB first
>> - cb->initiator_addrtype = GSS_C_AF_NULLADDR;
>> - cb->acceptor_addrtype = GSS_C_AF_NULLADDR;
>> + cb->initiator_addrtype = GSS_C_AF_UNSPEC;
>> + cb->acceptor_addrtype = GSS_C_AF_UNSPEC;
>
> This looks wrong to me -- as you already mentioned -- this violates RFC
> 2744, section 3.11, last sentence:
>> or omit addressing information, specifying
>> GSS_C_AF_NULLADDR as the address-types.
>
>> /* release initiator address */
>> - if (cb->initiator_addrtype != GSS_C_AF_NULLADDR) {
>> + if (cb->initiator_addrtype != GSS_C_AF_NULLADDR &&
>> + cb->initiator_addrtype != GSS_C_AF_UNSPEC) {
>> resetGSSBuffer(&(cb->initiator_address));
>> }
>> /* release a
RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos
Hello, Could you please review the following patch: JBS: https://bugs.openjdk.java.net/browse/JDK-8245527 Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v0/ The Windows LDAP server with LdapEnforceChannelBinding=2 uses the tls-server-end-point channel binding (based on the TLS server certificate). The channel binding data is calculated as following : • The client calculates a hash of the TLS server certificate. The hash algorithm is selected on the base of the certificate signature algorithm. Also, the client should use SHA-256 algorithm, in case of the certificate signature algorithm is SHA1 or MD5 based • The channel binding information is the same as defined in rfc4121 [1] with small corrections: • initiator and acceptor addresses should be set to NULL • initiator and acceptor address types should be zero. It contradicts to the “Using Channel Bindings in GSS-API” document [2] that say that the address type should be set to GSS_C_AF_NULLADDR=0xFF, instead of GSS_C_AF_UNSPEC=0x00. This patch introduces changes in the LDAP, SASL and JGSS modules to generate channel binding data automatically if requested by an application. This patch reuses existing org.ietf.jgss.ChannelBinding class implementation but changes initial unspecified address type from CHANNEL_BINDING_AF_NULL_ADDR to CHANNEL_BINDING_AF_UNSPEC. The patch introduces new environment property “com.sun.jndi.ldap.tls.cbtype” that indicates Channel Binding type that should be used in the LDAPS connection over JGSS/Kerberos. Right now "tls-server-end-point" Channel Binding type is supported only. The client extracts server certificate from the underlying TLS connection and creates Channel Binding data for JGSS/Kerberos authentication if application indicates com.sun.jndi.ldap.tls.cbtype=tls-server-end-point property. Client application should also specify existing "com.sun.jndi.ldap.connect.timeout” property to force and wait TLS handshake completion before JGSS/Kerberos authentication data is generated. [1] - https://tools.ietf.org/html/rfc4121#section-4.1.1.2 [2] - https://docs.oracle.com/cd/E19120-01/open.solaris/819-2145/overview-52/index.html Initial discussion of this issue is available at security-dev list: https://mail.openjdk.java.net/pipermail/security-dev/2019-December/021052.html https://mail.openjdk.java.net/pipermail/security-dev/2020-January/021140.html https://mail.openjdk.java.net/pipermail/security-dev/2020-February/021278.html https://mail.openjdk.java.net/pipermail/security-dev/2020-May/021864.html
