On Mon, 2019-08-19 at 14:51 +0900, Michael Paquier wrote:
> On Fri, Aug 16, 2019 at 02:11:57PM -0400, Jonathan S. Katz wrote:
> > To be pedantic, +1 on the channel_binding param.
>
> Seems like we are moving in this direction then. I don't object to
> the introduction of this parameter.
OK, new patch attached. Seems like everyone is in agreement that we
need a channel_binding param.
Regards,
Jeff Davis
From 0eb8e76d08a64979c3070761efab95d61c4ff887 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Tue, 20 Aug 2019 18:59:23 -0700
Subject: [PATCH] Add libpq parameter 'channel_binding'.
Allow clients to require channel binding to enhance security against
untrusted servers.
Author: Jeff Davis
Discussion: https://postgr.es/m/227015d8417f2b4fef03f8966dbfa5cbcc4f44da.camel%40j-davis.com
---
doc/src/sgml/libpq.sgml | 19 +++++++++++++++++++
src/interfaces/libpq/fe-auth.c | 30 +++++++++++++++++++++++++++++-
src/interfaces/libpq/fe-connect.c | 28 ++++++++++++++++++++++++++++
src/interfaces/libpq/libpq-int.h | 2 ++
4 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index e7295abda28..1785cb1bcc5 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1118,6 +1118,25 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</listitem>
</varlistentry>
+ <varlistentry id="libpq-connect-channel-binding" xreflabel="channel_binding">
+ <term><literal>channel_binding</literal></term>
+ <listitem>
+ <para>
+ A setting of <literal>require</literal> means that the connection must
+ employ channel binding; and that the client will not respond to
+ requests from the server for cleartext password or MD5
+ authentication. The default setting is <literal>prefer</literal>,
+ which employs channel binding if available.
+ </para>
+ <para>
+ Channel binding is a method for the server to authenticate itself to
+ the client. It is only supported over SSL connections
+ with <productname>PostgreSQL</productname> 11.0 or later servers using
+ the <literal>scram-sha-256</literal> authentication method.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-connect-connect-timeout" xreflabel="connect_timeout">
<term><literal>connect_timeout</literal></term>
<listitem>
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index ab227421b3b..5df30913337 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -423,6 +423,13 @@ pg_SASL_init(PGconn *conn, int payloadlen)
initPQExpBuffer(&mechanism_buf);
+ if (conn->channel_binding[0] == 'r' && !conn->ssl_in_use)
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("Channel binding required, but SSL not in use\n"));
+ goto error;
+ }
+
if (conn->sasl_state)
{
printfPQExpBuffer(&conn->errorMessage,
@@ -488,7 +495,8 @@ pg_SASL_init(PGconn *conn, int payloadlen)
goto error;
}
}
- else if (strcmp(mechanism_buf.data, SCRAM_SHA_256_NAME) == 0 &&
+ else if (conn->channel_binding[0] != 'r' &&
+ strcmp(mechanism_buf.data, SCRAM_SHA_256_NAME) == 0 &&
!selected_mechanism)
selected_mechanism = SCRAM_SHA_256_NAME;
}
@@ -565,6 +573,9 @@ pg_SASL_init(PGconn *conn, int payloadlen)
if (initialresponse)
free(initialresponse);
+ if (strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0)
+ conn->channel_bound = true;
+
return STATUS_OK;
error:
@@ -791,6 +802,12 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
switch (areq)
{
case AUTH_REQ_OK:
+ if (conn->channel_binding[0] == 'r' && !conn->channel_bound)
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("Channel binding required but not offered by server\n"));
+ return STATUS_ERROR;
+ }
break;
case AUTH_REQ_KRB4:
@@ -919,6 +936,17 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
{
char *password;
+ if (conn->channel_binding[0] == 'r')
+ {
+ if (areq == AUTH_REQ_PASSWORD)
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("Channel binding required but server requested cleartext password authentication\n"));
+ else
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("Channel binding required but server requested MD5 authentication\n"));
+ return STATUS_ERROR;
+ }
+
conn->password_needed = true;
password = conn->connhost[conn->whichhost].password;
if (password == NULL)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index fa5af18ffac..e9627c06ec1 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 DefaultTty ""
#define DefaultOption ""
#define DefaultAuthtype ""
+#define DefaultChannelBinding "prefer"
#define DefaultTargetSessionAttrs "any"
#ifdef USE_SSL
#define DefaultSSLMode "prefer"
@@ -211,6 +212,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"Database-Password-File", "", 64,
offsetof(struct pg_conn, pgpassfile)},
+ {"channel_binding", "PGCHANNELBINDING", NULL, NULL,
+ "Channel-Binding", "", 7, /* sizeof("require") */
+ offsetof(struct pg_conn, channel_binding)},
+
{"connect_timeout", "PGCONNECT_TIMEOUT", NULL, NULL,
"Connect-timeout", "", 10, /* strlen(INT32_MAX) == 10 */
offsetof(struct pg_conn, connect_timeout)},
@@ -556,6 +561,7 @@ pqDropServerData(PGconn *conn)
conn->last_sqlstate[0] = '\0';
conn->auth_req_received = false;
conn->password_needed = false;
+ conn->channel_bound = false;
conn->write_failed = false;
if (conn->write_err_msg)
free(conn->write_err_msg);
@@ -1197,6 +1203,28 @@ connectOptions2(PGconn *conn)
}
}
+ /*
+ * validate channel_binding option
+ */
+ if (conn->channel_binding)
+ {
+ if (strcmp(conn->channel_binding, "prefer") != 0
+ && strcmp(conn->channel_binding, "require") != 0)
+ {
+ conn->status = CONNECTION_BAD;
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid channel_binding value: \"%s\"\n"),
+ conn->channel_binding);
+ return false;
+ }
+ }
+ else
+ {
+ conn->channel_binding = strdup(DefaultChannelBinding);
+ if (!conn->channel_binding)
+ goto oom_error;
+ }
+
/*
* validate sslmode option
*/
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index d37bb3ce404..70a5b445d72 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -347,6 +347,7 @@ struct pg_conn
char *pguser; /* Postgres username and password, if any */
char *pgpass;
char *pgpassfile; /* path to a file containing password(s) */
+ char *channel_binding; /* channel binding mode (require,prefer) */
char *keepalives; /* use TCP keepalives? */
char *keepalives_idle; /* time between TCP keepalives */
char *keepalives_interval; /* time between TCP keepalive
@@ -410,6 +411,7 @@ struct pg_conn
int sversion; /* server version, e.g. 70401 for 7.4.1 */
bool auth_req_received; /* true if any type of auth req received */
bool password_needed; /* true if server demanded a password */
+ bool channel_bound; /* true if channel_binding happened */
bool sigpipe_so; /* have we masked SIGPIPE via SO_NOSIGPIPE? */
bool sigpipe_flag; /* can we mask SIGPIPE via MSG_NOSIGNAL? */
bool write_failed; /* have we had a write failure on sock? */
--
2.17.1
