Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-27 Thread Peter Eisentraut 
On 6/23/18 17:09, Bruce Momjian wrote:
> On Wed, Jun  6, 2018 at 01:16:11PM -0700, Steven Fackler wrote:
>> TLS 1.3, (which is currently in a draft state, but is theoretically being
>> finalized soon) does not support the TLS channel binding algorithms [1]. From
> 
> Uh, according to this article, TLS 1.3 was finalized in March:
> 
>   
> https://www.theregister.co.uk/2018/03/27/with_tls_13_signed_off_its_implementation_time/

More generally, is our TLS 1.3 support sound?  For instance, I've read
about new cipher suites, so one question is, do the existing
configuration settings that control such things even still work?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-24 Thread Michael Paquier 
On Tue, Jun 19, 2018 at 09:19:40AM +0900, Michael Paquier wrote:
> As Peter and Heikki have worked as well on all those features with me,
> are there any objection to discard this open item?  I looked again at
> the patch this morning and it is true that OpenSSL's history makes
> things harder, so keeping code consistent and simple with their last LTS
> version is appealing.

Okay, in consequence, I have moved this open item to the non-bugs
section.
--
Michael


signature.asc
Description: PGP signature

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-24 Thread Alvaro Hernandez 



On 24/06/18 18:49, Dave Cramer wrote:



On 29 May 2018 at 22:48, Michael Paquier > wrote:


On Tue, May 29, 2018 at 10:33:03PM -0400, Heikki Linnakangas wrote:
> Hmm. I think Peter went through this in commits ac3ff8b1d8 and
054e8c6cdb.
> If you got that working now, I suppose we could do that, but I'm
actually
> inclined to just stick to the current, more straightforward
code, and
> require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been
around for
> several years now. It's not available on all the popular
platforms and
> distributions yet, but I don't want to bend over backwards to
support those.

I think that this mainly boils down to how much Postgres JDBC wants to
get support here as some vendors can maintain oldest versions of
OpenSSL
for a long time.  The extra code is not that much complicated by the
way, still it is true that HEAD is cleaner with its simplicity.


I'm unclear what this has to do with JDBC ? JDBC doesn't use OpenSSL

Alvaro ?




    It's only indirectly related. It does matter on what servers JDBC 
would be able to connect to (using SCRAM + channel binding). Only those 
with tls-server-end-point will be able to use CB with JDBC, and that is, 
as of today, only OpenSSL 1.0.2 or higher, which is not available on 
some older distributions.




    Álvaro

--

Alvaro Hernandez


---
OnGres

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-24 Thread Dave Cramer 
On 29 May 2018 at 22:48, Michael Paquier  wrote:

> On Tue, May 29, 2018 at 10:33:03PM -0400, Heikki Linnakangas wrote:
> > Hmm. I think Peter went through this in commits ac3ff8b1d8 and
> 054e8c6cdb.
> > If you got that working now, I suppose we could do that, but I'm actually
> > inclined to just stick to the current, more straightforward code, and
> > require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been around for
> > several years now. It's not available on all the popular platforms and
> > distributions yet, but I don't want to bend over backwards to support
> those.
>
> I think that this mainly boils down to how much Postgres JDBC wants to
> get support here as some vendors can maintain oldest versions of OpenSSL
> for a long time.  The extra code is not that much complicated by the
> way, still it is true that HEAD is cleaner with its simplicity.
>
>
I'm unclear what this has to do with JDBC ? JDBC doesn't use OpenSSL

Alvaro ?



Dave Cramer

da...@postgresintl.com
www.postgresintl.com

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-23 Thread Bruce Momjian 
On Wed, Jun  6, 2018 at 01:16:11PM -0700, Steven Fackler wrote:
> TLS 1.3, (which is currently in a draft state, but is theoretically being
> finalized soon) does not support the TLS channel binding algorithms [1]. From

Uh, according to this article, TLS 1.3 was finalized in March:

  
https://www.theregister.co.uk/2018/03/27/with_tls_13_signed_off_its_implementation_time/

-- 
  Bruce Momjian  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 +

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-18 Thread Michael Paquier 
On Sat, Jun 09, 2018 at 09:28:17AM +0900, Michael Paquier wrote:
> I am still not completely sure what is the correct course of action
> here.  Heikki and Peter and not much in favor of adding more complexity
> here as OpenSSL has a long history of having a non-linear history across
> platforms.  On the other side, PGDG provides packages down to RHEL6, and
> there are surely servers which use it as backend.

As Peter and Heikki have worked as well on all those features with me,
are there any objection to discard this open item?  I looked again at
the patch this morning and it is true that OpenSSL's history makes
things harder, so keeping code consistent and simple with their last LTS
version is appealing.
--
Michael


signature.asc
Description: PGP signature

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-11 Thread Michael Paquier 
On Mon, Jun 11, 2018 at 10:47:23AM -0400, Peter Eisentraut wrote:
> I think we'll just have to wait for an updated RFC on channel bindings
> for TLS 1.3.
> 
> Perhaps we should change PostgreSQL 11 to not advertise channel binding
> when TLS 1.3 is used?

Yeah, that's what we should do and I would vote for doing nothing as
long as we are not sure how the TLS is shaped at the end, as we could as
well be able to use only be-tls-end-point so -PLUS can be advertised.

From a technical point of view, the decision-making can happen with
Port->ssl->version by looking for TLS1_3_VERSION which is new as of
OpenSSL 1.1.1 so that's very fresh (1.1.1 beta 5 is out as of today).
--
Michael


signature.asc
Description: PGP signature

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-11 Thread Peter Eisentraut 
On 6/6/18 16:16, Steven Fackler wrote:
> TLS 1.3, (which is currently in a draft state, but is theoretically
> being finalized soon) does not support the TLS channel binding
> algorithms [1]. From talking with one of the people working on the TLS
> 1.3 standard, tls-unique is seen as particularly problematic. There's
> some discussion on the IETF mailing lists from a couple of years ago [2].

I think we'll just have to wait for an updated RFC on channel bindings
for TLS 1.3.

Perhaps we should change PostgreSQL 11 to not advertise channel binding
when TLS 1.3 is used?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-06 Thread Steven Fackler 
On Wed, Jun 6, 2018 at 2:21 PM Michael Paquier  wrote:

Thanks for the pointers, Steven.  You should avoid top-posting on this
> list, this is not the style used on the Postgres lists.
>

Ah sorry about that! Hopefully this looks better.


> Does this mean that tls-server-end-point goes into unsupported mode?
> The emails you mention (thanks!), talk about only tls-unique while the
> RFCs mention all channel binding types.
>

That's the part that I'm unsure about - tls-server-end-point doesn't seem
particularly objectionable. I asked for some clarification from the person
that I was talking to earlier.


> Please let me think about this one, I am not completely sure yet what
> that would mean for libpq and the backend code.
>

On the backend, you can use SSL_session_reused to check if a session was
resumed, and then use SSL_get_peer_finished if it wasn't and
SSL_get_finished if it was. The libpq frontend library doesn't need to
worry about it since it never attempts to reuse sessions.

Steven

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-06 Thread Michael Paquier 
On Wed, Jun 06, 2018 at 01:16:11PM -0700, Steven Fackler wrote:

Thanks for the pointers, Steven.  You should avoid top-posting on this
list, this is not the style used on the Postgres lists.

> TLS 1.3, (which is currently in a draft state, but is theoretically being
> finalized soon) does not support the TLS channel binding algorithms [1].
> From talking with one of the people working on the TLS 1.3 standard,
> tls-unique is seen as particularly problematic. There's some discussion on
> the IETF mailing lists from a couple of years ago [2].

Well, it seems that we are going this API to decide if an SSL
implementation supports channel binding or not then:
https://www.postgresql.org/message-id/20180122072936.GB1772%40paquier.xyz
And for TLS 1.3 it would need to use SSL_get_version() or such with the
connection context.

> Ignoring that line of the draft, the current tls-unique implementation in
> Postgres is currently incorrect for TLS 1.3 handshakes anyway since the
> server sends the first Finished message rather than the client [3].

Does this mean that tls-server-end-point goes into unsupported mode?
The emails you mention (thanks!), talk about only tls-unique while the
RFCs mention all channel binding types.

> This is also the case for TLS 1.2 handshakes with session resumption [4].

Please let me think about this one, I am not completely sure yet what
that would mean for libpq and the backend code.
--
Michael


signature.asc
Description: PGP signature

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-06 Thread Steven Fackler 
TLS 1.3, (which is currently in a draft state, but is theoretically being
finalized soon) does not support the TLS channel binding algorithms [1].
>From talking with one of the people working on the TLS 1.3 standard,
tls-unique is seen as particularly problematic. There's some discussion on
the IETF mailing lists from a couple of years ago [2].

Ignoring that line of the draft, the current tls-unique implementation in
Postgres is currently incorrect for TLS 1.3 handshakes anyway since the
server sends the first Finished message rather than the client [3]. This is
also the case for TLS 1.2 handshakes with session resumption [4].

Steven

[1]: https://tools.ietf.org/html/draft-ietf-tls-tls13-28#appendix-C.5
[2]: https://www.ietf.org/mail-archive/web/tls/current/msg18257.html
[3]: https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-2
[4]: https://tools.ietf.org/html/rfc5246#section-7.3

On Wed, Jun 6, 2018 at 12:37 PM Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 6/6/18 12:37, Alvaro Herrera wrote:
> > If SCRAM channel binding is an important aspect to security, and the
> > older OpenSSL versions will still be around in servers for some time
> > yet, it seems like it behooves us to go the extra mile and provide an
> > implementation that works with such existing servers.  Looking at
> > yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
> > which appears to have openssl 1.0.0.
>
> There are two channel binding types: tls-unique and
> tls-server-end-point.  Of the two, tls-unique is the "better" one.  We
> do support that without a problem.  tls-server-end-point is for SSL
> implementations that cannot support tls-unique, because the SSL library
> does not expose the required information.  Most prominently, this is for
> JDBC.
>
> So currently, we support channel binding using tls-unique just fine
> between libpq and a server.  And we support tls-server-end-point between
> JDBC and a server using new-ish OpenSSL.  We don't support any channel
> binding between for example JDBC and a server on CentOS 6.  But that's
> not a regression, it's just not there.
>
> As Heikki was saying, the proposed patch seems to tread into the
> portability problem territory that caused the previous attempt to fail
> and had to be reverted.  I am not that interested in trying that again
> without new insights.  I don't think we are going to do ourselves a
> favor if we start meddling with that again.  There are dozens of OpenSSL
> variants out there, and the version history is nonlinear.
>
> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-06 Thread Peter Eisentraut 
On 6/6/18 12:37, Alvaro Herrera wrote:
> If SCRAM channel binding is an important aspect to security, and the
> older OpenSSL versions will still be around in servers for some time
> yet, it seems like it behooves us to go the extra mile and provide an
> implementation that works with such existing servers.  Looking at
> yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
> which appears to have openssl 1.0.0.

There are two channel binding types: tls-unique and
tls-server-end-point.  Of the two, tls-unique is the "better" one.  We
do support that without a problem.  tls-server-end-point is for SSL
implementations that cannot support tls-unique, because the SSL library
does not expose the required information.  Most prominently, this is for
JDBC.

So currently, we support channel binding using tls-unique just fine
between libpq and a server.  And we support tls-server-end-point between
JDBC and a server using new-ish OpenSSL.  We don't support any channel
binding between for example JDBC and a server on CentOS 6.  But that's
not a regression, it's just not there.

As Heikki was saying, the proposed patch seems to tread into the
portability problem territory that caused the previous attempt to fail
and had to be reverted.  I am not that interested in trying that again
without new insights.  I don't think we are going to do ourselves a
favor if we start meddling with that again.  There are dozens of OpenSSL
variants out there, and the version history is nonlinear.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-06 Thread Tom Lane 
Alvaro Herrera  writes:
> If SCRAM channel binding is an important aspect to security, and the
> older OpenSSL versions will still be around in servers for some time
> yet, it seems like it behooves us to go the extra mile and provide an
> implementation that works with such existing servers.  Looking at
> yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
> which appears to have openssl 1.0.0.

Not sure if the difference is relevant here, but an up-to-date RHEL6
installation contains 1.0.1e:

$ rpm -q openssl
openssl-1.0.1e-57.el6.x86_64
openssl-1.0.1e-57.el6.i686

regards, tom lane

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-06-06 Thread Alvaro Herrera 
On 2018-May-29, Michael Paquier wrote:

> On Tue, May 29, 2018 at 10:33:03PM -0400, Heikki Linnakangas wrote:
> > Hmm. I think Peter went through this in commits ac3ff8b1d8 and 054e8c6cdb.
> > If you got that working now, I suppose we could do that, but I'm actually
> > inclined to just stick to the current, more straightforward code, and
> > require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been around for
> > several years now. It's not available on all the popular platforms and
> > distributions yet, but I don't want to bend over backwards to support those.
> 
> I think that this mainly boils down to how much Postgres JDBC wants to
> get support here as some vendors can maintain oldest versions of OpenSSL
> for a long time.  The extra code is not that much complicated by the
> way, still it is true that HEAD is cleaner with its simplicity.
> 
> Steven, as the initial reporter, what's your take?

If SCRAM channel binding is an important aspect to security, and the
older OpenSSL versions will still be around in servers for some time
yet, it seems like it behooves us to go the extra mile and provide an
implementation that works with such existing servers.  Looking at
yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
which appears to have openssl 1.0.0.

Anyway, even if I'm wrong, this thread has stalled.  I hereby sprinkle
this thread with magic RMT dust for it to get fixed soon.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-05-29 Thread Heikki Linnakangas 

On 29/05/18 17:02, Michael Paquier wrote:

Currently, the SCRAM channel binding tls-server-end-point is supported
only with OpenSSL 1.0.2 and newer versions as we rely on
X509_get_signature_nid to get the certificate signature ID, which is the
official way of upstream to get this information as all the contents of
X509 are shadowed since this version.


Hmm. I think Peter went through this in commits ac3ff8b1d8 and 
054e8c6cdb. If you got that working now, I suppose we could do that, but 
I'm actually inclined to just stick to the current, more straightforward 
code, and require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been 
around for several years now. It's not available on all the popular 
platforms and distributions yet, but I don't want to bend over backwards 
to support those.


[1] 
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ac3ff8b1d8f98da38c53a701e6397931080a39cf
[2] 
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=054e8c6cdb7f4261869e49d3ed7705cca475182e


- Heikki

Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

2018-05-29 Thread Michael Paquier 
Hi all,

Currently, the SCRAM channel binding tls-server-end-point is supported
only with OpenSSL 1.0.2 and newer versions as we rely on
X509_get_signature_nid to get the certificate signature ID, which is the
official way of upstream to get this information as all the contents of
X509 are shadowed since this version.

I think that this matters for users of JDBC with environments shipping
and maintaining longer than upstream versions of OpenSSL like 1.0.1 and
1.0.0 (Redhat is one of them).

Stephen Fackler, who is in CC, has reported to me off-list that it is
possible to get easily this information by looking at
X509->sig_alg->algorithm, which to easily get tls-server-end-point
support for OpenSSL 1.0.0 and 1.0.1.

The official way to get the hashing algorithm is then to apply
OBJ_find_sigid_algs, which is only available since 1.0.0.  Stephen has
also pointed me out that we could use EVP_get_digestbynid, but it is
surprising to see that working correctly if you particularly look at
upstream commit ee1d9ec which introduced a sort of hack to fix links
between the digest and signature algorithms in OpenSSL 1.0.0 and above.
Anyway, it seems to me that it would not be a good bet to rely on an
upstream hack as we may get trapped later on if the handlings for
EVP_get_digestbynid get reworked by upstream again.  So I propose to
rely on OBJ_find_sigid_algs which is the official interface to get the
signature algorithm as far as I understand.

While hacking on that, I also noticed that OBJ_find_sigid_algs can crash
when using OpenSSL 1.0.0 if a NULL input is used with its 2nd and 3rd
arguments, which is an issue fixed by upstream in commit 177f27d,
present since 1.0.1 but bit 1.0.0, so we need an idle field to get that
to work properly with OpenSSL 1.0.0.

Attached is the result of this investigation, which I have tested using
OpenSSL from 0.9.8 to 1.0.2 compiled manually.  0.9.8 fails any
connection attempt with tls-server-end-point, while 1.0.0, 1.0.1 and
1.0.2 succeed properly.

An open item is added as well.

One of the versions I discussed with Stephen is here by the way, and I
am attaching it to this thread as well for transparency (see
end-point-support-stephen.patch):
https://github.com/sfackler/postgres/commit/93383be85358a1ee03f34f34de45058e238964d1

Thanks,
--
Michael
From 93383be85358a1ee03f34f34de45058e238964d1 Mon Sep 17 00:00:00 2001
From: Steven Fackler 
Date: Mon, 28 May 2018 14:53:38 -0700
Subject: [PATCH] Support tls-server-end-point on old OpenSSL

X509_get_signature_nid was added in 1.0.2, but the fields required to
get to the signare algorithm are public on versions before that, so we
can just shim out the function when missing.

OBJ_find_sigid_algs was added in 1.0.0, but (somewhat surprisingly)
EVP_get_digestbynid works when given the NID for a signature algorithm
so we can use that instead.
---
 src/backend/libpq/be-secure-openssl.c| 34 ++--
 src/interfaces/libpq/fe-secure-openssl.c | 40 +++-
 src/test/ssl/t/002_scram.pl  | 24 +++---
 3 files changed, 39 insertions(+), 59 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 48b468f62f..53660105ec 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1123,16 +1123,26 @@ be_tls_get_peer_finished(Port *port, size_t *len)
 	return result;
 }
 
+#ifndef HAVE_X509_GET_SIGNATURE_NID
+/*
+ * This function was added in OpenSSL 1.0.2, but all of the fields it accesses
+ * are public in older versions, so we can just redefine it when missing.
+ */
+static int
+X509_get_signature_nid(const X509 *x)
+{
+	return OBJ_obj2nid(x->sig_alg->algorithm);
+}
+#endif
+
 char *
 be_tls_get_certificate_hash(Port *port, size_t *len)
 {
-#ifdef HAVE_X509_GET_SIGNATURE_NID
 	X509	   *server_cert;
 	char	   *cert_hash;
-	const EVP_MD *algo_type = NULL;
+	const EVP_MD *algo_type;
 	unsigned char hash[EVP_MAX_MD_SIZE];	/* size for SHA-512 */
 	unsigned int hash_size;
-	int			algo_nid;
 
 	*len = 0;
 	server_cert = SSL_get_certificate(port->ssl);
@@ -1143,8 +1153,8 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
 	 * Get the signature algorithm of the certificate to determine the hash
 	 * algorithm to use for the result.
 	 */
-	if (!OBJ_find_sigid_algs(X509_get_signature_nid(server_cert),
-			 _nid, NULL))
+	algo_type = EVP_get_digestbynid(X509_get_signature_nid(server_cert));
+	if (!algo_type)
 		elog(ERROR, "could not determine server certificate signature algorithm");
 
 	/*
@@ -1153,18 +1163,12 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
 	 * (https://tools.ietf.org/html/rfc5929#section-4.1).  If something else
 	 * is used, the same hash as the signature algorithm is used.
 	 */
-	switch (algo_nid)
+	switch (EVP_MD_type(algo_type))
 	{
 		case NID_md5:
 		case NID_sha1:
 			algo_type = EVP_sha256();
 			break;
-		default:
-			algo_type = EVP_get_digestbynid(algo_nid);
-			if