]> granicus.if.org Git - postgresql/commitdiff
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