RFR: 8259707: LDAP channel binding does not work with StartTLS extension [v2]

[Alexey Bakhtin]

On Wed, 20 Jan 2021 15:54:41 GMT, Daniel Fuchs <dfuchs at 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