libpq: Add sslcertmode option to control client certificates
authorMichael Paquier <[email protected]>
Fri, 24 Mar 2023 04:34:26 +0000 (13:34 +0900)
committerMichael Paquier <[email protected]>
Fri, 24 Mar 2023 04:34:26 +0000 (13:34 +0900)
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, where a
configured client certificate is sent if the server requests one
(via one of its default locations or sslcert).  With the current
implementation, will happen whenever TLS is negotiated.
- "disable" causes the client to refuse to send a client certificate
even if sslcert is configured or if a client certificate is available in
one of its default locations.
- "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 helpful to troubleshoot
more complicated TLS setups.

sslcertmode=require requires SSL_CTX_set_cert_cb(), available since
OpenSSL 1.0.2.  Note that LibreSSL does not include it.

Using a connection parameter different than require_auth has come up as
the simplest design because certificate authentication does not rely
directly on any of the AUTH_REQ_* codes, and one may want to require a
certificate to be sent in combination of a given authentication method,
like SCRAM-SHA-256.

TAP tests are added in src/test/ssl/, some of them relying on sslinfo to
check if a certificate has been set.  These are compatible across all
the versions of OpenSSL supported on HEAD (currently down to 1.0.1).

Author: Jacob Champion
Reviewed-by: Aleksander Alekseev, Peter Eisentraut, David G. Johnston,
Michael Paquier
Discussion: https://postgr.es/m/9e5a8ccddb8355ea9fa4b75a1e3a9edc88a70cd3[email protected]

12 files changed:
configure
configure.ac
doc/src/sgml/libpq.sgml
meson.build
src/include/pg_config.h.in
src/interfaces/libpq/fe-auth.c
src/interfaces/libpq/fe-connect.c
src/interfaces/libpq/fe-secure-openssl.c
src/interfaces/libpq/libpq-int.h
src/test/ssl/t/001_ssltests.pl
src/test/ssl/t/003_sslinfo.pl
src/tools/msvc/Solution.pm

index e221dd5b0ff2cb02084c3f5cbfa606582b904f61..905be9568bf743d2b472194c66113f6eb8d37f9b 100755 (executable)
--- a/configure
+++ b/configure
@@ -12973,13 +12973,15 @@ 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 does not have
+  # SSL_CTX_set_cert_cb().
+  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
index 3aa6c15c13c363bf75eb9a058f8ff422fb692be4..8095dfcf1d1fe482756c8f555d520b79deae0091 100644 (file)
@@ -1373,8 +1373,9 @@ 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 does not have
+  # SSL_CTX_set_cert_cb().
+  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
index 9ee5532c076030c7a9f0f9ff857299ea3d6f0aef..8579dcac952c1a3fbd16d4291c8cde4d37f1a5f2 100644 (file)
@@ -1810,6 +1810,62 @@ 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 available
+            (default location or 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 the
+            client has one to send.
+           </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>
@@ -7986,6 +8042,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>
index 09f619b12fdc04361b077f1c611bc471efe9c09e..77ee015ea9c4a51a1b0045549445c04be120c124 100644 (file)
@@ -1221,8 +1221,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.
       ['X509_get_signature_nid'],
+      ['SSL_CTX_set_cert_cb'], # not in LibreSSL
 
       # 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
index 4882c7055903d262d12aef66eb133a23367bdba3..3665e799e7b926de8cb8174a6e94004401028412 100644 (file)
 /* 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
 
index fa95f8e6e961dae8dfe7c9e67addda0a0fe405d5..934e3f4f7ca89b95921d04ef1544d96e71c62152 100644 (file)
@@ -798,6 +798,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 an SSL certificate");
+           return false;
+       }
+       else if (!conn->ssl_cert_sent)
+       {
+           libpq_append_conn_error(conn, "server accepted connection without a valid SSL 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
index 7d2be5666485ce784a9def935a2e2d33d0b1e818..660775e0198310706406e9e58dae46ec4691664e 100644 (file)
@@ -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)},
@@ -1506,6 +1512,52 @@ 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, "%s value \"%s\" invalid when SSL support is not compiled in",
+                                   "sslcertmode", 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 certificate was actually requested, so "require" is
+        * useless.
+        */
+       if (strcmp(conn->sslcertmode, "require") == 0)
+       {
+           conn->status = CONNECTION_BAD;
+           libpq_append_conn_error(conn, "%s value \"%s\" is not supported (check OpenSSL version)",
+                                   "sslcertmode", conn->sslcertmode);
+           return false;
+       }
+#endif
+   }
+   else
+   {
+       conn->sslcertmode = strdup(DefaultSSLCertMode);
+       if (!conn->sslcertmode)
+           goto oom_error;
+   }
+
    /*
     * validate gssencmode option
     */
@@ -4238,6 +4290,7 @@ freePGconn(PGconn *conn)
        explicit_bzero(conn->sslpassword, strlen(conn->sslpassword));
        free(conn->sslpassword);
    }
+   free(conn->sslcertmode);
    free(conn->sslrootcert);
    free(conn->sslcrl);
    free(conn->sslcrldir);
index 6a4431ddfe99395a4771c26187a388b5e67bf40a..4d1e4009ef133dc4370fd7c4ad9837c6e319440f 100644 (file)
@@ -462,6 +462,34 @@ 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 +981,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 +1140,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;
index 1dc264fe544962e9cad44a840dfcffccac67a6ee..10187c31b9a90d6f35e18a1e0abf0b90975415b1 100644 (file)
@@ -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 */
index 3094e27af3a85a1371de8ad96525bb8a22597d21..dc43b8f81aaa77f03e162699c191fe79234f92c3 100644 (file)
@@ -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 SSL certificate/
+   : qr/sslcertmode value "require" is not supported/);
+
 # CRL tests
 
 # Invalid CRL filename is the same as no CRL, succeeds
@@ -538,6 +558,28 @@ $node->connect_ok(
    "certificate authorization succeeds with correct client cert in encrypted DER format"
 );
 
+# correct client cert with sslcertmode=allow or require
+if ($supports_sslcertmode_require)
+{
+   $node->connect_ok(
+       "$common_connstr user=ssltestuser sslcertmode=require sslcert=ssl/client.crt "
+         . sslkey('client.key'),
+       "certificate authorization succeeds with correct client cert and sslcertmode=require"
+   );
+}
+$node->connect_ok(
+   "$common_connstr user=ssltestuser sslcertmode=allow sslcert=ssl/client.crt "
+     . sslkey('client.key'),
+   "certificate authorization succeeds with correct client cert and sslcertmode=allow"
+);
+
+# client cert is not sent if sslcertmode=disable.
+$node->connect_fails(
+   "$common_connstr user=ssltestuser sslcertmode=disable sslcert=ssl/client.crt "
+     . sslkey('client.key'),
+   "certificate authorization fails with correct client cert and 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 "
index 3f498fff704ae74da98c1b36b7cb87b5a28b169d..c073625213e5942e73cfc20873ae7d0068bf1282 100644 (file)
@@ -43,6 +43,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;
 
@@ -166,4 +170,24 @@ $result = $node->safe_psql(
    connstr => $common_connstr);
 is($result, 'CA:FALSE|t', 'extract extension from cert');
 
+# Sanity tests for sslcertmode, using ssl_client_cert_present()
+my @cases = (
+   { opts => "sslcertmode=allow",                 present => 't' },
+   { opts => "sslcertmode=allow sslcert=invalid", present => 'f' },
+   { opts => "sslcertmode=disable",               present => 'f' },);
+if ($supports_sslcertmode_require)
+{
+   push(@cases, { opts => "sslcertmode=require", present => 't' });
+}
+
+foreach my $c (@cases)
+{
+   $result = $node->safe_psql(
+       "trustdb",
+       "SELECT ssl_client_cert_present();",
+       connstr => "$common_connstr dbname=trustdb $c->{'opts'}");
+   is($result, $c->{'present'},
+       "ssl_client_cert_present() for $c->{'opts'}");
+}
+
 done_testing();
index b59953e5b5983f402034dfe6cb6b11cf408dcedb..153be7be11ca34550a2a51ffa6b82fedb79eeca1 100644 (file)
@@ -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,
@@ -506,6 +507,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);