On Mon, Jun 13, 2022 at 10:00 PM Michael Paquier <mich...@paquier.xyz> wrote:
> > Personally I think the ability to provide a default of `!password` is
> > very compelling. Any allowlist we hardcode won't be future-proof (see
> > also my response to Laurenz upthread [1]).
>
> Hm, perhaps.  You could use that as a default at application level,
> but the default set in libpq should be backward-compatible (aka allow
> everything, even trust where the backend just sends AUTH_REQ_OK).
> This is unfortunate but there is a point in not breaking any user's
> application, as well, particularly with ldap, and note that we have to
> keep a certain amount of things backward-compatible as a very common
> practice of packaging with Postgres is to have libpq link to binaries
> across *multiple* major versions (Debian is one if I recall), with the
> newest version of libpq used for all.

I am 100% with you on this, but there's been a lot of chatter around
removing plaintext password support from libpq. I would much rather
see them rejected by default than removed entirely. !password would
provide an easy path for that.

> > I'm pretty motivated to provide the ability to say "I want cert auth
> > only, nothing else." Using a separate parameter would mean we'd need
> > something like `require_auth=none`, but I think that makes a certain
> > amount of sense.
>
> If the default of require_auth is backward-compatible and allows
> everything, using a different parameter for the certs won't matter
> anyway?

If you want cert authentication only, allowing "everything" will let
the server extract a password and then you're back at square one.
There needs to be a way to prohibit all explicit authentication
requests.

> My suggestion is to reword the error message so as the reason and the
> main error message can be treated as two independent things.  You are
> right to apply two times libpq_gettext(), once to "reason" and once to
> the main string.

Ah, thanks for the clarification. Done that way in v3.

> My point would be to either register a max flag in the set of
> AUTH_REQ_* in pqcomm.h so as we never go above 32 with an assertion to
> make sure that this would never overflow, but I would add a comment in
> pqcomm.h telling that we also do bitwise operations, relying on the
> number of AUTH_REQ_* flags, and that we'd better be careful once the
> number of flags gets higher than 32.  There is some margin, still that
> could be easily forgotten.

Makes sense; done.

v3 also removes "cert" from require_auth while I work on a replacement
connection option, fixes the bugs/suggestions pointed out upthread,
and adds a documentation first draft. I tried combining this with the
NOT work but it was too much to juggle, so that'll wait for a v4+,
along with require_auth=none and "cert mode".

Thanks for the detailed review!
--Jacob
commit c299b1abf0ffc477371cdd0d0d789e824da56328
Author: Jacob Champion <jchamp...@timescale.com>
Date:   Fri Jun 10 11:20:08 2022 -0700

    squash! libpq: let client reject unexpected auth methods
    
    Per review suggestions:
    
    - Fix a memory leak when parsing require_auth.
    - Add PGREQUIREAUTH to the envvars.
    - Remove the "cert" method from require_auth; this will be handled with
      a separate conninfo option later.
    - Fix builds without GSSAPI.
    - Test combinations of require_auth and channel_binding.
    - Statically assert when AUTH_REQ_* grows too large to hold safely in
      our allowed_auth_methods bitmask.
    - Improve ease of translation.
    - Add a first draft of documentation.

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 37ec3cb4e5..00990ce47d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1221,6 +1221,76 @@ 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>
+        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>
+        </variablelist>
+      </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="libpq-connect-channel-binding" 
xreflabel="channel_binding">
       <term><literal>channel_binding</literal></term>
       <listitem>
@@ -7761,6 +7831,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 b418283d5f..a47f7b2d91 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -161,6 +161,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.c b/src/interfaces/libpq/fe-auth.c
index 04f9bf4831..59c3575cc3 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -877,6 +877,9 @@ auth_description(AuthRequest areq)
 #define auth_allowed(conn, type) \
        (((conn)->allowed_auth_methods & (1 << (type))) != 0)
 
+StaticAssertDecl(AUTH_REQ_MAX < CHAR_BIT * 
sizeof(((PGconn){0}).allowed_auth_methods),
+                                "AUTH_REQ_MAX overflows the 
allowed_auth_methods bitmask");
+
 /*
  * Verify that the authentication request is expected, given the connection
  * parameters. This is especially important when the client wishes to
@@ -907,48 +910,24 @@ check_expected_areq(AuthRequest areq, PGconn *conn)
                                /*
                                 * No explicit authentication request was made 
by the server --
                                 * or perhaps it was made and not completed, in 
the case of
-                                * SCRAM -- but there are two special cases to 
check:
-                                *
-                                * 1) If the user allowed "cert", then as long 
as we sent a
-                                *    client certificate to the server in 
response to its
-                                *    TLS CertificateRequest, this check is 
satisfied.
-                                *
-                                * 2) If the user allowed "gss", then a 
GSS-encrypted channel
-                                *    also satisfies the check.
+                                * SCRAM -- but there is one special case to 
check. If the user
+                                * allowed "gss", then a GSS-encrypted channel 
also satisfies
+                                * the check.
                                 */
-                               if (conn->allowed_auth_method_cert)
-                               {
-                                       /*
-                                        * Trade off a little bit of complexity 
to try to get these
-                                        * error messages as precise as 
possible.
-                                        */
-                                       if (!conn->ssl_cert_requested)
-                                       {
-                                               reason = 
conn->allowed_auth_methods
-                                                       ? libpq_gettext("server 
did not complete authentication or request a certificate")
-                                                       : libpq_gettext("server 
did not request a certificate");
-                                               result = false;
-                                       }
-                                       else if (!conn->ssl_cert_sent)
-                                       {
-                                               reason = 
conn->allowed_auth_methods
-                                                       ? libpq_gettext("server 
did not complete authentication and accepted connection without a valid 
certificate")
-                                                       : libpq_gettext("server 
accepted connection without a valid certificate");
-                                               result = false;
-                                       }
-                               }
-                               else if (auth_allowed(conn, AUTH_REQ_GSS) && 
conn->gssenc)
+#ifdef ENABLE_GSS
+                               if (auth_allowed(conn, AUTH_REQ_GSS) && 
conn->gssenc)
                                {
                                        /*
-                                        * Special case: if implicit GSS auth 
has already been
-                                        * performed via GSS encryption, we 
don't need to have
-                                        * performed an AUTH_REQ_GSS exchange.
+                                        * If implicit GSS auth has already 
been performed via GSS
+                                        * encryption, we don't need to have 
performed an
+                                        * AUTH_REQ_GSS exchange.
                                         *
                                         * TODO: check this assumption. What 
mutual auth guarantees
                                         * are made in this case?
                                         */
                                }
                                else
+#endif
                                {
                                        reason = libpq_gettext("server did not 
complete authentication"),
                                        result = false;
@@ -982,12 +961,8 @@ check_expected_areq(AuthRequest areq, PGconn *conn)
        {
                if (reason)
                {
-                       /*
-                        * XXX double call to libpq_gettext() is probably not 
easy to
-                        * translate from English
-                        */
                        appendPQExpBuffer(&conn->errorMessage,
-                                                         libpq_gettext("auth 
method \"%s\" required, but %s\n"),
+                                                         libpq_gettext("auth 
method \"%s\" requirement failed: %s\n"),
                                                          conn->require_auth, 
reason);
                }
                else
diff --git a/src/interfaces/libpq/fe-connect.c 
b/src/interfaces/libpq/fe-connect.c
index c650687c9b..f2579ff7ee 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -310,7 +310,7 @@ static const internalPQconninfoOption PQconninfoOptions[] = 
{
                "Require-Peer", "", 10,
        offsetof(struct pg_conn, requirepeer)},
 
-       {"require_auth", NULL, NULL, NULL,
+       {"require_auth", "PGREQUIREAUTH", NULL, NULL,
                "Require-Auth", "", 14, /* sizeof("scram-sha-256") == 14 */
        offsetof(struct pg_conn, require_auth)},
 
@@ -1301,31 +1301,14 @@ connectOptions2(PGconn *conn)
                                bits |= (1 << AUTH_REQ_SASL_CONT);
                                bits |= (1 << AUTH_REQ_SASL_FIN);
                        }
-                       else if (strcmp(method, "cert") == 0)
-                       {
-                               /*
-                                * Special case: there is no AUTH_REQ code for 
certificate
-                                * authentication since it's done as part of 
the TLS handshake.
-                                * Make a note in conn, for 
check_expected_areq() to see later.
-                                */
-                               if (conn->allowed_auth_method_cert)
-                               {
-                                       conn->status = CONNECTION_BAD;
-                                       appendPQExpBuffer(&conn->errorMessage,
-                                                                         
libpq_gettext("require_auth method \"%s\" is specified more than once"),
-                                                                         
method);
-                                       return false;
-                               }
-
-                               conn->allowed_auth_method_cert = true;
-                               continue; /* avoid the bitmask manipulation 
below */
-                       }
                        else
                        {
                                conn->status = CONNECTION_BAD;
                                appendPQExpBuffer(&conn->errorMessage,
                                                                  
libpq_gettext("invalid require_auth method: \"%s\""),
                                                                  method);
+
+                               free(method);
                                return false;
                        }
 
@@ -1339,10 +1322,13 @@ connectOptions2(PGconn *conn)
                                appendPQExpBuffer(&conn->errorMessage,
                                                                  
libpq_gettext("require_auth method \"%s\" is specified more than once"),
                                                                  method);
+
+                               free(method);
                                return false;
                        }
 
                        conn->allowed_auth_methods |= bits;
+                       free(method);
                }
        }
 
diff --git a/src/interfaces/libpq/fe-secure-openssl.c 
b/src/interfaces/libpq/fe-secure-openssl.c
index 6fb9d48896..fc91cae7a2 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -477,32 +477,6 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
        return ok;
 }
 
-/*
- * 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 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;
-}
-
 /*
  * OpenSSL-specific wrapper around
  * pq_verify_peer_name_matches_certificate_name(), converting the ASN1_STRING
@@ -997,9 +971,6 @@ initialize_SSL(PGconn *conn)
                SSL_CTX_set_default_passwd_cb_userdata(SSL_context, conn);
        }
 
-       /* Set up a certificate selection callback. */
-       SSL_CTX_set_cert_cb(SSL_context, cert_cb, conn);
-
        /* Disable old protocol versions */
        SSL_CTX_set_options(SSL_context, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
 
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 62554fedde..6216af7f87 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -456,7 +456,6 @@ struct pg_conn
        char       *write_err_msg;      /* write error message, or NULL if OOM 
*/
 
        uint32          allowed_auth_methods;   /* bitmask of acceptable 
AuthRequest codes */
-       bool            allowed_auth_method_cert;       /* tracks "cert" which 
has no AuthRequest code */
 
        /* Transient state needed while establishing connection */
        PGTargetServerType target_server_type;  /* desired session properties */
@@ -522,8 +521,6 @@ 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/authentication/t/001_password.pl 
b/src/test/authentication/t/001_password.pl
index dab50774c4..dd27092134 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -83,9 +83,6 @@ test_role($node, 'md5_role', 'trust', 0,
        log_unlike => [qr/connection authenticated:/]);
 
 # All require_auth options should fail.
-$node->connect_fails("user=scram_role require_auth=cert",
-       "certificate authentication can be required: fails with trust auth",
-       expected_stderr => qr/server did not request a certificate/);
 $node->connect_fails("user=scram_role require_auth=gss",
        "GSS authentication can be required: fails with trust auth",
        expected_stderr => qr/server did not complete authentication/);
@@ -101,9 +98,9 @@ $node->connect_fails("user=scram_role require_auth=md5",
 $node->connect_fails("user=scram_role require_auth=scram-sha-256",
        "SCRAM authentication can be required: fails with trust auth",
        expected_stderr => qr/server did not complete authentication/);
-$node->connect_fails("user=scram_role require_auth=cert,scram-sha-256",
+$node->connect_fails("user=scram_role require_auth=password,scram-sha-256",
        "multiple authentication types can be required: fails with trust auth",
-       expected_stderr => qr/server did not complete authentication or request 
a certificate/);
+       expected_stderr => qr/server did not complete authentication/);
 
 # For plain "password" method, all users should also be able to connect.
 reset_pg_hba($node, 'password');
@@ -117,7 +114,7 @@ test_role($node, 'md5_role', 'password', 0,
 # require_auth should succeed with a plaintext password...
 $node->connect_ok("user=scram_role require_auth=password",
        "password authentication can be required: works with password auth");
-$node->connect_ok("user=scram_role require_auth=cert,password,md5",
+$node->connect_ok("user=scram_role require_auth=scram-sha-256,password,md5",
        "multiple authentication types can be required: works with password 
auth");
 
 # ...and fail for other auth types.
@@ -144,7 +141,7 @@ test_role($node, 'md5_role', 'scram-sha-256', 2,
 # require_auth should succeed with SCRAM...
 $node->connect_ok("user=scram_role require_auth=scram-sha-256",
        "SCRAM authentication can be required: works with SCRAM auth");
-$node->connect_ok("user=scram_role require_auth=password,scram-sha-256,cert",
+$node->connect_ok("user=scram_role require_auth=password,scram-sha-256,md5",
        "multiple authentication types can be required: works with SCRAM auth");
 
 # ...and fail for other auth types.
@@ -174,7 +171,7 @@ test_role($node, 'md5_role', 'md5', 0,
 # require_auth should succeed with MD5...
 $node->connect_ok("user=md5_role require_auth=md5",
        "MD5 authentication can be required: works with MD5 auth");
-$node->connect_ok("user=md5_role require_auth=md5,scram-sha-256,cert",
+$node->connect_ok("user=md5_role require_auth=md5,scram-sha-256,password",
        "multiple authentication types can be required: works with MD5 auth");
 
 # ...and fail for other auth types.
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 68a888e11a..c0b4a5739c 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -505,22 +505,6 @@ note "running server tests";
 $common_connstr =
   "$default_ssl_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require 
dbname=certdb hostaddr=$SERVERHOSTADDR host=localhost";
 
-# require_auth=cert should succeed against the certdb...
-$node->connect_ok(
-       "$common_connstr require_auth=cert user=ssltestuser 
sslcert=ssl/client.crt "
-       . sslkey('client.key'),
-       "certificate authentication can be required: works with cert auth");
-
-# ...and fail against the trustdb, if no certificate is provided.
-$node->connect_fails(
-       "$common_connstr require_auth=cert dbname=trustdb user=ssltestuser",
-       "certificate authentication can be required: fails with trust auth and 
no cert",
-       expected_stderr => qr/server accepted connection without a valid 
certificate/);
-$node->connect_fails(
-       "$common_connstr require_auth=scram-sha-256,cert dbname=trustdb 
user=ssltestuser",
-       "multiple authentication types can be required: fails with trust auth 
and no cert",
-       expected_stderr => qr/server did not complete authentication and 
accepted connection without a valid certificate/);
-
 # no client cert
 $node->connect_fails(
        "$common_connstr user=ssltestuser sslcert=invalid",
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 588f47a39b..9957a96e69 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -132,4 +132,29 @@ $node->connect_ok(
                qr/connection authenticated: identity="ssltestuser" 
method=scram-sha-256/
        ]);
 
+# channel_binding should continue to function 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/
+       );
+}
+
 done_testing();
From 731dbc040a0efa908af978d743de1d1e14fd55c6 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchamp...@vmware.com>
Date: Mon, 28 Feb 2022 09:40:43 -0800
Subject: [PATCH v3] 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.

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, and so the connection can be tied to the client in a
later audit.)

Deficiencies:
- This feature currently doesn't prevent unexpected certificate exchange
  or unexpected GSSAPI encryption.
- This is unlikely to be very forwards-compatible at the moment,
  especially with SASL/SCRAM.
- SSPI support is "implemented" but untested.
---
 doc/src/sgml/libpq.sgml                   |  80 +++++++++++++
 src/include/libpq/pqcomm.h                |   1 +
 src/interfaces/libpq/fe-auth-scram.c      |   1 +
 src/interfaces/libpq/fe-auth.c            | 133 ++++++++++++++++++++++
 src/interfaces/libpq/fe-connect.c         |  77 +++++++++++++
 src/interfaces/libpq/fe-secure-openssl.c  |   1 -
 src/interfaces/libpq/libpq-int.h          |   6 +
 src/test/authentication/t/001_password.pl |  62 ++++++++++
 src/test/kerberos/t/001_auth.pl           |  26 +++++
 src/test/ldap/t/001_auth.pl               |   6 +
 src/test/ssl/t/002_scram.pl               |  25 ++++
 11 files changed, 417 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 37ec3cb4e5..00990ce47d 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1221,6 +1221,76 @@ 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>
+        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>
+        </variablelist>
+      </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="libpq-connect-channel-binding" xreflabel="channel_binding">
       <term><literal>channel_binding</literal></term>
       <listitem>
@@ -7761,6 +7831,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 b418283d5f..a47f7b2d91 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -161,6 +161,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 e616200704..d918e8b87f 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -289,6 +289,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 0a072a36dc..59c3575cc3 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -138,7 +138,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;
 }
@@ -328,6 +331,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;
 }
@@ -836,6 +842,44 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
 	return ret;
 }
 
+/*
+ * Translate an AuthRequest into a human-readable description.
+ */
+static const char *
+auth_description(AuthRequest areq)
+{
+	switch (areq)
+	{
+		case AUTH_REQ_PASSWORD:
+			return libpq_gettext("a cleartext password");
+		case AUTH_REQ_MD5:
+			return libpq_gettext("a hashed password");
+		case AUTH_REQ_GSS:
+		case AUTH_REQ_GSS_CONT:
+			return libpq_gettext("GSSAPI authentication");
+		case AUTH_REQ_SSPI:
+			return libpq_gettext("SSPI authentication");
+		case AUTH_REQ_SCM_CREDS:
+			return libpq_gettext("UNIX socket credentials");
+		case AUTH_REQ_SASL:
+		case AUTH_REQ_SASL_CONT:
+		case AUTH_REQ_SASL_FIN:
+			return libpq_gettext("SASL authentication");
+	}
+
+	return libpq_gettext("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_allowed(conn, type) \
+	(((conn)->allowed_auth_methods & (1 << (type))) != 0)
+
+StaticAssertDecl(AUTH_REQ_MAX < CHAR_BIT * sizeof(((PGconn){0}).allowed_auth_methods),
+				 "AUTH_REQ_MAX overflows the allowed_auth_methods bitmask");
+
 /*
  * Verify that the authentication request is expected, given the connection
  * parameters. This is especially important when the client wishes to
@@ -845,6 +889,92 @@ static bool
 check_expected_areq(AuthRequest areq, PGconn *conn)
 {
 	bool		result = true;
+	char	   *reason = NULL;
+
+	/*
+	 * 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.
+				 */
+				if (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_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.
+					 *
+					 * TODO: check this assumption. What mutual auth guarantees
+					 * are made in this 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_SSPI:
+			case AUTH_REQ_GSS_CONT:
+			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_allowed(conn, areq);
+				break;
+
+			default:
+				result = false;
+				break;
+		}
+	}
+
+	if (!result)
+	{
+		if (reason)
+		{
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("auth method \"%s\" requirement failed: %s\n"),
+							  conn->require_auth, reason);
+		}
+		else
+		{
+			appendPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("auth method \"%s\" required, but server requested %s\n"),
+							  conn->require_auth,
+							  auth_description(areq));
+		}
+
+		return result;
+	}
 
 	/*
 	 * When channel_binding=require, we must protect against two cases: (1) we
@@ -1046,6 +1176,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 6e936bbff3..f2579ff7ee 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -310,6 +310,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)},
@@ -1255,6 +1259,79 @@ connectOptions2(PGconn *conn)
 		}
 	}
 
+	/*
+	 * parse and validate require_auth option
+	 */
+	if (conn->require_auth)
+	{
+		char	   *s = conn->require_auth;
+		bool		more = true;
+
+		while (more)
+		{
+			char	   *method;
+			uint32		bits;
+
+			method = parse_comma_separated_list(&s, &more);
+			if (method == NULL)
+				goto oom_error;
+
+			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
+			{
+				conn->status = CONNECTION_BAD;
+				appendPQExpBuffer(&conn->errorMessage,
+								  libpq_gettext("invalid require_auth method: \"%s\""),
+								  method);
+
+				free(method);
+				return false;
+			}
+
+			/*
+			 * Sanity check; a duplicated method probably indicates a typo in a
+			 * setting where typos are extremely risky.
+			 */
+			if ((conn->allowed_auth_methods & bits) == bits)
+			{
+				conn->status = CONNECTION_BAD;
+				appendPQExpBuffer(&conn->errorMessage,
+								  libpq_gettext("require_auth method \"%s\" is specified more than once"),
+								  method);
+
+				free(method);
+				return false;
+			}
+
+			conn->allowed_auth_methods |= bits;
+			free(method);
+		}
+	}
+
 	/*
 	 * validate channel_binding option
 	 */
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 8117cbd40f..fc91cae7a2 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -477,7 +477,6 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
 	return ok;
 }
 
-
 /*
  * OpenSSL-specific wrapper around
  * pq_verify_peer_name_matches_certificate_name(), converting the ASN1_STRING
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 3db6a17db4..6216af7f87 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -393,6 +393,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;
@@ -454,6 +455,8 @@ 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 */
 
+	uint32		allowed_auth_methods;	/* bitmask of acceptable AuthRequest codes */
+
 	/* Transient state needed while establishing connection */
 	PGTargetServerType target_server_type;	/* desired session properties */
 	bool		try_next_addr;	/* time to advance to next address/host? */
@@ -509,6 +512,9 @@ struct pg_conn
 	bool		error_result;	/* do we need to make an ERROR result? */
 	PGresult   *next_result;	/* next result (used in single-row mode) */
 
+	bool		client_finished_auth; /* have we finished our half of the
+									   * authentication exchange? */
+
 	/* Assorted state for SASL, SSL, GSS, etc */
 	const pg_fe_sasl_mech *sasl;
 	void	   *sasl_state;
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 3e3079c824..dd27092134 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -82,6 +82,26 @@ test_role($node, 'scram_role', 'trust', 0,
 test_role($node, 'md5_role', 'trust', 0,
 	log_unlike => [qr/connection authenticated:/]);
 
+# All require_auth options should fail.
+$node->connect_fails("user=scram_role require_auth=gss",
+	"GSS authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=sspi",
+	"GSS authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=password",
+	"password authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=md5",
+	"md5 authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=scram-sha-256",
+	"SCRAM authentication can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+$node->connect_fails("user=scram_role require_auth=password,scram-sha-256",
+	"multiple authentication types can be required: fails with trust auth",
+	expected_stderr => qr/server did not complete authentication/);
+
 # For plain "password" method, all users should also be able to connect.
 reset_pg_hba($node, 'password');
 test_role($node, 'scram_role', 'password', 0,
@@ -91,6 +111,20 @@ test_role($node, 'md5_role', 'password', 0,
 	log_like =>
 	  [qr/connection authenticated: identity="md5_role" method=password/]);
 
+# require_auth should succeed with a plaintext password...
+$node->connect_ok("user=scram_role require_auth=password",
+	"password authentication can be required: works with password auth");
+$node->connect_ok("user=scram_role require_auth=scram-sha-256,password,md5",
+	"multiple authentication types can be required: works with password auth");
+
+# ...and fail for other auth types.
+$node->connect_fails("user=scram_role require_auth=md5",
+	"md5 authentication can be required: fails with password auth",
+	expected_stderr => qr/server requested a cleartext password/);
+$node->connect_fails("user=scram_role require_auth=scram-sha-256",
+	"SCRAM authentication can be required: fails with password auth",
+	expected_stderr => qr/server requested a cleartext password/);
+
 # For "scram-sha-256" method, user "scram_role" should be able to connect.
 reset_pg_hba($node, 'scram-sha-256');
 test_role(
@@ -104,6 +138,20 @@ test_role(
 test_role($node, 'md5_role', 'scram-sha-256', 2,
 	log_unlike => [qr/connection authenticated:/]);
 
+# require_auth should succeed with SCRAM...
+$node->connect_ok("user=scram_role require_auth=scram-sha-256",
+	"SCRAM authentication can be required: works with SCRAM auth");
+$node->connect_ok("user=scram_role require_auth=password,scram-sha-256,md5",
+	"multiple authentication types can be required: works with SCRAM auth");
+
+# ...and fail for other auth types.
+$node->connect_fails("user=scram_role require_auth=password",
+	"password authentication can be required: fails with SCRAM auth",
+	expected_stderr => qr/server requested SASL authentication/);
+$node->connect_fails("user=scram_role require_auth=md5",
+	"md5 authentication can be required: fails with SCRAM auth",
+	expected_stderr => qr/server requested SASL authentication/);
+
 # Test that bad passwords are rejected.
 $ENV{"PGPASSWORD"} = 'badpass';
 test_role($node, 'scram_role', 'scram-sha-256', 2,
@@ -120,6 +168,20 @@ test_role($node, 'md5_role', 'md5', 0,
 	log_like =>
 	  [qr/connection authenticated: identity="md5_role" method=md5/]);
 
+# require_auth should succeed with MD5...
+$node->connect_ok("user=md5_role require_auth=md5",
+	"MD5 authentication can be required: works with MD5 auth");
+$node->connect_ok("user=md5_role require_auth=md5,scram-sha-256,password",
+	"multiple authentication types can be required: works with MD5 auth");
+
+# ...and fail for other auth types.
+$node->connect_fails("user=md5_role require_auth=password",
+	"password authentication can be required: fails with MD5 auth",
+	expected_stderr => qr/server requested a hashed password/);
+$node->connect_fails("user=md5_role require_auth=scram-sha-256",
+	"SCRAM authentication can be required: fails with MD5 auth",
+	expected_stderr => qr/server requested a hashed password/);
+
 # Tests for channel binding without SSL.
 # Using the password authentication method; channel binding can't work
 reset_pg_hba($node, 'password');
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 62e0542639..de4ddcbf69 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -307,6 +307,24 @@ test_query(
 	'gssencmode=require',
 	'sending 100K lines works');
 
+# require_auth=gss should succeed...
+$node->connect_ok(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr gssencmode=disable require_auth=gss",
+	"GSS authentication can be 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 can be requested: works with GSS auth with encryption");
+
+# ...and require_auth=sspi should fail.
+$node->connect_fails(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr gssencmode=disable require_auth=sspi",
+	"SSPI authentication can be requested: fails with GSS auth without encryption",
+	expected_stderr => qr/server requested GSSAPI authentication/);
+$node->connect_fails(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr gssencmode=require require_auth=sspi",
+	"SSPI authentication can be requested: fails with GSS auth with encryption",
+	expected_stderr => qr/server did not complete authentication/);
+
 unlink($node->data_dir . '/pg_hba.conf');
 $node->append_conf('pg_hba.conf',
 	qq{hostgssenc all all $hostaddr/32 gss map=mymap});
@@ -335,6 +353,14 @@ test_access(
 test_access($node, 'test1', 'SELECT true', 2, 'gssencmode=disable',
 	'fails with GSS encryption disabled and hostgssenc hba');
 
+# require_auth=gss should succeed.
+$node->connect_ok(
+	$node->connstr('postgres') . " user=test1 host=$host hostaddr=$hostaddr gssencmode=require require_auth=gss",
+	"GSS authentication can be 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 can be 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 86dff8bd1f..e7c67920a1 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -216,6 +216,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 can be required: works with ldap auth");
+$node->connect_fails("user=test1 require_auth=scram-sha-256",
+	"SCRAM authentication can be 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 588f47a39b..9957a96e69 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -132,4 +132,29 @@ $node->connect_ok(
 		qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/
 	]);
 
+# channel_binding should continue to function 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/
+	);
+}
+
 done_testing();
-- 
2.25.1

Reply via email to