From de41869b64d57160f58852eab20a27f248188135 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 2 Jan 2017 21:37:12 -0500 Subject: [PATCH] Allow SSL configuration to be updated at SIGHUP. It is no longer necessary to restart the server to enable, disable, or reconfigure SSL. Instead, we just create a new SSL_CTX struct (by re-reading all relevant files) whenever we get SIGHUP. Testing shows that this is fast enough that it shouldn't be a problem. In conjunction with that, downgrade the logic that complains about pg_hba.conf "hostssl" lines when SSL isn't active: now that's just a warning condition not an error. An issue that still needs to be addressed is what shall we do with passphrase-protected server keys? As this stands, the server would demand the passphrase again on every SIGHUP, which is certainly impractical. But the case was only barely supported before, so that does not seem a sufficient reason to hold up committing this patch. Andreas Karlsson, reviewed by Michael Banck and Michael Paquier Discussion: https://postgr.es/m/556A6E8A.9030400@proxel.se --- doc/src/sgml/client-auth.sgml | 4 +- doc/src/sgml/config.sgml | 84 +++-- doc/src/sgml/runtime.sgml | 19 +- src/backend/libpq/auth.c | 22 +- src/backend/libpq/be-secure-openssl.c | 315 ++++++++++++------ src/backend/libpq/be-secure.c | 23 +- src/backend/libpq/hba.c | 29 +- src/backend/postmaster/postmaster.c | 45 ++- src/backend/utils/misc/guc.c | 18 +- src/backend/utils/misc/postgresql.conf.sample | 15 +- src/include/libpq/libpq-be.h | 3 +- src/include/libpq/libpq.h | 2 +- src/test/ssl/ServerSetup.pm | 58 ++-- 13 files changed, 400 insertions(+), 237 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 960f5b5871..dda5891900 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -156,9 +156,11 @@ hostnossl database user To make use of this option the server must be built with SSL support. Furthermore, - SSL must be enabled at server start time + SSL must be enabled by setting the configuration parameter (see for more information). + Otherwise, the hostssl record is ignored except for + logging a warning that it cannot match any connections. diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 8d7b3bfd66..30dd54cd5d 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -958,10 +958,10 @@ include_dir 'conf.d' Enables SSL connections. Please read - before using this. The default - is off. This parameter can only be set at server - start. SSL communication is only possible with - TCP/IP connections. + before using this. + This parameter can only be set in the postgresql.conf + file or on the server command line. + The default is off. @@ -975,11 +975,16 @@ include_dir 'conf.d' Specifies the name of the file containing the SSL server certificate - authority (CA). The default is empty, meaning no CA file is loaded, - and client certificate verification is not performed. (In previous - releases of PostgreSQL, the name of this file was hard-coded - as root.crt.) Relative paths are relative to the - data directory. This parameter can only be set at server start. + authority (CA). + Relative paths are relative to the data directory. + This parameter can only be set in the postgresql.conf + file or on the server command line. + The default is empty, meaning no CA file is loaded, + and client certificate verification is not performed. + + + In previous releases of PostgreSQL, the name of this file was + hard-coded as root.crt. @@ -993,9 +998,10 @@ include_dir 'conf.d' Specifies the name of the file containing the SSL server certificate. - The default is server.crt. Relative paths are - relative to the data directory. This parameter can only be set at - server start. + Relative paths are relative to the data directory. + This parameter can only be set in the postgresql.conf + file or on the server command line. + The default is server.crt. @@ -1009,11 +1015,15 @@ include_dir 'conf.d' Specifies the name of the file containing the SSL server certificate - revocation list (CRL). The default is empty, meaning no CRL file is - loaded. (In previous releases of PostgreSQL, the name of this file was - hard-coded as root.crl.) Relative paths are - relative to the data directory. This parameter can only be set at - server start. + revocation list (CRL). + Relative paths are relative to the data directory. + This parameter can only be set in the postgresql.conf + file or on the server command line. + The default is empty, meaning no CRL file is loaded. + + + In previous releases of PostgreSQL, the name of this file was + hard-coded as root.crl. @@ -1027,9 +1037,10 @@ include_dir 'conf.d' Specifies the name of the file containing the SSL server private key. - The default is server.key. Relative paths are - relative to the data directory. This parameter can only be set at - server start. + Relative paths are relative to the data directory. + This parameter can only be set in the postgresql.conf + file or on the server command line. + The default is server.key. @@ -1046,10 +1057,12 @@ include_dir 'conf.d' used on secure connections. See the ciphers manual page in the OpenSSL package for the syntax of this setting - and a list of supported values. The default value is - HIGH:MEDIUM:+3DES:!aNULL. It is usually reasonable, - unless you have specific security requirements. This parameter can only - be set at server start. + and a list of supported values. + This parameter can only be set in the postgresql.conf + file or on the server command line. + The default value is HIGH:MEDIUM:+3DES:!aNULL. The + default is usually a reasonable choice unless you have specific + security requirements. @@ -1113,7 +1126,7 @@ include_dir 'conf.d' - ssl_prefer_server_ciphers (bool) + ssl_prefer_server_ciphers (boolean) ssl_prefer_server_ciphers configuration parameter @@ -1121,8 +1134,10 @@ include_dir 'conf.d' Specifies whether to use the server's SSL cipher preferences, rather - than the client's. The default is true. This parameter can only be - set at server start. + than the client's. + This parameter can only be set in the postgresql.conf + file or on the server command line. + The default is true. @@ -1145,19 +1160,18 @@ include_dir 'conf.d' Specifies the name of the curve to use in ECDH key exchange. It needs to be supported by all clients that connect. - It does not need to be same curve as used by server's Elliptic - Curve key. The default is prime256v1. This parameter - can only be set at server start. + It does not need to be the same curve used by the server's Elliptic + Curve key. + This parameter can only be set in the postgresql.conf + file or on the server command line. + The default is prime256v1. - OpenSSL names for most common curves: + OpenSSL names for the most common curves are: prime256v1 (NIST P-256), secp384r1 (NIST P-384), secp521r1 (NIST P-521). - - - The full list of available curves can be shown with the command openssl ecparam -list_curves. Not all of them are usable in TLS though. @@ -3003,7 +3017,7 @@ include_dir 'conf.d' - track_commit_timestamp (bool) + track_commit_timestamp (boolean) track_commit_timestamp configuration parameter diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index 787cfce987..65c7809332 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -2285,11 +2285,20 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433 - The files server.key, server.crt, - root.crt, and root.crl - (or their configured alternative names) - are only examined during server start; so you must restart - the server for changes in them to take effect. + The server reads these files at server start and whenever the server + configuration is reloaded. On Windows + systems, they are also re-read whenever a new backend process is spawned + for a new client connection. + + + + If an error in these files is detected at server start, the server will + refuse to start. But if an error is detected during a configuration + reload, the files are ignored and the old values continue to be used. + On Windows systems, if an error in these + files is detected at backend start, that backend will be unable to + establish an SSL connection. In all these cases, the error condition is + reported in the server log. diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 72306e639c..da7ae16d50 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -348,28 +348,22 @@ ClientAuthentication(Port *port) */ if (port->hba->clientcert) { + /* If we haven't loaded a root certificate store, fail */ + if (!secure_loaded_verify_locations()) + ereport(FATAL, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("client certificates can only be checked if a root certificate store is available"))); + /* - * When we parse pg_hba.conf, we have already made sure that we have - * been able to load a certificate store. Thus, if a certificate is - * present on the client, it has been verified against our root + * If we loaded a root certificate store, and if a certificate is + * present on the client, then it has been verified against our root * certificate store, and the connection would have been aborted * already if it didn't verify ok. */ -#ifdef USE_SSL if (!port->peer_cert_valid) - { ereport(FATAL, (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), errmsg("connection requires a valid client certificate"))); - } -#else - - /* - * hba.c makes sure hba->clientcert can't be set unless OpenSSL is - * present. - */ - Assert(false); -#endif } /* diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 668f217bba..4a39d7f746 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -77,12 +77,13 @@ static DH *generate_dh_parameters(int prime_len, int generator); static DH *tmp_dh_cb(SSL *s, int is_export, int keylength); static int verify_cb(int, X509_STORE_CTX *); static void info_cb(const SSL *ssl, int type, int args); -static void initialize_ecdh(void); +static bool initialize_ecdh(SSL_CTX *context, bool failOnError); static const char *SSLerrmessage(unsigned long ecode); static char *X509_NAME_to_cstring(X509_NAME *name); static SSL_CTX *SSL_context = NULL; +static bool SSL_initialized = false; /* ------------------------------------------------------------ */ /* Hardcoded values */ @@ -156,15 +157,20 @@ KWbuHn491xNO25CQWMtem80uKw+pTnisBRF/454n1Jnhub144YRBoN8CAQI=\n\ /* * Initialize global SSL context. + * + * If failOnError is true, report any errors as FATAL (so we don't return). + * Otherwise, log errors at LOG level and return -1 to indicate trouble. + * Returns 0 if OK. */ -void -be_tls_init(void) +int +be_tls_init(bool failOnError) { - struct stat buf; - STACK_OF(X509_NAME) *root_cert_list = NULL; + SSL_CTX *context; + struct stat buf; - if (!SSL_context) + /* This stuff need be done only once. */ + if (!SSL_initialized) { #ifdef HAVE_OPENSSL_INIT_SSL OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL); @@ -173,121 +179,157 @@ be_tls_init(void) SSL_library_init(); SSL_load_error_strings(); #endif + SSL_initialized = true; + } - /* - * We use SSLv23_method() because it can negotiate use of the highest - * mutually supported protocol version, while alternatives like - * TLSv1_2_method() permit only one specific version. Note that we - * don't actually allow SSL v2 or v3, only TLS protocols (see below). - */ - SSL_context = SSL_CTX_new(SSLv23_method()); - if (!SSL_context) - ereport(FATAL, - (errmsg("could not create SSL context: %s", - SSLerrmessage(ERR_get_error())))); + /* + * We use SSLv23_method() because it can negotiate use of the highest + * mutually supported protocol version, while alternatives like + * TLSv1_2_method() permit only one specific version. Note that we don't + * actually allow SSL v2 or v3, only TLS protocols (see below). + */ + context = SSL_CTX_new(SSLv23_method()); + if (!context) + { + ereport(failOnError ? FATAL : LOG, + (errmsg("could not create SSL context: %s", + SSLerrmessage(ERR_get_error())))); + goto error; + } - /* - * Disable OpenSSL's moving-write-buffer sanity check, because it - * causes unnecessary failures in nonblocking send cases. - */ - SSL_CTX_set_mode(SSL_context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); + /* + * Disable OpenSSL's moving-write-buffer sanity check, because it causes + * unnecessary failures in nonblocking send cases. + */ + SSL_CTX_set_mode(context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); - /* - * Load and verify server's certificate and private key - */ - if (SSL_CTX_use_certificate_chain_file(SSL_context, - ssl_cert_file) != 1) - ereport(FATAL, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("could not load server certificate file \"%s\": %s", - ssl_cert_file, SSLerrmessage(ERR_get_error())))); + /* + * Load and verify server's certificate and private key + */ + if (SSL_CTX_use_certificate_chain_file(context, ssl_cert_file) != 1) + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("could not load server certificate file \"%s\": %s", + ssl_cert_file, SSLerrmessage(ERR_get_error())))); + goto error; + } - if (stat(ssl_key_file, &buf) != 0) - ereport(FATAL, - (errcode_for_file_access(), - errmsg("could not access private key file \"%s\": %m", - ssl_key_file))); + if (stat(ssl_key_file, &buf) != 0) + { + ereport(failOnError ? FATAL : LOG, + (errcode_for_file_access(), + errmsg("could not access private key file \"%s\": %m", + ssl_key_file))); + goto error; + } - if (!S_ISREG(buf.st_mode)) - ereport(FATAL, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("private key file \"%s\" is not a regular file", - ssl_key_file))); + if (!S_ISREG(buf.st_mode)) + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("private key file \"%s\" is not a regular file", + ssl_key_file))); + goto error; + } - /* - * Refuse to load files owned by users other than us or root. - * - * XXX surely we can check this on Windows somehow, too. - */ + /* + * Refuse to load files owned by users other than us or root. + * + * XXX surely we can check this on Windows somehow, too. + */ #if !defined(WIN32) && !defined(__CYGWIN__) - if (buf.st_uid != geteuid() && buf.st_uid != 0) - ereport(FATAL, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("private key file \"%s\" must be owned by the database user or root", - ssl_key_file))); + if (buf.st_uid != geteuid() && buf.st_uid != 0) + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("private key file \"%s\" must be owned by the database user or root", + ssl_key_file))); + goto error; + } #endif - /* - * Require no public access to key file. If the file is owned by us, - * require mode 0600 or less. If owned by root, require 0640 or less - * to allow read access through our gid, or a supplementary gid that - * allows to read system-wide certificates. - * - * XXX temporarily suppress check when on Windows, because there may - * not be proper support for Unix-y file permissions. Need to think - * of a reasonable check to apply on Windows. (See also the data - * directory permission check in postmaster.c) - */ + /* + * Require no public access to key file. If the file is owned by us, + * require mode 0600 or less. If owned by root, require 0640 or less to + * allow read access through our gid, or a supplementary gid that allows + * to read system-wide certificates. + * + * XXX temporarily suppress check when on Windows, because there may not + * be proper support for Unix-y file permissions. Need to think of a + * reasonable check to apply on Windows. (See also the data directory + * permission check in postmaster.c) + */ #if !defined(WIN32) && !defined(__CYGWIN__) - if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) || - (buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO))) - ereport(FATAL, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("private key file \"%s\" has group or world access", - ssl_key_file), - errdetail("File must have permissions u=rw (0600) or less if owned by the database user, or permissions u=rw,g=r (0640) or less if owned by root."))); + if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) || + (buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO))) + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("private key file \"%s\" has group or world access", + ssl_key_file), + errdetail("File must have permissions u=rw (0600) or less if owned by the database user, or permissions u=rw,g=r (0640) or less if owned by root."))); + goto error; + } #endif - if (SSL_CTX_use_PrivateKey_file(SSL_context, - ssl_key_file, - SSL_FILETYPE_PEM) != 1) - ereport(FATAL, - (errmsg("could not load private key file \"%s\": %s", - ssl_key_file, SSLerrmessage(ERR_get_error())))); - - if (SSL_CTX_check_private_key(SSL_context) != 1) - ereport(FATAL, - (errmsg("check of private key failed: %s", - SSLerrmessage(ERR_get_error())))); + if (SSL_CTX_use_PrivateKey_file(context, + ssl_key_file, + SSL_FILETYPE_PEM) != 1) + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("could not load private key file \"%s\": %s", + ssl_key_file, SSLerrmessage(ERR_get_error())))); + goto error; + } + + if (SSL_CTX_check_private_key(context) != 1) + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("check of private key failed: %s", + SSLerrmessage(ERR_get_error())))); + goto error; } /* set up ephemeral DH keys, and disallow SSL v2/v3 while at it */ - SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb); - SSL_CTX_set_options(SSL_context, + SSL_CTX_set_tmp_dh_callback(context, tmp_dh_cb); + SSL_CTX_set_options(context, SSL_OP_SINGLE_DH_USE | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); /* set up ephemeral ECDH keys */ - initialize_ecdh(); + if (!initialize_ecdh(context, failOnError)) + goto error; /* set up the allowed cipher list */ - if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1) - elog(FATAL, "could not set the cipher list (no valid ciphers available)"); + if (SSL_CTX_set_cipher_list(context, SSLCipherSuites) != 1) + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("could not set the cipher list (no valid ciphers available)"))); + goto error; + } /* Let server choose order */ if (SSLPreferServerCiphers) - SSL_CTX_set_options(SSL_context, SSL_OP_CIPHER_SERVER_PREFERENCE); + SSL_CTX_set_options(context, SSL_OP_CIPHER_SERVER_PREFERENCE); /* * Load CA store, so we can verify client certificates if needed. */ if (ssl_ca_file[0]) { - if (SSL_CTX_load_verify_locations(SSL_context, ssl_ca_file, NULL) != 1 || + if (SSL_CTX_load_verify_locations(context, ssl_ca_file, NULL) != 1 || (root_cert_list = SSL_load_client_CA_file(ssl_ca_file)) == NULL) - ereport(FATAL, - (errmsg("could not load root certificate file \"%s\": %s", + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("could not load root certificate file \"%s\": %s", ssl_ca_file, SSLerrmessage(ERR_get_error())))); + goto error; + } } /*---------- @@ -297,7 +339,7 @@ be_tls_init(void) */ if (ssl_crl_file[0]) { - X509_STORE *cvstore = SSL_CTX_get_cert_store(SSL_context); + X509_STORE *cvstore = SSL_CTX_get_cert_store(context); if (cvstore) { @@ -310,15 +352,20 @@ be_tls_init(void) X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); #else ereport(LOG, - (errmsg("SSL certificate revocation list file \"%s\" ignored", - ssl_crl_file), - errdetail("SSL library does not support certificate revocation lists."))); + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("SSL certificate revocation list file \"%s\" ignored", + ssl_crl_file), + errdetail("SSL library does not support certificate revocation lists."))); #endif } else - ereport(FATAL, - (errmsg("could not load SSL certificate revocation list file \"%s\": %s", + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("could not load SSL certificate revocation list file \"%s\": %s", ssl_crl_file, SSLerrmessage(ERR_get_error())))); + goto error; + } } } @@ -329,21 +376,53 @@ be_tls_init(void) * presented. We might fail such connections later, depending on what * we find in pg_hba.conf. */ - SSL_CTX_set_verify(SSL_context, + SSL_CTX_set_verify(context, (SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE), verify_cb); - /* Set flag to remember CA store is successfully loaded */ - ssl_loaded_verify_locations = true; - /* * Tell OpenSSL to send the list of root certs we trust to clients in * CertificateRequests. This lets a client with a keystore select the * appropriate client certificate to send to us. */ - SSL_CTX_set_client_CA_list(SSL_context, root_cert_list); + SSL_CTX_set_client_CA_list(context, root_cert_list); } + + /* + * Success! Replace any existing SSL_context. + */ + if (SSL_context) + SSL_CTX_free(SSL_context); + + SSL_context = context; + + /* + * Set flag to remember whether CA store has been loaded into SSL_context. + */ + if (ssl_ca_file[0]) + ssl_loaded_verify_locations = true; + else + ssl_loaded_verify_locations = false; + + return 0; + +error: + if (context) + SSL_CTX_free(context); + return -1; +} + +/* + * Destroy global SSL context, if any. + */ +void +be_tls_destroy(void) +{ + if (SSL_context) + SSL_CTX_free(SSL_context); + SSL_context = NULL; + ssl_loaded_verify_locations = false; } /* @@ -360,6 +439,14 @@ be_tls_open_server(Port *port) Assert(!port->ssl); Assert(!port->peer); + if (!SSL_context) + { + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("could not initialize SSL connection: SSL context not set up"))); + return -1; + } + if (!(port->ssl = SSL_new(SSL_context))) { ereport(COMMERROR, @@ -743,7 +830,7 @@ my_BIO_s_socket(void) !BIO_meth_set_puts(my_bio_methods, BIO_meth_get_puts(biom)) || !BIO_meth_set_ctrl(my_bio_methods, BIO_meth_get_ctrl(biom)) || !BIO_meth_set_create(my_bio_methods, BIO_meth_get_create(biom)) || - !BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom)) || + !BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom)) || !BIO_meth_set_callback_ctrl(my_bio_methods, BIO_meth_get_callback_ctrl(biom))) { BIO_meth_free(my_bio_methods); @@ -1034,8 +1121,8 @@ info_cb(const SSL *ssl, int type, int args) } } -static void -initialize_ecdh(void) +static bool +initialize_ecdh(SSL_CTX *context, bool failOnError) { #ifndef OPENSSL_NO_ECDH EC_KEY *ecdh; @@ -1043,18 +1130,28 @@ initialize_ecdh(void) nid = OBJ_sn2nid(SSLECDHCurve); if (!nid) - ereport(FATAL, - (errmsg("ECDH: unrecognized curve name: %s", SSLECDHCurve))); + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("ECDH: unrecognized curve name: %s", SSLECDHCurve))); + return false; + } ecdh = EC_KEY_new_by_curve_name(nid); if (!ecdh) - ereport(FATAL, - (errmsg("ECDH: could not create key"))); + { + ereport(failOnError ? FATAL : LOG, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("ECDH: could not create key"))); + return false; + } - SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_ECDH_USE); - SSL_CTX_set_tmp_ecdh(SSL_context, ecdh); + SSL_CTX_set_options(context, SSL_OP_SINGLE_ECDH_USE); + SSL_CTX_set_tmp_ecdh(context, ecdh); EC_KEY_free(ecdh); #endif + + return true; } /* diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index b267507de9..4a6a0d6f58 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -63,16 +63,31 @@ bool SSLPreferServerCiphers; /* ------------------------------------------------------------ */ /* - * Initialize global context + * Initialize global context. + * + * If failOnError is true, report any errors as FATAL (so we don't return). + * Otherwise, log errors at LOG level and return -1 to indicate trouble. + * Returns 0 if OK. */ int -secure_initialize(void) +secure_initialize(bool failOnError) { #ifdef USE_SSL - be_tls_init(); + return be_tls_init(failOnError); +#else + return 0; #endif +} - return 0; +/* + * Destroy global context, if any. + */ +void +secure_destroy(void) +{ +#ifdef USE_SSL + be_tls_destroy(); +#endif } /* diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index f1e9a38c92..5b644d6452 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -870,28 +870,23 @@ parse_hba_line(List *line, int line_num, char *raw_line) if (token->string[4] == 's') /* "hostssl" */ { - /* SSL support must be actually active, else complain */ + parsedline->conntype = ctHostSSL; + /* Log a warning if SSL support is not active */ #ifdef USE_SSL - if (EnableSSL) - parsedline->conntype = ctHostSSL; - else - { + if (!EnableSSL) ereport(LOG, (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("hostssl requires SSL to be turned on"), + errmsg("hostssl record cannot match because SSL is disabled"), errhint("Set ssl = on in postgresql.conf."), errcontext("line %d of configuration file \"%s\"", line_num, HbaFileName))); - return NULL; - } #else ereport(LOG, (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("hostssl is not supported by this build"), + errmsg("hostssl record cannot match because SSL is not supported by this build"), errhint("Compile with --with-openssl to use SSL connections."), errcontext("line %d of configuration file \"%s\"", line_num, HbaFileName))); - return NULL; #endif } else if (token->string[4] == 'n') /* "hostnossl" */ @@ -1417,10 +1412,6 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num) } else if (strcmp(name, "clientcert") == 0) { - /* - * Since we require ctHostSSL, this really can never happen on - * non-SSL-enabled builds, so don't bother checking for USE_SSL. - */ if (hbaline->conntype != ctHostSSL) { ereport(LOG, @@ -1432,16 +1423,6 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num) } if (strcmp(val, "1") == 0) { - if (!secure_loaded_verify_locations()) - { - ereport(LOG, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("client certificates can only be checked if a root certificate store is available"), - errhint("Make sure the configuration parameter \"%s\" is set.", "ssl_ca_file"), - errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); - return false; - } hbaline->clientcert = true; } else diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 535f6c4e5a..02e6bb9dbd 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -368,6 +368,11 @@ static unsigned int random_seed = 0; static struct timeval random_start_time; #endif +#ifdef USE_SSL +/* Set when and if SSL has been initialized properly */ +static bool LoadedSSL = false; +#endif + #ifdef USE_BONJOUR static DNSServiceRef bonjour_sdref = NULL; #endif @@ -930,7 +935,10 @@ PostmasterMain(int argc, char *argv[]) */ #ifdef USE_SSL if (EnableSSL) - secure_initialize(); + { + (void) secure_initialize(true); + LoadedSSL = true; + } #endif /* @@ -1961,7 +1969,7 @@ ProcessStartupPacket(Port *port, bool SSLdone) #ifdef USE_SSL /* No SSL when disabled or on Unix sockets */ - if (!EnableSSL || IS_AF_UNIX(port->laddr.addr.ss_family)) + if (!LoadedSSL || IS_AF_UNIX(port->laddr.addr.ss_family)) SSLok = 'N'; else SSLok = 'S'; /* Support for SSL */ @@ -2498,13 +2506,30 @@ SIGHUP_handler(SIGNAL_ARGS) /* Reload authentication config files too */ if (!load_hba()) - ereport(WARNING, + ereport(LOG, (errmsg("pg_hba.conf not reloaded"))); if (!load_ident()) - ereport(WARNING, + ereport(LOG, (errmsg("pg_ident.conf not reloaded"))); +#ifdef USE_SSL + /* Reload SSL configuration as well */ + if (EnableSSL) + { + if (secure_initialize(false) == 0) + LoadedSSL = true; + else + ereport(LOG, + (errmsg("SSL context not reloaded"))); + } + else + { + secure_destroy(); + LoadedSSL = false; + } +#endif + #ifdef EXEC_BACKEND /* Update the starting-point file for future children */ write_nondefault_variables(PGC_SIGHUP); @@ -4733,12 +4758,22 @@ SubPostmasterMain(int argc, char *argv[]) * context structures contain function pointers and cannot be passed * through the parameter file. * + * If for some reason reload fails (maybe the user installed broken + * key files), soldier on without SSL; that's better than all + * connections becoming impossible. + * * XXX should we do this in all child processes? For the moment it's * enough to do it in backend children. */ #ifdef USE_SSL if (EnableSSL) - secure_initialize(); + { + if (secure_initialize(false) == 0) + LoadedSSL = true; + else + ereport(LOG, + (errmsg("SSL context could not be reloaded in child process"))); + } #endif /* diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 946ba9e73e..a5963b3d55 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -934,7 +934,7 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, { - {"ssl", PGC_POSTMASTER, CONN_AUTH_SECURITY, + {"ssl", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Enables SSL connections."), NULL }, @@ -943,7 +943,7 @@ static struct config_bool ConfigureNamesBool[] = check_ssl, NULL, NULL }, { - {"ssl_prefer_server_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY, + {"ssl_prefer_server_ciphers", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Give priority to server ciphersuite order."), NULL }, @@ -2304,7 +2304,7 @@ static struct config_int ConfigureNamesInt[] = GUC_UNIT_XBLOCKS }, &WalWriterFlushAfter, - (1024*1024) / XLOG_BLCKSZ, 0, INT_MAX, + (1024 * 1024) / XLOG_BLCKSZ, 0, INT_MAX, NULL, NULL, NULL }, @@ -3435,7 +3435,7 @@ static struct config_string ConfigureNamesString[] = }, { - {"ssl_cert_file", PGC_POSTMASTER, CONN_AUTH_SECURITY, + {"ssl_cert_file", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Location of the SSL server certificate file."), NULL }, @@ -3445,7 +3445,7 @@ static struct config_string ConfigureNamesString[] = }, { - {"ssl_key_file", PGC_POSTMASTER, CONN_AUTH_SECURITY, + {"ssl_key_file", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Location of the SSL server private key file."), NULL }, @@ -3455,7 +3455,7 @@ static struct config_string ConfigureNamesString[] = }, { - {"ssl_ca_file", PGC_POSTMASTER, CONN_AUTH_SECURITY, + {"ssl_ca_file", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Location of the SSL certificate authority file."), NULL }, @@ -3465,7 +3465,7 @@ static struct config_string ConfigureNamesString[] = }, { - {"ssl_crl_file", PGC_POSTMASTER, CONN_AUTH_SECURITY, + {"ssl_crl_file", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Location of the SSL certificate revocation list file."), NULL }, @@ -3507,7 +3507,7 @@ static struct config_string ConfigureNamesString[] = }, { - {"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY, + {"ssl_ciphers", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Sets the list of allowed SSL ciphers."), NULL, GUC_SUPERUSER_ONLY @@ -3522,7 +3522,7 @@ static struct config_string ConfigureNamesString[] = }, { - {"ssl_ecdh_curve", PGC_POSTMASTER, CONN_AUTH_SECURITY, + {"ssl_ecdh_curve", PGC_SIGHUP, CONN_AUTH_SECURITY, gettext_noop("Sets the curve to use for ECDH."), NULL, GUC_SUPERUSER_ONLY diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index ee8232f2f4..b3f29610d0 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -76,15 +76,14 @@ # - Security and Authentication - #authentication_timeout = 1min # 1s-600s -#ssl = off # (change requires restart) +#ssl = off #ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers - # (change requires restart) -#ssl_prefer_server_ciphers = on # (change requires restart) -#ssl_ecdh_curve = 'prime256v1' # (change requires restart) -#ssl_cert_file = 'server.crt' # (change requires restart) -#ssl_key_file = 'server.key' # (change requires restart) -#ssl_ca_file = '' # (change requires restart) -#ssl_crl_file = '' # (change requires restart) +#ssl_prefer_server_ciphers = on +#ssl_ecdh_curve = 'prime256v1' +#ssl_cert_file = 'server.crt' +#ssl_key_file = 'server.key' +#ssl_ca_file = '' +#ssl_crl_file = '' #password_encryption = md5 # md5 or plain #db_user_namespace = off #row_security = on diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 66647ad003..5dac9ceb18 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -199,7 +199,8 @@ typedef struct Port * These functions are implemented by the glue code specific to each * SSL implementation (e.g. be-secure-openssl.c) */ -extern void be_tls_init(void); +extern int be_tls_init(bool failOnError); +extern void be_tls_destroy(void); extern int be_tls_open_server(Port *port); extern void be_tls_close(Port *port); extern ssize_t be_tls_read(Port *port, void *ptr, size_t len, int *waitfor); diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h index 5fac8171ed..66ceb2b4a0 100644 --- a/src/include/libpq/libpq.h +++ b/src/include/libpq/libpq.h @@ -81,7 +81,7 @@ extern char *ssl_key_file; extern char *ssl_ca_file; extern char *ssl_crl_file; -extern int secure_initialize(void); +extern int secure_initialize(bool failOnError); extern bool secure_loaded_verify_locations(void); extern void secure_destroy(void); extern int secure_open_server(Port *port); diff --git a/src/test/ssl/ServerSetup.pm b/src/test/ssl/ServerSetup.pm index d312880f8b..20eaf76bff 100644 --- a/src/test/ssl/ServerSetup.pm +++ b/src/test/ssl/ServerSetup.pm @@ -70,7 +70,11 @@ sub configure_test_server_for_ssl close CONF; -# Copy all server certificates and keys, and client root cert, to the data dir + # ssl configuration will be placed here + open SSLCONF, ">$pgdata/sslconfig.conf"; + close SSLCONF; + + # Copy all server certificates and keys, and client root cert, to the data dir copy_files("ssl/server-*.crt", $pgdata); copy_files("ssl/server-*.key", $pgdata); chmod(0600, glob "$pgdata/server-*.key") or die $!; @@ -78,25 +82,14 @@ sub configure_test_server_for_ssl copy_files("ssl/root_ca.crt", $pgdata); copy_files("ssl/root+client.crl", $pgdata); - # Only accept SSL connections from localhost. Our tests don't depend on this - # but seems best to keep it as narrow as possible for security reasons. - # - # When connecting to certdb, also check the client certificate. - open HBA, ">$pgdata/pg_hba.conf"; - print HBA -"# TYPE DATABASE USER ADDRESS METHOD\n"; - print HBA -"hostssl trustdb ssltestuser $serverhost/32 trust\n"; - print HBA -"hostssl trustdb ssltestuser ::1/128 trust\n"; - print HBA -"hostssl certdb ssltestuser $serverhost/32 cert\n"; - print HBA -"hostssl certdb ssltestuser ::1/128 cert\n"; - close HBA; + # Stop and restart server to load new listen_addresses. + $node->restart; + + # Change pg_hba after restart because hostssl requires ssl=on + configure_hba_for_ssl($node, $serverhost); } -# Change the configuration to use given server cert file, and restart +# Change the configuration to use given server cert file, and reload # the server so that the configuration takes effect. sub switch_server_cert { @@ -105,7 +98,7 @@ sub switch_server_cert my $cafile = $_[2] || "root+client_ca"; my $pgdata = $node->data_dir; - diag "Restarting server with certfile \"$certfile\" and cafile \"$cafile\"..."; + diag "Reloading server with certfile \"$certfile\" and cafile \"$cafile\"..."; open SSLCONF, ">$pgdata/sslconfig.conf"; print SSLCONF "ssl=on\n"; @@ -115,6 +108,29 @@ sub switch_server_cert print SSLCONF "ssl_crl_file='root+client.crl'\n"; close SSLCONF; - # Stop and restart server to reload the new config. - $node->restart; + $node->reload; +} + +sub configure_hba_for_ssl +{ + my $node = $_[0]; + my $serverhost = $_[1]; + my $pgdata = $node->data_dir; + + # Only accept SSL connections from localhost. Our tests don't depend on this + # but seems best to keep it as narrow as possible for security reasons. + # + # When connecting to certdb, also check the client certificate. + open HBA, ">$pgdata/pg_hba.conf"; + print HBA +"# TYPE DATABASE USER ADDRESS METHOD\n"; + print HBA +"hostssl trustdb ssltestuser $serverhost/32 trust\n"; + print HBA +"hostssl trustdb ssltestuser ::1/128 trust\n"; + print HBA +"hostssl certdb ssltestuser $serverhost/32 cert\n"; + print HBA +"hostssl certdb ssltestuser ::1/128 cert\n"; + close HBA; } -- 2.40.0