From 79f31e85dbb7a67c62e7d2365afbf1e139fb43ff Mon Sep 17 00:00:00 2001 From: David Shaw Date: Fri, 20 Mar 2009 20:55:47 +0000 Subject: * 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. --- g10/ChangeLog | 8 ++++++++ g10/keyring.c | 34 ++++++++++++++++++++++------------ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/g10/ChangeLog b/g10/ChangeLog index ada348b2d..253336ec2 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,11 @@ +2009-03-20 David Shaw + + * 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-14 David Shaw * gpgv.c (strusage): Fix name of program in "Syntax" line. diff --git a/g10/keyring.c b/g10/keyring.c index 1259c0b1b..f4bca709e 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, 2007, 2009 Free Software Foundation, Inc. * * This file is part of GnuPG. * @@ -1212,10 +1212,17 @@ static int rename_tmp_file (const char *bakfname, const char *tmpfname, const char *fname, int secret ) { - int rc=0; + int rc=G10ERR_GENERAL; + + /* 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 + (ext4). */ + if(secret && iobuf_ioctl(NULL,4,0,(char*)tmpfname)!=0) + goto fail; /* invalidate close caches*/ - iobuf_ioctl (NULL, 2, 0, (char*)tmpfname ); + if(iobuf_ioctl (NULL, 2, 0, (char*)tmpfname )!=0) + goto fail; iobuf_ioctl (NULL, 2, 0, (char*)bakfname ); iobuf_ioctl (NULL, 2, 0, (char*)fname ); @@ -1245,15 +1252,7 @@ rename_tmp_file (const char *bakfname, const char *tmpfname, tmpfname, fname, strerror(errno) ); register_secured_file (fname); rc = G10ERR_RENAME_FILE; - 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 */ @@ -1275,6 +1274,17 @@ rename_tmp_file (const char *bakfname, const char *tmpfname, #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; } -- cgit v1.2.3