From: Timo Sirainen Date: Sat, 2 Nov 2013 13:27:28 +0000 (+0200) Subject: ssl-params: Added ssl_dh_parameters_length & removed ssl_parameters_regenerate setting. X-Git-Tag: 2.2.7~17 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=923e40fa9af9484f5d34d2cf3523ae1e675a6420;p=thirdparty%2Fdovecot%2Fcore.git ssl-params: Added ssl_dh_parameters_length & removed ssl_parameters_regenerate setting. ssl_parameters_regenerate was based on some text from GNUTLS documentation a long time ago, but there's really not much point in doing it. Ideally we should also support "openssl dhparam" input files, but for now there's the ssl_dh_parameters_length setting that can be used to specify the wanted DH parameters length. If the current ssl-parameters.dat has a different length, it's regenerated. We should probably at some point support also built-in DH parameters which are returned while the ssl-params runs. --- diff --git a/doc/example-config/conf.d/10-ssl.conf b/doc/example-config/conf.d/10-ssl.conf index cab9423f6a..7ae6b7a2bb 100644 --- a/doc/example-config/conf.d/10-ssl.conf +++ b/doc/example-config/conf.d/10-ssl.conf @@ -42,10 +42,8 @@ ssl_key = verify_client_cert == ctx2->verify_client_cert ? 0 : 1; } -static void ssl_params_corrupted(void) +static void ssl_params_corrupted(const char *reason) { - i_fatal("Corrupted SSL parameters file in state_dir: ssl-parameters.dat"); + i_fatal("Corrupted SSL ssl-parameters.dat in state_dir: %s", reason); } static void read_next(struct ssl_parameters *params, void *data, size_t size) @@ -175,7 +175,7 @@ static void read_next(struct ssl_parameters *params, void *data, size_t size) if ((ret = read_full(params->fd, data, size)) < 0) i_fatal("read(%s) failed: %m", params->path); if (ret == 0) - ssl_params_corrupted(); + ssl_params_corrupted("Truncated file"); } static bool read_dh_parameters_next(struct ssl_parameters *params) @@ -194,7 +194,7 @@ static bool read_dh_parameters_next(struct ssl_parameters *params) /* read data size. */ read_next(params, &len, sizeof(len)); if (len > 1024*100) /* should be enough? */ - ssl_params_corrupted(); + ssl_params_corrupted("File too large"); buf = i_malloc(len); read_next(params, buf, len); @@ -202,13 +202,15 @@ static bool read_dh_parameters_next(struct ssl_parameters *params) cbuf = buf; switch (bits) { case 512: + if (params->dh_512 != NULL) + ssl_params_corrupted("Duplicate 512bit parameters"); params->dh_512 = d2i_DHparams(NULL, &cbuf, len); break; - case 1024: - params->dh_1024 = d2i_DHparams(NULL, &cbuf, len); - break; default: - ssl_params_corrupted(); + if (params->dh_default != NULL) + ssl_params_corrupted("Duplicate default parameters"); + params->dh_default = d2i_DHparams(NULL, &cbuf, len); + break; } i_free(buf); @@ -221,9 +223,9 @@ static void ssl_free_parameters(struct ssl_parameters *params) DH_free(params->dh_512); params->dh_512 = NULL; } - if (params->dh_1024 != NULL) { - DH_free(params->dh_1024); - params->dh_1024 = NULL; + if (params->dh_default != NULL) { + DH_free(params->dh_default); + params->dh_default = NULL; } } @@ -250,7 +252,7 @@ static void ssl_refresh_parameters(struct ssl_parameters *params) i_fatal("read(%s) failed: %m", params->path); else if (ret != 0) { /* more data than expected */ - ssl_params_corrupted(); + ssl_params_corrupted("More data than expected"); } if (close(params->fd) < 0) @@ -840,12 +842,10 @@ static RSA *ssl_gen_rsa_key(SSL *ssl ATTR_UNUSED, static DH *ssl_tmp_dh_callback(SSL *ssl ATTR_UNUSED, int is_export, int keylength) { - /* Well, I'm not exactly sure why the logic in here is this. - It's the same as in Postfix, so it can't be too wrong. */ if (is_export && keylength == 512 && ssl_params.dh_512 != NULL) return ssl_params.dh_512; - return ssl_params.dh_1024; + return ssl_params.dh_default; } static void ssl_info_callback(const SSL *ssl, int where, int ret) diff --git a/src/ssl-params/ssl-params-openssl.c b/src/ssl-params/ssl-params-openssl.c index 3fc45f468b..206ab6c507 100644 --- a/src/ssl-params/ssl-params-openssl.c +++ b/src/ssl-params/ssl-params-openssl.c @@ -13,8 +13,6 @@ default.. */ #define DH_GENERATOR 2 -static int dh_param_bitsizes[] = { 512, 1024 }; - static const char *ssl_last_error(void) { unsigned long err; @@ -56,13 +54,12 @@ static void generate_dh_parameters(int bitsize, int fd, const char *fname) i_free(buf); } -void ssl_generate_parameters(int fd, const char *fname) +void ssl_generate_parameters(int fd, unsigned int dh_length, const char *fname) { - unsigned int i; int bits; - for (i = 0; i < N_ELEMENTS(dh_param_bitsizes); i++) - generate_dh_parameters(dh_param_bitsizes[i], fd, fname); + generate_dh_parameters(512, fd, fname); + generate_dh_parameters(dh_length, fd, fname); bits = 0; if (write_full(fd, &bits, sizeof(bits)) < 0) i_fatal("write_full() failed for file %s: %m", fname); diff --git a/src/ssl-params/ssl-params-settings.c b/src/ssl-params/ssl-params-settings.c index dda8e901d7..42cc210f4e 100644 --- a/src/ssl-params/ssl-params-settings.c +++ b/src/ssl-params/ssl-params-settings.c @@ -61,12 +61,14 @@ struct service_settings ssl_params_service_settings = { static const struct setting_define ssl_params_setting_defines[] = { DEF(SET_TIME, ssl_parameters_regenerate), + DEF(SET_UINT, ssl_dh_parameters_length), SETTING_DEFINE_LIST_END }; static const struct ssl_params_settings ssl_params_default_settings = { - .ssl_parameters_regenerate = 3600*24*7 + .ssl_parameters_regenerate = 0, + .ssl_dh_parameters_length = 1024 }; const struct setting_parser_info ssl_params_setting_parser_info = { diff --git a/src/ssl-params/ssl-params-settings.h b/src/ssl-params/ssl-params-settings.h index d539b77134..951c6a05ed 100644 --- a/src/ssl-params/ssl-params-settings.h +++ b/src/ssl-params/ssl-params-settings.h @@ -5,6 +5,7 @@ struct master_service; struct ssl_params_settings { unsigned int ssl_parameters_regenerate; + unsigned int ssl_dh_parameters_length; }; struct ssl_params_settings * diff --git a/src/ssl-params/ssl-params.c b/src/ssl-params/ssl-params.c index 751eb986d6..479249f58f 100644 --- a/src/ssl-params/ssl-params.c +++ b/src/ssl-params/ssl-params.c @@ -22,7 +22,7 @@ # include #endif -#define MAX_PARAM_FILE_SIZE 1024 +#define MAX_PARAM_FILE_SIZE 1024*1024 #define SSL_BUILD_PARAM_TIMEOUT_SECS (60*30) #define SSL_PARAMS_PRIORITY 15 @@ -31,11 +31,11 @@ struct ssl_params { struct ssl_params_settings set; time_t last_mtime; - struct timeout *to_rebuild; ssl_params_callback_t *callback; }; -static void ssl_params_if_unchanged(const char *path, time_t mtime) +static void ssl_params_if_unchanged(const char *path, time_t mtime, + unsigned int ssl_dh_parameters_length) { const char *temp_path; struct file_lock *lock; @@ -99,7 +99,7 @@ static void ssl_params_if_unchanged(const char *path, time_t mtime) i_info("Generating SSL parameters"); #ifdef HAVE_SSL - ssl_generate_parameters(fd, temp_path); + ssl_generate_parameters(fd, ssl_dh_parameters_length, temp_path); #endif if (rename(temp_path, path) < 0) @@ -129,9 +129,6 @@ static void ssl_params_close_listeners(void) static void ssl_params_rebuild(struct ssl_params *param) { - if (param->to_rebuild != NULL) - timeout_remove(¶m->to_rebuild); - switch (fork()) { case -1: i_fatal("fork() failed: %m"); @@ -139,7 +136,8 @@ static void ssl_params_rebuild(struct ssl_params *param) /* child - close listener fds so a long-running ssl-params doesn't cause Dovecot restart to fail */ ssl_params_close_listeners(); - ssl_params_if_unchanged(param->path, param->last_mtime); + ssl_params_if_unchanged(param->path, param->last_mtime, + param->set.ssl_dh_parameters_length); exit(0); default: /* parent */ @@ -147,27 +145,38 @@ static void ssl_params_rebuild(struct ssl_params *param) } } -static void ssl_params_set_timeout(struct ssl_params *param) +static bool +ssl_params_verify(struct ssl_params *param, + const unsigned char *data, size_t size) { - time_t next_rebuild, diff; - - if (param->to_rebuild != NULL) - timeout_remove(¶m->to_rebuild); - if (param->set.ssl_parameters_regenerate == 0) - return; - - next_rebuild = param->last_mtime + - param->set.ssl_parameters_regenerate; - - if (ioloop_time >= next_rebuild) { - ssl_params_rebuild(param); - return; + unsigned int bitsize, len; + bool found = FALSE; + + /* ... */ + while (size >= sizeof(bitsize)) { + memcpy(&bitsize, data, sizeof(bitsize)); + if (bitsize == 0) { + if (found) + return TRUE; + i_warning("Regenerating %s for ssl_dh_parameters_length=%u", + param->path, param->set.ssl_dh_parameters_length); + return FALSE; + } + data += sizeof(bitsize); + size -= sizeof(bitsize); + if (bitsize == param->set.ssl_dh_parameters_length) + found = TRUE; + + if (size < sizeof(len)) + break; + memcpy(&len, data, sizeof(len)); + if (len > size - sizeof(len)) + break; + data += sizeof(bitsize) + len; + size -= sizeof(bitsize) + len; } - - diff = next_rebuild - ioloop_time; - if (diff > INT_MAX / 1000) - diff = INT_MAX / 1000; - param->to_rebuild = timeout_add(diff * 1000, ssl_params_rebuild, param); + i_error("Corrupted %s", param->path); + return FALSE; } static int ssl_params_read(struct ssl_params *param) @@ -188,6 +197,7 @@ static int ssl_params_read(struct ssl_params *param) i_close_fd(&fd); return -1; } + param->last_mtime = st.st_mtime; if (st.st_size == 0 || st.st_size > MAX_PARAM_FILE_SIZE) { i_error("Corrupted file: %s", param->path); i_close_fd(&fd); @@ -202,9 +212,9 @@ static int ssl_params_read(struct ssl_params *param) else if (ret == 0) { i_error("File unexpectedly shrank: %s", param->path); ret = -1; + } else if (!ssl_params_verify(param, buffer, st.st_size)) { + ret = -1; } else { - param->last_mtime = st.st_mtime; - ssl_params_set_timeout(param); param->callback(buffer, st.st_size); } @@ -238,8 +248,6 @@ void ssl_params_deinit(struct ssl_params **_param) struct ssl_params *param = *_param; *_param = NULL; - if (param->to_rebuild != NULL) - timeout_remove(¶m->to_rebuild); i_free(param->path); i_free(param); } diff --git a/src/ssl-params/ssl-params.h b/src/ssl-params/ssl-params.h index e5eb070c04..19d8f6e9db 100644 --- a/src/ssl-params/ssl-params.h +++ b/src/ssl-params/ssl-params.h @@ -12,6 +12,6 @@ void ssl_params_deinit(struct ssl_params **param); void ssl_params_refresh(struct ssl_params *param); -void ssl_generate_parameters(int fd, const char *fname); +void ssl_generate_parameters(int fd, unsigned int dh_length, const char *fname); #endif