From 48a2c93a1886589d1a0e2a4a2173e0e81311200b Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 3 Aug 2016 15:31:27 +0200 Subject: [PATCH] gpg,gpgsm: Block signals during keyring/keybox update. * kbx/keybox-util.c (keybox_file_rename): Add arg BLOCK_SIGNALS. * kbx/keybox-update.c (rename_tmp_file): Block all signals when doing a double rename. * g10/keyring.c (rename_tmp_file): Block all signals during the double rename. -- This might fix Debian-bug-id: 831510 Signed-off-by: Werner Koch --- g10/keyring.c | 13 +++++-- kbx/keybox-update.c | 37 +++++++++++-------- kbx/keybox-util.c | 88 ++++++++++++++++++++++++++------------------- kbx/keybox.h | 3 +- 4 files changed, 88 insertions(+), 53 deletions(-) diff --git a/g10/keyring.c b/g10/keyring.c index 0611b2eb9..aa73290b2 100644 --- a/g10/keyring.c +++ b/g10/keyring.c @@ -1338,6 +1338,7 @@ static int rename_tmp_file (const char *bakfname, const char *tmpfname, const char *fname) { int rc = 0; + int block = 0; /* Invalidate close caches. */ if (iobuf_ioctl (NULL, IOBUF_IOCTL_INVALIDATE_CACHE, 0, (char*)tmpfname )) @@ -1349,12 +1350,18 @@ rename_tmp_file (const char *bakfname, const char *tmpfname, const char *fname) iobuf_ioctl (NULL, IOBUF_IOCTL_INVALIDATE_CACHE, 0, (char*)fname ); /* First make a backup file. */ - rc = keybox_file_rename (fname, bakfname); + block = 1; + rc = keybox_file_rename (fname, bakfname, &block); if (rc) goto fail; /* then rename the file */ - rc = keybox_file_rename (tmpfname, fname); + rc = keybox_file_rename (tmpfname, fname, NULL); + if (block) + { + gnupg_unblock_all_signals (); + block = 0; + } if (rc) { register_secured_file (fname); @@ -1379,6 +1386,8 @@ rename_tmp_file (const char *bakfname, const char *tmpfname, const char *fname) return 0; fail: + if (block) + gnupg_unblock_all_signals (); return rc; } diff --git a/kbx/keybox-update.c b/kbx/keybox-update.c index ff6590436..ec28b4c4a 100644 --- a/kbx/keybox-update.c +++ b/kbx/keybox-update.c @@ -97,6 +97,7 @@ rename_tmp_file (const char *bakfname, const char *tmpfname, const char *fname, int secret ) { int rc=0; + int block = 0; /* restrict the permissions for secret keyboxs */ #ifndef HAVE_DOSISH_SYSTEM @@ -119,27 +120,35 @@ rename_tmp_file (const char *bakfname, const char *tmpfname, /* First make a backup file except for secret keyboxes. */ if (!secret) { - rc = keybox_file_rename (fname, bakfname); + block = 1; + rc = keybox_file_rename (fname, bakfname, &block); if (rc) - return rc; + goto leave; } /* Then rename the file. */ - rc = keybox_file_rename (tmpfname, fname); - if (rc) + rc = keybox_file_rename (tmpfname, fname, NULL); + if (block) { - if (secret) - { -/* log_info ("WARNING: 2 files with confidential" */ -/* " information exists.\n"); */ -/* log_info ("%s is the unchanged one\n", fname ); */ -/* log_info ("%s is the new one\n", tmpfname ); */ -/* log_info ("Please fix this possible security flaw\n"); */ - } - return rc; + gnupg_unblock_all_signals (); + block = 0; } + /* if (rc) */ + /* { */ + /* if (secret) */ + /* { */ + /* log_info ("WARNING: 2 files with confidential" */ + /* " information exists.\n"); */ + /* log_info ("%s is the unchanged one\n", fname ); */ + /* log_info ("%s is the new one\n", tmpfname ); */ + /* log_info ("Please fix this possible security flaw\n"); */ + /* } */ + /* } */ - return 0; + leave: + if (block) + gnupg_unblock_all_signals (); + return rc; } diff --git a/kbx/keybox-util.c b/kbx/keybox-util.c index 13fedb32f..a2ca3f0ed 100644 --- a/kbx/keybox-util.c +++ b/kbx/keybox-util.c @@ -27,6 +27,7 @@ #endif #include "keybox-defs.h" +#include "utilproto.h" static void *(*alloc_func)(size_t n) = malloc; @@ -147,55 +148,70 @@ keybox_tmp_names (const char *filename, int for_keyring, } -/* Wrapper for rename(2) to handle Windows peculiarities. */ +/* Wrapper for rename(2) to handle Windows peculiarities. If + * BLOCK_SIGNALS is not NULL and points to a variable set to true, all + * signals will be blocked by calling gnupg_block_all_signals; the + * caller needs to call gnupg_unblock_all_signals if that variable is + * still set to true on return. */ gpg_error_t -keybox_file_rename (const char *oldname, const char *newname) +keybox_file_rename (const char *oldname, const char *newname, + int *block_signals) { gpg_error_t err = 0; + if (block_signals && *block_signals) + gnupg_block_all_signals (); + #ifdef HAVE_DOSISH_SYSTEM - int wtime = 0; + { + int wtime = 0; - gnupg_remove (newname); - again: - if (rename (oldname, newname)) - { - if (GetLastError () == ERROR_SHARING_VIOLATION) - { - /* Another process has the file open. We do not use a lock - * for read but instead we wait until the other process has - * closed the file. This may take long but that would also - * be the case with a dotlock approach for read and write. - * Note that we don't need this on Unix due to the inode - * concept. - * - * So let's wait until the rename has worked. The retry - * intervals are 50, 100, 200, 400, 800, 50ms, ... */ - if (!wtime || wtime >= 800) - wtime = 50; - else - wtime *= 2; + gnupg_remove (newname); + again: + if (rename (oldname, newname)) + { + if (GetLastError () == ERROR_SHARING_VIOLATION) + { + /* Another process has the file open. We do not use a + * lock for read but instead we wait until the other + * process has closed the file. This may take long but + * that would also be the case with a dotlock approach for + * read and write. Note that we don't need this on Unix + * due to the inode concept. + * + * So let's wait until the rename has worked. The retry + * intervals are 50, 100, 200, 400, 800, 50ms, ... */ + if (!wtime || wtime >= 800) + wtime = 50; + else + wtime *= 2; - if (wtime >= 800) - log_info ("waiting for file '%s' to become accessible ...\n", - oldname); - - Sleep (wtime); - goto again; - } - err = gpg_error_from_syserror (); - } + if (wtime >= 800) + log_info ("waiting for file '%s' to become accessible ...\n", + oldname); + Sleep (wtime); + goto again; + } + err = gpg_error_from_syserror (); + } + } #else /* Unix */ - + { #ifdef __riscos__ - gnupg_remove (newname); + gnupg_remove (newname); #endif - if (rename (oldname, newname) ) - err = gpg_error_from_syserror (); - + if (rename (oldname, newname) ) + err = gpg_error_from_syserror (); + } #endif /* Unix */ + if (block_signals && *block_signals && err) + { + gnupg_unblock_all_signals (); + *block_signals = 0; + } + if (err) log_error ("renaming '%s' to '%s' failed: %s\n", oldname, newname, gpg_strerror (err)); diff --git a/kbx/keybox.h b/kbx/keybox.h index bfc358620..6180a2fd7 100644 --- a/kbx/keybox.h +++ b/kbx/keybox.h @@ -134,7 +134,8 @@ void keybox_set_malloc_hooks ( void *(*new_alloc_func)(size_t n), gpg_error_t keybox_tmp_names (const char *filename, int for_keyring, char **r_bakname, char **r_tmpname); -gpg_error_t keybox_file_rename (const char *oldname, const char *newname); +gpg_error_t keybox_file_rename (const char *oldname, const char *newname, + int *block_signals); #ifdef __cplusplus