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

[Osipov, Michael]

Am 2020-06-30 um 14:22 schrieb Alexey Bakhtin:
> 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/

A few notes
> +                throw new NamingException(TlsChannelBinding.CHANNEL_BINDING_TYPE +
> +                        " property requires " +
> +                        "com.sun.jndi.ldap.connect.timeout property is set.");

"...requires ... to be set." The current text reads strange in English.

> +                    env.get("jdk.internal.sasl.tlschannelbinding") != null) {

and alike. Any reason to use the literal instead of the static symbol? 
Module import issues?

In TlsChannelBinding.java:

> +    // TLS channel binding type property
> +    public static final String CHANNEL_BINDING_TYPE =
> +            "com.sun.jndi.ldap.tls.cbtype";

and

> +    public static TlsChannelBindingType parseType(String prop) throws NamingException {

TLS channel binding is not tied to LDAP, it can be used with other 
protocols, even custom ones. I see no good reason to have the property 
contain jndi.ldap or use NamingException. IllegalArgumentException would 
be approriate here.
Also on this class you have the wonderful enum TlsChannelBindingType, 
but still use litrals throughout for the possible values. Why?
Why isn't this class part of the Sun SASL impl?

 > #parseType()

I would not use the term 'prop'. I miss the loose coupling here. 
Ideally, the class does not even know that is might be passed around 
with a env property from LDAP, SMTP, foo protocol to SASL. It is merely 
a record to contain the calculcated data. I'd remove all props from here.

TlsChannelBindingImpl.java:

I like this idea. This basically solves the problem with the same 
approach I have proposed from the first time, a specialized subimpl, but 
this time not for the address, but for the binding type. Nice!

I consider this webrev version much much better than v1. I really miss 
the TLS Channel Binding abstraction from LDAP. The current situation 
bascially means that no one with Java can implement TLS-CB with anything 
else than the Sun JNDI LDAP provider over SASL. Kind of disappointing.

Michael