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:
/messages/by-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

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

On Sat, May 27, 2017 at 5:59 PM, Álvaro Hernández Tortosa
<aht@8kdata.com> wrote:

On 25/05/17 17:17, Michael Paquier wrote:

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 ^_^

The thing would have been reviewed and rewritten a couple of times :)
So that would have unlikely (hopefully) finished in the shape of a
Java-C-like patch.

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.

Those are 5 lines of code each in OpenSSL, not that hard to maintain.
Those APIs are clunky by the way as there is no way to know in advance
the length of the message for memory allocation.

- 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.

I am wondering how other languages are regarding that. Python has
added a method in 2011:
https://hg.python.org/cpython/rev/cb44fef5ea1d

- 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:

RFC 5802, section 6 gives a good level of details on the matter:
https://tools.ietf.org/html/rfc5802#section-6
There is indeed interaction between SSL and scram, and the patch makes
use of USE_SSL for this purpose.

"
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)

The SSL hanshake happens only once at connection obtention, before the
authentication exchange happens. So there is no renegociation. SSL
renegotiation has been removed in PG anyway, disabled on
back-branches, and removed as well in TLS 1.3 (right? I don't recall
the spec in details).

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:
/messages/by-id/df8c6e27-4d8e-5281-96e5-131a4e638fc8@8kdata.com

The RFC says that any server implementing channel binding must
implement tls-unique, so having only end-point is not compliant.

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.

It seems to me as well that this gives us an indication that it
should be the default channel binding used, or if the exchange list
has no channel names included, the client can assume that tls-unique
will be used. Such an approach has as well the merit to keep the patch
simple. In short, I would advocate an incremental approach that adds
tls-unique first. This has value anyway as there is no need for
certificate configuration. This also does not require a message format
extension.
--
Michael

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

On Sat, May 27, 2017 at 5:59 PM, Álvaro Hernández Tortosa
<aht@8kdata.com> wrote:

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

It seems to me that the question is not just whether those APIs will
be available in future versions of OpenSSL, but whether they will be
available in every current and future version of every SSL
implementation that we may wish to use in core or that any client may
wish to use. We've talked before about being able to use the Windows
native SSL implementation rather than OpenSSL and it seems that there
would be significant advantages in having that capability. Meanwhile,
a client that reimplements the PostgreSQL wire protocol is free to use
any SSL implementation it likes. If channel binding can't work unless
both sides are speaking OpenSSL, then I'd say we've blown it.

Also, Heikki pointed out in his PGCon talk that there's currently no
way for libpq to insist on the use of SCRAM rather than plain password
authentication or md5. So, if someone trojans the server, they only
need to hack it to ask the client for plaintext authentication rather
than SCRAM and the client will cheerfully hand over the password with
no questions asked. Presumably, we need to add a connection option to
insist (a) on SCRAM or (b) SCRAM + channel binding, or else this isn't
going to actually provide us with any increased security. Even
without that, channel binding will still let the server verify that
it's really connected to the client, but that's not really doing much
for us in terms of security because the client (who has the password)
can always let someone else impersonate it perfectly by just handing
over the password. Channel binding can't prevent that. It *could*
prevent someone from impersonating the *server* but not if the server
can disable the check whenever it likes by ceasing to advertise
channel binding as an available option -- with no notification to the
client that anything has changed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, May 27, 2017 at 5:59 PM, Álvaro Hernández Tortosa
<aht@8kdata.com> wrote:

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

It seems to me that the question is not just whether those APIs will
be available in future versions of OpenSSL, but whether they will be
available in every current and future version of every SSL
implementation that we may wish to use in core or that any client may
wish to use. We've talked before about being able to use the Windows
native SSL implementation rather than OpenSSL and it seems that there
would be significant advantages in having that capability.

Another thing of the same sort that should be on our radar is making
use of Apple's TLS code on macOS. The handwriting on the wall is
unmistakable that they intend to stop shipping OpenSSL before long,
and I do not think we really want to be in a position of having to
bundle OpenSSL into our distribution on macOS.

I'm not volunteering to do that, mind you. But +1 for not tying new
features to any single TLS implementation.

regards, tom lane

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

Tom, all,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, May 27, 2017 at 5:59 PM, Álvaro Hernández Tortosa
<aht@8kdata.com> wrote:

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

It seems to me that the question is not just whether those APIs will
be available in future versions of OpenSSL, but whether they will be
available in every current and future version of every SSL
implementation that we may wish to use in core or that any client may
wish to use. We've talked before about being able to use the Windows
native SSL implementation rather than OpenSSL and it seems that there
would be significant advantages in having that capability.

Another thing of the same sort that should be on our radar is making
use of Apple's TLS code on macOS. The handwriting on the wall is
unmistakable that they intend to stop shipping OpenSSL before long,
and I do not think we really want to be in a position of having to
bundle OpenSSL into our distribution on macOS.

I'm not volunteering to do that, mind you. But +1 for not tying new
features to any single TLS implementation.

Work has been done in that area already, as well as for LibNSS support.

I've not had a chance to closely look over the proposed approach here,
but generally speaking, we need to be looking for a standard way to
generate and transmit the channel binding information across the
wire. The specific APIs are, certainly, going to be different between
different TLS implementations and I don't believe we need to be
particularly concerned with that (or if they change in the future), as
long as we abstract those APIs. Perhaps there's some question as to
just how to abstract them, but I'm at least a bit less concerned with
that as I expect we'll be able to adjust those abstraction layers later
(presuming they aren't exposed, which I wouldn't expect them to be).

I skimmed over RFC5929, which looks to be the correct RFC when it comes
to channel binding with TLS. Hopefully the various TLS implementations
work in a similar manner, following that RFC (or whichever is relevant).

If that wasn't the case then, for example, it wouldn't be possible to do
channel binding with a LibNSS client and an OpenSSL server or with other
combinations and I find that rather hard to believe.

Thanks!

Stephen

On Tue, May 30, 2017 at 8:14 AM, Stephen Frost <sfrost@snowman.net> wrote:

Work has been done in that area already, as well as for LibNSS support.

I've not had a chance to closely look over the proposed approach here,
but generally speaking, we need to be looking for a standard way to
generate and transmit the channel binding information across the
wire.

The RFC of SCRAM specifies that the client sends the type of channel
binding in its first message data, and then provides the contents of
the TLS message it generated in the challenge response.

The specific APIs are, certainly, going to be different between
different TLS implementations and I don't believe we need to be
particularly concerned with that (or if they change in the future), as
long as we abstract those APIs. Perhaps there's some question as to
just how to abstract them, but I'm at least a bit less concerned with
that as I expect we'll be able to adjust those abstraction layers later
(presuming they aren't exposed, which I wouldn't expect them to be).

The current patch fetches the TLS finish message data just after the
handshake in be_tls_open_server() using the dedicated OpenSSL API. We
could definitely go with a more generic routine on our side that
sticks with the concepts of be_tls_get_compression():
- For the backend, this routine returns an allocated string with the
contents of the expected TLS message, and its size:
char *be_tls_get_tls_finish(Port *, int *)
- For the frontend, a routine to get the generated TLS finish message:
char *pgtls_get_tls_finish(PGconn *, int *)

I skimmed over RFC5929, which looks to be the correct RFC when it comes
to channel binding with TLS. Hopefully the various TLS implementations
work in a similar manner, following that RFC (or whichever is relevant).

That's the one I use as a reference.

If that wasn't the case then, for example, it wouldn't be possible to do
channel binding with a LibNSS client and an OpenSSL server or with other
combinations and I find that rather hard to believe.

As far as I can see, Windows has some APIs to get the TLS finish message:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa382420(v=vs.85).aspx
On macos though it is another story, I am not seeing anything:
https://developer.apple.com/reference/security/secure_transport#symbols

Depending on the SSL implementation the server is compiled with, it
will be up to the backend to decide if it should advertise to the
client the -PLUS mechanism or not, so we can stay modular here.
--
Michael

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

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:

On Tue, May 30, 2017 at 8:14 AM, Stephen Frost <sfrost@snowman.net> wrote:

The specific APIs are, certainly, going to be different between
different TLS implementations and I don't believe we need to be
particularly concerned with that (or if they change in the future), as
long as we abstract those APIs. Perhaps there's some question as to
just how to abstract them, but I'm at least a bit less concerned with
that as I expect we'll be able to adjust those abstraction layers later
(presuming they aren't exposed, which I wouldn't expect them to be).

The current patch fetches the TLS finish message data just after the
handshake in be_tls_open_server() using the dedicated OpenSSL API. We
could definitely go with a more generic routine on our side that
sticks with the concepts of be_tls_get_compression():
- For the backend, this routine returns an allocated string with the
contents of the expected TLS message, and its size:
char *be_tls_get_tls_finish(Port *, int *)
- For the frontend, a routine to get the generated TLS finish message:
char *pgtls_get_tls_finish(PGconn *, int *)

Those look pretty reasonable to me and seem to go along with the
concepts from the RFC, making them hopefully the right API.

If that wasn't the case then, for example, it wouldn't be possible to do
channel binding with a LibNSS client and an OpenSSL server or with other
combinations and I find that rather hard to believe.

As far as I can see, Windows has some APIs to get the TLS finish message:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa382420(v=vs.85).aspx

Good.

On macos though it is another story, I am not seeing anything:
https://developer.apple.com/reference/security/secure_transport#symbols

That's a bit unfortunate, if that's the case. Perhaps Apple will see
fit to address that deficiency though.

Depending on the SSL implementation the server is compiled with, it
will be up to the backend to decide if it should advertise to the
client the -PLUS mechanism or not, so we can stay modular here.

Right, of course.

All-in-all, this sounds like it's heading in the right direction, at
least at a high level. Glad to see that there's been consideration of
other TLS implementations, and as such I don't think we need to be
overly concerned about the specifics of the OpenSSL API here.

Thanks!

Stephen

On 30 May 2017, at 18:25, Michael Paquier <michael.paquier@gmail.com> wrote:

On macos though it is another story, I am not seeing anything:
https://developer.apple.com/reference/security/secure_transport#symbols

The Secure Transport documentation unfortunately excel at being incomplete and
hard to search in. That being said, I think you’re right, AFAICT there is no
published API for this (SSLProcessFinished() seems like the best match but is
not public).

Depending on the SSL implementation the server is compiled with, it
will be up to the backend to decide if it should advertise to the
client the -PLUS mechanism or not, so we can stay modular here.

+1

cheers ./daniel

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

On 30 May 2017, at 16:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, May 27, 2017 at 5:59 PM, Álvaro Hernández Tortosa
<aht@8kdata.com> wrote:

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

It seems to me that the question is not just whether those APIs will
be available in future versions of OpenSSL, but whether they will be
available in every current and future version of every SSL
implementation that we may wish to use in core or that any client may
wish to use. We've talked before about being able to use the Windows
native SSL implementation rather than OpenSSL and it seems that there
would be significant advantages in having that capability.

Another thing of the same sort that should be on our radar is making
use of Apple's TLS code on macOS. The handwriting on the wall is
unmistakable that they intend to stop shipping OpenSSL before long,
and I do not think we really want to be in a position of having to
bundle OpenSSL into our distribution on macOS.

I'm not volunteering to do that, mind you. But +1 for not tying new
features to any single TLS implementation.

Big +1. The few settings we have already make it hard to provide other
implementations as drop-in replacements (Secure Transport doesn’t support
.crl files for example, only CRL loaded in Keychains).

cheers ./daniel

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

On Tue, May 30, 2017 at 1:00 PM, Stephen Frost <sfrost@snowman.net> wrote:

All-in-all, this sounds like it's heading in the right direction, at
least at a high level. Glad to see that there's been consideration of
other TLS implementations, and as such I don't think we need to be
overly concerned about the specifics of the OpenSSL API here.

That sounds like undue optimism to me. Unless somebody's tested that
Michael's proposed implementation, which uses undocumented OpenSSL
APIs, actually interoperates properly with a SCRAM + channel binding
implementation based on some other underlying SSL implementation, we
can't really know that it's going to work. It's not like we're
calling SSL_do_the_right_thing_for_channel_binding_thing_per_rfc5929().
We're calling SSL_do_something_undocumented() and hoping that
something_undocumented ==
the_right_thing_for_channel_binding_thing_per_rfc5929. Could be true,
but without actual interoperability testing it sounds pretty
speculative to me.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

On Tue, May 30, 2017 at 5:59 PM, Robert Haas <robertmhaas@gmail.com> wrote:

That sounds like undue optimism to me. Unless somebody's tested that
Michael's proposed implementation, which uses undocumented OpenSSL
APIs, actually interoperates properly with a SCRAM + channel binding
implementation based on some other underlying SSL implementation, we
can't really know that it's going to work. It's not like we're
calling SSL_do_the_right_thing_for_channel_binding_thing_per_rfc5929().
We're calling SSL_do_something_undocumented() and hoping that
something_undocumented ==
the_right_thing_for_channel_binding_thing_per_rfc5929. Could be true,
but without actual interoperability testing it sounds pretty
speculative to me.

Hm? Python is using that stuff for a certain amount of years now, for
the same goal of providing channel binding for SSL authentication. You
can look at this thread:
https://bugs.python.org/issue12551
So qualifying that as a speculative method sounds a quite hard word to me.
--
Michael

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

On 31/05/17 03:39, Michael Paquier wrote:

On Tue, May 30, 2017 at 5:59 PM, Robert Haas <robertmhaas@gmail.com> wrote:

That sounds like undue optimism to me. Unless somebody's tested that
Michael's proposed implementation, which uses undocumented OpenSSL
APIs, actually interoperates properly with a SCRAM + channel binding
implementation based on some other underlying SSL implementation, we
can't really know that it's going to work. It's not like we're
calling SSL_do_the_right_thing_for_channel_binding_thing_per_rfc5929().
We're calling SSL_do_something_undocumented() and hoping that
something_undocumented ==
the_right_thing_for_channel_binding_thing_per_rfc5929. Could be true,
but without actual interoperability testing it sounds pretty
speculative to me.

Hm? Python is using that stuff for a certain amount of years now, for
the same goal of providing channel binding for SSL authentication. You
can look at this thread:
https://bugs.python.org/issue12551
So qualifying that as a speculative method sounds a quite hard word to me.

Actually this Python patch seems to ultimately rely on the same
OpenSSL functions as your implementation.

I don't have anything against implementing tls-unique, specially as
per the RFC it is mandatory (if channel binding support is provided).
However, given the use of undocumented API, given that at least the Java
driver would have a very difficult time implementing it (basically
replacing Java's standard SSL stack with a completely external one,
BouncyCastle, which adds load, size and complexity), I would rather
focus the efforts on tls-server-end-point which only needs to access the
certificate data, something that most APIs/frameworks/languages seem to
cover much more naturally than tls-unique.

Both are equally safe and effective and so having support for both
seems sensible to me.

Á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

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:

but without actual interoperability testing it sounds pretty
speculative to me.

I'm all for interoperability testing.

When we have multiple implementations of TLS using different libraries
with various versions of PostgreSQL and libpq and are able to test those
against other versions of PostgreSQL and libpq compiled with other TLS
libraries, I'll be downright ecstatic. We are a small ways from that
right now, however, and I don't believe that we should be asking the
implementors of channel binding to also implement support for multiple
TLS libraries in PostgreSQL in order to test that their RFC-following
(at least, as far as they can tell) implementation actually works.

I'm not exactly sure what to characterize that as, given that the old
fall-back of "feature creep" feels woefully inadequate as a description.

Thanks!

Stephen

On Tue, May 30, 2017 at 11:49 PM, Stephen Frost <sfrost@snowman.net> wrote:

... and I don't believe that we should be asking the
implementors of channel binding to also implement support for multiple
TLS libraries in PostgreSQL in order to test that their RFC-following
(at least, as far as they can tell) implementation actually works.

You're of course free to believe what you wish, but that sounds
short-sighted to me. If we implement channel binding and it turns out
not to be interoperable with other SSL implementations, then what? We
can't change it later without breaking compatibility with our own
prior implementation. Note that Álvaro Hernández Tortosa said about
two hours before you sent this email that it doesn't seem possible to
implement something comparable in Java's standard SSL stack. If
that's the case, adopting this implementation is dooming everyone who
connects to the database server using JDBC to be unable to use channel
binding. And that's a large percentage of our user base.

And then what happens when we do add support for Windows SSL or macOS
SSL? It sounds like you're equally willing to throw people using
those implementations under the bus. So we'll end up with channel
binding that works only when everybody's running the same operating
system and nobody's using Java. Uggh. At that point you might as
well just go back to using SSL certificates to solve this problem. If
you use SSL certificates, then (1) it should work with any SSL
implementation and (2) the client can force SSL certificate
verification, whereas currently the client cannot force even the use
of SCRAM, let alone channel binding.

So what is being proposed here does not (yet, anyway) provide any
actual security and seems to have poor prospects for interoperability,
whereas we have an existing feature (SSL certificates) that has
neither of those problems. Are you sure you're not putting
buzzword-compliance ahead of real progress?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

On Tue, May 30, 2017 at 6:50 PM, Álvaro Hernández Tortosa
<aht@8kdata.com> wrote:

Actually this Python patch seems to ultimately rely on the same OpenSSL
functions as your implementation.

Yup. My point is that even if those APIs are undocumented, I think
that they are not going to disappear either.

I don't have anything against implementing tls-unique, specially as per
the RFC it is mandatory (if channel binding support is provided). However,
given the use of undocumented API, given that at least the Java driver would
have a very difficult time implementing it (basically replacing Java's
standard SSL stack with a completely external one, BouncyCastle, which adds
load, size and complexity), I would rather focus the efforts on
tls-server-end-point which only needs to access the certificate data,
something that most APIs/frameworks/languages seem to cover much more
naturally than tls-unique.

Point taken. Well, this brings us to the point where we should have
both tls-unique and end-point to allow JDBC to work with it. Now,
thinking about it, do we really need to advertise the available
channel binding types when the client chooses the -PLUS mechanism?
Wouldn't it make sense to let the client choose what it thinks is
better and just fail the exchange with the backend if the channel type
is not supported?

Both are equally safe and effective and so having support for both seems
sensible to me.

I have some automatic setups that would be really simplified by just
using libpq with SSL connections if there is channel binding. And that
involves certificate deployments, I think I am not the only one to see
that present even if JDBC just supports end-point.
--
Michael

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

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:

On Tue, May 30, 2017 at 11:49 PM, Stephen Frost <sfrost@snowman.net> wrote:

... and I don't believe that we should be asking the
implementors of channel binding to also implement support for multiple
TLS libraries in PostgreSQL in order to test that their RFC-following
(at least, as far as they can tell) implementation actually works.

You're of course free to believe what you wish, but that sounds
short-sighted to me. If we implement channel binding and it turns out
not to be interoperable with other SSL implementations, then what? We
can't change it later without breaking compatibility with our own
prior implementation. Note that Álvaro Hernández Tortosa said about
two hours before you sent this email that it doesn't seem possible to
implement something comparable in Java's standard SSL stack. If
that's the case, adopting this implementation is dooming everyone who
connects to the database server using JDBC to be unable to use channel
binding. And that's a large percentage of our user base.

I'm hopeful that we're closer to agreement here than we are the
opposite.

It was absolutely not my intent that the ongoing discussion between
Alvaro and Michael be stopped or changed, quite the opposite, in fact.

If your comments regarding the requirement that we have interoperability
testing of this feature before accepting it were intended to mean that
we need to make sure that the JDBC driver is able to work with the
chosen solution (or, perhaps, that we support multuple options, one of
which works with the JDBC changes), and that we review the other TLS
libraries with the goal of making sure that what we agree on in core
should work with those also, then that's great and exactly what I'd like
to see also.

If your comments regarding the requirement that we have interoperability
testing of this feature before accepting it were intended to mean that
we must have a complete alternative TLS solution, while that would
actually play a great deal to a goal I've had for a while to have PG
support multiple TLS implementations (LibNSS being top-of-mind, at least
for me, as I've mentioned a couple times), I can't agree that we should
require that before accepting channel binding as a feature. To do so
would be akin to requiring that we support multiple TLS implementations
before we agreed to support client-side certificates, in my opinion.

I would be rather surprised if the solution which Michael and Alvaro
come to results in a solution which requires us to break compatibility
down the road, though it's of course a risk we need to consider. The
ongoing discussion around finding a way to support both methods outlined
in the RFC sounds exactly correct to me and I encourage further
discussion there.

And then what happens when we do add support for Windows SSL or macOS
SSL? It sounds like you're equally willing to throw people using
those implementations under the bus. So we'll end up with channel
binding that works only when everybody's running the same operating
system and nobody's using Java. Uggh. At that point you might as
well just go back to using SSL certificates to solve this problem. If
you use SSL certificates, then (1) it should work with any SSL
implementation and (2) the client can force SSL certificate
verification, whereas currently the client cannot force even the use
of SCRAM, let alone channel binding.

I hope it's clear that it's not my intent to throw anyone "under the
bus." The only reason that "SSL certificates should work with any SSL
implementation" is because those SSL implementations are based on RFCs
which define how they should work and what I was encouraging up-thread
is exactly that we should be looking at RFCs to determine the right path
forward. There are cases, of course, where the RFCs have alternatives
and the ideal approach is to find a way to work with all of those
alternatives, or at least implement a solution which is flexible, such
that later changes could add support without breaking existing users.

I'm certainly on-board with the general idea of having a way for the
client to require both SCRAM and channel binding and I agree that's a
hole in our current system, but that is really an orthogonal concern.

So what is being proposed here does not (yet, anyway) provide any
actual security and seems to have poor prospects for interoperability,
whereas we have an existing feature (SSL certificates) that has
neither of those problems. Are you sure you're not putting
buzzword-compliance ahead of real progress?

Adding support for channel binding is quite important and valuable, in
its own right. I concur that we want to provide ways for the client to
require SCRAM and to require channel binding, but I don't see any
particular reason that adding support for channel binding has to be held
up behind that.

As for your question, I have to say that I find it inappropriate to
characterize channel binding as a "buzzword" or to suggest that this
particular feature which Michael and Alvaro are working to add to PG is
only being done to fulfill some marketing requirement or checkbox but
without adding any technical merit in its own right. Channel binding
isn't new, it's supported by quite a few different technologies, and we
would absolutely be better off including support for it, from an
entirely technical perspective. If I were one of the individuals
working on this feature, I'd be rather put-off and upset at the
suggestion that the good work they're doing is viewed by the PostgreSQL
community and one of our major committers as only being done to check a
box or to be "buzzword compliant" and ultimately without technical
merit.

Thankfully, I know both Michael and Alvaro and had excellent discussions
with them at PGCon last week, and I don't doubt that they will continue
their efforts regardless, but I worry about the signal it sends to
individuals who are not yet contributors or who have not gone through
the process of trying to contribute to PostgreSQL. We make it more than
difficult enough to get code into core, let's try to avoid discouraging
contributors by casting doubt on the technical merits of their efforts
when they're very clearly adding a useful feature, even if it isn't
perfect until we also add X, Y or Z. Instead, let's encourage their
efforts to complete the work they've started and then work with them to
add those other components necessary to have a complete solution.

Thanks!

Stephen

On Wed, May 31, 2017 at 7:59 PM, Stephen Frost <sfrost@snowman.net> wrote:

If your comments regarding the requirement that we have interoperability
testing of this feature before accepting it were intended to mean that
we need to make sure that the JDBC driver is able to work with the
chosen solution (or, perhaps, that we support multuple options, one of
which works with the JDBC changes), and that we review the other TLS
libraries with the goal of making sure that what we agree on in core
should work with those also, then that's great and exactly what I'd like
to see also.

OK, great. That's what I mean (mostly - see below).

If your comments regarding the requirement that we have interoperability
testing of this feature before accepting it were intended to mean that
we must have a complete alternative TLS solution, while that would
actually play a great deal to a goal I've had for a while to have PG
support multiple TLS implementations (LibNSS being top-of-mind, at least
for me, as I've mentioned a couple times), I can't agree that we should
require that before accepting channel binding as a feature. To do so
would be akin to requiring that we support multiple TLS implementations
before we agreed to support client-side certificates, in my opinion.

I don't think that we need to have a committed patch allowing multiple
SSL implementations before we accept a patch for channel binding, but
I think it might be a good idea for someone to hack either libpq or
the server enough to make it work with some other SSL implementation
(Windows SSL would probably be the best candidate) and test that what
we think will work for channel binding actually does work. It
wouldn't need to be a committable patch, but it should be enough to
make a successful connection in the laboratory. Now, there might also
be other ways besides that of testing that interoperability is
possible, so don't take me as being of the view that someone has to
necessarily do exactly that thing that I just said. But I do think
that we probably should do more than say "hey, we've got this
undocumented SSL API, and this other Windows API, and it looks like
they do about the same thing, so we're good". There's sometimes a big
difference between what sounds like it should work on paper and what
actually does work.

I would be rather surprised if the solution which Michael and Alvaro
come to results in a solution which requires us to break compatibility
down the road, though it's of course a risk we need to consider. The
ongoing discussion around finding a way to support both methods outlined
in the RFC sounds exactly correct to me and I encourage further
discussion there.

Sure, I have no objection to the discussion. Discussion is always
cool; what I'm worried about is a tls-unique implementation that is
supremely unportable, and there's no current evidence that the
currently-proposed patch is anything else. There is some optimism,
but optimism <> evidence.

I'm certainly on-board with the general idea of having a way for the
client to require both SCRAM and channel binding and I agree that's a
hole in our current system, but that is really an orthogonal concern.

Orthogonal but *very* closely related.

entirely technical perspective. If I were one of the individuals
working on this feature, I'd be rather put-off and upset at the
suggestion that the good work they're doing is viewed by the PostgreSQL
community and one of our major committers as only being done to check a
box or to be "buzzword compliant" and ultimately without technical
merit.

I did not say that the feature was "ultimately without technical
merit"; I said that the patch as submitted seemed like it might not
interoperate, and that without a libpq option to force it to be used
it wouldn't add any real security. I stand by those statements and I
think it is 100% appropriate for me to raise those issues. Other
people, including you, do the same thing about other patches all the
time. It is not a broadside against the contributors or the patch; it
is a short, specific list of concerns that are IMHO quite justifiable.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:

On Wed, May 31, 2017 at 7:59 PM, Stephen Frost <sfrost@snowman.net> wrote:

If your comments regarding the requirement that we have interoperability
testing of this feature before accepting it were intended to mean that
we need to make sure that the JDBC driver is able to work with the
chosen solution (or, perhaps, that we support multuple options, one of
which works with the JDBC changes), and that we review the other TLS
libraries with the goal of making sure that what we agree on in core
should work with those also, then that's great and exactly what I'd like
to see also.

OK, great. That's what I mean (mostly - see below).

Glad to hear it.

If your comments regarding the requirement that we have interoperability
testing of this feature before accepting it were intended to mean that
we must have a complete alternative TLS solution, while that would
actually play a great deal to a goal I've had for a while to have PG
support multiple TLS implementations (LibNSS being top-of-mind, at least
for me, as I've mentioned a couple times), I can't agree that we should
require that before accepting channel binding as a feature. To do so
would be akin to requiring that we support multiple TLS implementations
before we agreed to support client-side certificates, in my opinion.

I don't think that we need to have a committed patch allowing multiple
SSL implementations before we accept a patch for channel binding, but
I think it might be a good idea for someone to hack either libpq or
the server enough to make it work with some other SSL implementation
(Windows SSL would probably be the best candidate) and test that what
we think will work for channel binding actually does work. It
wouldn't need to be a committable patch, but it should be enough to
make a successful connection in the laboratory. Now, there might also
be other ways besides that of testing that interoperability is
possible, so don't take me as being of the view that someone has to
necessarily do exactly that thing that I just said. But I do think
that we probably should do more than say "hey, we've got this
undocumented SSL API, and this other Windows API, and it looks like
they do about the same thing, so we're good". There's sometimes a big
difference between what sounds like it should work on paper and what
actually does work.

I certainly wouldn't object to someone working on this, but I feel like
it's a good deal more work than perhaps you're realizing (and I tend to
think trying to use the Windows SSL implementation would increase the
level of effort, not minimize it). Perhaps it wouldn't be too bad to
write a one-off piece of code which just connects and then returns the
channel binding information on each side and then one could hand-check
that what's returned matches what it's supposed to based on the RFC, but
if it doesn't, then what? In the specific case we're talking about,
there's two approaches in the RFC and it seems like we should support
both because at least our current JDBC implementation only works with
one, and ideally we can allow for that to be extended to other methods,
but I wouldn't want to implement a method that only works on Windows SSL
because that implementation, for whatever reason, doesn't follow either
of the methods available in the RFC.

Thanks!

Stephen

On Wed, May 31, 2017 at 09:37:02AM -0400, Robert Haas wrote:

On Tue, May 30, 2017 at 11:49 PM, Stephen Frost <sfrost@snowman.net> wrote:

... and I don't believe that we should be asking the
implementors of channel binding to also implement support for multiple
TLS libraries in PostgreSQL in order to test that their RFC-following
(at least, as far as they can tell) implementation actually works.

You're of course free to believe what you wish, but that sounds
short-sighted to me. If we implement channel binding and it turns out
not to be interoperable with other SSL implementations, then what? We
can't change it later without breaking compatibility with our own
prior implementation. Note that �lvaro Hern�ndez Tortosa said about
two hours before you sent this email that it doesn't seem possible to
implement something comparable in Java's standard SSL stack. If
that's the case, adopting this implementation is dooming everyone who
connects to the database server using JDBC to be unable to use channel
binding. And that's a large percentage of our user base.

Just to step back, exactly how does channel binding work? Is each side
of the SSL connection hashing the password hash with the shared SSL
session secret in some way that each side knows the other end knows
the password hash, but not disclosing the secret or password hash? Is
there some other way JDBC can get that information?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

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