Add libpq parameter 'channel_binding'.
authorJeff Davis <jdavis@postgresql.org>
Mon, 23 Sep 2019 20:45:23 +0000 (13:45 -0700)
committerJeff Davis <jdavis@postgresql.org>
Mon, 23 Sep 2019 21:03:35 +0000 (14:03 -0700)
Allow clients to require channel binding to enhance security against
untrusted servers.

Author: Jeff Davis
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/227015d8417f2b4fef03f8966dbfa5cbcc4f44da.camel%40j-davis.com

doc/src/sgml/libpq.sgml
src/interfaces/libpq/fe-auth-scram.c
src/interfaces/libpq/fe-auth.c
src/interfaces/libpq/fe-auth.h
src/interfaces/libpq/fe-connect.c
src/interfaces/libpq/libpq-int.h
src/test/authentication/t/001_password.pl
src/test/ssl/t/002_scram.pl
src/test/ssl/t/SSLServer.pm

index 1189341ca15dbe4f572ade6221cfafee67f18d33..c58527b0c3b98c09a0cf3b1d9313487ffc67c41b 100644 (file)
@@ -1122,6 +1122,28 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       </listitem>
      </varlistentry>
 
+     <varlistentry id="libpq-connect-channel-binding" xreflabel="channel_binding">
+      <term><literal>channel_binding</literal></term>
+      <listitem>
+      <para>
+        This option controls the client's use of channel binding. A setting
+        of <literal>require</literal> means that the connection must employ
+        channel binding, <literal>prefer</literal> means that the client will
+        choose channel binding if available, and <literal>disable</literal>
+        prevents the use of channel binding. The default
+        is <literal>prefer</literal> if
+        <productname>PostgreSQL</productname> is compiled with SSL support;
+        otherwise the default is <literal>disable</literal>.
+      </para>
+      <para>
+        Channel binding is a method for the server to authenticate itself to
+        the client. It is only supported over SSL connections
+        with <productname>PostgreSQL</productname> 11 or later servers using
+        the <literal>SCRAM</literal> authentication method.
+      </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="libpq-connect-connect-timeout" xreflabel="connect_timeout">
       <term><literal>connect_timeout</literal></term>
       <listitem>
@@ -6864,6 +6886,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
      </para>
     </listitem>
 
+    <listitem>
+     <para>
+      <indexterm>
+       <primary><envar>PGCHANNELBINDING</envar></primary>
+      </indexterm>
+      <envar>PGCHANNELBINDING</envar> behaves the same as the <xref
+      linkend="libpq-connect-channel-binding"/> connection parameter.
+     </para>
+    </listitem>
+
     <listitem>
      <para>
       <indexterm>
index 7a8335bf9f82539e71c168a223aa504f7a4e118b..693739c544278c4ea78441123dc72fee273407c2 100644 (file)
@@ -119,6 +119,35 @@ pg_fe_scram_init(PGconn *conn,
    return state;
 }
 
+/*
+ * Return true if channel binding was employed and the SCRAM exchange
+ * completed. This should be used after a successful exchange to determine
+ * whether the server authenticated itself to the client.
+ *
+ * Note that the caller must also ensure that the exchange was actually
+ * successful.
+ */
+bool
+pg_fe_scram_channel_bound(void *opaq)
+{
+   fe_scram_state *state = (fe_scram_state *) opaq;
+
+   /* no SCRAM exchange done */
+   if (state == NULL)
+       return false;
+
+   /* SCRAM exchange not completed */
+   if (state->state != FE_SCRAM_FINISHED)
+       return false;
+
+   /* channel binding mechanism not used */
+   if (strcmp(state->sasl_mechanism, SCRAM_SHA_256_PLUS_NAME) != 0)
+       return false;
+
+   /* all clear! */
+   return true;
+}
+
 /*
  * Free SCRAM exchange status
  */
@@ -225,9 +254,7 @@ pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
 
            /*
             * Verify server signature, to make sure we're talking to the
-            * genuine server.  XXX: A fake server could simply not require
-            * authentication, though.  There is currently no option in libpq
-            * to reject a connection, if SCRAM authentication did not happen.
+            * genuine server.
             */
            if (verify_server_signature(state))
                *success = true;
@@ -358,7 +385,8 @@ build_client_first_message(fe_scram_state *state)
        appendPQExpBufferStr(&buf, "p=tls-server-end-point");
    }
 #ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
-   else if (conn->ssl_in_use)
+   else if (conn->channel_binding[0] != 'd' && /* disable */
+            conn->ssl_in_use)
    {
        /*
         * Client supports channel binding, but thinks the server does not.
@@ -369,7 +397,7 @@ build_client_first_message(fe_scram_state *state)
    else
    {
        /*
-        * Client does not support channel binding.
+        * Client does not support channel binding, or has disabled it.
         */
        appendPQExpBufferChar(&buf, 'n');
    }
@@ -498,7 +526,8 @@ build_client_final_message(fe_scram_state *state)
 #endif                         /* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
    }
 #ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
-   else if (conn->ssl_in_use)
+   else if (conn->channel_binding[0] != 'd' && /* disable */
+            conn->ssl_in_use)
        appendPQExpBufferStr(&buf, "c=eSws");   /* base64 of "y,," */
 #endif
    else
index ab227421b3ba125f829707d26f12dd595542d20d..cd29e8bd126e971c23de8774e963779c523e927c 100644 (file)
@@ -423,6 +423,14 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 
    initPQExpBuffer(&mechanism_buf);
 
+   if (conn->channel_binding[0] == 'r' &&  /* require */
+       !conn->ssl_in_use)
+   {
+       printfPQExpBuffer(&conn->errorMessage,
+                         libpq_gettext("Channel binding required, but SSL not in use\n"));
+       goto error;
+   }
+
    if (conn->sasl_state)
    {
        printfPQExpBuffer(&conn->errorMessage,
@@ -454,10 +462,10 @@ pg_SASL_init(PGconn *conn, int payloadlen)
 
        /*
         * Select the mechanism to use.  Pick SCRAM-SHA-256-PLUS over anything
-        * else if a channel binding type is set and if the client supports
-        * it. Pick SCRAM-SHA-256 if nothing else has already been picked.  If
-        * we add more mechanisms, a more refined priority mechanism might
-        * become necessary.
+        * else if a channel binding type is set and if the client supports it
+        * (and did not set channel_binding=disable). Pick SCRAM-SHA-256 if
+        * nothing else has already been picked.  If we add more mechanisms, a
+        * more refined priority mechanism might become necessary.
         */
        if (strcmp(mechanism_buf.data, SCRAM_SHA_256_PLUS_NAME) == 0)
        {
@@ -466,10 +474,11 @@ pg_SASL_init(PGconn *conn, int payloadlen)
                /*
                 * The server has offered SCRAM-SHA-256-PLUS, which is only
                 * supported by the client if a hash of the peer certificate
-                * can be created.
+                * can be created, and if channel_binding is not disabled.
                 */
 #ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
-               selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
+               if (conn->channel_binding[0] != 'd')    /* disable */
+                   selected_mechanism = SCRAM_SHA_256_PLUS_NAME;
 #endif
            }
            else
@@ -493,6 +502,14 @@ pg_SASL_init(PGconn *conn, int payloadlen)
            selected_mechanism = SCRAM_SHA_256_NAME;
    }
 
+   if (conn->channel_binding[0] == 'r' &&  /* require */
+       strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) != 0)
+   {
+       printfPQExpBuffer(&conn->errorMessage,
+                         libpq_gettext("channel binding is required, but server did not offer an authentication method that supports channel binding\n"));
+       goto error;
+   }
+
    if (!selected_mechanism)
    {
        printfPQExpBuffer(&conn->errorMessage,
@@ -774,6 +791,50 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
    return ret;
 }
 
+/*
+ * Verify that the authentication request is expected, given the connection
+ * parameters. This is especially important when the client wishes to
+ * authenticate the server before any sensitive information is exchanged.
+ */
+static bool
+check_expected_areq(AuthRequest areq, PGconn *conn)
+{
+   bool        result = true;
+
+   /*
+    * When channel_binding=require, we must protect against two cases: (1) we
+    * must not respond to non-SASL authentication requests, which might leak
+    * information such as the client's password; and (2) even if we receive
+    * AUTH_REQ_OK, we still must ensure that channel binding has happened in
+    * order to authenticate the server.
+    */
+   if (conn->channel_binding[0] == 'r' /* require */ )
+   {
+       switch (areq)
+       {
+           case AUTH_REQ_SASL:
+           case AUTH_REQ_SASL_CONT:
+           case AUTH_REQ_SASL_FIN:
+               break;
+           case AUTH_REQ_OK:
+               if (!pg_fe_scram_channel_bound(conn->sasl_state))
+               {
+                   printfPQExpBuffer(&conn->errorMessage,
+                                     libpq_gettext("Channel binding required, but server authenticated client without channel binding\n"));
+                   result = false;
+               }
+               break;
+           default:
+               printfPQExpBuffer(&conn->errorMessage,
+                                 libpq_gettext("Channel binding required but not supported by server's authentication request\n"));
+               result = false;
+               break;
+       }
+   }
+
+   return result;
+}
+
 /*
  * pg_fe_sendauth
  *     client demux routine for processing an authentication request
@@ -788,6 +849,9 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
 int
 pg_fe_sendauth(AuthRequest areq, int payloadlen, PGconn *conn)
 {
+   if (!check_expected_areq(areq, conn))
+       return STATUS_ERROR;
+
    switch (areq)
    {
        case AUTH_REQ_OK:
index 122ba5ccbafc82d6c8173c332a403fa0c2254b67..2f1af53fb08198bee008ef00fc6a658390a08319 100644 (file)
@@ -26,6 +26,7 @@ extern char *pg_fe_getauthname(PQExpBuffer errorMessage);
 extern void *pg_fe_scram_init(PGconn *conn,
                              const char *password,
                              const char *sasl_mechanism);
+extern bool pg_fe_scram_channel_bound(void *opaq);
 extern void pg_fe_scram_free(void *opaq);
 extern void pg_fe_scram_exchange(void *opaq, char *input, int inputlen,
                                 char **output, int *outputlen,
index 8ba0159313cb826507e42552b6c3dcb42b454e7d..f91f0f2efe7f286f66a0641b484bfb03527c1a0b 100644 (file)
@@ -124,6 +124,11 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
 #define DefaultTty     ""
 #define DefaultOption  ""
 #define DefaultAuthtype          ""
+#ifdef USE_SSL
+#define DefaultChannelBinding  "prefer"
+#else
+#define DefaultChannelBinding  "disable"
+#endif
 #define DefaultTargetSessionAttrs  "any"
 #ifdef USE_SSL
 #define DefaultSSLMode "prefer"
@@ -211,6 +216,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
        "Database-Password-File", "", 64,
    offsetof(struct pg_conn, pgpassfile)},
 
+   {"channel_binding", "PGCHANNELBINDING", NULL, NULL,
+       "Channel-Binding", "", 7,   /* sizeof("require") */
+   offsetof(struct pg_conn, channel_binding)},
+
    {"connect_timeout", "PGCONNECT_TIMEOUT", NULL, NULL,
        "Connect-timeout", "", 10,  /* strlen(INT32_MAX) == 10 */
    offsetof(struct pg_conn, connect_timeout)},
@@ -1197,6 +1206,29 @@ connectOptions2(PGconn *conn)
        }
    }
 
+   /*
+    * validate channel_binding option
+    */
+   if (conn->channel_binding)
+   {
+       if (strcmp(conn->channel_binding, "disable") != 0
+           && strcmp(conn->channel_binding, "prefer") != 0
+           && strcmp(conn->channel_binding, "require") != 0)
+       {
+           conn->status = CONNECTION_BAD;
+           printfPQExpBuffer(&conn->errorMessage,
+                             libpq_gettext("invalid channel_binding value: \"%s\"\n"),
+                             conn->channel_binding);
+           return false;
+       }
+   }
+   else
+   {
+       conn->channel_binding = strdup(DefaultChannelBinding);
+       if (!conn->channel_binding)
+           goto oom_error;
+   }
+
    /*
     * validate sslmode option
     */
@@ -3485,10 +3517,11 @@ keep_going:                     /* We will come back to here until there is
        case CONNECTION_SETENV:
            {
                /*
-                * Do post-connection housekeeping (only needed in protocol 2.0).
+                * Do post-connection housekeeping (only needed in protocol
+                * 2.0).
                 *
-                * We pretend that the connection is OK for the duration of these
-                * queries.
+                * We pretend that the connection is OK for the duration of
+                * these queries.
                 */
                conn->status = CONNECTION_OK;
 
@@ -3905,6 +3938,8 @@ freePGconn(PGconn *conn)
    }
    if (conn->pgpassfile)
        free(conn->pgpassfile);
+   if (conn->channel_binding)
+       free(conn->channel_binding);
    if (conn->keepalives)
        free(conn->keepalives);
    if (conn->keepalives_idle)
index d37bb3ce40482e03784b389da544dac05f29d6a5..64468ab4dab96bc765b6a8a7e20b17c9e2a1aff4 100644 (file)
@@ -347,6 +347,8 @@ struct pg_conn
    char       *pguser;         /* Postgres username and password, if any */
    char       *pgpass;
    char       *pgpassfile;     /* path to a file containing password(s) */
+   char       *channel_binding;    /* channel binding mode
+                                    * (require,prefer,disable) */
    char       *keepalives;     /* use TCP keepalives? */
    char       *keepalives_idle;    /* time between TCP keepalives */
    char       *keepalives_interval;    /* time between TCP keepalive
index 3a3b0eb7e80c1923699b943202f46646ea7ef15d..aae6de8b345cb12adcfe463dd1991de214a76ea2 100644 (file)
@@ -17,7 +17,7 @@ if ($windows_os)
 }
 else
 {
-   plan tests => 8;
+   plan tests => 10;
 }
 
 
@@ -86,3 +86,13 @@ test_role($node, 'md5_role',   'scram-sha-256', 2);
 reset_pg_hba($node, 'md5');
 test_role($node, 'scram_role', 'md5', 0);
 test_role($node, 'md5_role',   'md5', 0);
+
+# Tests for channel binding without SSL.
+# Using the password authentication method; channel binding can't work
+reset_pg_hba($node, 'password');
+$ENV{"PGCHANNELBINDING"} = 'require';
+test_role($node, 'scram_role', 'scram-sha-256', 2);
+# SSL not in use; channel binding still can't work
+reset_pg_hba($node, 'scram-sha-256');
+$ENV{"PGCHANNELBINDING"} = 'require';
+test_role($node, 'scram_role', 'scram-sha-256', 2);
index 7c4b821cb7874c92001fb791c56f4591352a5e06..5fa2dbde1c144d80463c6034bcfcef19cf800cd9 100644 (file)
@@ -18,7 +18,7 @@ if ($ENV{with_openssl} ne 'yes')
    plan skip_all => 'SSL not supported by this build';
 }
 
-my $number_of_tests = 1;
+my $number_of_tests = 9;
 
 # This is the hostname used to connect to the server.
 my $SERVERHOSTADDR = '127.0.0.1';
@@ -44,9 +44,42 @@ configure_test_server_for_ssl($node, $SERVERHOSTADDR, "scram-sha-256",
 switch_server_cert($node, 'server-cn-only');
 $ENV{PGPASSWORD} = "pass";
 $common_connstr =
-  "user=ssltestuser dbname=trustdb sslmode=require sslcert=invalid sslrootcert=invalid hostaddr=$SERVERHOSTADDR";
+  "dbname=trustdb sslmode=require sslcert=invalid sslrootcert=invalid hostaddr=$SERVERHOSTADDR";
 
 # Default settings
-test_connect_ok($common_connstr, '', "Basic SCRAM authentication with SSL");
+test_connect_ok($common_connstr, "user=ssltestuser",
+   "Basic SCRAM authentication with SSL");
+
+# Test channel_binding
+test_connect_fails(
+   $common_connstr,
+   "user=ssltestuser channel_binding=invalid_value",
+   qr/invalid channel_binding value: "invalid_value"/,
+   "SCRAM with SSL and channel_binding=invalid_value");
+test_connect_ok(
+   $common_connstr,
+   "user=ssltestuser channel_binding=disable",
+   "SCRAM with SSL and channel_binding=disable");
+test_connect_ok(
+   $common_connstr,
+   "user=ssltestuser channel_binding=require",
+   "SCRAM with SSL and channel_binding=require");
+
+# Now test when the user has an MD5-encrypted password; should fail
+test_connect_fails(
+   $common_connstr,
+   "user=md5testuser channel_binding=require",
+   qr/Channel binding required but not supported by server's authentication request/,
+   "MD5 with SSL and channel_binding=require");
+
+# Now test with auth method 'cert' by connecting to 'certdb'. Should
+# fail, because channel binding is not performed.
+copy("ssl/client.key", "ssl/client_tmp.key");
+chmod 0600, "ssl/client_tmp.key";
+test_connect_fails(
+   "sslcert=ssl/client.crt sslkey=ssl/client_tmp.key hostaddr=$SERVERHOSTADDR",
+   "dbname=certdb user=ssltestuser channel_binding=require",
+   qr/Channel binding required, but server authenticated client without channel binding/,
+   "Cert authentication and channel_binding=require");
 
 done_testing($number_of_tests);
index d25c38dbbc7f1271f119ded38ebfde52e85ecb73..005955a2ff736ea094c4bee504c47a66f8566026 100644 (file)
@@ -102,6 +102,7 @@ sub configure_test_server_for_ssl
 
    # Create test users and databases
    $node->psql('postgres', "CREATE USER ssltestuser");
+   $node->psql('postgres', "CREATE USER md5testuser");
    $node->psql('postgres', "CREATE USER anotheruser");
    $node->psql('postgres', "CREATE USER yetanotheruser");
    $node->psql('postgres', "CREATE DATABASE trustdb");
@@ -114,6 +115,10 @@ sub configure_test_server_for_ssl
        $node->psql('postgres',
            "SET password_encryption='$password_enc'; ALTER USER ssltestuser PASSWORD '$password';"
        );
+       # A special user that always has an md5-encrypted password
+       $node->psql('postgres',
+           "SET password_encryption='md5'; ALTER USER md5testuser PASSWORD '$password';"
+       );
        $node->psql('postgres',
            "SET password_encryption='$password_enc'; ALTER USER anotheruser PASSWORD '$password';"
        );
@@ -128,7 +133,7 @@ sub configure_test_server_for_ssl
    print $conf "log_statement=all\n";
 
    # enable SSL and set up server key
-   print $conf "include 'sslconfig.conf'";
+   print $conf "include 'sslconfig.conf'\n";
 
    close $conf;
 
@@ -186,6 +191,8 @@ sub configure_hba_for_ssl
    open my $hba, '>', "$pgdata/pg_hba.conf";
    print $hba
      "# TYPE  DATABASE        USER            ADDRESS                 METHOD             OPTIONS\n";
+   print $hba
+     "hostssl trustdb         md5testuser     $serverhost/32            md5\n";
    print $hba
      "hostssl trustdb         all             $serverhost/32            $authmethod\n";
    print $hba