From 69f74e3cced093c9ae2cce830e31c3fd76a8a06b Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 6 Oct 2016 16:22:29 +0300 Subject: [PATCH 1/1] Always use 2048 bit DH parameters for OpenSSL ephemeral DH ciphers. 1024 bits is considered weak these days, but OpenSSL always passes 1024 as the key length to the tmp_dh callback. All the code to handle other key lengths as in fact dead. To remedy those issues: * Only include hard-coded 2048-bit parameters. * Set the parameters directly with SSL_CTX_set_tmp_dh(), without the callback * The file for the DH parameters is now called "dh_params.pem", instead of "dh1024.pem" (the files for other key lengths, dh512.pem, dh2048.pem, etc. were never actually used) Per report by Nicolas Guini Discussion: --- src/backend/libpq/be-secure-openssl.c | 198 +++++++++------------------------- 1 file changed, 48 insertions(+), 150 deletions(-) diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 668f217..fc7dd6a 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -65,18 +65,19 @@ #include "tcop/tcopprot.h" #include "utils/memutils.h" +/* name of the file containing DH parameters */ +#define DH_PARAMS_FILE "dh_params.pem" static int my_sock_read(BIO *h, char *buf, int size); static int my_sock_write(BIO *h, const char *buf, int size); static BIO_METHOD *my_BIO_s_socket(void); static int my_SSL_set_fd(Port *port, int fd); -static DH *load_dh_file(int keylength); +static DH *load_dh_file(void); static DH *load_dh_buffer(const char *, size_t); -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_dh(void); static void initialize_ecdh(void); static const char *SSLerrmessage(unsigned long ecode); @@ -94,14 +95,14 @@ static SSL_CTX *SSL_context = NULL; * sessions even if the static private key is compromised, * so we are *highly* motivated to ensure that we can use * EDH even if the DBA... or an attacker... deletes the - * $DataDir/dh*.pem files. + * $DataDir/dh_params.pem file. * * We could refuse SSL connections unless a good DH parameter * file exists, but some clients may quietly renegotiate an * unsecured connection without fully informing the user. * Very uncool. * - * Alternatively, the backend could attempt to load these files + * Alternatively, the backend could attempt to load the file * on startup if SSL is enabled - and refuse to start if any * do not exist - but this would tend to piss off DBAs. * @@ -111,19 +112,6 @@ static SSL_CTX *SSL_context = NULL; * for suggestions. */ -static const char file_dh512[] = -"-----BEGIN DH PARAMETERS-----\n\ -MEYCQQD1Kv884bEpQBgRjXyEpwpy1obEAxnIByl6ypUM2Zafq9AKUJsCRtMIPWak\n\ -XUGfnHy9iUsiGSa6q6Jew1XpKgVfAgEC\n\ ------END DH PARAMETERS-----\n"; - -static const char file_dh1024[] = -"-----BEGIN DH PARAMETERS-----\n\ -MIGHAoGBAPSI/VhOSdvNILSd5JEHNmszbDgNRR0PfIizHHxbLY7288kjwEPwpVsY\n\ -jY67VYy4XTjTNP18F1dDox0YbN4zISy1Kv884bEpQBgRjXyEpwpy1obEAxnIByl6\n\ -ypUM2Zafq9AKUJsCRtMIPWakXUGfnHy9iUsiGSa6q6Jew1XpL3jHAgEC\n\ ------END DH PARAMETERS-----\n"; - static const char file_dh2048[] = "-----BEGIN DH PARAMETERS-----\n\ MIIBCAKCAQEA9kJXtwh/CBdyorrWqULzBej5UxE5T7bxbrlLOCDaAadWoxTpj0BV\n\ @@ -134,21 +122,6 @@ Q6MdGGzeMyEstSr/POGxKUAYEY18hKcKctaGxAMZyAcpesqVDNmWn6vQClCbAkbT\n\ CD1mpF1Bn5x8vYlLIhkmuquiXsNV6TILOwIBAg==\n\ -----END DH PARAMETERS-----\n"; -static const char file_dh4096[] = -"-----BEGIN DH PARAMETERS-----\n\ -MIICCAKCAgEA+hRyUsFN4VpJ1O8JLcCo/VWr19k3BCgJ4uk+d+KhehjdRqNDNyOQ\n\ -l/MOyQNQfWXPeGKmOmIig6Ev/nm6Nf9Z2B1h3R4hExf+zTiHnvVPeRBhjdQi81rt\n\ -Xeoh6TNrSBIKIHfUJWBh3va0TxxjQIs6IZOLeVNRLMqzeylWqMf49HsIXqbcokUS\n\ -Vt1BkvLdW48j8PPv5DsKRN3tloTxqDJGo9tKvj1Fuk74A+Xda1kNhB7KFlqMyN98\n\ -VETEJ6c7KpfOo30mnK30wqw3S8OtaIR/maYX72tGOno2ehFDkq3pnPtEbD2CScxc\n\ -alJC+EL7RPk5c/tgeTvCngvc1KZn92Y//EI7G9tPZtylj2b56sHtMftIoYJ9+ODM\n\ -sccD5Piz/rejE3Ome8EOOceUSCYAhXn8b3qvxVI1ddd1pED6FHRhFvLrZxFvBEM9\n\ -ERRMp5QqOaHJkM+Dxv8Cj6MqrCbfC4u+ZErxodzuusgDgvZiLF22uxMZbobFWyte\n\ -OvOzKGtwcTqO/1wV5gKkzu1ZVswVUQd5Gg8lJicwqRWyyNRczDDoG9jVDxmogKTH\n\ -AaqLulO7R8Ifa1SwF2DteSGVtgWEN8gDpN3RBmmPTDngyF2DHb5qmpnznwtFKdTL\n\ -KWbuHn491xNO25CQWMtem80uKw+pTnisBRF/454n1Jnhub144YRBoN8CAQI=\n\ ------END DH PARAMETERS-----\n"; - /* ------------------------------------------------------------ */ /* Public interface */ @@ -261,13 +234,12 @@ be_tls_init(void) SSLerrmessage(ERR_get_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); + /* disallow SSL v2/v3 */ SSL_CTX_set_options(SSL_context, - SSL_OP_SINGLE_DH_USE | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); - /* set up ephemeral ECDH keys */ + /* set up ephemeral DH and ECDH keys */ + initialize_dh(); initialize_ecdh(); /* set up the allowed cipher list */ @@ -800,43 +772,32 @@ err: * what we expect it to contain. */ static DH * -load_dh_file(int keylength) +load_dh_file(void) { FILE *fp; - char fnbuf[MAXPGPATH]; + char *filename = DH_PARAMS_FILE; DH *dh = NULL; int codes; /* attempt to open file. It's not an error if it doesn't exist. */ - snprintf(fnbuf, sizeof(fnbuf), "dh%d.pem", keylength); - if ((fp = fopen(fnbuf, "r")) == NULL) + if ((fp = fopen(filename, "r")) == NULL) return NULL; -/* flock(fileno(fp), LOCK_SH); */ dh = PEM_read_DHparams(fp, NULL, NULL, NULL); -/* flock(fileno(fp), LOCK_UN); */ fclose(fp); - /* is the prime the correct size? */ - if (dh != NULL && 8 * DH_size(dh) < keylength) - { - elog(LOG, "DH errors (%s): %d bits expected, %d bits found", - fnbuf, keylength, 8 * DH_size(dh)); - dh = NULL; - } - /* make sure the DH parameters are usable */ if (dh != NULL) { if (DH_check(dh, &codes) == 0) { - elog(LOG, "DH_check error (%s): %s", fnbuf, + elog(LOG, "DH_check error (%s): %s", filename, SSLerrmessage(ERR_get_error())); return NULL; } if (codes & DH_CHECK_P_NOT_PRIME) { - elog(LOG, "DH error (%s): p is not prime", fnbuf); + elog(LOG, "DH error (%s): p is not prime", filename); return NULL; } if ((codes & DH_NOT_SUITABLE_GENERATOR) && @@ -844,7 +805,7 @@ load_dh_file(int keylength) { elog(LOG, "DH error (%s): neither suitable generator or safe prime", - fnbuf); + filename); return NULL; } } @@ -878,102 +839,6 @@ load_dh_buffer(const char *buffer, size_t len) } /* - * Generate DH parameters. - * - * Last resort if we can't load precomputed nor hardcoded - * parameters. - */ -static DH * -generate_dh_parameters(int prime_len, int generator) -{ - DH *dh; - - if ((dh = DH_new()) == NULL) - return NULL; - - if (DH_generate_parameters_ex(dh, prime_len, generator, NULL)) - return dh; - - DH_free(dh); - return NULL; -} - -/* - * Generate an ephemeral DH key. Because this can take a long - * time to compute, we can use precomputed parameters of the - * common key sizes. - * - * Since few sites will bother to precompute these parameter - * files, we also provide a fallback to the parameters provided - * by the OpenSSL project. - * - * These values can be static (once loaded or computed) since - * the OpenSSL library can efficiently generate random keys from - * the information provided. - */ -static DH * -tmp_dh_cb(SSL *s, int is_export, int keylength) -{ - DH *r = NULL; - static DH *dh = NULL; - static DH *dh512 = NULL; - static DH *dh1024 = NULL; - static DH *dh2048 = NULL; - static DH *dh4096 = NULL; - - switch (keylength) - { - case 512: - if (dh512 == NULL) - dh512 = load_dh_file(keylength); - if (dh512 == NULL) - dh512 = load_dh_buffer(file_dh512, sizeof file_dh512); - r = dh512; - break; - - case 1024: - if (dh1024 == NULL) - dh1024 = load_dh_file(keylength); - if (dh1024 == NULL) - dh1024 = load_dh_buffer(file_dh1024, sizeof file_dh1024); - r = dh1024; - break; - - case 2048: - if (dh2048 == NULL) - dh2048 = load_dh_file(keylength); - if (dh2048 == NULL) - dh2048 = load_dh_buffer(file_dh2048, sizeof file_dh2048); - r = dh2048; - break; - - case 4096: - if (dh4096 == NULL) - dh4096 = load_dh_file(keylength); - if (dh4096 == NULL) - dh4096 = load_dh_buffer(file_dh4096, sizeof file_dh4096); - r = dh4096; - break; - - default: - if (dh == NULL) - dh = load_dh_file(keylength); - r = dh; - } - - /* this may take a long time, but it may be necessary... */ - if (r == NULL || 8 * DH_size(r) < keylength) - { - ereport(DEBUG2, - (errmsg_internal("DH: generating parameters (%d bits)", - keylength))); - r = generate_dh_parameters(keylength, DH_GENERATOR_2); - } - - return r; -} - -/* * Certificate verification callback * * This callback allows us to log intermediate problems during @@ -1034,6 +899,39 @@ info_cb(const SSL *ssl, int type, int args) } } +/* + * Obtain DH parameters for generating ephemeral DH keys. Because + * this can take a long time to compute, these must be precomputed. + * + * Since few sites will bother to precompute these parameter files, + * we also also provide a fallback to the parameters provided by the + * OpenSSL project. + * + * These values can be static (once loaded or computed) since the + * OpenSSL library can efficiently generate random keys from the + * information provided. + */ +static void +initialize_dh(void) +{ + DH *dh; + + dh = load_dh_file(); + if (!dh) + dh = load_dh_buffer(file_dh2048, sizeof file_dh2048); + if (!dh) + ereport(FATAL, + (errmsg("DH: could not create key"))); + + SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE); + SSL_CTX_set_tmp_dh(SSL_context, dh); +} + +/* + * Set ECDH parameters for generating ephemeral Elliptic Curve DH + * keys. This is much simpler than the DH parameters, as we just + * need to provide the name of the curve to OpenSSL. + */ static void initialize_ecdh(void) { -- 2.9.3