dovecot-2.2: ssl-params: Added ssl_dh_parameters_length & remove...
dovecot at dovecot.org
dovecot at dovecot.org
Sat Nov 2 15:27:59 EET 2013
details: http://hg.dovecot.org/dovecot-2.2/rev/43ab5abeb8f0
changeset: 16912:43ab5abeb8f0
user: Timo Sirainen <tss at iki.fi>
date: Sat Nov 02 15:27:28 2013 +0200
description:
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.
diffstat:
doc/example-config/conf.d/10-ssl.conf | 6 +--
src/login-common/ssl-proxy-openssl.c | 32 ++++++++--------
src/ssl-params/ssl-params-openssl.c | 9 +---
src/ssl-params/ssl-params-settings.c | 4 +-
src/ssl-params/ssl-params-settings.h | 1 +
src/ssl-params/ssl-params.c | 66 +++++++++++++++++++---------------
src/ssl-params/ssl-params.h | 2 +-
7 files changed, 63 insertions(+), 57 deletions(-)
diffs (truncated from 324 to 300 lines):
diff -r 07c083496665 -r 43ab5abeb8f0 doc/example-config/conf.d/10-ssl.conf
--- a/doc/example-config/conf.d/10-ssl.conf Sat Nov 02 15:18:15 2013 +0200
+++ b/doc/example-config/conf.d/10-ssl.conf Sat Nov 02 15:27:28 2013 +0200
@@ -42,10 +42,8 @@
# auth_ssl_username_from_cert=yes.
#ssl_cert_username_field = commonName
-# How often to regenerate the SSL parameters file. Generation is quite CPU
-# intensive operation. The value is in hours, 0 disables regeneration
-# entirely.
-#ssl_parameters_regenerate = 168
+# DH parameters length to use.
+#ssl_dh_parameters_length = 1024
# SSL protocols to use
#ssl_protocols = !SSLv2
diff -r 07c083496665 -r 43ab5abeb8f0 src/login-common/ssl-proxy-openssl.c
--- a/src/login-common/ssl-proxy-openssl.c Sat Nov 02 15:18:15 2013 +0200
+++ b/src/login-common/ssl-proxy-openssl.c Sat Nov 02 15:27:28 2013 +0200
@@ -86,7 +86,7 @@
time_t last_refresh;
int fd;
- DH *dh_512, *dh_1024;
+ DH *dh_512, *dh_default;
};
struct ssl_server_context {
@@ -163,9 +163,9 @@
return ctx1->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 @@
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 @@
/* 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 @@
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);
+ default:
+ if (params->dh_default != NULL)
+ ssl_params_corrupted("Duplicate default parameters");
+ params->dh_default = d2i_DHparams(NULL, &cbuf, len);
break;
- default:
- ssl_params_corrupted();
}
i_free(buf);
@@ -221,9 +223,9 @@
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 @@
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 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 -r 07c083496665 -r 43ab5abeb8f0 src/ssl-params/ssl-params-openssl.c
--- a/src/ssl-params/ssl-params-openssl.c Sat Nov 02 15:18:15 2013 +0200
+++ b/src/ssl-params/ssl-params-openssl.c Sat Nov 02 15:27:28 2013 +0200
@@ -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 @@
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 -r 07c083496665 -r 43ab5abeb8f0 src/ssl-params/ssl-params-settings.c
--- a/src/ssl-params/ssl-params-settings.c Sat Nov 02 15:18:15 2013 +0200
+++ b/src/ssl-params/ssl-params-settings.c Sat Nov 02 15:27:28 2013 +0200
@@ -61,12 +61,14 @@
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 -r 07c083496665 -r 43ab5abeb8f0 src/ssl-params/ssl-params-settings.h
--- a/src/ssl-params/ssl-params-settings.h Sat Nov 02 15:18:15 2013 +0200
+++ b/src/ssl-params/ssl-params-settings.h Sat Nov 02 15:27:28 2013 +0200
@@ -5,6 +5,7 @@
struct ssl_params_settings {
unsigned int ssl_parameters_regenerate;
+ unsigned int ssl_dh_parameters_length;
};
struct ssl_params_settings *
diff -r 07c083496665 -r 43ab5abeb8f0 src/ssl-params/ssl-params.c
--- a/src/ssl-params/ssl-params.c Sat Nov 02 15:18:15 2013 +0200
+++ b/src/ssl-params/ssl-params.c Sat Nov 02 15:27:28 2013 +0200
@@ -22,7 +22,7 @@
# include <sys/resource.h>
#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_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 @@
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_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 @@
/* 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_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;
+ unsigned int bitsize, len;
+ bool found = FALSE;
- if (param->to_rebuild != NULL)
- timeout_remove(¶m->to_rebuild);
- if (param->set.ssl_parameters_regenerate == 0)
- return;
+ /* <bitsize><length><data>... */
+ 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;
- next_rebuild = param->last_mtime +
- param->set.ssl_parameters_regenerate;
-
- if (ioloop_time >= next_rebuild) {
- ssl_params_rebuild(param);
- return;
+ 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 @@
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 @@
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;
More information about the dovecot-cvs
mailing list