On Fri, May 18, 2018 at 10:46:46AM +0900, Michael Paquier wrote: > From a security point of view, 1) is important for libpq, but I am not > much enthusiast about 2) as a whole. The backend has proper support for > channel binding, hence other drivers speaking the protocol could have > their own restriction mechanisms.
So, I have been playing with libpq to address point 1), and added a new connection parameter called channel_binding_mode which can be set to 'prefer', which is what libpq uses now, and 'require'. The patch has two important parts: 1) If a server does not support channel binding, still it is sending back a SCRAM authentication, but the client still wants to enforce the use of channel binding, then libpq reacts as follows: $ psql -d "dbname=foo channel_binding_mode=require" psql: channel binding required for SASL authentication but no valid mechanism could be selected This requires pg_SASL_init() to be patched after the SASL mechanism has been selected. That error can be triggered with a v10 server with whom a SCRAM authentication is done, as well as with a v11 server where SSL is not used. Some people may use sslmode=prefer in combination to channel_binding_mode=require, in which case an error should be raised if the SSL connection cannot be achieved first. That addresses a bit of the craziness of sslmode=prefer... 2) If client wants to use channel binding, but the server is trying to enforce another protocol than SCRAM, like MD5, trust, gssapi or such, then the following error happens: $ psql -d "dbname=foo channel_binding_mode=require" psql: channel binding required for authentication but no valid protocol are used In this case, it seems to me that the best bet is to patch pg_fe_sendauth() and filter by message types. In the attached, I have added the parameter and some documentation. I have not added tests, but some things could be tested in the SSL suite: - Check for incorrect values in the parameter. - Test connection without SCRAM with "require" - Test connection without SSL but SCRAM with "require" I have not put much thoughts into the documentation, but the patch is rather simple so hopefully that helps in making progress in the discussion. -- Michael
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 498b8df988..aedc65b63f 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1238,6 +1238,49 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</listitem>
</varlistentry>
+ <varlistentry id="libpq-channel-binding-mode" xreflabel="channel_binding_mode">
+ <term><literal>channel_binding_mode</literal></term>
+ <listitem>
+ <para>
+ This option determines whether channel binding should be used for
+ authentication or not. As of now, channel binding is only supported
+ with SCRAM authentication. There are two modes:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>prefer</literal> (default)</term>
+ <listitem>
+ <para>
+ Try to use channel binding if available but rely on the server
+ depending on what types of SASL mechanisms are published during
+ the message exchange. This is the default, and it can be
+ considered as insecure as this does not protect from servers
+ attempting downgrade authentication attacks to a client.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>require</literal></term>
+ <listitem>
+ <para>
+ Reject any connection attempts to a server if channel binding
+ is not supported by the protocol used. Channel binding being
+ supported only for SCRAM in the context of an SSL connection,
+ a connection with this option enabled would fail except in this
+ authentication context. This protects from downgrade attacks
+ and ensures that a SCRAM connection with channel binding is
+ able to transparently achieve MITM protection with
+ <application>libpq</application>.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-scram-channel-binding" xreflabel="scram_channel_binding">
<term><literal>scram_channel_binding</literal></term>
<listitem>
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 3b2073a47f..e23ab83c1b 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -547,6 +547,25 @@ pg_SASL_init(PGconn *conn, int payloadlen)
goto error;
}
+ /*
+ * If client is willing to enforce the use the channel binding but
+ * it has not been previously selected, because it was either not
+ * published by the server or could not be selected, then complain
+ * back. If SSL is not used for this connection, still complain
+ * similarly, as the client may want channel binding but forgot
+ * to set it up to do so which could be the case with sslmode=prefer
+ * for example. This protects from any kind of downgrade attacks
+ * from rogue servers attempting to bypass channel binding.
+ */
+ if (conn->channel_binding_mode &&
+ strcmp(conn->channel_binding_mode, "require") == 0 &&
+ strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) != 0)
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("channel binding required for SASL authentication but no valid mechanism could be selected\n"));
+ goto error;
+ }
+
/*
* Now that the SASL mechanism has been chosen for the exchange,
* initialize its state information.
@@ -835,6 +854,24 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
int
pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
{
+ /*
+ * As of now, channel binding is only used for SASL authentication.
+ * Hence if a server is not willing to use channel binding but
+ * client wants to enforce its use, then check that valid
+ * message types only are used.
+ */
+ if (areq != AUTH_REQ_SASL &&
+ areq != AUTH_REQ_SASL_CONT &&
+ areq != AUTH_REQ_SASL_FIN &&
+ areq != AUTH_REQ_OK &&
+ conn->channel_binding_mode &&
+ strcmp(conn->channel_binding_mode, "require") == 0)
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("channel binding required for authentication but invalid protocol is used\n"));
+ return STATUS_ERROR;
+ }
+
switch (areq)
{
case AUTH_REQ_OK:
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index a7e969d7c1..b4df044ebf 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -124,6 +124,7 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
#define DefaultAuthtype ""
#define DefaultTargetSessionAttrs "any"
#define DefaultSCRAMChannelBinding SCRAM_CHANNEL_BINDING_TLS_UNIQUE
+#define DefaultChannelBindingMode "prefer"
#ifdef USE_SSL
#define DefaultSSLMode "prefer"
#else
@@ -269,6 +270,11 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
21, /* sizeof("tls-server-end-point") == 21 */
offsetof(struct pg_conn, scram_channel_binding)},
+ {"channel_binding_mode", NULL, DefaultChannelBindingMode, NULL,
+ "Channel-Binding-Mode", "D",
+ 8, /* sizeof("require") == 8 */
+ offsetof(struct pg_conn, channel_binding_mode)},
+
/*
* ssl options are allowed even without client SSL support because the
* client can still handle SSL modes "disable" and "allow". Other
@@ -1166,6 +1172,20 @@ connectOptions2(PGconn *conn)
goto oom_error;
}
+ /* validate channel_binding_mode option */
+ if (conn->channel_binding_mode)
+ {
+ if (strcmp(conn->channel_binding_mode, "prefer") != 0 &&
+ strcmp(conn->channel_binding_mode, "require") != 0)
+ {
+ conn->status = CONNECTION_BAD;
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid channel_binding_mode value: \"%s\"\n"),
+ conn->channel_binding_mode);
+ return false;
+ }
+ }
+
/*
* Resolve special "auto" client_encoding from the locale
*/
@@ -3478,6 +3498,8 @@ freePGconn(PGconn *conn)
free(conn->keepalives_count);
if (conn->scram_channel_binding)
free(conn->scram_channel_binding);
+ if (conn->channel_binding_mode)
+ free(conn->channel_binding_mode);
if (conn->sslmode)
free(conn->sslmode);
if (conn->sslcert)
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 9a586ff25a..9d4192b7d8 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -350,6 +350,7 @@ struct pg_conn
char *keepalives_count; /* maximum number of TCP keepalive
* retransmits */
char *scram_channel_binding; /* SCRAM channel binding type */
+ char *channel_binding_mode; /* channel binding mode */
char *sslmode; /* SSL mode (require,prefer,allow,disable) */
char *sslcompression; /* SSL compression (0 or 1) */
char *sslkey; /* client key filename */
signature.asc
Description: PGP signature
