From 5d2cb48f463c328e0405fd8b675e1f85111fbd59 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sat, 19 Nov 2016 00:31:21 +0800 Subject: [PATCH] Use atomic key generation in all cases --- dbutil.c | 17 +++++++++++++++++ dbutil.h | 2 ++ dropbearkey.c | 2 +- gensignkey.c | 39 ++++++++++++++++++++++++++++++++++----- gensignkey.h | 2 +- svr-kex.c | 47 ++--------------------------------------------- 6 files changed, 57 insertions(+), 52 deletions(-) diff --git a/dbutil.c b/dbutil.c index 22d798b..830e8d2 100644 --- a/dbutil.c +++ b/dbutil.c @@ -681,4 +681,21 @@ time_t monotonic_now() { return time(NULL); } +void fsync_parent_dir(const char* fn) { +#ifdef HAVE_LIBGEN_H + char *fn_dir = m_strdup(fn); + char *dir = dirname(fn_dir); + int dirfd = open(dir, O_RDONLY); + if (dirfd != -1) { + if (fsync(dirfd) != 0) { + TRACE(("fsync of directory %s failed: %s", dir, strerror(errno))) + } + m_close(dirfd); + } else { + TRACE(("error opening directory %s for fsync: %s", dir, strerror(errno))) + } + + free(fn_dir); +#endif +} diff --git a/dbutil.h b/dbutil.h index 9bcc875..d83b20a 100644 --- a/dbutil.h +++ b/dbutil.h @@ -89,4 +89,6 @@ time_t monotonic_now(void); char * expand_homedir_path(const char *inpath); +void fsync_parent_dir(const char* fn); + #endif /* DROPBEAR_DBUTIL_H_ */ diff --git a/dropbearkey.c b/dropbearkey.c index 6ea68b4..a0d315b 100644 --- a/dropbearkey.c +++ b/dropbearkey.c @@ -241,7 +241,7 @@ int main(int argc, char ** argv) { } fprintf(stderr, "Generating key, this may take a while...\n"); - if (signkey_generate(keytype, bits, filename) == DROPBEAR_FAILURE) + if (signkey_generate(keytype, bits, filename, 0) == DROPBEAR_FAILURE) { dropbear_exit("Failed to generate key.\n"); } diff --git a/gensignkey.c b/gensignkey.c index 55facc3..4691de0 100644 --- a/gensignkey.c +++ b/gensignkey.c @@ -76,10 +76,12 @@ static int get_default_bits(enum signkey_type keytype) } } -int signkey_generate(enum signkey_type keytype, int bits, const char* filename) +/* if skip_exist is set it will silently return if the key file exists */ +int signkey_generate(enum signkey_type keytype, int bits, const char* filename, int skip_exist) { sign_key * key = NULL; buffer *buf = NULL; + char *fn_temp = NULL; int ret = DROPBEAR_FAILURE; if (bits == 0) { @@ -126,10 +128,37 @@ int signkey_generate(enum signkey_type keytype, int bits, const char* filename) sign_key_free(key); key = NULL; buf_setpos(buf, 0); - ret = buf_writefile(buf, filename); - buf_burn(buf); - buf_free(buf); - buf = NULL; + fn_temp = m_malloc(strlen(filename) + 30); + snprintf(fn_temp, strlen(filename)+30, "%s.tmp%d", filename, getpid()); + ret = buf_writefile(buf, fn_temp); + + if (ret == DROPBEAR_FAILURE) { + goto out; + } + + if (link(fn_temp, filename) < 0) { + /* If generating keys on connection (skipexist) it's OK to get EEXIST + - we probably just lost a race with another connection to generate the key */ + if (!(skip_exist && errno == EEXIST)) { + dropbear_log(LOG_ERR, "Failed moving key file to %s: %s", filename, + strerror(errno)); + /* XXX fallback to non-atomic copy for some filesystems? */ + ret = DROPBEAR_FAILURE; + goto out; + } + } + +out: + if (buf) { + buf_burn(buf); + buf_free(buf); + } + + if (fn_temp) { + unlink(fn_temp); + m_free(fn_temp); + } + return ret; } diff --git a/gensignkey.h b/gensignkey.h index 508eca0..1cba8d3 100644 --- a/gensignkey.h +++ b/gensignkey.h @@ -3,6 +3,6 @@ #include "signkey.h" -int signkey_generate(enum signkey_type type, int bits, const char* filename); +int signkey_generate(enum signkey_type type, int bits, const char* filename, int skip_exist); #endif diff --git a/svr-kex.c b/svr-kex.c index fba9760..7108f64 100644 --- a/svr-kex.c +++ b/svr-kex.c @@ -93,29 +93,9 @@ void recv_msg_kexdh_init() { #if DROPBEAR_DELAY_HOSTKEY -static void fsync_parent_dir(const char* fn) { -#ifdef HAVE_LIBGEN_H - char *fn_dir = m_strdup(fn); - char *dir = dirname(fn_dir); - int dirfd = open(dir, O_RDONLY); - - if (dirfd != -1) { - if (fsync(dirfd) != 0) { - TRACE(("fsync of directory %s failed: %s", dir, strerror(errno))) - } - m_close(dirfd); - } else { - TRACE(("error opening directory %s for fsync: %s", dir, strerror(errno))) - } - - free(fn_dir); -#endif -} - static void svr_ensure_hostkey() { const char* fn = NULL; - char *fn_temp = NULL; enum signkey_type type = ses.newkeys->algo_hostkey; void **hostkey = signkey_key_ptr(svr_opts.hostkey, type); int ret = DROPBEAR_FAILURE; @@ -151,28 +131,10 @@ static void svr_ensure_hostkey() { return; } - fn_temp = m_malloc(strlen(fn) + 20); - snprintf(fn_temp, strlen(fn)+20, "%s.tmp%d", fn, getpid()); - - if (signkey_generate(type, 0, fn_temp) == DROPBEAR_FAILURE) { + if (signkey_generate(type, 0, fn, 1) == DROPBEAR_FAILURE) { goto out; } - - if (link(fn_temp, fn) < 0) { - /* It's OK to get EEXIST - we probably just lost a race - with another connection to generate the key */ - if (errno != EEXIST) { - dropbear_log(LOG_ERR, "Failed moving key file to %s: %s", fn, - strerror(errno)); - /* XXX fallback to non-atomic copy for some filesystems? */ - goto out; - } - } - - /* ensure directory update is flushed to disk, otherwise we can end up - with zero-byte hostkey files if the power goes off */ - fsync_parent_dir(fn); - + ret = readhostkey(fn, svr_opts.hostkey, &type); if (ret == DROPBEAR_SUCCESS) { @@ -190,11 +152,6 @@ static void svr_ensure_hostkey() { } out: - if (fn_temp) { - unlink(fn_temp); - m_free(fn_temp); - } - if (ret == DROPBEAR_FAILURE) { dropbear_exit("Couldn't read or generate hostkey %s", fn);