On Wed, 20 Jan 2021 15:54:41 GMT, Daniel Fuchs <dfu...@openjdk.org> 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<String, Object>) 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<String, Object>) 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
