[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-131a4e638...@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



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to