From 36f40ce2dc66f1a36d6a12f7a0352e1c5bf1063e Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 24 Mar 2023 13:34:26 +0900 Subject: libpq: Add sslcertmode option to control client certificates The sslcertmode option controls whether the server is allowed and/or required to request a certificate from the client. There are three modes: - "allow" is the default and follows the current behavior, 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.camel@vmware.com --- src/interfaces/libpq/fe-auth.c | 19 ++++++++++++ src/interfaces/libpq/fe-connect.c | 53 ++++++++++++++++++++++++++++++++ src/interfaces/libpq/fe-secure-openssl.c | 40 +++++++++++++++++++++++- src/interfaces/libpq/libpq-int.h | 3 ++ 4 files changed, 114 insertions(+), 1 deletion(-) (limited to 'src/interfaces/libpq') diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index fa95f8e6e9..934e3f4f7c 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -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 diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 7d2be56664..660775e019 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -125,8 +125,10 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options, #define DefaultTargetSessionAttrs "any" #ifdef USE_SSL #define DefaultSSLMode "prefer" +#define DefaultSSLCertMode "allow" #else #define DefaultSSLMode "disable" +#define DefaultSSLCertMode "disable" #endif #ifdef ENABLE_GSS #include "fe-gssapi-common.h" @@ -283,6 +285,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "SSL-Client-Key", "", 64, offsetof(struct pg_conn, sslkey)}, + {"sslcertmode", "PGSSLCERTMODE", NULL, NULL, + "SSL-Client-Cert-Mode", "", 8, /* sizeof("disable") == 8 */ + offsetof(struct pg_conn, sslcertmode)}, + {"sslpassword", NULL, NULL, NULL, "SSL-Client-Key-Password", "*", 20, offsetof(struct pg_conn, sslpassword)}, @@ -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); diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index 6a4431ddfe..4d1e4009ef 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -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; diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 1dc264fe54..10187c31b9 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -384,6 +384,7 @@ struct pg_conn char *sslkey; /* client key filename */ char *sslcert; /* client certificate filename */ char *sslpassword; /* client key file password */ + char *sslcertmode; /* client cert mode (require,allow,disable) */ char *sslrootcert; /* root certificate filename */ char *sslcrl; /* certificate revocation list filename */ char *sslcrldir; /* certificate revocation list directory name */ @@ -527,6 +528,8 @@ struct pg_conn /* SSL structures */ bool ssl_in_use; + bool ssl_cert_requested; /* Did the server ask us for a cert? */ + bool ssl_cert_sent; /* Did we send one in reply? */ #ifdef USE_SSL bool allow_ssl_try; /* Allowed to try SSL negotiation */ -- cgit v1.2.1