> On Jun 5, 2020, at 11:03 PM, Alexey Bakhtin <ale...@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
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.
>
> Regards
> Alexey
>
>> On 5 Jun 2020, at 06:41, Weijun Wang <weijun.w...@oracle.com> wrote:
>>
>> Hi Alexey,
>>
>> It's so unfortunate that different addressType must be used. I'm OK with the
>> new TlsChannelBindingImpl class.
>>
>> One thing I'm not comfortable is the
>> java.security.sasl/share/classes/module-info.java change. We've struggled
>> hard to avoid these kind of secret tunnels. Is it possible to pass the
>> tlsCB.getData() directly to the SASL mechanism? The property name is clear
>> enough to avoid any misuse.
>>
>> Another question: can an application user set the
>> "com.sun.security.sasl.tlschannelbinding" property? and can they read it
>> after it's set internally? I cannot think of a good attack now but at least
>> it seems they have no need to access that property value. If we keep it
>> internal then we also have the chance to modify the approach later.
>>
>> Thanks,
>> Max
>>
>>
>>> On Jun 5, 2020, at 2:15 AM, Alexey Bakhtin <ale...@azul.com> wrote:
>>>
>>> Hello,
>>>
>>> Could you please review new version of the patch:
>>> http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v3/
>>>
>>> I’ve moved all logic for creating TLS Channel Binding data out of
>>> GssKrb5Client.java file.
>>> All data is prepared inside TlsChannelBinding class.
>>> Internal property name is renamed to
>>> “com.sun.security.sasl.tlschannelbinding”.
>>> TlsChannelBinding object is still used to pass channel binding data from
>>> LdapClient to GssKrb5Client.
>>> I do not change it to byte[] because of TlsChannelBinding class is still
>>> used for internal property name.
>>> Also, I’ve updated TlsChannelBinding class to select SHA-256 hash algorithm
>>> if it can not be derived
>>> from the certificate signature name.
>>>
>>> Regards
>>> Alexey
>>>
>>>
>>>> On 26 May 2020, at 17:50, Weijun Wang <weijun.w...@oracle.com> wrote:
>>>>
>>>> I have a question on GssKrb5Client.java:
>>>>
>>>> Do you think it's a good idea to let the SASL mechanism understand what
>>>> TLS binding is? Instead of passing the whole TlsChannelBinding object
>>>> through a SASL property, is it possible to only pass the actual cbData?
>>>> After all, the name "com.sun.security.sasl.channelbinding" suggests it's
>>>> just a general ChannelBinding which is independent with any application
>>>> level info.
>>>>
>>>> From my reading of the code change, it looks like this cbData can be
>>>> calculated on the LDAP side.
>>>>
>>>> Thanks,
>>>> Max
>>>>
>>>>> On May 21, 2020, at 3:35 PM, Alexey Bakhtin <ale...@azul.com> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you please review the following patch:
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8245527
>>>>> Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v0/
>>>>>
>>>>> The Windows LDAP server with LdapEnforceChannelBinding=2 uses the
>>>>> tls-server-end-point channel binding
>>>>> (based on the TLS server certificate). The channel binding data is
>>>>> calculated as following :
>>>>> • The client calculates a hash of the TLS server certificate.
>>>>> The hash algorithm is selected on the base of the certificate
>>>>> signature algorithm.
>>>>> Also, the client should use SHA-256 algorithm, in case of the
>>>>> certificate signature algorithm is SHA1 or MD5 based
>>>>> • The channel binding information is the same as defined in rfc4121 [1]
>>>>> with small corrections:
>>>>> • initiator and acceptor addresses should be set to NULL
>>>>> • initiator and acceptor address types should be zero.
>>>>> It contradicts to the “Using Channel Bindings in GSS-API”
>>>>> document [2] that say that
>>>>> the address type should be set to GSS_C_AF_NULLADDR=0xFF,
>>>>> instead of GSS_C_AF_UNSPEC=0x00.
>>>>>
>>>>> This patch introduces changes in the LDAP, SASL and JGSS modules
>>>>> to generate channel binding data automatically if requested by an
>>>>> application.
>>>>> This patch reuses existing org.ietf.jgss.ChannelBinding class
>>>>> implementation but changes
>>>>> initial unspecified address type from CHANNEL_BINDING_AF_NULL_ADDR to
>>>>> CHANNEL_BINDING_AF_UNSPEC.
>>>>> The patch introduces new environment property
>>>>> “com.sun.jndi.ldap.tls.cbtype” that indicates
>>>>> Channel Binding type that should be used in the LDAPS connection over
>>>>> JGSS/Kerberos.
>>>>> Right now "tls-server-end-point" Channel Binding type is supported only.
>>>>> The client extracts server certificate from the underlying TLS connection
>>>>> and creates
>>>>> Channel Binding data for JGSS/Kerberos authentication if application
>>>>> indicates
>>>>> com.sun.jndi.ldap.tls.cbtype=tls-server-end-point property.
>>>>> Client application should also specify existing
>>>>> "com.sun.jndi.ldap.connect.timeout” property
>>>>> to force and wait TLS handshake completion before JGSS/Kerberos
>>>>> authentication data is generated.
>>>>>
>>>>> [1] - https://tools.ietf.org/html/rfc4121#section-4.1.1.2
>>>>>
>>>>> [2] -
>>>>> https://docs.oracle.com/cd/E19120-01/open.solaris/819-2145/overview-52/index.html
>>>>>
>>>>> Initial discussion of this issue is available at security-dev list:
>>>>> https://mail.openjdk.java.net/pipermail/security-dev/2019-December/021052.html
>>>>> https://mail.openjdk.java.net/pipermail/security-dev/2020-January/021140.html
>>>>> https://mail.openjdk.java.net/pipermail/security-dev/2020-February/021278.html
>>>>> https://mail.openjdk.java.net/pipermail/security-dev/2020-May/021864.html
>>>
>
