Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Martin Balao
Hi,

Here it is my proposal for JDK-6491070 (Support for RFC 5929-Channel Bindings: e.g. public API to obtain TLS finished message) [1]:


Notes:
 
 * Implementation based on Channel Bindings for TLS (RFC 5929) [2]

 * Only "tls-unique" currently supported

Look forward to your comments.

Kind regards,
Martin.-

--
Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Martin Balao
Hi,

Here it is an update for the proposed TLS Channel Bindings support in OpenJDK:


Changes since v01:

 * getTlsChannelBinding API changed to return null by default (if not implemented), instead of throwing an UnsupportedOperationException.

 * "tls-server-end-point" TLS channel binding now supported.

Kind regards,
Martin.-

On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao <[hidden email]> wrote:
Hi,

Here it is my proposal for JDK-6491070 (Support for RFC 5929-Channel Bindings: e.g. public API to obtain TLS finished message) [1]:


Notes:
 
 * Implementation based on Channel Bindings for TLS (RFC 5929) [2]

 * Only "tls-unique" currently supported

Look forward to your comments.

Kind regards,
Martin.-

--

Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Xuelei Fan-2
Hi Marin,

Sorry for the delay.

There are a few protocols that can benefits from exporting SSL/TLS
handshake materials, including RFC 5929, RFC 5056, token binding and TLS
1.3 itself.  Can we define a general API so as to exposing the handshake
materials, so as to mitigate the inflating of JSSE APIs?  I may suggest
make further evaluation before move on to following design and code.

 >
http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
I have two concerns about the design:

1. Channel binding may be not always required.
SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);

The SunJSSE provider happens to cache the finished messages in its
implementation so you can use it for tls-unique, but it may not be true
for other provider or other channel bindings.  Need to define a more
reliable approach to get the handshake materials.

If the channel binding is not required, it may be not necessary to
expose the handshake materials.  Need to define a solution to indicate
the need of the exporting.

2. No way to know the update of the underlying handshake materials.
If renegotiation can takes place, need to define a interface to indicate
that so that application can response accordingly.  See section 3 and 7
of RFC 5929.

Thanks,
Xuelei

On 7/31/2017 8:53 AM, Martin Balao wrote:

> Hi,
>
> Here it is an update for the proposed TLS Channel Bindings support in
> OpenJDK:
>
>   *
> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/ 
> (browse online)
>   *
> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip 
> (download)
>
> Changes since v01:
>
>   * getTlsChannelBinding API changed to return null by default (if not
> implemented), instead of throwing an UnsupportedOperationException.
>
>   * "tls-server-end-point" TLS channel binding now supported.
>
> Kind regards,
> Martin.-
>
> On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi,
>
>     Here it is my proposal for JDK-6491070 (Support for RFC 5929-Channel
>     Bindings: e.g. public API to obtain TLS finished message) [1]:
>
>       *
>     http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>     <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>       *
>     http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>     <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>
>     Notes:
>       * Implementation based on Channel Bindings for TLS (RFC 5929) [2]
>
>       * Only "tls-unique" currently supported
>
>     Look forward to your comments.
>
>     Kind regards,
>     Martin.-
>
>     --
>     [1] - https://bugs.openjdk.java.net/browse/JDK-6491070
>     <https://bugs.openjdk.java.net/browse/JDK-6491070>
>     [2] - https://tools.ietf.org/html/rfc5929
>     <https://tools.ietf.org/html/rfc5929>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Bernd Eckenfels-4
How about only passing it to an extended handshake listener. The material does not have to be cached (the app can do it if needed) and renegotiation works the same way. This can also be helpful for things like logging the master secret (for wireshark decryption). It can even handle auditing of session resumptions 


From: security-dev <[hidden email]> on behalf of Xuelei Fan <[hidden email]>
Sent: Saturday, August 26, 2017 11:25:50 PM
To: Martin Balao; [hidden email]
Subject: Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)
 
Hi Marin,

Sorry for the delay.

There are a few protocols that can benefits from exporting SSL/TLS
handshake materials, including RFC 5929, RFC 5056, token binding and TLS
1.3 itself.  Can we define a general API so as to exposing the handshake
materials, so as to mitigate the inflating of JSSE APIs?  I may suggest
make further evaluation before move on to following design and code.

 >
http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
I have two concerns about the design:

1. Channel binding may be not always required.
SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);

The SunJSSE provider happens to cache the finished messages in its
implementation so you can use it for tls-unique, but it may not be true
for other provider or other channel bindings.  Need to define a more
reliable approach to get the handshake materials.

If the channel binding is not required, it may be not necessary to
expose the handshake materials.  Need to define a solution to indicate
the need of the exporting.

2. No way to know the update of the underlying handshake materials.
If renegotiation can takes place, need to define a interface to indicate
that so that application can response accordingly.  See section 3 and 7
of RFC 5929.

Thanks,
Xuelei

On 7/31/2017 8:53 AM, Martin Balao wrote:
> Hi,
>
> Here it is an update for the proposed TLS Channel Bindings support in
> OpenJDK:
>
>   *
> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
> (browse online)
>   *
> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
> (download)
>
> Changes since v01:
>
>   * getTlsChannelBinding API changed to return null by default (if not
> implemented), instead of throwing an UnsupportedOperationException.
>
>   * "tls-server-end-point" TLS channel binding now supported.
>
> Kind regards,
> Martin.-
>
> On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao <[hidden email]
> <[hidden email]>> wrote:
>
>     Hi,
>
>     Here it is my proposal for JDK-6491070 (Support for RFC 5929-Channel
>     Bindings: e.g. public API to obtain TLS finished message) [1]:
>
>       *
>     http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>     <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>       *
>     http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>     <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>
>     Notes:
>       * Implementation based on Channel Bindings for TLS (RFC 5929) [2]
>
>       * Only "tls-unique" currently supported
>
>     Look forward to your comments.
>
>     Kind regards,
>     Martin.-
>
>     --
>     [1] - https://bugs.openjdk.java.net/browse/JDK-6491070
>     <https://bugs.openjdk.java.net/browse/JDK-6491070>
>     [2] - https://tools.ietf.org/html/rfc5929
>     <https://tools.ietf.org/html/rfc5929>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Martin Balao
In reply to this post by Xuelei Fan-2
Hi Xuelei,

>There are a few protocols that can benefits from exporting SSL/TLS handshake materials, including RFC 5929, RFC 5056, token binding and TLS 1.3 itself.  Can we define a general API so as to exposing the handshake materials, so as to mitigate the inflating of JSSE APIs?  I may suggest make further evaluation before move on to following design and code.

Do you prefer an API like "public byte[] getTlsHandshakeMaterial(String materialType)" (in SSLSocket and SSLEngine) where "materialType" can eventually be "clientFinishedMessage"/"finishedMessage" or "serverFinishedMessage"/"peerFinishedMessage"? I cannot think of "serverCertificate" or "masterKey" as this is more related to a Session and not neccessarily to a handshake. getTlsHandshakeMaterial would be a lower level API and would move the burden of knowing which information is required for "tls-unique" TLS channel binding to the API consumer. Looks more like the OpenSSL approach (instead of the Python, SSLBoring or GnuTls approaches). However, OpenSSL have specific methods for each piece of information instead of a generic and parametrized one. I.e.: SSL_get_finished or SSL_get_peer_finished. What other information do you expect the Handshaker to provide?

>The SunJSSE provider happens to cache the finished messages in its implementation so you can use it for tls-unique, but it may not be true for other provider or other channel bindings.  Need to define a more reliable approach to get the handshake materials.

I focused on SunJSSE provider. I'm not sure about how other providers may implement this API and where they can get the required information from, without knowing their internals. In regard to SunJSSE and "tls-unique" binding type, I leveraged on existing data. If data weren't already there, I would have to figure out how to get it from the handshake -doing the same that was already done would have been an option-. Do you prefer the Handshaker to provide a function to get different information and not just the finished hash? (as for the public SSLSocket/SSLEngine "getTlsHandshakeMaterial" API). Which other information may be useful to get from the Handshaker? What do you mean by reliable? (given that this is all SunJSSE internal and we have no external dependencies).

In regard to other channel bindings, it'll depend on the binding type the way in which the information is obtained. I.e.: "tls-unique" SunJSSE implementation leverages on cached finished messages. However, "tls-server-end-point" leverages on stored certificates that are obtained from the Session (not from the handshaker). Is there any specific channel binding you are concerned with?

>If the channel binding is not required, it may be not necessary to expose the handshake materials.  Need to define a solution to indicate the need of the exporting.

Do you mean a lower layer knowing if the upper layer is going to require that information and decide to provide it or not based on that knowledge? I think I didn't get your point here.

>2. No way to know the update of the underlying handshake materials.
>If renegotiation can takes place, need to define a interface to indicate that so that application can response accordingly.  See section 3 and 7 of RFC 5929.

I intentionally skipped this -at the cost of a spurious authentication- to avoid adding complexity to the API. An spurious authentication -which does not appear likely to me- can easily be retried by the application. The RFC 5929 suggests APIs through which the application can *control* the flow (i.e.: hold a renegotitation). This would expose JSSE internals. This is more than notifying. Notification, in my opinion, adds no value: what if the application already used the binding token before receiving the notification? The spurious authentication will happen anyways and has to be handled -i.e. retried-. It's just a timing issue. The real value is controlling the flow as the RFC suggests, but at the cost of exposing JSSE internals.

Kind regards,
Martin.-

On Sat, Aug 26, 2017 at 5:25 PM, Xuelei Fan <[hidden email]> wrote:
Hi Marin,

Sorry for the delay.

There are a few protocols that can benefits from exporting SSL/TLS handshake materials, including RFC 5929, RFC 5056, token binding and TLS 1.3 itself.  Can we define a general API so as to exposing the handshake materials, so as to mitigate the inflating of JSSE APIs?  I may suggest make further evaluation before move on to following design and code.

> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
I have two concerns about the design:

1. Channel binding may be not always required.
SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);

The SunJSSE provider happens to cache the finished messages in its implementation so you can use it for tls-unique, but it may not be true for other provider or other channel bindings.  Need to define a more reliable approach to get the handshake materials.

If the channel binding is not required, it may be not necessary to expose the handshake materials.  Need to define a solution to indicate the need of the exporting.

2. No way to know the update of the underlying handshake materials.
If renegotiation can takes place, need to define a interface to indicate that so that application can response accordingly.  See section 3 and 7 of RFC 5929.

Thanks,
Xuelei

On 7/31/2017 8:53 AM, Martin Balao wrote:
Hi,

Here it is an update for the proposed TLS Channel Bindings support in OpenJDK:

  * http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/ (browse online)
  * http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip (download)

Changes since v01:

  * getTlsChannelBinding API changed to return null by default (if not implemented), instead of throwing an UnsupportedOperationException.

  * "tls-server-end-point" TLS channel binding now supported.

Kind regards,
Martin.-

On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao <[hidden email] <mailto:[hidden email]>> wrote:

    Hi,

    Here it is my proposal for JDK-6491070 (Support for RFC 5929-Channel
    Bindings: e.g. public API to obtain TLS finished message) [1]:

      *
    http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
    <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
      *
    http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
    <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>

    Notes:
      * Implementation based on Channel Bindings for TLS (RFC 5929) [2]

      * Only "tls-unique" currently supported

    Look forward to your comments.

    Kind regards,
    Martin.-

    --
    [1] - https://bugs.openjdk.java.net/browse/JDK-6491070
    <https://bugs.openjdk.java.net/browse/JDK-6491070>
    [2] - https://tools.ietf.org/html/rfc5929
    <https://tools.ietf.org/html/rfc5929>



Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Xuelei Fan-2
On 8/26/2017 2:56 PM, Bernd Eckenfels wrote:
 > How about only passing it to an extended handshake listener. The
 > material does not have to be cached (the app can do it if needed) and
 > renegotiation works the same way. This can also be helpful for things
 > like logging the master secret (for wireshark decryption). It can even
 > handle auditing of session resumptions
Martin, what do you think about Bernd's proposal above and similar
callback mechanism?

More comment inlines.

On 8/29/2017 11:50 AM, Martin Balao wrote:

> Hi Xuelei,
>
>  >There are a few protocols that can benefits from exporting SSL/TLS
> handshake materials, including RFC 5929, RFC 5056, token binding and TLS
> 1.3 itself.  Can we define a general API so as to exposing the handshake
> materials, so as to mitigate the inflating of JSSE APIs?  I may suggest
> make further evaluation before move on to following design and code.
>
> Do you prefer an API like "public byte[] getTlsHandshakeMaterial(String
> materialType)" (in SSLSocket and SSLEngine) where "materialType" can
> eventually be "clientFinishedMessage"/"finishedMessage" or
> "serverFinishedMessage"/"peerFinishedMessage"?
The problem of the APIs like that is, when applications call the method,
it is not always return the expected result, and the implementation may
have to cache the message even if an application never use it.  See more
in the following example.

> I cannot think of
> "serverCertificate" or "masterKey" as this is more related to a Session
> and not neccessarily to a handshake. getTlsHandshakeMaterial would be a
> lower level API and would move the burden of knowing which information
> is required for "tls-unique" TLS channel binding to the API consumer.
> Looks more like the OpenSSL approach (instead of the Python, SSLBoring
> or GnuTls approaches). However, OpenSSL have specific methods for each
> piece of information instead of a generic and parametrized one. I.e.:
> SSL_get_finished or SSL_get_peer_finished. What other information do you
> expect the Handshaker to provide?
>
>  >The SunJSSE provider happens to cache the finished messages in its
> implementation so you can use it for tls-unique, but it may not be true
> for other provider or other channel bindings.  Need to define a more
> reliable approach to get the handshake materials.
>
> I focused on SunJSSE provider. I'm not sure about how other providers
> may implement this API and where they can get the required information
> from, without knowing their internals. In regard to SunJSSE and
> "tls-unique" binding type, I leveraged on existing data. If data weren't
> already there, I would have to figure out how to get it from the
> handshake -doing the same that was already done would have been an
> option-. Do you prefer the Handshaker to provide a function to get
> different information and not just the finished hash? (as for the public
> SSLSocket/SSLEngine "getTlsHandshakeMaterial" API). Which other
> information may be useful to get from the Handshaker? What do you mean
> by reliable? (given that this is all SunJSSE internal and we have no
> external dependencies).
>
Let consider the use of the API.
    byte[] getTlsChannelBinding("tls_unique");

I'm confusing when I try to use it by myself:
1. provider does not implement this method
    return null or empty?

It happens because an old provider should still work in new JDK, but old
provider does not implement new APIs, or a new provider does not support
this feature.

2. the method is called before handshaking
    return null or empty?

3. the method is called during handshaking
    return null, empty or the channel binding value?

4. the method is called at the same time the handshaking completed?
    return the channel binding value?

5. the method is called after the handshaking
    return the channel binding value?

6. the method is called during renegoitation
    return null, empty, the old binding value, or the new binding value?

7. the method is called after handshaking
    return old binding value, or the new binding value?

8. the method is called after the initial handshaking, but the binding
value is changed shortly after because of renegotiation.
    how could application use the binding value?

We need a clear define of the behavior of the method.  It could be
complicated if the method is designed as getTlsChannelBinding("tls_unique").

I feel that handshake material should be captured when
1. it is requested to capture the handshake material, and
2. the handshake material get produced.

For the getTlsChannelBinding("tls_unique") API, it is unknown:
1. Is it required to capture the handshake material?
2. Is the handshake material produced?

The two points could result in a few unexpected problems, as the above 8
items that we may want to consider.

> In regard to other channel bindings, it'll depend on the binding type
> the way in which the information is obtained. I.e.: "tls-unique" SunJSSE
> implementation leverages on cached finished messages. However,
> "tls-server-end-point" leverages on stored certificates that are
> obtained from the Session (not from the handshaker). Is there any
> specific channel binding you are concerned with?
>
>  >If the channel binding is not required, it may be not necessary to
> expose the handshake materials.  Need to define a solution to indicate
> the need of the exporting.
>
> Do you mean a lower layer knowing if the upper layer is going to require
> that information and decide to provide it or not based on that
> knowledge? I think I didn't get your point here.
>
I mean, if an application want to support channel binding, the provider
can provider the channel binding service;  If the an application does
not want channel binding, the provider should be perform the channel
binding service.  The getTlsChannelBinding() make the provider MUST
perform channel binding cache or calculation no matter application want
it or not.

>  >2. No way to know the update of the underlying handshake materials.
>  >If renegotiation can takes place, need to define a interface to
> indicate that so that application can response accordingly.  See section
> 3 and 7 of RFC 5929.
>
> I intentionally skipped this -at the cost of a spurious authentication-
> to avoid adding complexity to the API. An spurious authentication -which
> does not appear likely to me- can easily be retried by the application.
> The RFC 5929 suggests APIs through which the application can *control*
> the flow (i.e.: hold a renegotitation). This would expose JSSE
> internals. This is more than notifying. Notification, in my opinion,
> adds no value: what if the application already used the binding token
> before receiving the notification? The spurious authentication will
> happen anyways and has to be handled -i.e. retried-. It's just a timing
> issue. The real value is controlling the flow as the RFC suggests, but
> at the cost of exposing JSSE internals.
>
My understanding, the block of the protocol is to make sure application
is performing the channel binding with the right value, or updating the
value accordingly if necessary.  If you skip this and when renegotiation
happen, the channel binding could be limited, or may not work as expected.

Thanks,
Xuelei

> Kind regards,
> Martin.-
>
> On Sat, Aug 26, 2017 at 5:25 PM, Xuelei Fan <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi Marin,
>
>     Sorry for the delay.
>
>     There are a few protocols that can benefits from exporting SSL/TLS
>     handshake materials, including RFC 5929, RFC 5056, token binding and
>     TLS 1.3 itself.  Can we define a general API so as to exposing the
>     handshake materials, so as to mitigate the inflating of JSSE APIs?
>     I may suggest make further evaluation before move on to following
>     design and code.
>
>      >
>     http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>     <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
>     I have two concerns about the design:
>
>     1. Channel binding may be not always required.
>     SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);
>
>     The SunJSSE provider happens to cache the finished messages in its
>     implementation so you can use it for tls-unique, but it may not be
>     true for other provider or other channel bindings.  Need to define a
>     more reliable approach to get the handshake materials.
>
>     If the channel binding is not required, it may be not necessary to
>     expose the handshake materials.  Need to define a solution to
>     indicate the need of the exporting.
>
>     2. No way to know the update of the underlying handshake materials.
>     If renegotiation can takes place, need to define a interface to
>     indicate that so that application can response accordingly.  See
>     section 3 and 7 of RFC 5929.
>
>     Thanks,
>     Xuelei
>
>     On 7/31/2017 8:53 AM, Martin Balao wrote:
>
>         Hi,
>
>         Here it is an update for the proposed TLS Channel Bindings
>         support in OpenJDK:
>
>            *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
>         (browse online)
>            *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>
>         (download)
>
>         Changes since v01:
>
>            * getTlsChannelBinding API changed to return null by default
>         (if not implemented), instead of throwing an
>         UnsupportedOperationException.
>
>            * "tls-server-end-point" TLS channel binding now supported.
>
>         Kind regards,
>         Martin.-
>
>         On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao <[hidden email]
>         <mailto:[hidden email]> <mailto:[hidden email]
>         <mailto:[hidden email]>>> wrote:
>
>              Hi,
>
>              Here it is my proposal for JDK-6491070 (Support for RFC
>         5929-Channel
>              Bindings: e.g. public API to obtain TLS finished message) [1]:
>
>                *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>            
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>
>                *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>            
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>
>
>              Notes:
>                * Implementation based on Channel Bindings for TLS (RFC
>         5929) [2]
>
>                * Only "tls-unique" currently supported
>
>              Look forward to your comments.
>
>              Kind regards,
>              Martin.-
>
>              --
>              [1] - https://bugs.openjdk.java.net/browse/JDK-6491070
>         <https://bugs.openjdk.java.net/browse/JDK-6491070>
>              <https://bugs.openjdk.java.net/browse/JDK-6491070
>         <https://bugs.openjdk.java.net/browse/JDK-6491070>>
>              [2] - https://tools.ietf.org/html/rfc5929
>         <https://tools.ietf.org/html/rfc5929>
>              <https://tools.ietf.org/html/rfc5929
>         <https://tools.ietf.org/html/rfc5929>>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Martin Balao
Hi,

The material is already cached in the handshaker if secure renegotiation is enabled. However, I agree with you that we are going to cache the value even when secure renegotiation is not enabled, thus, wasting roughly 12 bytes (as bytes for an empty array are already consumed). In fact, the exact case -adding a few conditionals to webrev.02- is the one in which secure renegotiation is disabled and extended master secret is enabled. GnuTls and OpenSSL -including its derivatives like Boring SSL and Python (through OpenSSL)- always cache this information.

As for the empty / null cases, the API consumer was expected to ask for the binding information after the TLS connection is established. It's on the API consumer knowledge that asking for the information before (i.e.: just after creating a disconnected socket) or while the handshake is taking place, makes no sense and no valid value will be obtained (either we define this as null or empty). For those providers that do not support this feature, the information wouldn't have been available after the handshake. However, I agree with you that before the handshake is completed there is no means of knowing if the provider does support this API. My first webrev (webrev.01) was throwing an UnsupportedOperationException to make this case explicit but I had doubts regarding the real value it provides for the API consumer. The proposed API was similar to Python, SSLBoring and GnuTLs. However, handshake listener callbacks -as Bernd suggests- and the idea of just exposing the handshake material (as a lower level API) sounds good to me. I can give it a try. I propose then to bring the handshake information as part of a HandshakeCompletedEvent instance, even though the callback registrant may not consume it. Would that work for you?

In regard to the handshake material update -which I assumed unlikely-, the point in which a renegotiation may take place (from the server side) is when reading data, not when writing. That cannot be controlled by the application because it's JSSE internal and not exposed. Thus, an application may read from the socket, get the handshake material and write a message using the binding value -which we can make sure that is the valid one at that point-. However, as soon as the application reads again from the socket, a renegotiation -if requested by the client- may be processed and the binding value gets updated. The higher level protocol may fail -because the binding value was already sent but not processed on the other side- and a re-try needed. This looks independent of whether we use the originally proposed API or the handshake listener callback interface (or even a sync callback), because the underlying problem is that the application cannot really control the renegotiation flow in the lower layer (as RFC 5929 suggest). The options I see are adding more complexity to the API and let the application control the renegotiation flow or live with that and expect the application to retry.

Thanks,
Martin.-

On Tue, Aug 29, 2017 at 4:34 PM, Xuelei Fan <[hidden email]> wrote:
On 8/26/2017 2:56 PM, Bernd Eckenfels wrote:
> How about only passing it to an extended handshake listener. The
> material does not have to be cached (the app can do it if needed) and
> renegotiation works the same way. This can also be helpful for things
> like logging the master secret (for wireshark decryption). It can even
> handle auditing of session resumptions
Martin, what do you think about Bernd's proposal above and similar callback mechanism?

More comment inlines.

On 8/29/2017 11:50 AM, Martin Balao wrote:
Hi Xuelei,

 >There are a few protocols that can benefits from exporting SSL/TLS handshake materials, including RFC 5929, RFC 5056, token binding and TLS 1.3 itself.  Can we define a general API so as to exposing the handshake materials, so as to mitigate the inflating of JSSE APIs?  I may suggest make further evaluation before move on to following design and code.

Do you prefer an API like "public byte[] getTlsHandshakeMaterial(String materialType)" (in SSLSocket and SSLEngine) where "materialType" can eventually be "clientFinishedMessage"/"finishedMessage" or "serverFinishedMessage"/"peerFinishedMessage"?
The problem of the APIs like that is, when applications call the method, it is not always return the expected result, and the implementation may have to cache the message even if an application never use it.  See more in the following example.

I cannot think of "serverCertificate" or "masterKey" as this is more related to a Session and not neccessarily to a handshake. getTlsHandshakeMaterial would be a lower level API and would move the burden of knowing which information is required for "tls-unique" TLS channel binding to the API consumer. Looks more like the OpenSSL approach (instead of the Python, SSLBoring or GnuTls approaches). However, OpenSSL have specific methods for each piece of information instead of a generic and parametrized one. I.e.: SSL_get_finished or SSL_get_peer_finished. What other information do you expect the Handshaker to provide?

 >The SunJSSE provider happens to cache the finished messages in its implementation so you can use it for tls-unique, but it may not be true for other provider or other channel bindings.  Need to define a more reliable approach to get the handshake materials.

I focused on SunJSSE provider. I'm not sure about how other providers may implement this API and where they can get the required information from, without knowing their internals. In regard to SunJSSE and "tls-unique" binding type, I leveraged on existing data. If data weren't already there, I would have to figure out how to get it from the handshake -doing the same that was already done would have been an option-. Do you prefer the Handshaker to provide a function to get different information and not just the finished hash? (as for the public SSLSocket/SSLEngine "getTlsHandshakeMaterial" API). Which other information may be useful to get from the Handshaker? What do you mean by reliable? (given that this is all SunJSSE internal and we have no external dependencies).

Let consider the use of the API.
   byte[] getTlsChannelBinding("tls_unique");

I'm confusing when I try to use it by myself:
1. provider does not implement this method
   return null or empty?

It happens because an old provider should still work in new JDK, but old provider does not implement new APIs, or a new provider does not support this feature.

2. the method is called before handshaking
   return null or empty?

3. the method is called during handshaking
   return null, empty or the channel binding value?

4. the method is called at the same time the handshaking completed?
   return the channel binding value?

5. the method is called after the handshaking
   return the channel binding value?

6. the method is called during renegoitation
   return null, empty, the old binding value, or the new binding value?

7. the method is called after handshaking
   return old binding value, or the new binding value?

8. the method is called after the initial handshaking, but the binding value is changed shortly after because of renegotiation.
   how could application use the binding value?

We need a clear define of the behavior of the method.  It could be complicated if the method is designed as getTlsChannelBinding("tls_unique").

I feel that handshake material should be captured when
1. it is requested to capture the handshake material, and
2. the handshake material get produced.

For the getTlsChannelBinding("tls_unique") API, it is unknown:
1. Is it required to capture the handshake material?
2. Is the handshake material produced?

The two points could result in a few unexpected problems, as the above 8 items that we may want to consider.

In regard to other channel bindings, it'll depend on the binding type the way in which the information is obtained. I.e.: "tls-unique" SunJSSE implementation leverages on cached finished messages. However, "tls-server-end-point" leverages on stored certificates that are obtained from the Session (not from the handshaker). Is there any specific channel binding you are concerned with?

 >If the channel binding is not required, it may be not necessary to expose the handshake materials.  Need to define a solution to indicate the need of the exporting.

Do you mean a lower layer knowing if the upper layer is going to require that information and decide to provide it or not based on that knowledge? I think I didn't get your point here.

I mean, if an application want to support channel binding, the provider can provider the channel binding service;  If the an application does not want channel binding, the provider should be perform the channel binding service.  The getTlsChannelBinding() make the provider MUST perform channel binding cache or calculation no matter application want it or not.

 >2. No way to know the update of the underlying handshake materials.
 >If renegotiation can takes place, need to define a interface to indicate that so that application can response accordingly.  See section 3 and 7 of RFC 5929.

I intentionally skipped this -at the cost of a spurious authentication- to avoid adding complexity to the API. An spurious authentication -which does not appear likely to me- can easily be retried by the application. The RFC 5929 suggests APIs through which the application can *control* the flow (i.e.: hold a renegotitation). This would expose JSSE internals. This is more than notifying. Notification, in my opinion, adds no value: what if the application already used the binding token before receiving the notification? The spurious authentication will happen anyways and has to be handled -i.e. retried-. It's just a timing issue. The real value is controlling the flow as the RFC suggests, but at the cost of exposing JSSE internals.

My understanding, the block of the protocol is to make sure application is performing the channel binding with the right value, or updating the value accordingly if necessary.  If you skip this and when renegotiation happen, the channel binding could be limited, or may not work as expected.

Thanks,
Xuelei

Kind regards,
Martin.-


On Sat, Aug 26, 2017 at 5:25 PM, Xuelei Fan <[hidden email] <mailto:[hidden email]>> wrote:

    Hi Marin,

    Sorry for the delay.

    There are a few protocols that can benefits from exporting SSL/TLS
    handshake materials, including RFC 5929, RFC 5056, token binding and
    TLS 1.3 itself.  Can we define a general API so as to exposing the
    handshake materials, so as to mitigate the inflating of JSSE APIs?     I may suggest make further evaluation before move on to following
    design and code.

     >
    http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
    <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
    I have two concerns about the design:

    1. Channel binding may be not always required.
    SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);

    The SunJSSE provider happens to cache the finished messages in its
    implementation so you can use it for tls-unique, but it may not be
    true for other provider or other channel bindings.  Need to define a
    more reliable approach to get the handshake materials.

    If the channel binding is not required, it may be not necessary to
    expose the handshake materials.  Need to define a solution to
    indicate the need of the exporting.

    2. No way to know the update of the underlying handshake materials.
    If renegotiation can takes place, need to define a interface to
    indicate that so that application can response accordingly.  See
    section 3 and 7 of RFC 5929.

    Thanks,
    Xuelei

    On 7/31/2017 8:53 AM, Martin Balao wrote:

        Hi,

        Here it is an update for the proposed TLS Channel Bindings
        support in OpenJDK:

           *
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
        (browse online)
           *
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>
        (download)

        Changes since v01:

           * getTlsChannelBinding API changed to return null by default
        (if not implemented), instead of throwing an
        UnsupportedOperationException.

           * "tls-server-end-point" TLS channel binding now supported.

        Kind regards,
        Martin.-

        On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao <[hidden email]
        <mailto:[hidden email]> <mailto:[hidden email]

        <mailto:[hidden email]>>> wrote:

             Hi,

             Here it is my proposal for JDK-6491070 (Support for RFC
        5929-Channel
             Bindings: e.g. public API to obtain TLS finished message) [1]:

               *
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
                    <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>
               *
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
                    <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>

             Notes:
               * Implementation based on Channel Bindings for TLS (RFC
        5929) [2]

               * Only "tls-unique" currently supported

             Look forward to your comments.

             Kind regards,
             Martin.-

             --
             [1] - https://bugs.openjdk.java.net/browse/JDK-6491070
        <https://bugs.openjdk.java.net/browse/JDK-6491070>
             <https://bugs.openjdk.java.net/browse/JDK-6491070
        <https://bugs.openjdk.java.net/browse/JDK-6491070>>
             [2] - https://tools.ietf.org/html/rfc5929
        <https://tools.ietf.org/html/rfc5929>
             <https://tools.ietf.org/html/rfc5929
        <https://tools.ietf.org/html/rfc5929>>




Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Xuelei Fan-2
The failure-and-retry mechanism could a nightmare for some applications.
  Please think more how could we avoid it.  If need more APIs, what the
update may looks like and how complicated it could be?

If required, Bernd's proposal can be extended a little bit to support
operations other than listening.

APIs maintain is very complicated, a good design may need more time
currently, but could save much more in the future.

Thanks,
Xuelei

On 8/31/2017 11:41 AM, Martin Balao wrote:

> Hi,
>
> The material is already cached in the handshaker if secure renegotiation
> is enabled. However, I agree with you that we are going to cache the
> value even when secure renegotiation is not enabled, thus, wasting
> roughly 12 bytes (as bytes for an empty array are already consumed). In
> fact, the exact case -adding a few conditionals to webrev.02- is the one
> in which secure renegotiation is disabled and extended master secret is
> enabled. GnuTls and OpenSSL -including its derivatives like Boring SSL
> and Python (through OpenSSL)- always cache this information.
>
> As for the empty / null cases, the API consumer was expected to ask for
> the binding information after the TLS connection is established. It's on
> the API consumer knowledge that asking for the information before (i.e.:
> just after creating a disconnected socket) or while the handshake is
> taking place, makes no sense and no valid value will be obtained (either
> we define this as null or empty). For those providers that do not
> support this feature, the information wouldn't have been available after
> the handshake. However, I agree with you that before the handshake is
> completed there is no means of knowing if the provider does support this
> API. My first webrev (webrev.01) was throwing an
> UnsupportedOperationException to make this case explicit but I had
> doubts regarding the real value it provides for the API consumer. The
> proposed API was similar to Python, SSLBoring and GnuTLs. However,
> handshake listener callbacks -as Bernd suggests- and the idea of just
> exposing the handshake material (as a lower level API) sounds good to
> me. I can give it a try. I propose then to bring the handshake
> information as part of a HandshakeCompletedEvent instance, even though
> the callback registrant may not consume it. Would that work for you?
>
> In regard to the handshake material update -which I assumed unlikely-,
> the point in which a renegotiation may take place (from the server side)
> is when reading data, not when writing. That cannot be controlled by the
> application because it's JSSE internal and not exposed. Thus, an
> application may read from the socket, get the handshake material and
> write a message using the binding value -which we can make sure that is
> the valid one at that point-. However, as soon as the application reads
> again from the socket, a renegotiation -if requested by the client- may
> be processed and the binding value gets updated. The higher level
> protocol may fail -because the binding value was already sent but not
> processed on the other side- and a re-try needed. This looks independent
> of whether we use the originally proposed API or the handshake listener
> callback interface (or even a sync callback), because the underlying
> problem is that the application cannot really control the renegotiation
> flow in the lower layer (as RFC 5929 suggest). The options I see are
> adding more complexity to the API and let the application control the
> renegotiation flow or live with that and expect the application to retry.
>
> Thanks,
> Martin.-
>
> On Tue, Aug 29, 2017 at 4:34 PM, Xuelei Fan <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 8/26/2017 2:56 PM, Bernd Eckenfels wrote:
>     > How about only passing it to an extended handshake listener. The
>     > material does not have to be cached (the app can do it if needed) and
>     > renegotiation works the same way. This can also be helpful for things
>     > like logging the master secret (for wireshark decryption). It can even
>     > handle auditing of session resumptions
>     Martin, what do you think about Bernd's proposal above and similar
>     callback mechanism?
>
>     More comment inlines.
>
>     On 8/29/2017 11:50 AM, Martin Balao wrote:
>
>         Hi Xuelei,
>
>           >There are a few protocols that can benefits from exporting
>         SSL/TLS handshake materials, including RFC 5929, RFC 5056, token
>         binding and TLS 1.3 itself.  Can we define a general API so as
>         to exposing the handshake materials, so as to mitigate the
>         inflating of JSSE APIs?  I may suggest make further evaluation
>         before move on to following design and code.
>
>         Do you prefer an API like "public byte[]
>         getTlsHandshakeMaterial(String materialType)" (in SSLSocket and
>         SSLEngine) where "materialType" can eventually be
>         "clientFinishedMessage"/"finishedMessage" or
>         "serverFinishedMessage"/"peerFinishedMessage"?
>
>     The problem of the APIs like that is, when applications call the
>     method, it is not always return the expected result, and the
>     implementation may have to cache the message even if an application
>     never use it.  See more in the following example.
>
>         I cannot think of "serverCertificate" or "masterKey" as this is
>         more related to a Session and not neccessarily to a handshake.
>         getTlsHandshakeMaterial would be a lower level API and would
>         move the burden of knowing which information is required for
>         "tls-unique" TLS channel binding to the API consumer. Looks more
>         like the OpenSSL approach (instead of the Python, SSLBoring or
>         GnuTls approaches). However, OpenSSL have specific methods for
>         each piece of information instead of a generic and parametrized
>         one. I.e.: SSL_get_finished or SSL_get_peer_finished. What other
>         information do you expect the Handshaker to provide?
>
>           >The SunJSSE provider happens to cache the finished messages
>         in its implementation so you can use it for tls-unique, but it
>         may not be true for other provider or other channel bindings.
>         Need to define a more reliable approach to get the handshake
>         materials.
>
>         I focused on SunJSSE provider. I'm not sure about how other
>         providers may implement this API and where they can get the
>         required information from, without knowing their internals. In
>         regard to SunJSSE and "tls-unique" binding type, I leveraged on
>         existing data. If data weren't already there, I would have to
>         figure out how to get it from the handshake -doing the same that
>         was already done would have been an option-. Do you prefer the
>         Handshaker to provide a function to get different information
>         and not just the finished hash? (as for the public
>         SSLSocket/SSLEngine "getTlsHandshakeMaterial" API). Which other
>         information may be useful to get from the Handshaker? What do
>         you mean by reliable? (given that this is all SunJSSE internal
>         and we have no external dependencies).
>
>     Let consider the use of the API.
>         byte[] getTlsChannelBinding("tls_unique");
>
>     I'm confusing when I try to use it by myself:
>     1. provider does not implement this method
>         return null or empty?
>
>     It happens because an old provider should still work in new JDK, but
>     old provider does not implement new APIs, or a new provider does not
>     support this feature.
>
>     2. the method is called before handshaking
>         return null or empty?
>
>     3. the method is called during handshaking
>         return null, empty or the channel binding value?
>
>     4. the method is called at the same time the handshaking completed?
>         return the channel binding value?
>
>     5. the method is called after the handshaking
>         return the channel binding value?
>
>     6. the method is called during renegoitation
>         return null, empty, the old binding value, or the new binding value?
>
>     7. the method is called after handshaking
>         return old binding value, or the new binding value?
>
>     8. the method is called after the initial handshaking, but the
>     binding value is changed shortly after because of renegotiation.
>         how could application use the binding value?
>
>     We need a clear define of the behavior of the method.  It could be
>     complicated if the method is designed as
>     getTlsChannelBinding("tls_unique").
>
>     I feel that handshake material should be captured when
>     1. it is requested to capture the handshake material, and
>     2. the handshake material get produced.
>
>     For the getTlsChannelBinding("tls_unique") API, it is unknown:
>     1. Is it required to capture the handshake material?
>     2. Is the handshake material produced?
>
>     The two points could result in a few unexpected problems, as the
>     above 8 items that we may want to consider.
>
>         In regard to other channel bindings, it'll depend on the binding
>         type the way in which the information is obtained. I.e.:
>         "tls-unique" SunJSSE implementation leverages on cached finished
>         messages. However, "tls-server-end-point" leverages on stored
>         certificates that are obtained from the Session (not from the
>         handshaker). Is there any specific channel binding you are
>         concerned with?
>
>           >If the channel binding is not required, it may be not
>         necessary to expose the handshake materials.  Need to define a
>         solution to indicate the need of the exporting.
>
>         Do you mean a lower layer knowing if the upper layer is going to
>         require that information and decide to provide it or not based
>         on that knowledge? I think I didn't get your point here.
>
>     I mean, if an application want to support channel binding, the
>     provider can provider the channel binding service;  If the an
>     application does not want channel binding, the provider should be
>     perform the channel binding service.  The getTlsChannelBinding()
>     make the provider MUST perform channel binding cache or calculation
>     no matter application want it or not.
>
>           >2. No way to know the update of the underlying handshake
>         materials.
>           >If renegotiation can takes place, need to define a interface
>         to indicate that so that application can response accordingly.
>         See section 3 and 7 of RFC 5929.
>
>         I intentionally skipped this -at the cost of a spurious
>         authentication- to avoid adding complexity to the API. An
>         spurious authentication -which does not appear likely to me- can
>         easily be retried by the application. The RFC 5929 suggests APIs
>         through which the application can *control* the flow (i.e.: hold
>         a renegotitation). This would expose JSSE internals. This is
>         more than notifying. Notification, in my opinion, adds no value:
>         what if the application already used the binding token before
>         receiving the notification? The spurious authentication will
>         happen anyways and has to be handled -i.e. retried-. It's just a
>         timing issue. The real value is controlling the flow as the RFC
>         suggests, but at the cost of exposing JSSE internals.
>
>     My understanding, the block of the protocol is to make sure
>     application is performing the channel binding with the right value,
>     or updating the value accordingly if necessary.  If you skip this
>     and when renegotiation happen, the channel binding could be limited,
>     or may not work as expected.
>
>     Thanks,
>     Xuelei
>
>         Kind regards,
>         Martin.-
>
>
>         On Sat, Aug 26, 2017 at 5:25 PM, Xuelei Fan
>         <[hidden email] <mailto:[hidden email]>
>         <mailto:[hidden email] <mailto:[hidden email]>>>
>         wrote:
>
>              Hi Marin,
>
>              Sorry for the delay.
>
>              There are a few protocols that can benefits from exporting
>         SSL/TLS
>              handshake materials, including RFC 5929, RFC 5056, token
>         binding and
>              TLS 1.3 itself.  Can we define a general API so as to
>         exposing the
>              handshake materials, so as to mitigate the inflating of
>         JSSE APIs?     I may suggest make further evaluation before move
>         on to following
>              design and code.
>
>               >
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
>            
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>>
>              I have two concerns about the design:
>
>              1. Channel binding may be not always required.
>              SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);
>
>              The SunJSSE provider happens to cache the finished messages
>         in its
>              implementation so you can use it for tls-unique, but it may
>         not be
>              true for other provider or other channel bindings.  Need to
>         define a
>              more reliable approach to get the handshake materials.
>
>              If the channel binding is not required, it may be not
>         necessary to
>              expose the handshake materials.  Need to define a solution to
>              indicate the need of the exporting.
>
>              2. No way to know the update of the underlying handshake
>         materials.
>              If renegotiation can takes place, need to define a interface to
>              indicate that so that application can response
>         accordingly.  See
>              section 3 and 7 of RFC 5929.
>
>              Thanks,
>              Xuelei
>
>              On 7/31/2017 8:53 AM, Martin Balao wrote:
>
>                  Hi,
>
>                  Here it is an update for the proposed TLS Channel Bindings
>                  support in OpenJDK:
>
>                     *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
>                
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>>
>                  (browse online)
>                     *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>
>                
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>>
>                  (download)
>
>                  Changes since v01:
>
>                     * getTlsChannelBinding API changed to return null by
>         default
>                  (if not implemented), instead of throwing an
>                  UnsupportedOperationException.
>
>                     * "tls-server-end-point" TLS channel binding now
>         supported.
>
>                  Kind regards,
>                  Martin.-
>
>                  On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao
>         <[hidden email] <mailto:[hidden email]>
>                  <mailto:[hidden email] <mailto:[hidden email]>>
>         <mailto:[hidden email] <mailto:[hidden email]>
>
>                  <mailto:[hidden email] <mailto:[hidden email]>>>>
>         wrote:
>
>                       Hi,
>
>                       Here it is my proposal for JDK-6491070 (Support
>         for RFC
>                  5929-Channel
>                       Bindings: e.g. public API to obtain TLS finished
>         message) [1]:
>
>                         *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>                
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>
>                            
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>                
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>>
>                         *
>         http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>                
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>
>                            
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>                
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>         <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>>
>
>                       Notes:
>                         * Implementation based on Channel Bindings for
>         TLS (RFC
>                  5929) [2]
>
>                         * Only "tls-unique" currently supported
>
>                       Look forward to your comments.
>
>                       Kind regards,
>                       Martin.-
>
>                       --
>                       [1] -
>         https://bugs.openjdk.java.net/browse/JDK-6491070
>         <https://bugs.openjdk.java.net/browse/JDK-6491070>
>                  <https://bugs.openjdk.java.net/browse/JDK-6491070
>         <https://bugs.openjdk.java.net/browse/JDK-6491070>>
>                       <https://bugs.openjdk.java.net/browse/JDK-6491070
>         <https://bugs.openjdk.java.net/browse/JDK-6491070>
>                  <https://bugs.openjdk.java.net/browse/JDK-6491070
>         <https://bugs.openjdk.java.net/browse/JDK-6491070>>>
>                       [2] - https://tools.ietf.org/html/rfc5929
>         <https://tools.ietf.org/html/rfc5929>
>                  <https://tools.ietf.org/html/rfc5929
>         <https://tools.ietf.org/html/rfc5929>>
>                       <https://tools.ietf.org/html/rfc5929
>         <https://tools.ietf.org/html/rfc5929>
>                  <https://tools.ietf.org/html/rfc5929
>         <https://tools.ietf.org/html/rfc5929>>>
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

David M. Lloyd-3
I can say that from the perspective of SASL that we don't need any
special indication about handshake, and it would be much easier to
just read the material off of the engine, socket, or session as
appropriate.

On Thu, Aug 31, 2017 at 8:32 PM, Xuelei Fan <[hidden email]> wrote:

> The failure-and-retry mechanism could a nightmare for some applications.
> Please think more how could we avoid it.  If need more APIs, what the update
> may looks like and how complicated it could be?
>
> If required, Bernd's proposal can be extended a little bit to support
> operations other than listening.
>
> APIs maintain is very complicated, a good design may need more time
> currently, but could save much more in the future.
>
> Thanks,
> Xuelei
>
> On 8/31/2017 11:41 AM, Martin Balao wrote:
>>
>> Hi,
>>
>> The material is already cached in the handshaker if secure renegotiation
>> is enabled. However, I agree with you that we are going to cache the value
>> even when secure renegotiation is not enabled, thus, wasting roughly 12
>> bytes (as bytes for an empty array are already consumed). In fact, the exact
>> case -adding a few conditionals to webrev.02- is the one in which secure
>> renegotiation is disabled and extended master secret is enabled. GnuTls and
>> OpenSSL -including its derivatives like Boring SSL and Python (through
>> OpenSSL)- always cache this information.
>>
>> As for the empty / null cases, the API consumer was expected to ask for
>> the binding information after the TLS connection is established. It's on the
>> API consumer knowledge that asking for the information before (i.e.: just
>> after creating a disconnected socket) or while the handshake is taking
>> place, makes no sense and no valid value will be obtained (either we define
>> this as null or empty). For those providers that do not support this
>> feature, the information wouldn't have been available after the handshake.
>> However, I agree with you that before the handshake is completed there is no
>> means of knowing if the provider does support this API. My first webrev
>> (webrev.01) was throwing an UnsupportedOperationException to make this case
>> explicit but I had doubts regarding the real value it provides for the API
>> consumer. The proposed API was similar to Python, SSLBoring and GnuTLs.
>> However, handshake listener callbacks -as Bernd suggests- and the idea of
>> just exposing the handshake material (as a lower level API) sounds good to
>> me. I can give it a try. I propose then to bring the handshake information
>> as part of a HandshakeCompletedEvent instance, even though the callback
>> registrant may not consume it. Would that work for you?
>>
>> In regard to the handshake material update -which I assumed unlikely-, the
>> point in which a renegotiation may take place (from the server side) is when
>> reading data, not when writing. That cannot be controlled by the application
>> because it's JSSE internal and not exposed. Thus, an application may read
>> from the socket, get the handshake material and write a message using the
>> binding value -which we can make sure that is the valid one at that point-.
>> However, as soon as the application reads again from the socket, a
>> renegotiation -if requested by the client- may be processed and the binding
>> value gets updated. The higher level protocol may fail -because the binding
>> value was already sent but not processed on the other side- and a re-try
>> needed. This looks independent of whether we use the originally proposed API
>> or the handshake listener callback interface (or even a sync callback),
>> because the underlying problem is that the application cannot really control
>> the renegotiation flow in the lower layer (as RFC 5929 suggest). The options
>> I see are adding more complexity to the API and let the application control
>> the renegotiation flow or live with that and expect the application to
>> retry.
>>
>> Thanks,
>> Martin.-
>>
>> On Tue, Aug 29, 2017 at 4:34 PM, Xuelei Fan <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>>     On 8/26/2017 2:56 PM, Bernd Eckenfels wrote:
>>     > How about only passing it to an extended handshake listener. The
>>     > material does not have to be cached (the app can do it if needed)
>> and
>>     > renegotiation works the same way. This can also be helpful for
>> things
>>     > like logging the master secret (for wireshark decryption). It can
>> even
>>     > handle auditing of session resumptions
>>     Martin, what do you think about Bernd's proposal above and similar
>>     callback mechanism?
>>
>>     More comment inlines.
>>
>>     On 8/29/2017 11:50 AM, Martin Balao wrote:
>>
>>         Hi Xuelei,
>>
>>           >There are a few protocols that can benefits from exporting
>>         SSL/TLS handshake materials, including RFC 5929, RFC 5056, token
>>         binding and TLS 1.3 itself.  Can we define a general API so as
>>         to exposing the handshake materials, so as to mitigate the
>>         inflating of JSSE APIs?  I may suggest make further evaluation
>>         before move on to following design and code.
>>
>>         Do you prefer an API like "public byte[]
>>         getTlsHandshakeMaterial(String materialType)" (in SSLSocket and
>>         SSLEngine) where "materialType" can eventually be
>>         "clientFinishedMessage"/"finishedMessage" or
>>         "serverFinishedMessage"/"peerFinishedMessage"?
>>
>>     The problem of the APIs like that is, when applications call the
>>     method, it is not always return the expected result, and the
>>     implementation may have to cache the message even if an application
>>     never use it.  See more in the following example.
>>
>>         I cannot think of "serverCertificate" or "masterKey" as this is
>>         more related to a Session and not neccessarily to a handshake.
>>         getTlsHandshakeMaterial would be a lower level API and would
>>         move the burden of knowing which information is required for
>>         "tls-unique" TLS channel binding to the API consumer. Looks more
>>         like the OpenSSL approach (instead of the Python, SSLBoring or
>>         GnuTls approaches). However, OpenSSL have specific methods for
>>         each piece of information instead of a generic and parametrized
>>         one. I.e.: SSL_get_finished or SSL_get_peer_finished. What other
>>         information do you expect the Handshaker to provide?
>>
>>           >The SunJSSE provider happens to cache the finished messages
>>         in its implementation so you can use it for tls-unique, but it
>>         may not be true for other provider or other channel bindings.
>> Need to define a more reliable approach to get the handshake
>>         materials.
>>
>>         I focused on SunJSSE provider. I'm not sure about how other
>>         providers may implement this API and where they can get the
>>         required information from, without knowing their internals. In
>>         regard to SunJSSE and "tls-unique" binding type, I leveraged on
>>         existing data. If data weren't already there, I would have to
>>         figure out how to get it from the handshake -doing the same that
>>         was already done would have been an option-. Do you prefer the
>>         Handshaker to provide a function to get different information
>>         and not just the finished hash? (as for the public
>>         SSLSocket/SSLEngine "getTlsHandshakeMaterial" API). Which other
>>         information may be useful to get from the Handshaker? What do
>>         you mean by reliable? (given that this is all SunJSSE internal
>>         and we have no external dependencies).
>>
>>     Let consider the use of the API.
>>         byte[] getTlsChannelBinding("tls_unique");
>>
>>     I'm confusing when I try to use it by myself:
>>     1. provider does not implement this method
>>         return null or empty?
>>
>>     It happens because an old provider should still work in new JDK, but
>>     old provider does not implement new APIs, or a new provider does not
>>     support this feature.
>>
>>     2. the method is called before handshaking
>>         return null or empty?
>>
>>     3. the method is called during handshaking
>>         return null, empty or the channel binding value?
>>
>>     4. the method is called at the same time the handshaking completed?
>>         return the channel binding value?
>>
>>     5. the method is called after the handshaking
>>         return the channel binding value?
>>
>>     6. the method is called during renegoitation
>>         return null, empty, the old binding value, or the new binding
>> value?
>>
>>     7. the method is called after handshaking
>>         return old binding value, or the new binding value?
>>
>>     8. the method is called after the initial handshaking, but the
>>     binding value is changed shortly after because of renegotiation.
>>         how could application use the binding value?
>>
>>     We need a clear define of the behavior of the method.  It could be
>>     complicated if the method is designed as
>>     getTlsChannelBinding("tls_unique").
>>
>>     I feel that handshake material should be captured when
>>     1. it is requested to capture the handshake material, and
>>     2. the handshake material get produced.
>>
>>     For the getTlsChannelBinding("tls_unique") API, it is unknown:
>>     1. Is it required to capture the handshake material?
>>     2. Is the handshake material produced?
>>
>>     The two points could result in a few unexpected problems, as the
>>     above 8 items that we may want to consider.
>>
>>         In regard to other channel bindings, it'll depend on the binding
>>         type the way in which the information is obtained. I.e.:
>>         "tls-unique" SunJSSE implementation leverages on cached finished
>>         messages. However, "tls-server-end-point" leverages on stored
>>         certificates that are obtained from the Session (not from the
>>         handshaker). Is there any specific channel binding you are
>>         concerned with?
>>
>>           >If the channel binding is not required, it may be not
>>         necessary to expose the handshake materials.  Need to define a
>>         solution to indicate the need of the exporting.
>>
>>         Do you mean a lower layer knowing if the upper layer is going to
>>         require that information and decide to provide it or not based
>>         on that knowledge? I think I didn't get your point here.
>>
>>     I mean, if an application want to support channel binding, the
>>     provider can provider the channel binding service;  If the an
>>     application does not want channel binding, the provider should be
>>     perform the channel binding service.  The getTlsChannelBinding()
>>     make the provider MUST perform channel binding cache or calculation
>>     no matter application want it or not.
>>
>>           >2. No way to know the update of the underlying handshake
>>         materials.
>>           >If renegotiation can takes place, need to define a interface
>>         to indicate that so that application can response accordingly.
>> See section 3 and 7 of RFC 5929.
>>
>>         I intentionally skipped this -at the cost of a spurious
>>         authentication- to avoid adding complexity to the API. An
>>         spurious authentication -which does not appear likely to me- can
>>         easily be retried by the application. The RFC 5929 suggests APIs
>>         through which the application can *control* the flow (i.e.: hold
>>         a renegotitation). This would expose JSSE internals. This is
>>         more than notifying. Notification, in my opinion, adds no value:
>>         what if the application already used the binding token before
>>         receiving the notification? The spurious authentication will
>>         happen anyways and has to be handled -i.e. retried-. It's just a
>>         timing issue. The real value is controlling the flow as the RFC
>>         suggests, but at the cost of exposing JSSE internals.
>>
>>     My understanding, the block of the protocol is to make sure
>>     application is performing the channel binding with the right value,
>>     or updating the value accordingly if necessary.  If you skip this
>>     and when renegotiation happen, the channel binding could be limited,
>>     or may not work as expected.
>>
>>     Thanks,
>>     Xuelei
>>
>>         Kind regards,
>>         Martin.-
>>
>>
>>         On Sat, Aug 26, 2017 at 5:25 PM, Xuelei Fan
>>         <[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>>
>>
>>         wrote:
>>
>>              Hi Marin,
>>
>>              Sorry for the delay.
>>
>>              There are a few protocols that can benefits from exporting
>>         SSL/TLS
>>              handshake materials, including RFC 5929, RFC 5056, token
>>         binding and
>>              TLS 1.3 itself.  Can we define a general API so as to
>>         exposing the
>>              handshake materials, so as to mitigate the inflating of
>>         JSSE APIs?     I may suggest make further evaluation before move
>>         on to following
>>              design and code.
>>
>>               >
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>>
>>              I have two concerns about the design:
>>
>>              1. Channel binding may be not always required.
>>              SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);
>>
>>              The SunJSSE provider happens to cache the finished messages
>>         in its
>>              implementation so you can use it for tls-unique, but it may
>>         not be
>>              true for other provider or other channel bindings.  Need to
>>         define a
>>              more reliable approach to get the handshake materials.
>>
>>              If the channel binding is not required, it may be not
>>         necessary to
>>              expose the handshake materials.  Need to define a solution to
>>              indicate the need of the exporting.
>>
>>              2. No way to know the update of the underlying handshake
>>         materials.
>>              If renegotiation can takes place, need to define a interface
>> to
>>              indicate that so that application can response
>>         accordingly.  See
>>              section 3 and 7 of RFC 5929.
>>
>>              Thanks,
>>              Xuelei
>>
>>              On 7/31/2017 8:53 AM, Martin Balao wrote:
>>
>>                  Hi,
>>
>>                  Here it is an update for the proposed TLS Channel
>> Bindings
>>                  support in OpenJDK:
>>
>>                     *
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>>
>>                  (browse online)
>>                     *
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>>
>>                  (download)
>>
>>                  Changes since v01:
>>
>>                     * getTlsChannelBinding API changed to return null by
>>         default
>>                  (if not implemented), instead of throwing an
>>                  UnsupportedOperationException.
>>
>>                     * "tls-server-end-point" TLS channel binding now
>>         supported.
>>
>>                  Kind regards,
>>                  Martin.-
>>
>>                  On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao
>>         <[hidden email] <mailto:[hidden email]>
>>                  <mailto:[hidden email] <mailto:[hidden email]>>
>>         <mailto:[hidden email] <mailto:[hidden email]>
>>
>>                  <mailto:[hidden email] <mailto:[hidden email]>>>>
>>
>>         wrote:
>>
>>                       Hi,
>>
>>                       Here it is my proposal for JDK-6491070 (Support
>>         for RFC
>>                  5929-Channel
>>                       Bindings: e.g. public API to obtain TLS finished
>>         message) [1]:
>>
>>                         *
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>>
>>                         *
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>>
>>
>>                       Notes:
>>                         * Implementation based on Channel Bindings for
>>         TLS (RFC
>>                  5929) [2]
>>
>>                         * Only "tls-unique" currently supported
>>
>>                       Look forward to your comments.
>>
>>                       Kind regards,
>>                       Martin.-
>>
>>                       --
>>                       [1] -
>>         https://bugs.openjdk.java.net/browse/JDK-6491070
>>         <https://bugs.openjdk.java.net/browse/JDK-6491070>
>>                  <https://bugs.openjdk.java.net/browse/JDK-6491070
>>         <https://bugs.openjdk.java.net/browse/JDK-6491070>>
>>                       <https://bugs.openjdk.java.net/browse/JDK-6491070
>>         <https://bugs.openjdk.java.net/browse/JDK-6491070>
>>                  <https://bugs.openjdk.java.net/browse/JDK-6491070
>>         <https://bugs.openjdk.java.net/browse/JDK-6491070>>>
>>                       [2] - https://tools.ietf.org/html/rfc5929
>>         <https://tools.ietf.org/html/rfc5929>
>>                  <https://tools.ietf.org/html/rfc5929
>>         <https://tools.ietf.org/html/rfc5929>>
>>                       <https://tools.ietf.org/html/rfc5929
>>         <https://tools.ietf.org/html/rfc5929>
>>                  <https://tools.ietf.org/html/rfc5929
>>         <https://tools.ietf.org/html/rfc5929>>>
>>
>>
>>
>>
>



--
- DML
Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Martin Balao
Hi,

Renegotiation requirements:

 * As a client:

  * Do not proceed upon a received HelloRequest message 
   * If a server sends a HelloRequest message, the client will notice it only when reading from the socket/engine. Because a HelloRequest message is a handshake protocol message (Record.ct_handshake), the client will call initHandshaker (SSLSocketImpl.java / SSLEngineImpl.java). This will trigger a renegotiation, initializating a new Handshaker and proceeding. The proposal would be to check if any "handshake renegotiation event" callback was registered and, if any, call it to let the API client decide how to proceed (instead of just initializating a Handshaker and moving forward).

  * Do not send ClientHello messages
   * Once a connection is established (i.e.: connectionState == cs_DATA), the client does not initializate a new Handshaker by its own initiative.

 * As a server:

  * Do not send HelloRequest messages
   * Once a connection is established (i.e.: connectionState == cs_DATA), the server does not initializate a new Handshaker by its own initiative.

  * Do not handle ClientHello messages
   * This case is analogous to the case in which a Client receives a HelloRequest from a server. The proposal would be the same.

The interface for a SSLSocket/SSLEngine API client to have control over renegotiations would be registering a callback. This callback will be synchronously called when the counterpart wants a renegotiation, and a true/false result is expected to decide whether to continue the renegotiation or dismiss it. 

In regard to the callback interface, "addHandshakeCompletedListener" cannot be used because it's for async notifications. The API client cannot stop the renegotiation flow unless a synch call is done. I've noticed that "setHandshakeApplicationProtocolSelector" method was added with a similar purpose than ours. 

So, I see two options:

 * Add an ad-hoc set/get callback method for handshake renegotiations

 * Add a generic interface for handshake event callbacks (synchronously called). It's unfortunate that "setHandshakeApplicationProtocolSelector" refactoring would require a hard API change at this point and thus unlikely. But we can prevent future method explosion from now on. The downside of this approach is that we would need to deal with callbacks that have different parameter and return types.

Does this work for you?

Kind regards,
Martin.-

PS: sorry for the delay.

On Fri, Sep 1, 2017 at 10:08 AM, David Lloyd <[hidden email]> wrote:
I can say that from the perspective of SASL that we don't need any
special indication about handshake, and it would be much easier to
just read the material off of the engine, socket, or session as
appropriate.

On Thu, Aug 31, 2017 at 8:32 PM, Xuelei Fan <[hidden email]> wrote:
> The failure-and-retry mechanism could a nightmare for some applications.
> Please think more how could we avoid it.  If need more APIs, what the update
> may looks like and how complicated it could be?
>
> If required, Bernd's proposal can be extended a little bit to support
> operations other than listening.
>
> APIs maintain is very complicated, a good design may need more time
> currently, but could save much more in the future.
>
> Thanks,
> Xuelei
>
> On 8/31/2017 11:41 AM, Martin Balao wrote:
>>
>> Hi,
>>
>> The material is already cached in the handshaker if secure renegotiation
>> is enabled. However, I agree with you that we are going to cache the value
>> even when secure renegotiation is not enabled, thus, wasting roughly 12
>> bytes (as bytes for an empty array are already consumed). In fact, the exact
>> case -adding a few conditionals to webrev.02- is the one in which secure
>> renegotiation is disabled and extended master secret is enabled. GnuTls and
>> OpenSSL -including its derivatives like Boring SSL and Python (through
>> OpenSSL)- always cache this information.
>>
>> As for the empty / null cases, the API consumer was expected to ask for
>> the binding information after the TLS connection is established. It's on the
>> API consumer knowledge that asking for the information before (i.e.: just
>> after creating a disconnected socket) or while the handshake is taking
>> place, makes no sense and no valid value will be obtained (either we define
>> this as null or empty). For those providers that do not support this
>> feature, the information wouldn't have been available after the handshake.
>> However, I agree with you that before the handshake is completed there is no
>> means of knowing if the provider does support this API. My first webrev
>> (webrev.01) was throwing an UnsupportedOperationException to make this case
>> explicit but I had doubts regarding the real value it provides for the API
>> consumer. The proposed API was similar to Python, SSLBoring and GnuTLs.
>> However, handshake listener callbacks -as Bernd suggests- and the idea of
>> just exposing the handshake material (as a lower level API) sounds good to
>> me. I can give it a try. I propose then to bring the handshake information
>> as part of a HandshakeCompletedEvent instance, even though the callback
>> registrant may not consume it. Would that work for you?
>>
>> In regard to the handshake material update -which I assumed unlikely-, the
>> point in which a renegotiation may take place (from the server side) is when
>> reading data, not when writing. That cannot be controlled by the application
>> because it's JSSE internal and not exposed. Thus, an application may read
>> from the socket, get the handshake material and write a message using the
>> binding value -which we can make sure that is the valid one at that point-.
>> However, as soon as the application reads again from the socket, a
>> renegotiation -if requested by the client- may be processed and the binding
>> value gets updated. The higher level protocol may fail -because the binding
>> value was already sent but not processed on the other side- and a re-try
>> needed. This looks independent of whether we use the originally proposed API
>> or the handshake listener callback interface (or even a sync callback),
>> because the underlying problem is that the application cannot really control
>> the renegotiation flow in the lower layer (as RFC 5929 suggest). The options
>> I see are adding more complexity to the API and let the application control
>> the renegotiation flow or live with that and expect the application to
>> retry.
>>
>> Thanks,
>> Martin.-
>>
>> On Tue, Aug 29, 2017 at 4:34 PM, Xuelei Fan <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>>     On 8/26/2017 2:56 PM, Bernd Eckenfels wrote:
>>     > How about only passing it to an extended handshake listener. The
>>     > material does not have to be cached (the app can do it if needed)
>> and
>>     > renegotiation works the same way. This can also be helpful for
>> things
>>     > like logging the master secret (for wireshark decryption). It can
>> even
>>     > handle auditing of session resumptions
>>     Martin, what do you think about Bernd's proposal above and similar
>>     callback mechanism?
>>
>>     More comment inlines.
>>
>>     On 8/29/2017 11:50 AM, Martin Balao wrote:
>>
>>         Hi Xuelei,
>>
>>           >There are a few protocols that can benefits from exporting
>>         SSL/TLS handshake materials, including RFC 5929, RFC 5056, token
>>         binding and TLS 1.3 itself.  Can we define a general API so as
>>         to exposing the handshake materials, so as to mitigate the
>>         inflating of JSSE APIs?  I may suggest make further evaluation
>>         before move on to following design and code.
>>
>>         Do you prefer an API like "public byte[]
>>         getTlsHandshakeMaterial(String materialType)" (in SSLSocket and
>>         SSLEngine) where "materialType" can eventually be
>>         "clientFinishedMessage"/"finishedMessage" or
>>         "serverFinishedMessage"/"peerFinishedMessage"?
>>
>>     The problem of the APIs like that is, when applications call the
>>     method, it is not always return the expected result, and the
>>     implementation may have to cache the message even if an application
>>     never use it.  See more in the following example.
>>
>>         I cannot think of "serverCertificate" or "masterKey" as this is
>>         more related to a Session and not neccessarily to a handshake.
>>         getTlsHandshakeMaterial would be a lower level API and would
>>         move the burden of knowing which information is required for
>>         "tls-unique" TLS channel binding to the API consumer. Looks more
>>         like the OpenSSL approach (instead of the Python, SSLBoring or
>>         GnuTls approaches). However, OpenSSL have specific methods for
>>         each piece of information instead of a generic and parametrized
>>         one. I.e.: SSL_get_finished or SSL_get_peer_finished. What other
>>         information do you expect the Handshaker to provide?
>>
>>           >The SunJSSE provider happens to cache the finished messages
>>         in its implementation so you can use it for tls-unique, but it
>>         may not be true for other provider or other channel bindings.
>> Need to define a more reliable approach to get the handshake
>>         materials.
>>
>>         I focused on SunJSSE provider. I'm not sure about how other
>>         providers may implement this API and where they can get the
>>         required information from, without knowing their internals. In
>>         regard to SunJSSE and "tls-unique" binding type, I leveraged on
>>         existing data. If data weren't already there, I would have to
>>         figure out how to get it from the handshake -doing the same that
>>         was already done would have been an option-. Do you prefer the
>>         Handshaker to provide a function to get different information
>>         and not just the finished hash? (as for the public
>>         SSLSocket/SSLEngine "getTlsHandshakeMaterial" API). Which other
>>         information may be useful to get from the Handshaker? What do
>>         you mean by reliable? (given that this is all SunJSSE internal
>>         and we have no external dependencies).
>>
>>     Let consider the use of the API.
>>         byte[] getTlsChannelBinding("tls_unique");
>>
>>     I'm confusing when I try to use it by myself:
>>     1. provider does not implement this method
>>         return null or empty?
>>
>>     It happens because an old provider should still work in new JDK, but
>>     old provider does not implement new APIs, or a new provider does not
>>     support this feature.
>>
>>     2. the method is called before handshaking
>>         return null or empty?
>>
>>     3. the method is called during handshaking
>>         return null, empty or the channel binding value?
>>
>>     4. the method is called at the same time the handshaking completed?
>>         return the channel binding value?
>>
>>     5. the method is called after the handshaking
>>         return the channel binding value?
>>
>>     6. the method is called during renegoitation
>>         return null, empty, the old binding value, or the new binding
>> value?
>>
>>     7. the method is called after handshaking
>>         return old binding value, or the new binding value?
>>
>>     8. the method is called after the initial handshaking, but the
>>     binding value is changed shortly after because of renegotiation.
>>         how could application use the binding value?
>>
>>     We need a clear define of the behavior of the method.  It could be
>>     complicated if the method is designed as
>>     getTlsChannelBinding("tls_unique").
>>
>>     I feel that handshake material should be captured when
>>     1. it is requested to capture the handshake material, and
>>     2. the handshake material get produced.
>>
>>     For the getTlsChannelBinding("tls_unique") API, it is unknown:
>>     1. Is it required to capture the handshake material?
>>     2. Is the handshake material produced?
>>
>>     The two points could result in a few unexpected problems, as the
>>     above 8 items that we may want to consider.
>>
>>         In regard to other channel bindings, it'll depend on the binding
>>         type the way in which the information is obtained. I.e.:
>>         "tls-unique" SunJSSE implementation leverages on cached finished
>>         messages. However, "tls-server-end-point" leverages on stored
>>         certificates that are obtained from the Session (not from the
>>         handshaker). Is there any specific channel binding you are
>>         concerned with?
>>
>>           >If the channel binding is not required, it may be not
>>         necessary to expose the handshake materials.  Need to define a
>>         solution to indicate the need of the exporting.
>>
>>         Do you mean a lower layer knowing if the upper layer is going to
>>         require that information and decide to provide it or not based
>>         on that knowledge? I think I didn't get your point here.
>>
>>     I mean, if an application want to support channel binding, the
>>     provider can provider the channel binding service;  If the an
>>     application does not want channel binding, the provider should be
>>     perform the channel binding service.  The getTlsChannelBinding()
>>     make the provider MUST perform channel binding cache or calculation
>>     no matter application want it or not.
>>
>>           >2. No way to know the update of the underlying handshake
>>         materials.
>>           >If renegotiation can takes place, need to define a interface
>>         to indicate that so that application can response accordingly.
>> See section 3 and 7 of RFC 5929.
>>
>>         I intentionally skipped this -at the cost of a spurious
>>         authentication- to avoid adding complexity to the API. An
>>         spurious authentication -which does not appear likely to me- can
>>         easily be retried by the application. The RFC 5929 suggests APIs
>>         through which the application can *control* the flow (i.e.: hold
>>         a renegotitation). This would expose JSSE internals. This is
>>         more than notifying. Notification, in my opinion, adds no value:
>>         what if the application already used the binding token before
>>         receiving the notification? The spurious authentication will
>>         happen anyways and has to be handled -i.e. retried-. It's just a
>>         timing issue. The real value is controlling the flow as the RFC
>>         suggests, but at the cost of exposing JSSE internals.
>>
>>     My understanding, the block of the protocol is to make sure
>>     application is performing the channel binding with the right value,
>>     or updating the value accordingly if necessary.  If you skip this
>>     and when renegotiation happen, the channel binding could be limited,
>>     or may not work as expected.
>>
>>     Thanks,
>>     Xuelei
>>
>>         Kind regards,
>>         Martin.-
>>
>>
>>         On Sat, Aug 26, 2017 at 5:25 PM, Xuelei Fan
>>         <[hidden email] <mailto:[hidden email]>
>>         <mailto:[hidden email] <mailto:[hidden email]>>>
>>
>>         wrote:
>>
>>              Hi Marin,
>>
>>              Sorry for the delay.
>>
>>              There are a few protocols that can benefits from exporting
>>         SSL/TLS
>>              handshake materials, including RFC 5929, RFC 5056, token
>>         binding and
>>              TLS 1.3 itself.  Can we define a general API so as to
>>         exposing the
>>              handshake materials, so as to mitigate the inflating of
>>         JSSE APIs?     I may suggest make further evaluation before move
>>         on to following
>>              design and code.
>>
>>               >
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>>
>>              I have two concerns about the design:
>>
>>              1. Channel binding may be not always required.
>>              SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);
>>
>>              The SunJSSE provider happens to cache the finished messages
>>         in its
>>              implementation so you can use it for tls-unique, but it may
>>         not be
>>              true for other provider or other channel bindings.  Need to
>>         define a
>>              more reliable approach to get the handshake materials.
>>
>>              If the channel binding is not required, it may be not
>>         necessary to
>>              expose the handshake materials.  Need to define a solution to
>>              indicate the need of the exporting.
>>
>>              2. No way to know the update of the underlying handshake
>>         materials.
>>              If renegotiation can takes place, need to define a interface
>> to
>>              indicate that so that application can response
>>         accordingly.  See
>>              section 3 and 7 of RFC 5929.
>>
>>              Thanks,
>>              Xuelei
>>
>>              On 7/31/2017 8:53 AM, Martin Balao wrote:
>>
>>                  Hi,
>>
>>                  Here it is an update for the proposed TLS Channel
>> Bindings
>>                  support in OpenJDK:
>>
>>                     *
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>>
>>                  (browse online)
>>                     *
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>>
>>                  (download)
>>
>>                  Changes since v01:
>>
>>                     * getTlsChannelBinding API changed to return null by
>>         default
>>                  (if not implemented), instead of throwing an
>>                  UnsupportedOperationException.
>>
>>                     * "tls-server-end-point" TLS channel binding now
>>         supported.
>>
>>                  Kind regards,
>>                  Martin.-
>>
>>                  On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao
>>         <[hidden email] <mailto:[hidden email]>
>>                  <mailto:[hidden email] <mailto:[hidden email]>>
>>         <mailto:[hidden email] <mailto:[hidden email]>
>>
>>                  <mailto:[hidden email] <mailto:[hidden email]>>>>
>>
>>         wrote:
>>
>>                       Hi,
>>
>>                       Here it is my proposal for JDK-6491070 (Support
>>         for RFC
>>                  5929-Channel
>>                       Bindings: e.g. public API to obtain TLS finished
>>         message) [1]:
>>
>>                         *
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>>
>>                         *
>>
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
>>
>> <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>>
>>
>>                       Notes:
>>                         * Implementation based on Channel Bindings for
>>         TLS (RFC
>>                  5929) [2]
>>
>>                         * Only "tls-unique" currently supported
>>
>>                       Look forward to your comments.
>>
>>                       Kind regards,
>>                       Martin.-
>>
>>                       --
>>                       [1] -
>>         https://bugs.openjdk.java.net/browse/JDK-6491070
>>         <https://bugs.openjdk.java.net/browse/JDK-6491070>
>>                  <https://bugs.openjdk.java.net/browse/JDK-6491070
>>         <https://bugs.openjdk.java.net/browse/JDK-6491070>>
>>                       <https://bugs.openjdk.java.net/browse/JDK-6491070
>>         <https://bugs.openjdk.java.net/browse/JDK-6491070>
>>                  <https://bugs.openjdk.java.net/browse/JDK-6491070
>>         <https://bugs.openjdk.java.net/browse/JDK-6491070>>>
>>                       [2] - https://tools.ietf.org/html/rfc5929
>>         <https://tools.ietf.org/html/rfc5929>
>>                  <https://tools.ietf.org/html/rfc5929
>>         <https://tools.ietf.org/html/rfc5929>>
>>                       <https://tools.ietf.org/html/rfc5929
>>         <https://tools.ietf.org/html/rfc5929>
>>                  <https://tools.ietf.org/html/rfc5929
>>         <https://tools.ietf.org/html/rfc5929>>>
>>
>>
>>
>>
>



--
- DML

Reply | Threaded
Open this post in threaded view
|

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

Martin Balao
In reply to this post by Xuelei Fan-2
Hi Xuelei,

I would like to propose Webrev 03:


What's new in Webrev 03:

 * New interface to expose Finished messages verify data handshake material (from both client and server) through a HandshakeListener. It's now up to the API client to figure out which is the tls-unique binding value from that data. TlsChannelBindingTest.java is an example of how to use the proposed API.

 * The new API allows disabling renegotiations (TLS handshake protocol).

 * Added HandshakeCompletedListener to SSLEngine as in SSLSocket. It's necessary to be able to get the server certificate used on a handshake for the certificate binding value.

 * HandshakeListener and HandshakeCompletedListener could be merged into HandshakeCompletedListener, but I think the name "Completed" does not fit -particularly for the callback that gets rid of renegotiations-. Obviously renaming HandshakeCompletedListener to HandshakeListener would be a major and breaking API change.

 * TlsChannelBindingTest.java updated to the new API.

I've done testing running the following test categories and didn't notice any regression:

 * javax/net/ssl
 * sun/security/ssl

Look forward to your comments.

Kind regards,
Martin.-

On Thu, Aug 31, 2017 at 10:32 PM, Xuelei Fan <[hidden email]> wrote:
The failure-and-retry mechanism could a nightmare for some applications.  Please think more how could we avoid it.  If need more APIs, what the update may looks like and how complicated it could be?

If required, Bernd's proposal can be extended a little bit to support operations other than listening.

APIs maintain is very complicated, a good design may need more time currently, but could save much more in the future.

Thanks,
Xuelei

On 8/31/2017 11:41 AM, Martin Balao wrote:
Hi,

The material is already cached in the handshaker if secure renegotiation is enabled. However, I agree with you that we are going to cache the value even when secure renegotiation is not enabled, thus, wasting roughly 12 bytes (as bytes for an empty array are already consumed). In fact, the exact case -adding a few conditionals to webrev.02- is the one in which secure renegotiation is disabled and extended master secret is enabled. GnuTls and OpenSSL -including its derivatives like Boring SSL and Python (through OpenSSL)- always cache this information.

As for the empty / null cases, the API consumer was expected to ask for the binding information after the TLS connection is established. It's on the API consumer knowledge that asking for the information before (i.e.: just after creating a disconnected socket) or while the handshake is taking place, makes no sense and no valid value will be obtained (either we define this as null or empty). For those providers that do not support this feature, the information wouldn't have been available after the handshake. However, I agree with you that before the handshake is completed there is no means of knowing if the provider does support this API. My first webrev (webrev.01) was throwing an UnsupportedOperationException to make this case explicit but I had doubts regarding the real value it provides for the API consumer. The proposed API was similar to Python, SSLBoring and GnuTLs. However, handshake listener callbacks -as Bernd suggests- and the idea of just exposing the handshake material (as a lower level API) sounds good to me. I can give it a try. I propose then to bring the handshake information as part of a HandshakeCompletedEvent instance, even though the callback registrant may not consume it. Would that work for you?

In regard to the handshake material update -which I assumed unlikely-, the point in which a renegotiation may take place (from the server side) is when reading data, not when writing. That cannot be controlled by the application because it's JSSE internal and not exposed. Thus, an application may read from the socket, get the handshake material and write a message using the binding value -which we can make sure that is the valid one at that point-. However, as soon as the application reads again from the socket, a renegotiation -if requested by the client- may be processed and the binding value gets updated. The higher level protocol may fail -because the binding value was already sent but not processed on the other side- and a re-try needed. This looks independent of whether we use the originally proposed API or the handshake listener callback interface (or even a sync callback), because the underlying problem is that the application cannot really control the renegotiation flow in the lower layer (as RFC 5929 suggest). The options I see are adding more complexity to the API and let the application control the renegotiation flow or live with that and expect the application to retry.

Thanks,
Martin.-

On Tue, Aug 29, 2017 at 4:34 PM, Xuelei Fan <[hidden email] <mailto:[hidden email]>> wrote:

    On 8/26/2017 2:56 PM, Bernd Eckenfels wrote:
    > How about only passing it to an extended handshake listener. The
    > material does not have to be cached (the app can do it if needed) and
    > renegotiation works the same way. This can also be helpful for things
    > like logging the master secret (for wireshark decryption). It can even
    > handle auditing of session resumptions
    Martin, what do you think about Bernd's proposal above and similar
    callback mechanism?

    More comment inlines.

    On 8/29/2017 11:50 AM, Martin Balao wrote:

        Hi Xuelei,

          >There are a few protocols that can benefits from exporting
        SSL/TLS handshake materials, including RFC 5929, RFC 5056, token
        binding and TLS 1.3 itself.  Can we define a general API so as
        to exposing the handshake materials, so as to mitigate the
        inflating of JSSE APIs?  I may suggest make further evaluation
        before move on to following design and code.

        Do you prefer an API like "public byte[]
        getTlsHandshakeMaterial(String materialType)" (in SSLSocket and
        SSLEngine) where "materialType" can eventually be
        "clientFinishedMessage"/"finishedMessage" or
        "serverFinishedMessage"/"peerFinishedMessage"?

    The problem of the APIs like that is, when applications call the
    method, it is not always return the expected result, and the
    implementation may have to cache the message even if an application
    never use it.  See more in the following example.

        I cannot think of "serverCertificate" or "masterKey" as this is
        more related to a Session and not neccessarily to a handshake.
        getTlsHandshakeMaterial would be a lower level API and would
        move the burden of knowing which information is required for
        "tls-unique" TLS channel binding to the API consumer. Looks more
        like the OpenSSL approach (instead of the Python, SSLBoring or
        GnuTls approaches). However, OpenSSL have specific methods for
        each piece of information instead of a generic and parametrized
        one. I.e.: SSL_get_finished or SSL_get_peer_finished. What other
        information do you expect the Handshaker to provide?

          >The SunJSSE provider happens to cache the finished messages
        in its implementation so you can use it for tls-unique, but it
        may not be true for other provider or other channel bindings.         Need to define a more reliable approach to get the handshake
        materials.

        I focused on SunJSSE provider. I'm not sure about how other
        providers may implement this API and where they can get the
        required information from, without knowing their internals. In
        regard to SunJSSE and "tls-unique" binding type, I leveraged on
        existing data. If data weren't already there, I would have to
        figure out how to get it from the handshake -doing the same that
        was already done would have been an option-. Do you prefer the
        Handshaker to provide a function to get different information
        and not just the finished hash? (as for the public
        SSLSocket/SSLEngine "getTlsHandshakeMaterial" API). Which other
        information may be useful to get from the Handshaker? What do
        you mean by reliable? (given that this is all SunJSSE internal
        and we have no external dependencies).

    Let consider the use of the API.
        byte[] getTlsChannelBinding("tls_unique");

    I'm confusing when I try to use it by myself:
    1. provider does not implement this method
        return null or empty?

    It happens because an old provider should still work in new JDK, but
    old provider does not implement new APIs, or a new provider does not
    support this feature.

    2. the method is called before handshaking
        return null or empty?

    3. the method is called during handshaking
        return null, empty or the channel binding value?

    4. the method is called at the same time the handshaking completed?
        return the channel binding value?

    5. the method is called after the handshaking
        return the channel binding value?

    6. the method is called during renegoitation
        return null, empty, the old binding value, or the new binding value?

    7. the method is called after handshaking
        return old binding value, or the new binding value?

    8. the method is called after the initial handshaking, but the
    binding value is changed shortly after because of renegotiation.
        how could application use the binding value?

    We need a clear define of the behavior of the method.  It could be
    complicated if the method is designed as
    getTlsChannelBinding("tls_unique").

    I feel that handshake material should be captured when
    1. it is requested to capture the handshake material, and
    2. the handshake material get produced.

    For the getTlsChannelBinding("tls_unique") API, it is unknown:
    1. Is it required to capture the handshake material?
    2. Is the handshake material produced?

    The two points could result in a few unexpected problems, as the
    above 8 items that we may want to consider.

        In regard to other channel bindings, it'll depend on the binding
        type the way in which the information is obtained. I.e.:
        "tls-unique" SunJSSE implementation leverages on cached finished
        messages. However, "tls-server-end-point" leverages on stored
        certificates that are obtained from the Session (not from the
        handshaker). Is there any specific channel binding you are
        concerned with?

          >If the channel binding is not required, it may be not
        necessary to expose the handshake materials.  Need to define a
        solution to indicate the need of the exporting.

        Do you mean a lower layer knowing if the upper layer is going to
        require that information and decide to provide it or not based
        on that knowledge? I think I didn't get your point here.

    I mean, if an application want to support channel binding, the
    provider can provider the channel binding service;  If the an
    application does not want channel binding, the provider should be
    perform the channel binding service.  The getTlsChannelBinding()
    make the provider MUST perform channel binding cache or calculation
    no matter application want it or not.

          >2. No way to know the update of the underlying handshake
        materials.
          >If renegotiation can takes place, need to define a interface
        to indicate that so that application can response accordingly.         See section 3 and 7 of RFC 5929.

        I intentionally skipped this -at the cost of a spurious
        authentication- to avoid adding complexity to the API. An
        spurious authentication -which does not appear likely to me- can
        easily be retried by the application. The RFC 5929 suggests APIs
        through which the application can *control* the flow (i.e.: hold
        a renegotitation). This would expose JSSE internals. This is
        more than notifying. Notification, in my opinion, adds no value:
        what if the application already used the binding token before
        receiving the notification? The spurious authentication will
        happen anyways and has to be handled -i.e. retried-. It's just a
        timing issue. The real value is controlling the flow as the RFC
        suggests, but at the cost of exposing JSSE internals.

    My understanding, the block of the protocol is to make sure
    application is performing the channel binding with the right value,
    or updating the value accordingly if necessary.  If you skip this
    and when renegotiation happen, the channel binding could be limited,
    or may not work as expected.

    Thanks,
    Xuelei

        Kind regards,
        Martin.-


        On Sat, Aug 26, 2017 at 5:25 PM, Xuelei Fan
        <[hidden email] <mailto:[hidden email]>
        <mailto:[hidden email] <mailto:[hidden email]>>>

        wrote:

             Hi Marin,

             Sorry for the delay.

             There are a few protocols that can benefits from exporting
        SSL/TLS
             handshake materials, including RFC 5929, RFC 5056, token
        binding and
             TLS 1.3 itself.  Can we define a general API so as to
        exposing the
             handshake materials, so as to mitigate the inflating of
        JSSE APIs?     I may suggest make further evaluation before move
        on to following
             design and code.

              >
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
                    <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>>
             I have two concerns about the design:

             1. Channel binding may be not always required.
             SSLSocket/SSLEngine.getTlsChannelBinding(String bindingType);

             The SunJSSE provider happens to cache the finished messages
        in its
             implementation so you can use it for tls-unique, but it may
        not be
             true for other provider or other channel bindings.  Need to
        define a
             more reliable approach to get the handshake materials.

             If the channel binding is not required, it may be not
        necessary to
             expose the handshake materials.  Need to define a solution to
             indicate the need of the exporting.

             2. No way to know the update of the underlying handshake
        materials.
             If renegotiation can takes place, need to define a interface to
             indicate that so that application can response
        accordingly.  See
             section 3 and 7 of RFC 5929.

             Thanks,
             Xuelei

             On 7/31/2017 8:53 AM, Martin Balao wrote:

                 Hi,

                 Here it is an update for the proposed TLS Channel Bindings
                 support in OpenJDK:

                    *
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>
                        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/>>
                 (browse online)
                    *
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>
                        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.02/6491070.webrev.02.zip>>
                 (download)

                 Changes since v01:

                    * getTlsChannelBinding API changed to return null by
        default
                 (if not implemented), instead of throwing an
                 UnsupportedOperationException.

                    * "tls-server-end-point" TLS channel binding now
        supported.

                 Kind regards,
                 Martin.-

                 On Wed, Jul 26, 2017 at 4:12 PM, Martin Balao
        <[hidden email] <mailto:[hidden email]>
                 <mailto:[hidden email] <mailto:[hidden email]>>
        <mailto:[hidden email] <mailto:[hidden email]>

                 <mailto:[hidden email] <mailto:[hidden email]>>>>

        wrote:

                      Hi,

                      Here it is my proposal for JDK-6491070 (Support
        for RFC
                 5929-Channel
                      Bindings: e.g. public API to obtain TLS finished
        message) [1]:

                        *
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
                        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>
                                    <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>
                        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/>>>
                        *
        http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
                        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>
                                    <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>
                        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip
        <http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6491070/webrev.01/6491070.webrev.01.zip>>>

                      Notes:
                        * Implementation based on Channel Bindings for
        TLS (RFC
                 5929) [2]

                        * Only "tls-unique" currently supported

                      Look forward to your comments.

                      Kind regards,
                      Martin.-

                      --
                      [1] -
        https://bugs.openjdk.java.net/browse/JDK-6491070
        <https://bugs.openjdk.java.net/browse/JDK-6491070>
                 <https://bugs.openjdk.java.net/browse/JDK-6491070
        <https://bugs.openjdk.java.net/browse/JDK-6491070>>
                      <https://bugs.openjdk.java.net/browse/JDK-6491070
        <https://bugs.openjdk.java.net/browse/JDK-6491070>
                 <https://bugs.openjdk.java.net/browse/JDK-6491070
        <https://bugs.openjdk.java.net/browse/JDK-6491070>>>
                      [2] - https://tools.ietf.org/html/rfc5929
        <https://tools.ietf.org/html/rfc5929>
                 <https://tools.ietf.org/html/rfc5929
        <https://tools.ietf.org/html/rfc5929>>
                      <https://tools.ietf.org/html/rfc5929
        <https://tools.ietf.org/html/rfc5929>
                 <https://tools.ietf.org/html/rfc5929
        <https://tools.ietf.org/html/rfc5929>>>