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/5/20 12:45 PM, Daniel Fuchs wrote:
Hi Alexey,

On 05/06/2020 17:33, Alexey Bakhtin wrote:
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 ?

Yes - I am OK with that.

Also - you will need a test. Ideally we'd want a test that verifies
that setting the new property works as expected.

Best regards,

-- daniel


Thank you
Alexey

Reply via email to