RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

[Alexey Bakhtin]

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 <weijun.wang at oracle.com> wrote:
> 
> 
> 
>> On Jun 6, 2020, at 2:41 PM, Weijun Wang <weijun.wang at oracle.com> wrote:
>> 
>> 
>> 
>>> On Jun 5, 2020, at 11:03 PM, Alexey Bakhtin <alexey at azul.com> 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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Message signed with OpenPGP
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20200606/934d4b9a/signature.asc>