On Fri, Mar 10, 2023 at 3:16 PM Jacob Champion <jchamp...@timescale.com> wrote:
> > Could you send a new patch with all these adjustments? That would
> > help a lot.
>
> Will do!
Here's a v16:
- updated 0001 patch message
- all test names should have commas rather than colons now
- new test for an empty require_auth
- new SSPI suite (note that it doesn't run by default on Cirrus, due
to the use of PG_TEST_USE_UNIX_SOCKETS)
- fixed errant comma at EOL
Thanks,
--Jacob
From 8cc020598c7c939e3a45088f18ca04ea801fc87e Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Tue, 18 Oct 2022 16:55:36 -0700
Subject: [PATCH v16 3/3] require_auth: decouple SASL and SCRAM
Rather than assume that an AUTH_REQ_SASL* code refers to SCRAM-SHA-256,
future-proof by separating the single allowlist into a list of allowed
authentication request codes and a list of allowed SASL mechanisms.
The require_auth check is now separated into two tiers. The
AUTH_REQ_SASL* codes are always allowed. If the server sends one,
responsibility for the check then falls to pg_SASL_init(), which
compares the selected mechanism against the list of allowed mechanisms.
(Other SCRAM code is already responsible for rejecting unexpected or
out-of-order AUTH_REQ_SASL_* codes, so that's not explicitly handled
with this addition.)
Since there's only one recognized SASL mechanism, conn->sasl_mechs
currently only points at static hardcoded lists. Whenever a second
mechanism is added, the list will need to be managed dynamically.
---
src/interfaces/libpq/fe-auth.c | 34 +++++++++++++++++++
src/interfaces/libpq/fe-connect.c | 41 +++++++++++++++++++----
src/interfaces/libpq/libpq-int.h | 3 +-
src/test/authentication/t/001_password.pl | 14 +++++---
src/test/ssl/t/002_scram.pl | 6 ++++
5 files changed, 86 insertions(+), 12 deletions(-)
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 53c7d30eff..7927aebed8 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -522,6 +522,40 @@ pg_SASL_init(PGconn *conn, int payloadlen)
goto error;
}
+ /*
+ * Before going ahead with any SASL exchange, ensure that the user has
+ * allowed (or, alternatively, has not forbidden) this particular mechanism.
+ *
+ * In a hypothetical future where a server responds with multiple SASL
+ * mechanism families, we would need to instead consult this list up above,
+ * during mechanism negotiation. We don't live in that world yet. The server
+ * presents one auth method and we decide whether that's acceptable or not.
+ */
+ if (conn->sasl_mechs)
+ {
+ const char **mech;
+ bool found = false;
+
+ Assert(conn->require_auth);
+
+ for (mech = conn->sasl_mechs; *mech; mech++)
+ {
+ if (strcmp(*mech, selected_mechanism) == 0)
+ {
+ found = true;
+ break;
+ }
+ }
+
+ if ((conn->sasl_mechs_denied && found)
+ || (!conn->sasl_mechs_denied && !found))
+ {
+ libpq_append_conn_error(conn, "auth method \"%s\" requirement failed: server requested unacceptable SASL mechanism \"%s\"",
+ conn->require_auth, selected_mechanism);
+ goto error;
+ }
+ }
+
if (conn->channel_binding[0] == 'r' && /* require */
strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) != 0)
{
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index cbadb3f6af..a048793b46 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1258,12 +1258,25 @@ connectOptions2(PGconn *conn)
more;
bool negated = false;
+ static const uint32 default_methods = (
+ 1 << AUTH_REQ_SASL
+ | 1 << AUTH_REQ_SASL_CONT
+ | 1 << AUTH_REQ_SASL_FIN
+ );
+ static const char *no_mechs[] = { NULL };
+
/*
- * By default, start from an empty set of allowed options and add to
+ * By default, start from a minimum set of allowed options and add to
* it.
+ *
+ * NB: The SASL method codes are always "allowed" here. If the server
+ * requests SASL auth, pg_SASL_init() will enforce adherence to the
+ * sasl_mechs list, which by default is empty.
*/
conn->auth_required = true;
- conn->allowed_auth_methods = 0;
+ conn->allowed_auth_methods = default_methods;
+ conn->sasl_mechs = no_mechs;
+ conn->sasl_mechs_denied = false;
for (first = true, more = true; more; first = false)
{
@@ -1290,6 +1303,9 @@ connectOptions2(PGconn *conn)
*/
conn->auth_required = false;
conn->allowed_auth_methods = -1;
+
+ /* conn->sasl_mechs is now a list of denied mechanisms. */
+ conn->sasl_mechs_denied = true;
}
else if (!negated)
{
@@ -1334,10 +1350,23 @@ connectOptions2(PGconn *conn)
}
else if (strcmp(method, "scram-sha-256") == 0)
{
- /* This currently assumes that SCRAM is the only SASL method. */
- bits = (1 << AUTH_REQ_SASL);
- bits |= (1 << AUTH_REQ_SASL_CONT);
- bits |= (1 << AUTH_REQ_SASL_FIN);
+ static const char *scram_mechs[] = {
+ SCRAM_SHA_256_NAME,
+ SCRAM_SHA_256_PLUS_NAME,
+ NULL /* list terminator */
+ };
+
+ /*
+ * This currently assumes that SCRAM is the only SASL method.
+ * Once a second mechanism is added, this code will need to add
+ * to the list instead of replacing it wholesale.
+ */
+ if (conn->sasl_mechs[0])
+ goto duplicate;
+ conn->sasl_mechs = scram_mechs;
+
+ free(part);
+ continue; /* avoid the bitmask manipulation below */
}
else if (strcmp(method, "creds") == 0)
{
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index f1f1d973cc..ab26292586 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -465,7 +465,8 @@ struct pg_conn
* codes */
bool client_finished_auth; /* have we finished our half of the
* authentication exchange? */
-
+ const char **sasl_mechs; /* list of allowed/denied SASL mechanisms */
+ bool sasl_mechs_denied; /* is the sasl_mechs list forbidden? */
/* Transient state needed while establishing connection */
PGTargetServerType target_server_type; /* desired session properties */
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index cba5d7d648..015532893c 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -301,30 +301,34 @@ $node->connect_fails(
"user=scram_role require_auth=password",
"password authentication required, fails with SCRAM auth",
expected_stderr =>
- qr/auth method "password" requirement failed: server requested SASL authentication/
+ qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
);
$node->connect_fails(
"user=scram_role require_auth=md5",
"md5 authentication required, fails with SCRAM auth",
expected_stderr =>
- qr/auth method "md5" requirement failed: server requested SASL authentication/
+ qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
);
$node->connect_fails(
"user=scram_role require_auth=none",
"all authentication forbidden, fails with SCRAM auth",
expected_stderr =>
- qr/auth method "none" requirement failed: server requested SASL authentication/
+ qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
);
# Authentication fails if SCRAM authentication is forbidden.
$node->connect_fails(
"user=scram_role require_auth=!scram-sha-256",
"SCRAM authentication forbidden, fails with SCRAM auth",
- expected_stderr => qr/server requested SASL authentication/);
+ expected_stderr =>
+ qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
+);
$node->connect_fails(
"user=scram_role require_auth=!password,!md5,!scram-sha-256",
"multiple authentication types forbidden, fails with SCRAM auth",
- expected_stderr => qr/server requested SASL authentication/);
+ expected_stderr =>
+ qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256"/
+);
# Test that bad passwords are rejected.
$ENV{"PGPASSWORD"} = 'badpass';
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 8038135697..173ac8d86b 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -157,6 +157,12 @@ if ($supports_tls_server_end_point)
"$common_connstr user=ssltestuser channel_binding=require require_auth=scram-sha-256",
"SCRAM with SSL, channel_binding=require, and require_auth=scram-sha-256"
);
+ $node->connect_fails(
+ "$common_connstr user=ssltestuser channel_binding=require require_auth=password",
+ "SCRAM with SSL, channel_binding=require, and require_auth=password",
+ expected_stderr =>
+ qr/server requested unacceptable SASL mechanism "SCRAM-SHA-256-PLUS"/
+ );
}
else
{
--
2.25.1
From 437add51a7b617d720334011745619404fba3f9e Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Mon, 13 Mar 2023 12:05:39 -0700
Subject: [PATCH v16 1/3] libpq: let client reject unexpected auth methods
The require_auth connection option allows the client to choose a list of
acceptable authentication types for use with the server. There is no
negotiation: if the server does not present one of the allowed
authentication requests, the connection fails. Additionally, all methods
in the list may be negated, e.g. '!password', in which case the server
must NOT use the listed authentication type. The special method "none"
allows/disallows the use of unauthenticated connections (but it does not
govern transport-level authentication via TLS or GSSAPI).
Internally, the patch expands the role of check_expected_areq() to
ensure that the incoming request is compatible with conn->require_auth.
It also introduces a new flag, conn->client_finished_auth, which is set
by various authentication routines when the client side of the handshake
is finished. This signals to check_expected_areq() that an OK message
from the server is expected, and allows the client to complain if the
server forgoes authentication entirely.
(Since the client can't generally prove that the server is actually
doing the work of authentication, this last part is mostly useful for
SCRAM without channel binding. It could also provide a client with a
decent signal that, at the very least, it's not connecting to a database
with trust auth; this allows proxies to ensure that their clients are
actually using their assigned credentials.)
Deficiencies:
- This is unlikely to be very forwards-compatible at the moment,
especially with SASL/SCRAM.
---
doc/src/sgml/libpq.sgml | 115 +++++++++++
src/include/libpq/pqcomm.h | 1 +
src/interfaces/libpq/fe-auth-scram.c | 1 +
src/interfaces/libpq/fe-auth.c | 139 +++++++++++++
src/interfaces/libpq/fe-connect.c | 170 ++++++++++++++++
src/interfaces/libpq/libpq-int.h | 9 +
src/test/authentication/meson.build | 1 +
src/test/authentication/t/001_password.pl | 232 ++++++++++++++++++++++
src/test/authentication/t/005_sspi.pl | 40 ++++
src/test/kerberos/t/001_auth.pl | 36 ++++
src/test/ldap/t/001_auth.pl | 6 +
src/test/ssl/t/002_scram.pl | 28 +++
12 files changed, 778 insertions(+)
create mode 100644 src/test/authentication/t/005_sspi.pl
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 3ccd8ff942..3706d349ab 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1220,6 +1220,111 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</listitem>
</varlistentry>
+ <varlistentry id="libpq-connect-require-auth" xreflabel="require_auth">
+ <term><literal>require_auth</literal></term>
+ <listitem>
+ <para>
+ Specifies the authentication method that the client requires from the
+ server. If the server does not use the required method to authenticate
+ the client, or if the authentication handshake is not fully completed by
+ the server, the connection will fail. A comma-separated list of methods
+ may also be provided, of which the server must use exactly one in order
+ for the connection to succeed. By default, any authentication method is
+ accepted, and the server is free to skip authentication altogether.
+ </para>
+ <para>
+ Methods may be negated with the addition of a <literal>!</literal>
+ prefix, in which case the server must <emphasis>not</emphasis> attempt
+ the listed method; any other method is accepted, and the server is free
+ not to authenticate the client at all. If a comma-separated list is
+ provided, the server may not attempt <emphasis>any</emphasis> of the
+ listed negated methods. Negated and non-negated forms may not be
+ combined in the same setting.
+ </para>
+ <para>
+ As a final special case, the <literal>none</literal> method requires the
+ server not to use an authentication challenge. (It may also be negated,
+ to require some form of authentication.)
+ </para>
+ <para>
+ The following methods may be specified:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>password</literal></term>
+ <listitem>
+ <para>
+ The server must request plaintext password authentication.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>md5</literal></term>
+ <listitem>
+ <para>
+ The server must request MD5 hashed password authentication.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>gss</literal></term>
+ <listitem>
+ <para>
+ The server must either request a Kerberos handshake via
+ <acronym>GSSAPI</acronym> or establish a
+ <acronym>GSS</acronym>-encrypted channel (see also
+ <xref linkend="libpq-connect-gssencmode" />).
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>sspi</literal></term>
+ <listitem>
+ <para>
+ The server must request Windows <acronym>SSPI</acronym>
+ authentication.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>scram-sha-256</literal></term>
+ <listitem>
+ <para>
+ The server must successfully complete a SCRAM-SHA-256 authentication
+ exchange with the client.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>creds</literal></term>
+ <listitem>
+ <para>
+ The server must request SCM credential authentication (deprecated
+ as of <productname>PostgreSQL</productname> 9.1).
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>none</literal></term>
+ <listitem>
+ <para>
+ The server must not prompt the client for an authentication
+ exchange. (This does not prohibit client certificate authentication
+ via TLS, nor GSS authentication via its encrypted transport.)
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-connect-channel-binding" xreflabel="channel_binding">
<term><literal>channel_binding</literal></term>
<listitem>
@@ -7774,6 +7879,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
</para>
</listitem>
+ <listitem>
+ <para>
+ <indexterm>
+ <primary><envar>PGREQUIREAUTH</envar></primary>
+ </indexterm>
+ <envar>PGREQUIREAUTH</envar> behaves the same as the <xref
+ linkend="libpq-connect-require-auth"/> connection parameter.
+ </para>
+ </listitem>
+
<listitem>
<para>
<indexterm>
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 66ba359390..5268d442ab 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -123,6 +123,7 @@ extern PGDLLIMPORT bool Db_user_namespace;
#define AUTH_REQ_SASL 10 /* Begin SASL authentication */
#define AUTH_REQ_SASL_CONT 11 /* Continue SASL authentication */
#define AUTH_REQ_SASL_FIN 12 /* Final SASL message */
+#define AUTH_REQ_MAX AUTH_REQ_SASL_FIN /* maximum AUTH_REQ_* value */
typedef uint32 AuthRequest;
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 12c3d0bc33..277f72b280 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -282,6 +282,7 @@ scram_exchange(void *opaq, char *input, int inputlen,
}
*done = true;
state->state = FE_SCRAM_FINISHED;
+ state->conn->client_finished_auth = true;
break;
default:
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index ab454e6cd0..8ce5b60a3d 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -136,7 +136,10 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
}
if (maj_stat == GSS_S_COMPLETE)
+ {
+ conn->client_finished_auth = true;
gss_release_name(&lmin_s, &conn->gtarg_nam);
+ }
return STATUS_OK;
}
@@ -321,6 +324,9 @@ pg_SSPI_continue(PGconn *conn, int payloadlen)
FreeContextBuffer(outbuf.pBuffers[0].pvBuffer);
}
+ if (r == SEC_E_OK)
+ conn->client_finished_auth = true;
+
/* Cleanup is handled by the code in freePGconn() */
return STATUS_OK;
}
@@ -735,6 +741,8 @@ pg_local_sendauth(PGconn *conn)
strerror_r(errno, sebuf, sizeof(sebuf)));
return STATUS_ERROR;
}
+
+ conn->client_finished_auth = true;
return STATUS_OK;
#else
libpq_append_conn_error(conn, "SCM_CRED authentication method not supported");
@@ -805,6 +813,41 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
return ret;
}
+/*
+ * Translate a disallowed AuthRequest code into an error message.
+ */
+static const char *
+auth_method_description(AuthRequest areq)
+{
+ switch (areq)
+ {
+ case AUTH_REQ_PASSWORD:
+ return libpq_gettext("server requested a cleartext password");
+ case AUTH_REQ_MD5:
+ return libpq_gettext("server requested a hashed password");
+ case AUTH_REQ_GSS:
+ case AUTH_REQ_GSS_CONT:
+ return libpq_gettext("server requested GSSAPI authentication");
+ case AUTH_REQ_SSPI:
+ return libpq_gettext("server requested SSPI authentication");
+ case AUTH_REQ_SCM_CREDS:
+ return libpq_gettext("server requested UNIX socket credentials");
+ case AUTH_REQ_SASL:
+ case AUTH_REQ_SASL_CONT:
+ case AUTH_REQ_SASL_FIN:
+ return libpq_gettext("server requested SASL authentication");
+ }
+
+ return libpq_gettext("server requested an unknown authentication type");
+}
+
+/*
+ * Convenience macro for checking the allowed_auth_methods bitmask. Caller must
+ * ensure that type is not greater than 31 (high bit of the bitmask).
+ */
+#define auth_method_allowed(conn, type) \
+ (((conn)->allowed_auth_methods & (1 << (type))) != 0)
+
/*
* Verify that the authentication request is expected, given the connection
* parameters. This is especially important when the client wishes to
@@ -814,6 +857,99 @@ static bool
check_expected_areq(AuthRequest areq, PGconn *conn)
{
bool result = true;
+ const char *reason = NULL;
+
+ StaticAssertDecl((sizeof(conn->allowed_auth_methods) * CHAR_BIT) > AUTH_REQ_MAX,
+ "AUTH_REQ_MAX overflows the allowed_auth_methods bitmask");
+
+ /*
+ * If the user required a specific auth method, or specified an allowed
+ * set, then reject all others here, and make sure the server actually
+ * completes an authentication exchange.
+ */
+ if (conn->require_auth)
+ {
+ switch (areq)
+ {
+ case AUTH_REQ_OK:
+
+ /*
+ * Check to make sure we've actually finished our exchange (or
+ * else that the user has allowed an authentication-less
+ * connection).
+ *
+ * If the user has allowed both SCRAM and unauthenticated
+ * (trust) connections, then this check will silently accept
+ * partial SCRAM exchanges, where a misbehaving server does
+ * not provide its verifier before sending an OK. This is
+ * consistent with historical behavior, but it may be a point
+ * to revisit in the future, since it could allow a server
+ * that doesn't know the user's password to silently harvest
+ * material for a brute force attack.
+ */
+ if (!conn->auth_required || conn->client_finished_auth)
+ break;
+
+ /*
+ * No explicit authentication request was made by the server
+ * -- or perhaps it was made and not completed, in the case of
+ * SCRAM -- but there is one special case to check. If the
+ * user allowed "gss", then a GSS-encrypted channel also
+ * satisfies the check.
+ */
+#ifdef ENABLE_GSS
+ if (auth_method_allowed(conn, AUTH_REQ_GSS) && conn->gssenc)
+ {
+ /*
+ * If implicit GSS auth has already been performed via GSS
+ * encryption, we don't need to have performed an
+ * AUTH_REQ_GSS exchange. This allows require_auth=gss to
+ * be combined with gssencmode, since there won't be an
+ * explicit authentication request in that case.
+ */
+ }
+ else
+#endif
+ {
+ reason = libpq_gettext("server did not complete authentication");
+ result = false;
+ }
+
+ break;
+
+ case AUTH_REQ_PASSWORD:
+ case AUTH_REQ_MD5:
+ case AUTH_REQ_GSS:
+ case AUTH_REQ_GSS_CONT:
+ case AUTH_REQ_SSPI:
+ case AUTH_REQ_SCM_CREDS:
+ case AUTH_REQ_SASL:
+ case AUTH_REQ_SASL_CONT:
+ case AUTH_REQ_SASL_FIN:
+
+ /*
+ * We don't handle these with the default case, to avoid
+ * bit-shifting past the end of the allowed_auth_methods mask
+ * if the server sends an unexpected AuthRequest.
+ */
+ result = auth_method_allowed(conn, areq);
+ break;
+
+ default:
+ result = false;
+ break;
+ }
+ }
+
+ if (!result)
+ {
+ if (!reason)
+ reason = auth_method_description(areq);
+
+ libpq_append_conn_error(conn, "auth method \"%s\" requirement failed: %s",
+ conn->require_auth, reason);
+ return result;
+ }
/*
* When channel_binding=require, we must protect against two cases: (1) we
@@ -1008,6 +1144,9 @@ pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
"fe_sendauth: error sending password authentication\n");
return STATUS_ERROR;
}
+
+ /* We expect no further authentication requests. */
+ conn->client_finished_auth = true;
break;
}
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 5638b223cb..dd4b98e099 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -307,6 +307,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"Require-Peer", "", 10,
offsetof(struct pg_conn, requirepeer)},
+ {"require_auth", "PGREQUIREAUTH", NULL, NULL,
+ "Require-Auth", "", 14, /* sizeof("scram-sha-256") == 14 */
+ offsetof(struct pg_conn, require_auth)},
+
{"ssl_min_protocol_version", "PGSSLMINPROTOCOLVERSION", "TLSv1.2", NULL,
"SSL-Minimum-Protocol-Version", "", 8, /* sizeof("TLSv1.x") == 8 */
offsetof(struct pg_conn, ssl_min_protocol_version)},
@@ -595,6 +599,7 @@ pqDropServerData(PGconn *conn)
/* Reset assorted other per-connection state */
conn->last_sqlstate[0] = '\0';
conn->auth_req_received = false;
+ conn->client_finished_auth = false;
conn->password_needed = false;
conn->write_failed = false;
free(conn->write_err_msg);
@@ -1237,6 +1242,170 @@ connectOptions2(PGconn *conn)
}
}
+ /*
+ * parse and validate require_auth option
+ */
+ if (conn->require_auth && conn->require_auth[0])
+ {
+ char *s = conn->require_auth;
+ bool first,
+ more;
+ bool negated = false;
+
+ /*
+ * By default, start from an empty set of allowed options and add to
+ * it.
+ */
+ conn->auth_required = true;
+ conn->allowed_auth_methods = 0;
+
+ for (first = true, more = true; more; first = false)
+ {
+ char *method,
+ *part;
+ uint32 bits;
+
+ part = parse_comma_separated_list(&s, &more);
+ if (part == NULL)
+ goto oom_error;
+
+ /*
+ * Check for negation, e.g. '!password'. If one element is
+ * negated, they all have to be.
+ */
+ method = part;
+ if (*method == '!')
+ {
+ if (first)
+ {
+ /*
+ * Switch to a permissive set of allowed options, and
+ * subtract from it.
+ */
+ conn->auth_required = false;
+ conn->allowed_auth_methods = -1;
+ }
+ else if (!negated)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "negative require_auth method \"%s\" cannot be mixed with non-negative methods",
+ method);
+
+ free(part);
+ return false;
+ }
+
+ negated = true;
+ method++;
+ }
+ else if (negated)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "require_auth method \"%s\" cannot be mixed with negative methods",
+ method);
+
+ free(part);
+ return false;
+ }
+
+ if (strcmp(method, "password") == 0)
+ {
+ bits = (1 << AUTH_REQ_PASSWORD);
+ }
+ else if (strcmp(method, "md5") == 0)
+ {
+ bits = (1 << AUTH_REQ_MD5);
+ }
+ else if (strcmp(method, "gss") == 0)
+ {
+ bits = (1 << AUTH_REQ_GSS);
+ bits |= (1 << AUTH_REQ_GSS_CONT);
+ }
+ else if (strcmp(method, "sspi") == 0)
+ {
+ bits = (1 << AUTH_REQ_SSPI);
+ bits |= (1 << AUTH_REQ_GSS_CONT);
+ }
+ else if (strcmp(method, "scram-sha-256") == 0)
+ {
+ /* This currently assumes that SCRAM is the only SASL method. */
+ bits = (1 << AUTH_REQ_SASL);
+ bits |= (1 << AUTH_REQ_SASL_CONT);
+ bits |= (1 << AUTH_REQ_SASL_FIN);
+ }
+ else if (strcmp(method, "creds") == 0)
+ {
+ bits = (1 << AUTH_REQ_SCM_CREDS);
+ }
+ else if (strcmp(method, "none") == 0)
+ {
+ /*
+ * Special case: let the user explicitly allow (or disallow)
+ * connections where the server does not send an explicit
+ * authentication challenge, such as "trust" and "cert" auth.
+ */
+ if (negated) /* "!none" */
+ {
+ if (conn->auth_required)
+ goto duplicate;
+
+ conn->auth_required = true;
+ }
+ else /* "none" */
+ {
+ if (!conn->auth_required)
+ goto duplicate;
+
+ conn->auth_required = false;
+ }
+
+ free(part);
+ continue; /* avoid the bitmask manipulation below */
+ }
+ else
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "invalid require_auth method: \"%s\"",
+ method);
+
+ free(part);
+ return false;
+ }
+
+ /* Update the bitmask. */
+ if (negated)
+ {
+ if ((conn->allowed_auth_methods & bits) == 0)
+ goto duplicate;
+
+ conn->allowed_auth_methods &= ~bits;
+ }
+ else
+ {
+ if ((conn->allowed_auth_methods & bits) == bits)
+ goto duplicate;
+
+ conn->allowed_auth_methods |= bits;
+ }
+
+ free(part);
+ continue;
+
+ duplicate:
+
+ /*
+ * A duplicated method probably indicates a typo in a setting
+ * where typos are extremely risky.
+ */
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "require_auth method \"%s\" is specified more than once",
+ part);
+
+ free(part);
+ return false;
+ }
+ }
+
/*
* validate channel_binding option
*/
@@ -4055,6 +4224,7 @@ freePGconn(PGconn *conn)
free(conn->sslcompression);
free(conn->sslsni);
free(conn->requirepeer);
+ free(conn->require_auth);
free(conn->ssl_min_protocol_version);
free(conn->ssl_max_protocol_version);
free(conn->gssencmode);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index d7ec5ed429..1dc264fe54 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -396,6 +396,7 @@ struct pg_conn
char *ssl_min_protocol_version; /* minimum TLS protocol version */
char *ssl_max_protocol_version; /* maximum TLS protocol version */
char *target_session_attrs; /* desired session properties */
+ char *require_auth; /* name of the expected auth method */
/* Optional file to write trace info to */
FILE *Pfdebug;
@@ -457,6 +458,14 @@ struct pg_conn
bool write_failed; /* have we had a write failure on sock? */
char *write_err_msg; /* write error message, or NULL if OOM */
+ bool auth_required; /* require an authentication challenge from
+ * the server? */
+ uint32 allowed_auth_methods; /* bitmask of acceptable AuthRequest
+ * codes */
+ bool client_finished_auth; /* have we finished our half of the
+ * authentication exchange? */
+
+
/* Transient state needed while establishing connection */
PGTargetServerType target_server_type; /* desired session properties */
bool try_next_addr; /* time to advance to next address/host? */
diff --git a/src/test/authentication/meson.build b/src/test/authentication/meson.build
index 3fe279fc10..e501041785 100644
--- a/src/test/authentication/meson.build
+++ b/src/test/authentication/meson.build
@@ -10,6 +10,7 @@ tests += {
't/002_saslprep.pl',
't/003_peer.pl',
't/004_file_inclusion.pl',
+ 't/005_sspi.pl',
],
},
}
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index a2fde1408b..cba5d7d648 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -115,6 +115,114 @@ is($res, 't',
"users with trust authentication use SYSTEM_USER = NULL in parallel workers"
);
+# Explicitly specifying an empty require_auth (the default) should always
+# succeed.
+$node->connect_ok("user=scram_role require_auth=",
+ "empty require_auth succeeds");
+
+# All these values of require_auth should fail, as trust is expected.
+$node->connect_fails(
+ "user=scram_role require_auth=gss",
+ "GSS authentication required, fails with trust auth",
+ expected_stderr =>
+ qr/auth method "gss" requirement failed: server did not complete authentication/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=sspi",
+ "SSPI authentication required, fails with trust auth",
+ expected_stderr =>
+ qr/auth method "sspi" requirement failed: server did not complete authentication/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=password",
+ "password authentication required, fails with trust auth",
+ expected_stderr =>
+ qr/auth method "password" requirement failed: server did not complete authentication/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=md5",
+ "MD5 authentication required, fails with trust auth",
+ expected_stderr =>
+ qr/auth method "md5" requirement failed: server did not complete authentication/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=scram-sha-256",
+ "SCRAM authentication required, fails with trust auth",
+ expected_stderr =>
+ qr/auth method "scram-sha-256" requirement failed: server did not complete authentication/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=password,scram-sha-256",
+ "password and SCRAM authentication required, fails with trust auth",
+ expected_stderr =>
+ qr/auth method "password,scram-sha-256" requirement failed: server did not complete authentication/
+);
+
+# These negative patterns of require_auth should succeed.
+$node->connect_ok("user=scram_role require_auth=!gss",
+ "GSS authentication can be forbidden, succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!sspi",
+ "SSPI authentication can be forbidden, succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!password",
+ "password authentication can be forbidden, succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!md5",
+ "md5 authentication can be forbidden, succeeds with trust auth");
+$node->connect_ok("user=scram_role require_auth=!scram-sha-256",
+ "SCRAM authentication can be forbidden, succeeds with trust auth");
+$node->connect_ok(
+ "user=scram_role require_auth=!password,!scram-sha-256",
+ "multiple authentication types forbidden, succeeds with trust auth");
+
+# require_auth=[!]none should interact correctly with trust auth.
+$node->connect_ok("user=scram_role require_auth=none",
+ "all authentication types forbidden, succeeds with trust auth");
+$node->connect_fails(
+ "user=scram_role require_auth=!none",
+ "any authentication types required, fails with trust auth",
+ expected_stderr => qr/server did not complete authentication/);
+
+# Negative and positive require_auth methods can't be mixed.
+$node->connect_fails(
+ "user=scram_role require_auth=scram-sha-256,!md5",
+ "negative require_auth methods cannot be mixed with positive ones",
+ expected_stderr =>
+ qr/negative require_auth method "!md5" cannot be mixed with non-negative methods/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=!password,!none,scram-sha-256",
+ "positive require_auth methods cannot be mixed with negative one",
+ expected_stderr =>
+ qr/require_auth method "scram-sha-256" cannot be mixed with negative methods/
+);
+
+# require_auth methods cannot have duplicated values.
+$node->connect_fails(
+ "user=scram_role require_auth=password,md5,password",
+ "require_auth methods cannot include duplicates, positive case",
+ expected_stderr =>
+ qr/require_auth method "password" is specified more than once/);
+$node->connect_fails(
+ "user=scram_role require_auth=!password,!md5,!password",
+ "require_auth methods cannot be duplicated, negative case",
+ expected_stderr =>
+ qr/require_auth method "!password" is specified more than once/);
+$node->connect_fails(
+ "user=scram_role require_auth=none,md5,none",
+ "require_auth methods cannot be duplicated, none case",
+ expected_stderr =>
+ qr/require_auth method "none" is specified more than once/);
+$node->connect_fails(
+ "user=scram_role require_auth=!none,!md5,!none",
+ "require_auth methods cannot be duplicated, !none case",
+ expected_stderr =>
+ qr/require_auth method "!none" is specified more than once/);
+
+# Unknown value defined in require_auth.
+$node->connect_fails(
+ "user=scram_role require_auth=none,abcdefg",
+ "unknown require_auth methods are rejected",
+ expected_stderr => qr/invalid require_auth method: "abcdefg"/);
+
# For plain "password" method, all users should also be able to connect.
reset_pg_hba($node, 'all', 'all', 'password');
test_conn($node, 'user=scram_role', 'password', 0,
@@ -124,6 +232,47 @@ test_conn($node, 'user=md5_role', 'password', 0,
log_like =>
[qr/connection authenticated: identity="md5_role" method=password/]);
+# require_auth succeeds here with a plaintext password.
+$node->connect_ok("user=scram_role require_auth=password",
+ "password authentication required, works with password auth");
+$node->connect_ok("user=scram_role require_auth=!none",
+ "any authentication required, works with password auth");
+$node->connect_ok(
+ "user=scram_role require_auth=scram-sha-256,password,md5",
+ "multiple authentication types required, works with password auth");
+
+# require_auth fails for other authentication types.
+$node->connect_fails(
+ "user=scram_role require_auth=md5",
+ "md5 authentication required, fails with password auth",
+ expected_stderr =>
+ qr/auth method "md5" requirement failed: server requested a cleartext password/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=scram-sha-256",
+ "SCRAM authentication required, fails with password auth",
+ expected_stderr =>
+ qr/auth method "scram-sha-256" requirement failed: server requested a cleartext password/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=none",
+ "all authentication forbidden, fails with password auth",
+ expected_stderr =>
+ qr/auth method "none" requirement failed: server requested a cleartext password/
+);
+
+# Disallowing password authentication fails, even if requested by server.
+$node->connect_fails(
+ "user=scram_role require_auth=!password",
+ "password authentication forbidden, fails with password auth",
+ expected_stderr => qr/server requested a cleartext password/);
+$node->connect_fails(
+ "user=scram_role require_auth=!password,!md5,!scram-sha-256",
+ "multiple authentication types forbidden, fails with password auth",
+ expected_stderr =>
+ qr/ method "!password,!md5,!scram-sha-256" requirement failed: server requested a cleartext password/
+);
+
# For "scram-sha-256" method, user "scram_role" should be able to connect.
reset_pg_hba($node, 'all', 'all', 'scram-sha-256');
test_conn(
@@ -137,6 +286,46 @@ test_conn(
test_conn($node, 'user=md5_role', 'scram-sha-256', 2,
log_unlike => [qr/connection authenticated:/]);
+# require_auth should succeeds with SCRAM when it is required.
+$node->connect_ok(
+ "user=scram_role require_auth=scram-sha-256",
+ "SCRAM authentication required, works with SCRAM auth");
+$node->connect_ok("user=scram_role require_auth=!none",
+ "any authentication required, works with SCRAM auth");
+$node->connect_ok(
+ "user=scram_role require_auth=password,scram-sha-256,md5",
+ "multiple authentication types required, works with SCRAM auth");
+
+# Authentication fails for other authentication types.
+$node->connect_fails(
+ "user=scram_role require_auth=password",
+ "password authentication required, fails with SCRAM auth",
+ expected_stderr =>
+ qr/auth method "password" requirement failed: server requested SASL authentication/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=md5",
+ "md5 authentication required, fails with SCRAM auth",
+ expected_stderr =>
+ qr/auth method "md5" requirement failed: server requested SASL authentication/
+);
+$node->connect_fails(
+ "user=scram_role require_auth=none",
+ "all authentication forbidden, fails with SCRAM auth",
+ expected_stderr =>
+ qr/auth method "none" requirement failed: server requested SASL authentication/
+);
+
+# Authentication fails if SCRAM authentication is forbidden.
+$node->connect_fails(
+ "user=scram_role require_auth=!scram-sha-256",
+ "SCRAM authentication forbidden, fails with SCRAM auth",
+ expected_stderr => qr/server requested SASL authentication/);
+$node->connect_fails(
+ "user=scram_role require_auth=!password,!md5,!scram-sha-256",
+ "multiple authentication types forbidden, fails with SCRAM auth",
+ expected_stderr => qr/server requested SASL authentication/);
+
# Test that bad passwords are rejected.
$ENV{"PGPASSWORD"} = 'badpass';
test_conn($node, 'user=scram_role', 'scram-sha-256', 2,
@@ -153,6 +342,49 @@ test_conn($node, 'user=md5_role', 'md5', 0,
log_like =>
[qr/connection authenticated: identity="md5_role" method=md5/]);
+# require_auth succeeds with MD5 required.
+$node->connect_ok("user=md5_role require_auth=md5",
+ "MD5 authentication required, works with MD5 auth");
+$node->connect_ok("user=md5_role require_auth=!none",
+ "any authentication required, works with MD5 auth");
+$node->connect_ok(
+ "user=md5_role require_auth=md5,scram-sha-256,password",
+ "multiple authentication types required, works with MD5 auth");
+
+# Authentication fails if other types are required.
+$node->connect_fails(
+ "user=md5_role require_auth=password",
+ "password authentication required, fails with MD5 auth",
+ expected_stderr =>
+ qr/auth method "password" requirement failed: server requested a hashed password/
+);
+$node->connect_fails(
+ "user=md5_role require_auth=scram-sha-256",
+ "SCRAM authentication required, fails with MD5 auth",
+ expected_stderr =>
+ qr/auth method "scram-sha-256" requirement failed: server requested a hashed password/
+);
+$node->connect_fails(
+ "user=md5_role require_auth=none",
+ "all authentication types forbidden, fails with MD5 auth",
+ expected_stderr =>
+ qr/auth method "none" requirement failed: server requested a hashed password/
+);
+
+# Authentication fails if MD5 is forbidden.
+$node->connect_fails(
+ "user=md5_role require_auth=!md5",
+ "password authentication forbidden, fails with MD5 auth",
+ expected_stderr =>
+ qr/auth method "!md5" requirement failed: server requested a hashed password/
+);
+$node->connect_fails(
+ "user=md5_role require_auth=!password,!md5,!scram-sha-256",
+ "multiple authentication types forbidden, fails with MD5 auth",
+ expected_stderr =>
+ qr/auth method "!password,!md5,!scram-sha-256" requirement failed: server requested a hashed password/
+);
+
# Test SYSTEM_USER <> NULL with parallel workers.
$node->safe_psql(
'postgres',
diff --git a/src/test/authentication/t/005_sspi.pl b/src/test/authentication/t/005_sspi.pl
new file mode 100644
index 0000000000..c6b52ff3c9
--- /dev/null
+++ b/src/test/authentication/t/005_sspi.pl
@@ -0,0 +1,40 @@
+
+# Copyright (c) 2021-2023, PostgreSQL Global Development Group
+
+# Tests targeting SSPI on Windows.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+if (!$windows_os || $use_unix_sockets)
+{
+ plan skip_all =>
+ "SSPI tests require Windows (without PG_TEST_USE_UNIX_SOCKETS)";
+}
+
+# Initialize primary node
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init;
+$node->append_conf('postgresql.conf', "log_connections = on\n");
+$node->start;
+
+# SSPI is set up by default. Make sure it interacts correctly with require_auth.
+$node->connect_ok("require_auth=sspi",
+ "SSPI authentication required, works with SSPI auth");
+$node->connect_fails(
+ "require_auth=!sspi",
+ "SSPI authentication forbidden, fails with SSPI auth",
+ expected_stderr =>
+ qr/auth method "!sspi" requirement failed: server requested SSPI authentication/
+);
+$node->connect_fails(
+ "require_auth=scram-sha-256",
+ "SCRAM authentication required, fails with SSPI auth",
+ expected_stderr =>
+ qr/auth method "scram-sha-256" requirement failed: server requested SSPI authentication/
+);
+
+done_testing();
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 3bc4ad7dd3..ce7a323d0e 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -337,6 +337,32 @@ test_query(
'gssencmode=require',
'sending 100K lines works');
+# require_auth=gss succeeds if required.
+$node->connect_ok(
+ $node->connstr('postgres')
+ . " user=test1 host=$host hostaddr=$hostaddr gssencmode=disable require_auth=gss",
+ "GSS authentication requested, works with GSS auth without encryption");
+$node->connect_ok(
+ $node->connstr('postgres')
+ . " user=test1 host=$host hostaddr=$hostaddr gssencmode=require require_auth=gss",
+ "GSS authentication requested, works with GSS auth with encryption");
+
+# require_auth=sspi fails if required.
+$node->connect_fails(
+ $node->connstr('postgres')
+ . " user=test1 host=$host hostaddr=$hostaddr gssencmode=disable require_auth=sspi",
+ "SSPI authentication requested, fails with GSS auth without encryption",
+ expected_stderr =>
+ qr/auth method "sspi" requirement failed: server requested GSSAPI authentication/
+);
+$node->connect_fails(
+ $node->connstr('postgres')
+ . " user=test1 host=$host hostaddr=$hostaddr gssencmode=require require_auth=sspi",
+ "SSPI authentication requested, fails with GSS auth with encryption",
+ expected_stderr =>
+ qr/auth method "sspi" requirement failed: server did not complete authentication/
+);
+
# Test that SYSTEM_USER works.
test_query($node, 'test1', 'SELECT SYSTEM_USER;',
qr/^gss:test1\@$realm$/s, 'gssencmode=require', 'testing system_user');
@@ -382,6 +408,16 @@ test_access(
test_access($node, 'test1', 'SELECT true', 2, 'gssencmode=disable',
'fails with GSS encryption disabled and hostgssenc hba');
+# require_auth=gss succeeds if required.
+$node->connect_ok(
+ $node->connstr('postgres')
+ . " user=test1 host=$host hostaddr=$hostaddr gssencmode=require require_auth=gss",
+ "GSS authentication requested, works with GSS encryption");
+$node->connect_ok(
+ $node->connstr('postgres')
+ . " user=test1 host=$host hostaddr=$hostaddr gssencmode=require require_auth=gss,scram-sha-256",
+ "multiple authentication types requested, works with GSS encryption");
+
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
qq{hostnogssenc all all $hostaddr/32 gss map=mymap});
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index f3ed806ec2..1e027ced01 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -101,6 +101,12 @@ test_access(
qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/
],);
+# require_auth=password should complete successfully; other methods should fail.
+$node->connect_ok("user=test1 require_auth=password",
+ "password authentication required, works with ldap auth");
+$node->connect_fails("user=test1 require_auth=scram-sha-256",
+ "SCRAM authentication required, fails with ldap auth");
+
note "search+bind";
unlink($node->data_dir . '/pg_hba.conf');
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 1d3905d3a1..8038135697 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -140,6 +140,34 @@ $node->connect_ok(
qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/
]);
+# channel_binding should continue to work independently of require_auth.
+$node->connect_ok(
+ "$common_connstr user=ssltestuser channel_binding=disable require_auth=scram-sha-256",
+ "SCRAM with SSL, channel_binding=disable, and require_auth=scram-sha-256"
+);
+$node->connect_fails(
+ "$common_connstr user=md5testuser require_auth=md5 channel_binding=require",
+ "channel_binding can fail even when require_auth succeeds",
+ expected_stderr =>
+ qr/channel binding required but not supported by server's authentication request/
+);
+if ($supports_tls_server_end_point)
+{
+ $node->connect_ok(
+ "$common_connstr user=ssltestuser channel_binding=require require_auth=scram-sha-256",
+ "SCRAM with SSL, channel_binding=require, and require_auth=scram-sha-256"
+ );
+}
+else
+{
+ $node->connect_fails(
+ "$common_connstr user=ssltestuser channel_binding=require require_auth=scram-sha-256",
+ "SCRAM with SSL, channel_binding=require, and require_auth=scram-sha-256",
+ expected_stderr =>
+ qr/channel binding is required, but server did not offer an authentication method that supports channel binding/
+ );
+}
+
# Now test with a server certificate that uses the RSA-PSS algorithm.
# This checks that the certificate can be loaded and that channel binding
# works. (see bug #17760)
--
2.25.1
From d621f5b3cb948b64678a67b2a70ee366d690b38d Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Fri, 24 Jun 2022 15:40:42 -0700
Subject: [PATCH v16 2/3] Add sslcertmode option for client certificates
The sslcertmode option controls whether the server is allowed and/or
required to request a certificate from the client. There are three
modes:
- "allow" is the default and follows the current behavior -- a
configured sslcert is sent if the server requests one (which, with
the current implementation, will happen whenever TLS is negotiated).
- "disable" causes the client to refuse to send a client certificate
even if an sslcert is configured.
- "require" causes the client to fail if a client certificate is never
sent and the server opens a connection anyway. This doesn't add any
additional security, since there is no guarantee that the server is
validating the certificate correctly, but it may help troubleshoot
more complicated TLS setups.
sslcertmode=require needs the OpenSSL implementation to support
SSL_CTX_set_cert_cb(). Notably, LibreSSL does not.
---
configure | 11 ++--
configure.ac | 4 +-
doc/src/sgml/libpq.sgml | 64 ++++++++++++++++++++++++
meson.build | 3 +-
src/include/pg_config.h.in | 3 ++
src/interfaces/libpq/fe-auth.c | 19 +++++++
src/interfaces/libpq/fe-connect.c | 51 +++++++++++++++++++
src/interfaces/libpq/fe-secure-openssl.c | 39 ++++++++++++++-
src/interfaces/libpq/libpq-int.h | 3 ++
src/test/ssl/t/001_ssltests.pl | 43 ++++++++++++++++
src/tools/msvc/Solution.pm | 9 ++++
11 files changed, 240 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index e35769ea73..92896d2d83 100755
--- a/configure
+++ b/configure
@@ -12973,13 +12973,14 @@ else
fi
fi
- # Function introduced in OpenSSL 1.0.2.
- for ac_func in X509_get_signature_nid
+ # Functions introduced in OpenSSL 1.0.2. LibreSSL doesn't have all of these.
+ for ac_func in X509_get_signature_nid SSL_CTX_set_cert_cb
do :
- ac_fn_c_check_func "$LINENO" "X509_get_signature_nid" "ac_cv_func_X509_get_signature_nid"
-if test "x$ac_cv_func_X509_get_signature_nid" = xyes; then :
+ as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
+ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
+if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
cat >>confdefs.h <<_ACEOF
-#define HAVE_X509_GET_SIGNATURE_NID 1
+#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
_ACEOF
fi
diff --git a/configure.ac b/configure.ac
index af23c15cb2..34af40f621 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1373,8 +1373,8 @@ if test "$with_ssl" = openssl ; then
AC_SEARCH_LIBS(CRYPTO_new_ex_data, [eay32 crypto], [], [AC_MSG_ERROR([library 'eay32' or 'crypto' is required for OpenSSL])])
AC_SEARCH_LIBS(SSL_new, [ssleay32 ssl], [], [AC_MSG_ERROR([library 'ssleay32' or 'ssl' is required for OpenSSL])])
fi
- # Function introduced in OpenSSL 1.0.2.
- AC_CHECK_FUNCS([X509_get_signature_nid])
+ # Functions introduced in OpenSSL 1.0.2. LibreSSL doesn't have all of these.
+ AC_CHECK_FUNCS([X509_get_signature_nid SSL_CTX_set_cert_cb])
# Functions introduced in OpenSSL 1.1.0. We used to check for
# OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
# defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 3706d349ab..ac1c379f2a 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1820,6 +1820,60 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</listitem>
</varlistentry>
+ <varlistentry id="libpq-connect-sslcertmode" xreflabel="sslcertmode">
+ <term><literal>sslcertmode</literal></term>
+ <listitem>
+ <para>
+ This option determines whether a client certificate may be sent to the
+ server, and whether the server is required to request one. There are
+ three modes:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>disable</literal></term>
+ <listitem>
+ <para>
+ a client certificate is never sent, even if one is provided via
+ <xref linkend="libpq-connect-sslcert" />
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>allow</literal> (default)</term>
+ <listitem>
+ <para>
+ a certificate may be sent, if the server requests one and it has
+ been provided via <literal>sslcert</literal>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>require</literal></term>
+ <listitem>
+ <para>
+ the server <emphasis>must</emphasis> request a certificate. The
+ connection will fail if the client does not send a certificate and
+ the server successfully authenticates the client anyway.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+
+ <note>
+ <para>
+ <literal>sslcertmode=require</literal> doesn't add any additional
+ security, since there is no guarantee that the server is validating the
+ certificate correctly; PostgreSQL servers generally request TLS
+ certificates from clients whether they validate them or not. The option
+ may be useful when troubleshooting more complicated TLS setups.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-connect-sslrootcert" xreflabel="sslrootcert">
<term><literal>sslrootcert</literal></term>
<listitem>
@@ -7996,6 +8050,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
</para>
</listitem>
+ <listitem>
+ <para>
+ <indexterm>
+ <primary><envar>PGSSLCERTMODE</envar></primary>
+ </indexterm>
+ <envar>PGSSLCERTMODE</envar> behaves the same as the <xref
+ linkend="libpq-connect-sslcertmode"/> connection parameter.
+ </para>
+ </listitem>
+
<listitem>
<para>
<indexterm>
diff --git a/meson.build b/meson.build
index 8208815c96..70b64ea4b7 100644
--- a/meson.build
+++ b/meson.build
@@ -1213,8 +1213,9 @@ if sslopt in ['auto', 'openssl']
['CRYPTO_new_ex_data', {'required': true}],
['SSL_new', {'required': true}],
- # Function introduced in OpenSSL 1.0.2.
+ # Functions introduced in OpenSSL 1.0.2. LibreSSL doesn't have all of these.
['X509_get_signature_nid'],
+ ['SSL_CTX_set_cert_cb'],
# Functions introduced in OpenSSL 1.1.0. We used to check for
# OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 20c82f5979..c5a6762fc1 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -394,6 +394,9 @@
/* Define to 1 if you have spinlocks. */
#undef HAVE_SPINLOCKS
+/* Define to 1 if you have the `SSL_CTX_set_cert_cb' function. */
+#undef HAVE_SSL_CTX_SET_CERT_CB
+
/* Define to 1 if stdbool.h conforms to C99. */
#undef HAVE_STDBOOL_H
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 8ce5b60a3d..53c7d30eff 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -862,6 +862,25 @@ check_expected_areq(AuthRequest areq, PGconn *conn)
StaticAssertDecl((sizeof(conn->allowed_auth_methods) * CHAR_BIT) > AUTH_REQ_MAX,
"AUTH_REQ_MAX overflows the allowed_auth_methods bitmask");
+ if (conn->sslcertmode[0] == 'r' /* require */
+ && areq == AUTH_REQ_OK)
+ {
+ /*
+ * Trade off a little bit of complexity to try to get these error
+ * messages as precise as possible.
+ */
+ if (!conn->ssl_cert_requested)
+ {
+ libpq_append_conn_error(conn, "server did not request a certificate");
+ return false;
+ }
+ else if (!conn->ssl_cert_sent)
+ {
+ libpq_append_conn_error(conn, "server accepted connection without a valid certificate");
+ return false;
+ }
+ }
+
/*
* If the user required a specific auth method, or specified an allowed
* set, then reject all others here, and make sure the server actually
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index dd4b98e099..cbadb3f6af 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -125,8 +125,10 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
#define DefaultTargetSessionAttrs "any"
#ifdef USE_SSL
#define DefaultSSLMode "prefer"
+#define DefaultSSLCertMode "allow"
#else
#define DefaultSSLMode "disable"
+#define DefaultSSLCertMode "disable"
#endif
#ifdef ENABLE_GSS
#include "fe-gssapi-common.h"
@@ -283,6 +285,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"SSL-Client-Key", "", 64,
offsetof(struct pg_conn, sslkey)},
+ {"sslcertmode", "PGSSLCERTMODE", NULL, NULL,
+ "SSL-Client-Cert-Mode", "", 8, /* sizeof("disable") == 8 */
+ offsetof(struct pg_conn, sslcertmode)},
+
{"sslpassword", NULL, NULL, NULL,
"SSL-Client-Key-Password", "*", 20,
offsetof(struct pg_conn, sslpassword)},
@@ -1510,6 +1516,51 @@ connectOptions2(PGconn *conn)
return false;
}
+ /*
+ * validate sslcertmode option
+ */
+ if (conn->sslcertmode)
+ {
+ if (strcmp(conn->sslcertmode, "disable") != 0 &&
+ strcmp(conn->sslcertmode, "allow") != 0 &&
+ strcmp(conn->sslcertmode, "require") != 0)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
+ "sslcertmode", conn->sslcertmode);
+ return false;
+ }
+#ifndef USE_SSL
+ if (strcmp(conn->sslcertmode, "require") == 0)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "sslcertmode value \"%s\" invalid when SSL support is not compiled in",
+ conn->sslcertmode);
+ return false;
+ }
+#endif
+#ifndef HAVE_SSL_CTX_SET_CERT_CB
+ /*
+ * Without a certificate callback, the current implementation can't
+ * figure out if a certficate was actually requested, so "require" is
+ * useless.
+ */
+ if (strcmp(conn->sslcertmode, "require") == 0)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "sslcertmode value \"%s\" is not supported (check OpenSSL version)",
+ conn->sslcertmode);
+ return false;
+ }
+#endif
+ }
+ else
+ {
+ conn->sslcertmode = strdup(DefaultSSLCertMode);
+ if (!conn->sslcertmode)
+ goto oom_error;
+ }
+
/*
* validate gssencmode option
*/
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 6a4431ddfe..b88d9da3e2 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -462,6 +462,33 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
return ok;
}
+#ifdef HAVE_SSL_CTX_SET_CERT_CB
+/*
+ * Certificate selection callback
+ *
+ * This callback lets us choose the client certificate we send to the server
+ * after seeing its CertificateRequest. We only support sending a single
+ * hard-coded certificate via sslcert, so we don't actually set any certificates
+ * here; we just use it to record whether or not the server has actually asked
+ * for one and whether we have one to send.
+ */
+static int
+cert_cb(SSL *ssl, void *arg)
+{
+ PGconn *conn = arg;
+ conn->ssl_cert_requested = true;
+
+ /* Do we have a certificate loaded to send back? */
+ if (SSL_get_certificate(ssl))
+ conn->ssl_cert_sent = true;
+
+ /*
+ * Tell OpenSSL that the callback succeeded; we're not required to actually
+ * make any changes to the SSL handle.
+ */
+ return 1;
+}
+#endif
/*
* OpenSSL-specific wrapper around
@@ -953,6 +980,11 @@ initialize_SSL(PGconn *conn)
SSL_CTX_set_default_passwd_cb_userdata(SSL_context, conn);
}
+#ifdef HAVE_SSL_CTX_SET_CERT_CB
+ /* Set up a certificate selection callback. */
+ SSL_CTX_set_cert_cb(SSL_context, cert_cb, conn);
+#endif
+
/* Disable old protocol versions */
SSL_CTX_set_options(SSL_context, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
@@ -1107,7 +1139,12 @@ initialize_SSL(PGconn *conn)
else
fnbuf[0] = '\0';
- if (fnbuf[0] == '\0')
+ if (conn->sslcertmode[0] == 'd') /* disable */
+ {
+ /* don't send a client cert even if we have one */
+ have_cert = false;
+ }
+ else if (fnbuf[0] == '\0')
{
/* no home directory, proceed without a client cert */
have_cert = false;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 1dc264fe54..f1f1d973cc 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -384,6 +384,7 @@ struct pg_conn
char *sslkey; /* client key filename */
char *sslcert; /* client certificate filename */
char *sslpassword; /* client key file password */
+ char *sslcertmode; /* client cert mode (require,allow,disable) */
char *sslrootcert; /* root certificate filename */
char *sslcrl; /* certificate revocation list filename */
char *sslcrldir; /* certificate revocation list directory name */
@@ -527,6 +528,8 @@ struct pg_conn
/* SSL structures */
bool ssl_in_use;
+ bool ssl_cert_requested; /* Did the server ask us for a cert? */
+ bool ssl_cert_sent; /* Did we send one in reply? */
#ifdef USE_SSL
bool allow_ssl_try; /* Allowed to try SSL negotiation */
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 3094e27af3..4617f06f86 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -42,6 +42,10 @@ my $SERVERHOSTADDR = '127.0.0.1';
# This is the pattern to use in pg_hba.conf to match incoming connections.
my $SERVERHOSTCIDR = '127.0.0.1/32';
+# Determine whether build supports sslcertmode=require.
+my $supports_sslcertmode_require =
+ check_pg_config("#define HAVE_SSL_CTX_SET_CERT_CB 1");
+
# Allocation of base connection string shared among multiple tests.
my $common_connstr;
@@ -191,6 +195,22 @@ $node->connect_ok(
"$common_connstr sslrootcert=ssl/both-cas-2.crt sslmode=verify-ca",
"cert root file that contains two certificates, order 2");
+# sslcertmode=allow and =disable should both work without a client certificate.
+$node->connect_ok(
+ "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require sslcertmode=disable",
+ "connect with sslcertmode=disable");
+$node->connect_ok(
+ "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require sslcertmode=allow",
+ "connect with sslcertmode=allow");
+
+# sslcertmode=require, however, should fail.
+$node->connect_fails(
+ "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require sslcertmode=require",
+ "connect with sslcertmode=require fails without a client certificate",
+ expected_stderr => $supports_sslcertmode_require
+ ? qr/server accepted connection without a valid certificate/
+ : qr/sslcertmode value "require" is not supported/);
+
# CRL tests
# Invalid CRL filename is the same as no CRL, succeeds
@@ -538,6 +558,29 @@ $node->connect_ok(
"certificate authorization succeeds with correct client cert in encrypted DER format"
);
+# correct client cert with required/allowed certificate authentication
+if ($supports_sslcertmode_require)
+{
+ $node->connect_ok(
+ "$common_connstr user=ssltestuser sslcertmode=require sslcert=ssl/client.crt "
+ . sslkey('client.key'),
+ "certificate authorization succeeds with sslcertmode=require"
+ );
+}
+$node->connect_ok(
+ "$common_connstr user=ssltestuser sslcertmode=allow sslcert=ssl/client.crt "
+ . sslkey('client.key'),
+ "certificate authorization succeeds with sslcertmode=allow"
+);
+
+# client cert isn't sent if certificate authentication is disabled
+$node->connect_fails(
+ "$common_connstr user=ssltestuser sslcertmode=disable sslcert=ssl/client.crt "
+ . sslkey('client.key'),
+ "certificate authorization fails with sslcertmode=disable",
+ expected_stderr => qr/connection requires a valid client certificate/
+);
+
# correct client cert in encrypted PEM with wrong password
$node->connect_fails(
"$common_connstr user=ssltestuser sslcert=ssl/client.crt "
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 5eaea6355e..1ec1bac552 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -327,6 +327,7 @@ sub GenerateFiles
HAVE_SETPROCTITLE_FAST => undef,
HAVE_SOCKLEN_T => 1,
HAVE_SPINLOCKS => 1,
+ HAVE_SSL_CTX_SET_CERT_CB => undef,
HAVE_STDBOOL_H => 1,
HAVE_STDINT_H => 1,
HAVE_STDLIB_H => 1,
@@ -507,6 +508,14 @@ sub GenerateFiles
$define{HAVE_HMAC_CTX_NEW} = 1;
$define{HAVE_OPENSSL_INIT_SSL} = 1;
}
+
+ # Symbols needed with OpenSSL 1.0.2 and above.
+ if ( ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
+ || ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0')
+ || ($digit1 >= '1' && $digit2 >= '0' && $digit3 >= '2'))
+ {
+ $define{HAVE_SSL_CTX_SET_CERT_CB} = 1;
+ }
}
$self->GenerateConfigHeader('src/include/pg_config.h', \%define, 1);
--
2.25.1
