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

[Weijun Wang]

> On Jul 3, 2020, at 7:32 PM, Alexey Bakhtin <ale...@azul.com> wrote:
> 
> Hello All,
> 
> Thank you for review.
> 
>> 1. If the change in java.security.jgss/share/classes/module-info.java is 
>> unavoidable, can we create a sub-package for this single class so that we 
>> only need to export it?
> 
> As suggested by Max I’ve moved TlsChannelBindingImpl class into sub-package, 
> so module-info.java exports TlsChannelBindingImpl only.

Thanks.

> 
>> 
>> 2. Is GSSContextImpl::setChannelBinding really necessary? I don't know if 
>> there are people using null to erase a CB set earlier.
> 
> I think these changes could be useful to exclude situations when application 
> trying to set Channel Binding with GSSContext::setChannelBinding and 
> “com.sun.jndi.ldap.tls.cbtype” property altogether. I can remove it, if you 
> think it is not necessary.

I would suggest removing it. At least for the SASL GSS-API mech, it seems the 
GSSContext object will not be leaked and no one has a chance to call 
setChannelBinding again on it.

There is no spec saying setChannelBinding() can only be called once, so I'd 
rather we don't enforce that, although you might say there is no need to call 
it twice.

> 
> Also, I've fixed Exception text and parseType(String prop) parameter name as 
> suggested by Michael.
> Unfortunately, I can not completely exclude usage of the literal names 
> because of module import issues. Fixed in the TlsChannelBinding class only.
> 
> Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v10/

Thanks,
Max

> 
> Regards
> Alexey
>