pgsql: Remove support for tls-unique channel binding. - Mailing list pgsql-committers

From Heikki Linnakangas
Subject pgsql: Remove support for tls-unique channel binding.
Date
Msg-id E1fmGki-0003dN-6d@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Remove support for tls-unique channel binding.

There are some problems with the tls-unique channel binding type. It's not
supported by all SSL libraries, and strictly speaking it's not defined for
TLS 1.3 at all, even though at least in OpenSSL, the functions used for it
still seem to work with TLS 1.3 connections. And since we had no
mechanism to negotiate what channel binding type to use, there would be
awkward interoperability issues if a server only supported some channel
binding types. tls-server-end-point seems feasible to support with any SSL
library, so let's just stick to that.

This removes the scram_channel_binding libpq option altogether, since there
is now only one supported channel binding type.

This also removes all the channel binding tests from the SSL test suite.
They were really just testing the scram_channel_binding option, which
is now gone. Channel binding is used if both client and server support it,
so it is used in the existing tests. It would be good to have some tests
specifically for channel binding, to make sure it really is used, and the
different combinations of a client and a server that support or doesn't
support it. The current set of settings we have make it hard to write such
tests, but I did test those things manually, by disabling
HAVE_BE_TLS_GET_CERTIFICATE_HASH and/or
HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH.

I also removed the SCRAM_CHANNEL_BINDING_TLS_END_POINT constant. This is a
matter of taste, but IMO it's more readable to just use the
"tls-server-end-point" string.

Refactor the checks on whether the SSL library supports the functions
needed for tls-server-end-point channel binding. Now the server won't
advertise, and the client won't choose, the SCRAM-SHA-256-PLUS variant, if
compiled with an OpenSSL version too old to support it.

In the passing, add some sanity checks to check that the chosen SASL
mechanism, SCRAM-SHA-256 or SCRAM-SHA-256-PLUS, matches whether the SCRAM
exchange used channel binding or not. For example, if the client selects
the non-channel-binding variant SCRAM-SHA-256, but in the SCRAM message
uses channel binding anyway. It's harmless from a security point of view,
I believe, and I'm not sure if there are some other conditions that would
cause the connection to fail, but it seems better to be strict about these
things and check explicitly.

Discussion: https://www.postgresql.org/message-id/ec787074-2305-c6f4-86aa-6902f98485a4%40iki.fi

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/77291139c7c1dffac61e8db88cef98933677db18

Modified Files
--------------
doc/src/sgml/libpq.sgml                  |  28 ----
doc/src/sgml/protocol.sgml               |  26 +---
doc/src/sgml/release-11.sgml             |   5 +-
src/backend/libpq/auth-scram.c           | 223 +++++++++++++++++++++----------
src/backend/libpq/auth.c                 |  62 +++------
src/backend/libpq/be-secure-openssl.c    |  27 +---
src/include/common/scram-common.h        |   4 -
src/include/libpq/libpq-be.h             |  15 +--
src/include/libpq/scram.h                |   4 +-
src/interfaces/libpq/fe-auth-scram.c     |  79 ++++-------
src/interfaces/libpq/fe-auth.c           |  25 +++-
src/interfaces/libpq/fe-connect.c        |   8 --
src/interfaces/libpq/fe-secure-openssl.c |  28 +---
src/interfaces/libpq/libpq-int.h         |  15 +--
src/test/ssl/t/002_scram.pl              |  33 +----
15 files changed, 246 insertions(+), 336 deletions(-)


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Update version 11 release notes.
Next
From: Heikki Linnakangas
Date:
Subject: pgsql: Remove now unused check for HAVE_X509_GET_SIGNATURE_NID intest.