diff options
| -rw-r--r-- | ChangeLog | 4 | ||||
| -rw-r--r-- | agent/findkey.c | 2 | ||||
| -rw-r--r-- | common/ChangeLog | 9 | ||||
| -rw-r--r-- | common/iobuf.c | 106 | ||||
| -rw-r--r-- | configure.ac | 2 | ||||
| -rw-r--r-- | g10/ChangeLog | 13 | ||||
| -rw-r--r-- | g10/gpg.c | 2 | ||||
| -rw-r--r-- | g10/keyring.c | 51 |
8 files changed, 153 insertions, 36 deletions
@@ -1,3 +1,7 @@ +2009-04-01 Werner Koch <wk@g10code.com> + + * configure.ac: Test for fsync. + 2009-03-18 Werner Koch <wk@g10code.com> * configure.ac: Test for getrlimit. diff --git a/agent/findkey.c b/agent/findkey.c index 5bea198dc..867e19634 100644 --- a/agent/findkey.c +++ b/agent/findkey.c @@ -50,7 +50,7 @@ struct try_unprotect_arg_s /* Write an S-expression formatted key to our key storage. With FORCE - pased as true an existing key with the given GRIP will get + passed as true an existing key with the given GRIP will get overwritten. */ int agent_write_private_key (const unsigned char *grip, diff --git a/common/ChangeLog b/common/ChangeLog index af804f490..6ada73158 100644 --- a/common/ChangeLog +++ b/common/ChangeLog @@ -1,5 +1,14 @@ 2009-04-01 Werner Koch <wk@g10code.com> + * iobuf.c: Port David's changes from 1.4: + (fd_cache_invalidate): Pass return code from close back. + (direct_open, iobuf_ioctl): Check that eturn value. + (fd_cache_synchronize): New. + (iobuf_ioctl): Add new sub command 4 (fsync). + + * iobuf.c (fd_cache_strcmp): New. Taken from 1.4. + (fd_cache_invalidate, fd_cache_close, fd_cache_open): Use it. + * exechelp.c (gnupg_spawn_process): Implement new flag bit 6. * sysutils.c (gnupg_allow_set_foregound_window): Allow the use of ASFW_ANY. diff --git a/common/iobuf.c b/common/iobuf.c index 6c493b512..4ec151f5f 100644 --- a/common/iobuf.c +++ b/common/iobuf.c @@ -1,6 +1,6 @@ /* iobuf.c - File Handling for OpenPGP. * Copyright (C) 1998, 1999, 2000, 2001, 2003, 2004, 2006, - * 2007, 2008 Free Software Foundation, Inc. + * 2007, 2008, 2009 Free Software Foundation, Inc. * * This file is part of GnuPG. * @@ -48,7 +48,8 @@ test "armored_key_8192" in armor.test! */ #define IOBUF_BUFFER_SIZE 8192 -/* We don't want to use the STDIO based backend. */ +/* We don't want to use the STDIO based backend. If you change this + be aware that there is no fsync support for the stdio backend. */ #undef FILE_FILTER_USES_STDIO /*-- End configurable part. --*/ @@ -187,13 +188,32 @@ static int translate_file_handle (int fd, int for_write); #ifndef FILE_FILTER_USES_STDIO +/* This is a replacement for strcmp. Under W32 it does not + distinguish between backslash and slash. */ +static int +fd_cache_strcmp (const char *a, const char *b) +{ +#ifdef HAVE_DOSISH_SYSTEM + for (; *a && *b; a++, b++) + { + if (*a != *b && !((*a == '/' && *b == '\\') + || (*a == '\\' && *b == '/')) ) + break; + } + return *(const unsigned char *)a - *(const unsigned char *)b; +#else + return strcmp (a, b); +#endif +} + /* * Invalidate (i.e. close) a cached iobuf */ -static void +static int fd_cache_invalidate (const char *fname) { close_cache_t cc; + int rc = 0; assert (fname); if (DBG_IOBUF) @@ -201,18 +221,52 @@ fd_cache_invalidate (const char *fname) for (cc = close_cache; cc; cc = cc->next) { - if (cc->fp != INVALID_FP && !strcmp (cc->fname, fname)) + if (cc->fp != INVALID_FP && !fd_cache_strcmp (cc->fname, fname)) { if (DBG_IOBUF) log_debug (" did (%s)\n", cc->fname); #ifdef HAVE_W32_SYSTEM - CloseHandle (cc->fp); + if (!CloseHandle (cc->fp)) + rc = -1; #else - close (cc->fp); + rc = close (cc->fp); #endif cc->fp = INVALID_FP; } } + return rc; +} + + +/* Try to sync changes to the disk. This is to avoid data loss during + a system crash in write/close/rename cycle on some file + systems. */ +static int +fd_cache_synchronize (const char *fname) +{ + int err = 0; + +#ifdef HAVE_FSYNC + close_cache_t cc; + + if (DBG_IOBUF) + log_debug ("fd_cache_synchronize (%s)\n", fname); + + for (cc=close_cache; cc; cc = cc->next ) + { + if (cc->fp != INVALID_FP && !fd_cache_strcmp (cc->fname, fname)) + { + if (DBG_IOBUF) + log_debug (" did (%s)\n", cc->fname); + + err = fsync (cc->fp); + } + } +#else + (void)fname; +#endif /*HAVE_FSYNC*/ + + return err; } @@ -226,19 +280,21 @@ direct_open (const char *fname, const char *mode) /* Note, that we do not handle all mode combinations */ /* According to the ReactOS source it seems that open() of the - * standard MSW32 crt does open the file in share mode which is + * standard MSW32 crt does open the file in shared mode which is * something new for MS applications ;-) */ if (strchr (mode, '+')) { - fd_cache_invalidate (fname); + if (fd_cache_invalidate (fname)) + return INVALID_FP; da = GENERIC_READ | GENERIC_WRITE; cd = OPEN_EXISTING; sm = FILE_SHARE_READ | FILE_SHARE_WRITE; } else if (strchr (mode, 'w')) { - fd_cache_invalidate (fname); + if (fd_cache_invalidate (fname)) + return INVALID_FP; da = GENERIC_WRITE; cd = CREATE_ALWAYS; sm = FILE_SHARE_WRITE; @@ -259,12 +315,14 @@ direct_open (const char *fname, const char *mode) /* Note, that we do not handle all mode combinations */ if (strchr (mode, '+')) { - fd_cache_invalidate (fname); + if (fd_cache_invalidate (fname)) + return INVALID_FP; oflag = O_RDWR; } else if (strchr (mode, 'w')) { - fd_cache_invalidate (fname); + if (fd_cache_invalidate (fname)) + return INVALID_FP; oflag = O_WRONLY | O_CREAT | O_TRUNC; } else @@ -318,7 +376,7 @@ fd_cache_close (const char *fname, fp_or_fd_t fp) /* try to reuse a slot */ for (cc = close_cache; cc; cc = cc->next) { - if (cc->fp == INVALID_FP && !strcmp (cc->fname, fname)) + if (cc->fp == INVALID_FP && !fd_cache_strcmp (cc->fname, fname)) { cc->fp = fp; if (DBG_IOBUF) @@ -347,7 +405,7 @@ fd_cache_open (const char *fname, const char *mode) assert (fname); for (cc = close_cache; cc; cc = cc->next) { - if (cc->fp != INVALID_FP && !strcmp (cc->fname, fname)) + if (cc->fp != INVALID_FP && !fd_cache_strcmp (cc->fname, fname)) { fp_or_fd_t fp = cc->fp; cc->fp = INVALID_FP; @@ -1449,7 +1507,8 @@ iobuf_ioctl (iobuf_t a, int cmd, int intval, void *ptrval) if (!a && !intval && ptrval) { #ifndef FILE_FILTER_USES_STDIO - fd_cache_invalidate (ptrval); + if (fd_cache_invalidate (ptrval)) + return -1; #endif return 0; } @@ -1477,6 +1536,24 @@ iobuf_ioctl (iobuf_t a, int cmd, int intval, void *ptrval) } #endif } + else if (cmd == 4) + { + /* Do a fsync on the open fd and return any errors to the caller + of iobuf_ioctl. Note that we work on a file name here. */ + if (DBG_IOBUF) + log_debug ("iobuf-*.*: ioctl `%s' fsync\n", + ptrval? (const char*)ptrval:"<null>"); + + if (!a && !intval && ptrval) + { +#ifndef FILE_FILTER_USES_STDIO + return fd_cache_synchronize (ptrval); +#else + return 0; +#endif + } + } + return -1; } @@ -2310,7 +2387,6 @@ iobuf_get_fname (iobuf_t a) file_filter_ctx_t *b = a->filter_ov; return b->fname; } - return NULL; } diff --git a/configure.ac b/configure.ac index 0dd26cdbe..31a3516e9 100644 --- a/configure.ac +++ b/configure.ac @@ -1062,7 +1062,7 @@ AC_CHECK_FUNCS([unsetenv getpwnam getpwuid fcntl ftruncate]) AC_CHECK_FUNCS([gettimeofday getrusage getrlimit setrlimit clock_gettime]) AC_CHECK_FUNCS([atexit raise getpagesize strftime nl_langinfo setlocale]) AC_CHECK_FUNCS([waitpid wait4 sigaction sigprocmask pipe stat getaddrinfo]) -AC_CHECK_FUNCS([ttyname rand ftello]) +AC_CHECK_FUNCS([ttyname rand ftello fsync]) AC_CHECK_TYPES([struct sigaction, sigset_t],,,[#include <signal.h>]) diff --git a/g10/ChangeLog b/g10/ChangeLog index 50dca45ca..cf9a8c919 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,16 @@ +2009-04-01 Werner Koch <wk@g10code.com> + + * gpg.c (main): Properly handle UTF8 usernames with --sign-key and + --lsign-key. From 1.4, David 2008-12-21. + +2009-03-20 David Shaw <dshaw@jabberwocky.com> (wk) + + * keyring.c (rename_tmp_file): Force a fsync (via iobuf_ioctl) on + secret keyring files to be extra safe on filesystems that may not + sync data and metadata together (ext4). Also check return code + from the cache invalidation to make sure we're safe over NFS and + similar. + 2009-03-31 Werner Koch <wk@g10code.com> * passphrase.c (ask_passphrase): Use percent_plus_unescape. @@ -3562,7 +3562,7 @@ main (int argc, char **argv) append_to_strlist( &sl, "save" ); username = make_username( fname ); - keyedit_menu(fname, locusr, sl, 0, 0 ); + keyedit_menu (username, locusr, sl, 0, 0 ); xfree(username); free_strlist(sl); break; diff --git a/g10/keyring.c b/g10/keyring.c index ca2513198..00a5bb986 100644 --- a/g10/keyring.c +++ b/g10/keyring.c @@ -1,5 +1,5 @@ /* keyring.c - keyring file handling - * Copyright (C) 2001, 2004 Free Software Foundation, Inc. + * Copyright (C) 2001, 2004, 2009 Free Software Foundation, Inc. * * This file is part of GnuPG. * @@ -1219,10 +1219,23 @@ static int rename_tmp_file (const char *bakfname, const char *tmpfname, const char *fname, int secret ) { - int rc=0; + int rc = 0; - /* invalidate close caches*/ - iobuf_ioctl (NULL, 2, 0, (char*)tmpfname ); + /* It's a secret keyring, so let's force a fsync just to be safe on + filesystems that may not sync data and metadata together + (e.g. ext4). */ + if (secret && iobuf_ioctl (NULL, 4, 0, (char*)tmpfname)) + { + rc = gpg_error_from_syserror (); + goto fail; + } + + /* Invalidate close caches. */ + if (iobuf_ioctl (NULL, 2, 0, (char*)tmpfname )) + { + rc = gpg_error_from_syserror (); + goto fail; + } iobuf_ioctl (NULL, 2, 0, (char*)bakfname ); iobuf_ioctl (NULL, 2, 0, (char*)fname ); @@ -1253,15 +1266,7 @@ rename_tmp_file (const char *bakfname, const char *tmpfname, log_error (_("renaming `%s' to `%s' failed: %s\n"), tmpfname, fname, strerror(errno) ); register_secured_file (fname); - 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; + goto fail; } /* Now make sure the file has the same permissions as the original */ @@ -1272,17 +1277,27 @@ rename_tmp_file (const char *bakfname, const char *tmpfname, statbuf.st_mode=S_IRUSR | S_IWUSR; - if(((secret && !opt.preserve_permissions) || - (stat(bakfname,&statbuf)==0)) && - (chmod(fname,statbuf.st_mode)==0)) + if (((secret && !opt.preserve_permissions) + || !stat (bakfname,&statbuf)) + && !chmod (fname,statbuf.st_mode)) ; else - log_error("WARNING: unable to restore permissions to `%s': %s", - fname,strerror(errno)); + log_error ("WARNING: unable to restore permissions to `%s': %s", + fname, strerror(errno)); } #endif return 0; + + fail: + 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; } |