Re-allow SSL passphrase prompt at server start, but not thereafter.

Leave OpenSSL's default passphrase collection callback in place during
the first call of secure_initialize() in server startup.  Although that
doesn't work terribly well in daemon contexts, some people feel we should
not break it for anyone who was successfully using it before.  We still
block passphrase demands during SIGHUP, meaning that you can't adjust SSL
configuration on-the-fly if you used a passphrase, but this is no worse
than what it was before commit de41869b6.  And we block passphrase demands
during EXEC_BACKEND reloads; that behavior wasn't useful either, but at
least now it's documented.

Tweak some related log messages for more readability, and avoid issuing
essentially duplicate messages about reload failure caused by a passphrase.

Discussion: https://postgr.es/m/29982.1483412575@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2017-01-04 12:43:52 -05:00
parent 0fad355bec
commit 6667d9a6d7
6 changed files with 72 additions and 53 deletions

View File

@ -2159,8 +2159,13 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
</para> </para>
<para> <para>
The private key cannot be protected with a passphrase, as there is no If the private key is protected with a passphrase, the
way to supply the passphrase to the server. server will prompt for the passphrase and will not start until it has
been entered.
Using a passphrase also disables the ability to change the server's SSL
configuration without a server restart.
Furthermore, passphrase-protected private keys cannot be used at all
on Windows.
</para> </para>
<para> <para>
@ -2293,9 +2298,9 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
<para> <para>
If an error in these files is detected at server start, the server will 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 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. reload, the files are ignored and the old SSL configuration continues to
On <systemitem class="osname">Windows</> systems, if an error in these be used. On <systemitem class="osname">Windows</> systems, if an error in
files is detected at backend start, that backend will be unable to 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 establish an SSL connection. In all these cases, the error condition is
reported in the server log. reported in the server log.
</para> </para>
@ -2314,8 +2319,8 @@ openssl req -new -text -out server.req
you enter the local host name as <quote>Common Name</>; the challenge you enter the local host name as <quote>Common Name</>; the challenge
password can be left blank. The program will generate a key that is password can be left blank. The program will generate a key that is
passphrase protected; it will not accept a passphrase that is less passphrase protected; it will not accept a passphrase that is less
than four characters long. To remove the passphrase again (as you must), than four characters long. To remove the passphrase again (as you must
next run the commands: if you want automatic start-up of the server), next run the commands:
<programlisting> <programlisting>
openssl rsa -in privkey.pem -out server.key openssl rsa -in privkey.pem -out server.key
rm privkey.pem rm privkey.pem

View File

@ -78,13 +78,14 @@ static DH *tmp_dh_cb(SSL *s, int is_export, int keylength);
static int ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata); static int ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
static int verify_cb(int, X509_STORE_CTX *); static int verify_cb(int, X509_STORE_CTX *);
static void info_cb(const SSL *ssl, int type, int args); static void info_cb(const SSL *ssl, int type, int args);
static bool initialize_ecdh(SSL_CTX *context, bool failOnError); static bool initialize_ecdh(SSL_CTX *context, bool isServerStart);
static const char *SSLerrmessage(unsigned long ecode); static const char *SSLerrmessage(unsigned long ecode);
static char *X509_NAME_to_cstring(X509_NAME *name); static char *X509_NAME_to_cstring(X509_NAME *name);
static SSL_CTX *SSL_context = NULL; static SSL_CTX *SSL_context = NULL;
static bool SSL_initialized = false; static bool SSL_initialized = false;
static bool ssl_passwd_cb_called = false;
/* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */
/* Hardcoded values */ /* Hardcoded values */
@ -159,12 +160,12 @@ KWbuHn491xNO25CQWMtem80uKw+pTnisBRF/454n1Jnhub144YRBoN8CAQI=\n\
/* /*
* Initialize global SSL context. * Initialize global SSL context.
* *
* If failOnError is true, report any errors as FATAL (so we don't return). * If isServerStart is true, report any errors as FATAL (so we don't return).
* Otherwise, log errors at LOG level and return -1 to indicate trouble. * Otherwise, log errors at LOG level and return -1 to indicate trouble,
* Returns 0 if OK. * preserving the old SSL state if any. Returns 0 if OK.
*/ */
int int
be_tls_init(bool failOnError) be_tls_init(bool isServerStart)
{ {
STACK_OF(X509_NAME) *root_cert_list = NULL; STACK_OF(X509_NAME) *root_cert_list = NULL;
SSL_CTX *context; SSL_CTX *context;
@ -192,7 +193,7 @@ be_tls_init(bool failOnError)
context = SSL_CTX_new(SSLv23_method()); context = SSL_CTX_new(SSLv23_method());
if (!context) if (!context)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errmsg("could not create SSL context: %s", (errmsg("could not create SSL context: %s",
SSLerrmessage(ERR_get_error())))); SSLerrmessage(ERR_get_error()))));
goto error; goto error;
@ -205,16 +206,21 @@ be_tls_init(bool failOnError)
SSL_CTX_set_mode(context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); SSL_CTX_set_mode(context, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
/* /*
* Override OpenSSL's default handling of passphrase-protected files. * If reloading, override OpenSSL's default handling of
* passphrase-protected files, because we don't want to prompt for a
* passphrase in an already-running server. (Not that the default
* handling is very desirable during server start either, but some people
* insist we need to keep it.)
*/ */
SSL_CTX_set_default_passwd_cb(context, ssl_passwd_cb); if (!isServerStart)
SSL_CTX_set_default_passwd_cb(context, ssl_passwd_cb);
/* /*
* Load and verify server's certificate and private key * Load and verify server's certificate and private key
*/ */
if (SSL_CTX_use_certificate_chain_file(context, ssl_cert_file) != 1) if (SSL_CTX_use_certificate_chain_file(context, ssl_cert_file) != 1)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("could not load server certificate file \"%s\": %s", errmsg("could not load server certificate file \"%s\": %s",
ssl_cert_file, SSLerrmessage(ERR_get_error())))); ssl_cert_file, SSLerrmessage(ERR_get_error()))));
@ -223,7 +229,7 @@ be_tls_init(bool failOnError)
if (stat(ssl_key_file, &buf) != 0) if (stat(ssl_key_file, &buf) != 0)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode_for_file_access(), (errcode_for_file_access(),
errmsg("could not access private key file \"%s\": %m", errmsg("could not access private key file \"%s\": %m",
ssl_key_file))); ssl_key_file)));
@ -232,7 +238,7 @@ be_tls_init(bool failOnError)
if (!S_ISREG(buf.st_mode)) if (!S_ISREG(buf.st_mode))
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("private key file \"%s\" is not a regular file", errmsg("private key file \"%s\" is not a regular file",
ssl_key_file))); ssl_key_file)));
@ -240,14 +246,14 @@ be_tls_init(bool failOnError)
} }
/* /*
* Refuse to load files owned by users other than us or root. * Refuse to load key files owned by users other than us or root.
* *
* XXX surely we can check this on Windows somehow, too. * XXX surely we can check this on Windows somehow, too.
*/ */
#if !defined(WIN32) && !defined(__CYGWIN__) #if !defined(WIN32) && !defined(__CYGWIN__)
if (buf.st_uid != geteuid() && buf.st_uid != 0) if (buf.st_uid != geteuid() && buf.st_uid != 0)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("private key file \"%s\" must be owned by the database user or root", errmsg("private key file \"%s\" must be owned by the database user or root",
ssl_key_file))); ssl_key_file)));
@ -270,7 +276,7 @@ be_tls_init(bool failOnError)
if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) || 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))) (buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO)))
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("private key file \"%s\" has group or world access", errmsg("private key file \"%s\" has group or world access",
ssl_key_file), ssl_key_file),
@ -279,20 +285,31 @@ be_tls_init(bool failOnError)
} }
#endif #endif
/*
* OK, try to load the private key file.
*/
ssl_passwd_cb_called = false;
if (SSL_CTX_use_PrivateKey_file(context, if (SSL_CTX_use_PrivateKey_file(context,
ssl_key_file, ssl_key_file,
SSL_FILETYPE_PEM) != 1) SSL_FILETYPE_PEM) != 1)
{ {
ereport(failOnError ? FATAL : LOG, if (ssl_passwd_cb_called)
(errcode(ERRCODE_CONFIG_FILE_ERROR), ereport(isServerStart ? FATAL : LOG,
errmsg("could not load private key file \"%s\": %s", (errcode(ERRCODE_CONFIG_FILE_ERROR),
ssl_key_file, SSLerrmessage(ERR_get_error())))); errmsg("private key file \"%s\" cannot be reloaded because it requires a passphrase",
ssl_key_file)));
else
ereport(isServerStart ? 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; goto error;
} }
if (SSL_CTX_check_private_key(context) != 1) if (SSL_CTX_check_private_key(context) != 1)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("check of private key failed: %s", errmsg("check of private key failed: %s",
SSLerrmessage(ERR_get_error())))); SSLerrmessage(ERR_get_error()))));
@ -306,13 +323,13 @@ be_tls_init(bool failOnError)
SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
/* set up ephemeral ECDH keys */ /* set up ephemeral ECDH keys */
if (!initialize_ecdh(context, failOnError)) if (!initialize_ecdh(context, isServerStart))
goto error; goto error;
/* set up the allowed cipher list */ /* set up the allowed cipher list */
if (SSL_CTX_set_cipher_list(context, SSLCipherSuites) != 1) if (SSL_CTX_set_cipher_list(context, SSLCipherSuites) != 1)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("could not set the cipher list (no valid ciphers available)"))); errmsg("could not set the cipher list (no valid ciphers available)")));
goto error; goto error;
@ -330,7 +347,7 @@ be_tls_init(bool failOnError)
if (SSL_CTX_load_verify_locations(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) (root_cert_list = SSL_load_client_CA_file(ssl_ca_file)) == NULL)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("could not load root certificate file \"%s\": %s", errmsg("could not load root certificate file \"%s\": %s",
ssl_ca_file, SSLerrmessage(ERR_get_error())))); ssl_ca_file, SSLerrmessage(ERR_get_error()))));
@ -366,7 +383,7 @@ be_tls_init(bool failOnError)
} }
else else
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("could not load SSL certificate revocation list file \"%s\": %s", errmsg("could not load SSL certificate revocation list file \"%s\": %s",
ssl_crl_file, SSLerrmessage(ERR_get_error())))); ssl_crl_file, SSLerrmessage(ERR_get_error()))));
@ -1071,19 +1088,16 @@ tmp_dh_cb(SSL *s, int is_export, int keylength)
* *
* If OpenSSL is told to use a passphrase-protected server key, by default * If OpenSSL is told to use a passphrase-protected server key, by default
* it will issue a prompt on /dev/tty and try to read a key from there. * it will issue a prompt on /dev/tty and try to read a key from there.
* That's completely no good for a postmaster SIGHUP cycle, not to mention * That's no good during a postmaster SIGHUP cycle, not to mention SSL context
* SSL context reload in an EXEC_BACKEND postmaster child. So override it * reload in an EXEC_BACKEND postmaster child. So override it with this dummy
* with this dummy function that just returns an empty passphrase, * function that just returns an empty passphrase, guaranteeing failure.
* guaranteeing failure. Later we might think about collecting a passphrase
* at server start and feeding it to OpenSSL repeatedly, but we'd still
* need this callback for that.
*/ */
static int static int
ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata) ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata)
{ {
ereport(LOG, /* Set flag to change the error message we'll report */
(errcode(ERRCODE_CONFIG_FILE_ERROR), ssl_passwd_cb_called = true;
errmsg("server's private key file requires a passphrase"))); /* And return empty string */
Assert(size > 0); Assert(size > 0);
buf[0] = '\0'; buf[0] = '\0';
return 0; return 0;
@ -1151,7 +1165,7 @@ info_cb(const SSL *ssl, int type, int args)
} }
static bool static bool
initialize_ecdh(SSL_CTX *context, bool failOnError) initialize_ecdh(SSL_CTX *context, bool isServerStart)
{ {
#ifndef OPENSSL_NO_ECDH #ifndef OPENSSL_NO_ECDH
EC_KEY *ecdh; EC_KEY *ecdh;
@ -1160,7 +1174,7 @@ initialize_ecdh(SSL_CTX *context, bool failOnError)
nid = OBJ_sn2nid(SSLECDHCurve); nid = OBJ_sn2nid(SSLECDHCurve);
if (!nid) if (!nid)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("ECDH: unrecognized curve name: %s", SSLECDHCurve))); errmsg("ECDH: unrecognized curve name: %s", SSLECDHCurve)));
return false; return false;
@ -1169,7 +1183,7 @@ initialize_ecdh(SSL_CTX *context, bool failOnError)
ecdh = EC_KEY_new_by_curve_name(nid); ecdh = EC_KEY_new_by_curve_name(nid);
if (!ecdh) if (!ecdh)
{ {
ereport(failOnError ? FATAL : LOG, ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR), (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("ECDH: could not create key"))); errmsg("ECDH: could not create key")));
return false; return false;

View File

@ -65,15 +65,15 @@ bool SSLPreferServerCiphers;
/* /*
* Initialize global context. * Initialize global context.
* *
* If failOnError is true, report any errors as FATAL (so we don't return). * If isServerStart is true, report any errors as FATAL (so we don't return).
* Otherwise, log errors at LOG level and return -1 to indicate trouble. * Otherwise, log errors at LOG level and return -1 to indicate trouble,
* Returns 0 if OK. * preserving the old SSL state if any. Returns 0 if OK.
*/ */
int int
secure_initialize(bool failOnError) secure_initialize(bool isServerStart)
{ {
#ifdef USE_SSL #ifdef USE_SSL
return be_tls_init(failOnError); return be_tls_init(isServerStart);
#else #else
return 0; return 0;
#endif #endif

View File

@ -2507,11 +2507,11 @@ SIGHUP_handler(SIGNAL_ARGS)
/* Reload authentication config files too */ /* Reload authentication config files too */
if (!load_hba()) if (!load_hba())
ereport(LOG, ereport(LOG,
(errmsg("pg_hba.conf not reloaded"))); (errmsg("pg_hba.conf was not reloaded")));
if (!load_ident()) if (!load_ident())
ereport(LOG, ereport(LOG,
(errmsg("pg_ident.conf not reloaded"))); (errmsg("pg_ident.conf was not reloaded")));
#ifdef USE_SSL #ifdef USE_SSL
/* Reload SSL configuration as well */ /* Reload SSL configuration as well */
@ -2521,7 +2521,7 @@ SIGHUP_handler(SIGNAL_ARGS)
LoadedSSL = true; LoadedSSL = true;
else else
ereport(LOG, ereport(LOG,
(errmsg("SSL context not reloaded"))); (errmsg("SSL configuration was not reloaded")));
} }
else else
{ {
@ -4772,7 +4772,7 @@ SubPostmasterMain(int argc, char *argv[])
LoadedSSL = true; LoadedSSL = true;
else else
ereport(LOG, ereport(LOG,
(errmsg("SSL context could not be reloaded in child process"))); (errmsg("SSL configuration could not be loaded in child process")));
} }
#endif #endif

View File

@ -199,7 +199,7 @@ typedef struct Port
* These functions are implemented by the glue code specific to each * These functions are implemented by the glue code specific to each
* SSL implementation (e.g. be-secure-openssl.c) * SSL implementation (e.g. be-secure-openssl.c)
*/ */
extern int be_tls_init(bool failOnError); extern int be_tls_init(bool isServerStart);
extern void be_tls_destroy(void); extern void be_tls_destroy(void);
extern int be_tls_open_server(Port *port); extern int be_tls_open_server(Port *port);
extern void be_tls_close(Port *port); extern void be_tls_close(Port *port);

View File

@ -81,7 +81,7 @@ extern char *ssl_key_file;
extern char *ssl_ca_file; extern char *ssl_ca_file;
extern char *ssl_crl_file; extern char *ssl_crl_file;
extern int secure_initialize(bool failOnError); extern int secure_initialize(bool isServerStart);
extern bool secure_loaded_verify_locations(void); extern bool secure_loaded_verify_locations(void);
extern void secure_destroy(void); extern void secure_destroy(void);
extern int secure_open_server(Port *port); extern int secure_open_server(Port *port);