Re: [HACKERS] Channel binding support for SCRAM-SHA-256

[cross-posting to pgjdbc]

On 25/05/17 17:17, Michael Paquier wrote:
> Hi all,
>
> Please find attached a patch to add support for channel binding for
> SCRAM, to mitigate MITM attacks using this protocol. Per RFC5802
> (https://tools.ietf.org/html/rfc5802), servers supporting channel
> binding need to add support for tls-unique, and this is what this
> patch does.

This is awesome, Michael :) Thank you! You have prevented me
eventually writing the patch and having then PostgreSQL source tainted
with Java-to-C "transpiled" code ^_^
>
> As defined in RFC5056 (exactly here =>
> https://tools.ietf.org/html/rfc5056#section-4.1), servers can use the
> TLS finish message to determine if the client is actually the same as
> the one who initiated the connection, to eliminate the risk of MITM
> attacks. OpenSSL offers two undocumented APIs for this purpose:
> - SSL_get_finished() to get the latest TLS finish message that the
> client has sent.
> - SSL_get_peer_finished(), to get the latest TLS finish message
> expected by the server.
> So basically what is needed here is saving the latest message
> generated by client in libpq using get_finished(), send it to the
> server using the SASL exchange messages (value sent with the final
> client message), and then compare it with what the server expected.
> Connection is successful if what the client has sent and what the
> server expects match.
>
> There is also a clear documentation about what is expected from the
> client and the server in terms of how both should behave using the
> first client message https://tools.ietf.org/html/rfc5802#section-6. So
> I have tried to follow it, reviews are welcome particularly regarding
> that. The implementation is done in such a way that channel binding is
> used in the context of an SSL connection, which is what the RFCs
> expect from an implementation.

After a deeper analysis, I have some concerns/comments here:

- tls-unique, as you mentioned, uses two undocumented APIs. This raises
a small flag about the stability and future of those APIs.

- More importantly, some drivers (not libpq-based) may have unexpected
difficulties implementing tls-unique. In particular, there is not an API
in Java to get the finished message. I expect other languages to maybe
have similar limitations. For Java I see two options:
* Also implement tls-server-end-point, which rather relies on the
server certificate. This information seems to be exposed as part of the
Java APIs:
https://docs.oracle.com/javase/8/docs/api/java/security/cert/Certificate.html#getEncoded--
* Use the BouncyCastle library (http://bouncycastle.org/), which
explicitly supports tls-unique
(https://www.bouncycastle.org/docs/pkixdocs1.5on/org/bouncycastle/est/TLSUniqueProvider.html#getTLSUnique()
,
https://www.bouncycastle.org/docs/tlsdocs1.5on/org/bouncycastle/tls/ChannelBinding.html#tls_unique
). This would require, however, non-trivial changes to the driver and, I
expect, a lot of extra effort.

- It seems to me that tls-unique might be a bit fragile. In particular,
it requires the server to be aware of TSL renegotiations and avoid
performing one while the SCRAM authentication is being performed. I
don't know if this is already guaranteed by the current patch, but it
seems to me it requires complex interactions between levels of
abstraction that are quite separate (SSL and SCRAM). This is explained
by the RFC:

"
server applications MUST NOT request TLS renegotiation during phases of
the application protocol during which application-layer authentication
occurs
"
(https://tools.ietf.org/html/rfc5929#section-3.1)

Based on this facts, I'd suggest to either implement
tls-server-end-point or implement both tls-server-end-point and
tls-unique. The latter would require a more complete protocol message to
advertise separately SCRAM mechanisms and channel binding names. One of
such structures could be the one I suggested earlier:
https://www.postgresql.org/message-id/df8c6e27-4d8e-5281-96e5-131a4e638fc8@8kdata.com

>
> Before even that, the server needs to send to the client the list of
> SASL mechanisms that are supported. This adds SCRAM-SHA-256-PLUS if
> the server has been built with SSL support to the list.

And I'd say the list of channel binding names supported.

Álvaro

--

Álvaro Hernández Tortosa

-----------
<8K>data