* 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.

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  <[email protected]>
+
+	* 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  <[email protected]>
 
 	* 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;
 }