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 <wk@gnupg.org>
This commit is contained in:
Werner Koch 2016-08-03 15:31:27 +02:00
parent 3a2421c940
commit 48a2c93a18
No known key found for this signature in database
GPG Key ID: E3FDFF218E45B72B
4 changed files with 88 additions and 53 deletions

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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));

View File

@ -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