Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Implement channel binding tls-server-end-point for SCRAM

  1. pgsql: Implement channel binding tls-server-end-point for SCRAM

    Peter Eisentraut <peter_e@gmx.net> — 2018-01-04T20:36:41Z

    Implement channel binding tls-server-end-point for SCRAM

This adds a second standard channel binding type for SCRAM.  It is
mainly intended for third-party clients that cannot implement
tls-unique, for example JDBC.

Author: Michael Paquier <michael.paquier@gmail.com>

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/d3fb72ea6de58d285e278459bca9d7cdf7f6a38b

Modified Files
--------------
doc/src/sgml/protocol.sgml               | 17 ++++---
src/backend/libpq/auth-scram.c           | 20 ++++++--
src/backend/libpq/be-secure-openssl.c    | 61 ++++++++++++++++++++++++
src/include/common/scram-common.h        |  1 +
src/include/libpq/libpq-be.h             |  1 +
src/interfaces/libpq/fe-auth-scram.c     | 15 ++++++
src/interfaces/libpq/fe-secure-openssl.c | 80 ++++++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-int.h         |  1 +
src/test/ssl/t/002_scram.pl              |  5 +-
9 files changed, 189 insertions(+), 12 deletions(-)

  2. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-01-04T21:02:56Z

    Peter Eisentraut <peter_e@gmx.net> writes:
> Implement channel binding tls-server-end-point for SCRAM

Buildfarm doesn't like this one bit.

			regards, tom lane

  3. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-01-04T21:09:53Z

    On Fri, Jan 5, 2018 at 9:36 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> Implement channel binding tls-server-end-point for SCRAM

FYI some BF animals are saying:

libpq/be-secure-openssl.o: In function `be_tls_get_certificate_hash':
/home/pgbuildfarm/buildroot-termite/HEAD/pgsql.build/../pgsql/src/backend/libpq/be-secure-openssl.c:1268:
undefined reference to `X509_get_signature_nid'

-- 
Thomas Munro
http://www.enterprisedb.com

  4. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Robert Haas <robertmhaas@gmail.com> — 2018-01-05T13:47:44Z

    On Thu, Jan 4, 2018 at 4:09 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Fri, Jan 5, 2018 at 9:36 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> Implement channel binding tls-server-end-point for SCRAM
>
> FYI some BF animals are saying:
>
> libpq/be-secure-openssl.o: In function `be_tls_get_certificate_hash':
> /home/pgbuildfarm/buildroot-termite/HEAD/pgsql.build/../pgsql/src/backend/libpq/be-secure-openssl.c:1268:
> undefined reference to `X509_get_signature_nid'

The SSL tests on chipmunk failed in the last run.  I assume that's
probably the fault of this patch, or one of the follow-on commits:

# Running: psql -X -A -t -c SELECT 'connected with user=ssltestuser
dbname=trustdb sslmode=require hostaddr=127.0.0.1
scram_channel_binding=tls-server-end-point' -d user=ssltestuser
dbname=trustdb sslmode=require hostaddr=127.0.0.1
scram_channel_binding=tls-server-end-point
psql: channel binding type "tls-server-end-point" is not supported by this build
not ok 4 - SCRAM authentication with tls-server-end-point as channel binding

#   Failed test 'SCRAM authentication with tls-server-end-point as
channel binding'
#   at /home/pgbfarm/buildroot/HEAD/pgsql.build/src/test/ssl/ServerSetup.pm
line 64.
# Running: psql -X -A -t -c SELECT 'connected with user=ssltestuser
dbname=trustdb sslmode=require hostaddr=127.0.0.1
scram_channel_binding=not-exists' -d user=ssltestuser dbname=trustdb
sslmode=require hostaddr=127.0.0.1 scram_channel_binding=not-exists
psql: FATAL:  unsupported SCRAM channel-binding type
ok 5 - SCRAM authentication with invalid channel binding
### Stopping node "master" using mode immediate
# Running: pg_ctl -D
/home/pgbfarm/buildroot/HEAD/pgsql.build/src/test/ssl/tmp_check/t_002_scram_master_data/pgdata
-m immediate stop
waiting for server to shut down.... done
server stopped
# No postmaster PID for node "master"
# Looks like you failed 1 test of 5.

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

  5. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Michael Paquier <michael.paquier@gmail.com> — 2018-01-05T14:28:53Z

    On Fri, Jan 5, 2018 at 10:47 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> The SSL tests on chipmunk failed in the last run.  I assume that's
> probably the fault of this patch, or one of the follow-on commits:

Thanks for the heads-up, Robert. I did not notice the failure. That's
the fault of 054e8c6c. Raspbian is using OpenSSL 1.0.1t (package list
can be downloaded in
http://archive.raspbian.org/raspbian/dists/wheezy/main/binary-armhf/Packages
for 38MB), which does not have the necessary facilities to implement
tls-server-end-point as upstream has added necessary APIs only in
1.0.2.

In order to do things cleanly, we should make this TAP test
conditional on the version of OpenSSL. There have been discussions in
the past to make a module dedicated to that, but no clear patch or
approach has showed up. This can be retrieved with SSLeay_version() or
"openssl version", but that seems not fun nor stable to rely on
openssl to be in PATH. I don't see disabling this test helping either,
but we could consider that without an appropriate module to track
dependencies in a build with its versions. I would be personally fine
with having an environment variable switch I could use to enable the
test as well as I use already a script to run all regression tests in
the tree (src/test/ssl is not run by default as it is unsecure for
shared environments, without counting on meltdowns).

Thoughts from others?
-- 
Michael

  6. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-01-05T14:56:37Z

    On 1/5/18 09:28, Michael Paquier wrote:
> In order to do things cleanly, we should make this TAP test
> conditional on the version of OpenSSL.

How about looking into pg_config.h, like in the attached patch?

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

  7. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2018-01-05T17:56:27Z

    Peter Eisentraut wrote:
> On 1/5/18 09:28, Michael Paquier wrote:
> > In order to do things cleanly, we should make this TAP test
> > conditional on the version of OpenSSL.
> 
> How about looking into pg_config.h, like in the attached patch?

I suppose if this starts to spread, we'll come up with a better approach
... maybe generating a Perl file with variables that can be slurped more
ellegantly, or something like that.  (We mentioned the need for
config-based test selection re. patches for new SSL implementations.)

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

  8. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Michael Paquier <michael.paquier@gmail.com> — 2018-01-06T00:10:51Z

    On Sat, Jan 6, 2018 at 2:56 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Peter Eisentraut wrote:
>> On 1/5/18 09:28, Michael Paquier wrote:
>> > In order to do things cleanly, we should make this TAP test
>> > conditional on the version of OpenSSL.
>>
>> How about looking into pg_config.h, like in the attached patch?

+# Determine whether build supports tls-server-end-point.
+open my $pg_config_h, '<', '../../include/pg_config.h' or die "$!";
+my $supports_tls_server_end_point = (grep {/^#define
HAVE_X509_GET_SIGNATURE_NID 1/} <$pg_config_h>);
+close $pg_config_h;

Good idea as a whole, but I don't think this is the right approach. As
we include $(bindir) in PATH when running the prove command, why not
getting the include path from "pg_config --includedir"?

> I suppose if this starts to spread, we'll come up with a better approach
> ... maybe generating a Perl file with variables that can be slurped more
> ellegantly, or something like that.  (We mentioned the need for
> config-based test selection re. patches for new SSL implementations.)

One case I have in mind where this would be nice to have is
020_pg_receivewal.pl to have tests depending on if PG is built with
zlib or not. So we definitely want something more. At least I do. I
agree that the most elegant approach would be to generate pg_config.h
from this variable set, and not feed on parsing pg_config.h directly.
Or we could just live with an API in TestLib.pm which is able to get
the wanted information as Peter is doing but for a wanted variable
from pg_config. I could use that for my test case with HAVE_LIBZ as
well.
--
Michael

  9. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Michael Paquier <michael.paquier@gmail.com> — 2018-01-08T13:14:11Z

    On Sat, Jan 06, 2018 at 09:10:51AM +0900, Michael Paquier wrote:
> On Sat, Jan 6, 2018 at 2:56 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Good idea as a whole, but I don't think this is the right approach. As
> we include $(bindir) in PATH when running the prove command, why not
> getting the include path from "pg_config --includedir"?

So, I have been looking at that, and I propose the following
counter-patch which implements this idea by adding a new routine as
TestLib::config_check which is able to check within pg_config.h if a
given regexp matches or not for the installation on which TAP tests are
being run. I have tested with Postgres compiled with both OpenSSL 1.0.1
and 1.0.2, in which case the connection test respectively fails and
passes, causing the test to be correctly handled. This is based on
Peter's patch upthread.
--
Michael

  10. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Simon Riggs <simon@2ndquadrant.com> — 2018-01-08T15:44:11Z

    On 4 January 2018 at 21:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> Implement channel binding tls-server-end-point for SCRAM
>
> Buildfarm doesn't like this one bit.

Can't we automate these messages? Seems strange to send manual emails
every time. We do know who the commits are coming from and we have
their email address.

It would be useful to get automatic message giving a summary of
buildfarm results at 15, 30 and 60 minute intervals, even if it is
just ALL CLEAR.

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

  11. Buildfarm status monitoring (was Re: pgsql: Implement channel binding tls-server-end-point for SCRAM)

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-01-08T16:01:53Z

    Simon Riggs <simon@2ndquadrant.com> writes:
> On 4 January 2018 at 21:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Peter Eisentraut <peter_e@gmx.net> writes:
>>> Implement channel binding tls-server-end-point for SCRAM

>> Buildfarm doesn't like this one bit.

> Can't we automate these messages?

It's not that easy.  First, the buildfarm gets random failures all the
time, due to this and that.  Second, if several commits have occurred
since the critter's last run, it requires some human judgment to figure
out which commit is probably to blame.

You could ameliorate the first problem by waiting for multiple failures
to show up ... but the longer you wait, the worse the second problem
becomes (and the less useful the report would be anyway).

> It would be useful to get automatic message giving a summary of
> buildfarm results at 15, 30 and 60 minute intervals, even if it is
> just ALL CLEAR.

The raw result of that would be too noisy to be useful.  I've wondered
about getting the buildfarm status page to filter out the more obvious
classes of "random failure" --- git pull failures would be one, and
another would be if "no space left on device" appears anywhere in any
of the report's log files.  Don't know how far that would get us, though.

			regards, tom lane

  12. Re: Buildfarm status monitoring (was Re: pgsql: Implement channel binding tls-server-end-point for SCRAM)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-01-08T17:02:52Z

    
On 01/08/2018 11:01 AM, Tom Lane wrote:
> Simon Riggs <simon@2ndquadrant.com> writes:
>> On 4 January 2018 at 21:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Peter Eisentraut <peter_e@gmx.net> writes:
>>>> Implement channel binding tls-server-end-point for SCRAM
>>> Buildfarm doesn't like this one bit.
>> Can't we automate these messages?
> It's not that easy.  First, the buildfarm gets random failures all the
> time, due to this and that.  Second, if several commits have occurred
> since the critter's last run, it requires some human judgment to figure
> out which commit is probably to blame.
>
> You could ameliorate the first problem by waiting for multiple failures
> to show up ... but the longer you wait, the worse the second problem
> becomes (and the less useful the report would be anyway).
>
>> It would be useful to get automatic message giving a summary of
>> buildfarm results at 15, 30 and 60 minute intervals, even if it is
>> just ALL CLEAR.
> The raw result of that would be too noisy to be useful.  I've wondered
> about getting the buildfarm status page to filter out the more obvious
> classes of "random failure" --- git pull failures would be one, and
> another would be if "no space left on device" appears anywhere in any
> of the report's log files.  Don't know how far that would get us, though.
>
> 			



Without triangulating via something like git-bisect I suspect we'd very
soon find any automated system very tiresome indeed.

cheers

andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  13. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-01-09T17:49:11Z

    On 1/8/18 08:14, Michael Paquier wrote:
> On Sat, Jan 06, 2018 at 09:10:51AM +0900, Michael Paquier wrote:
>> On Sat, Jan 6, 2018 at 2:56 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> Good idea as a whole, but I don't think this is the right approach. As
>> we include $(bindir) in PATH when running the prove command, why not
>> getting the include path from "pg_config --includedir"?
> 
> So, I have been looking at that, and I propose the following
> counter-patch which implements this idea by adding a new routine as
> TestLib::config_check which is able to check within pg_config.h if a
> given regexp matches or not for the installation on which TAP tests are
> being run. I have tested with Postgres compiled with both OpenSSL 1.0.1
> and 1.0.2, in which case the connection test respectively fails and
> passes, causing the test to be correctly handled. This is based on
> Peter's patch upthread.

Committed.  I like counter-patches.

(I renamed the function a bit to check_pg_config, to make the naming
similar to other functions in TestLib.)

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

  14. Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

    Michael Paquier <michael.paquier@gmail.com> — 2018-01-10T01:19:08Z

    On Tue, Jan 09, 2018 at 12:49:11PM -0500, Peter Eisentraut wrote:
> Committed.  I like counter-patches.
> 
> (I renamed the function a bit to check_pg_config, to make the naming
> similar to other functions in TestLib.)

Thanks for committing. I think that you should have authorship as well,
which is something that c3d41ccf does not mention directly.
--
Michael