[svn] GnuPG - r4963 - branches/STABLE-BRANCH-1-4/g10

[svn author dshaw]

Author: dshaw
Date: 2009-03-20 21:55:47 +0100 (Fri, 20 Mar 2009)
New Revision: 4963

Modified:
   branches/STABLE-BRANCH-1-4/g10/ChangeLog
   branches/STABLE-BRANCH-1-4/g10/keyring.c
Log:
* 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.


Modified: branches/STABLE-BRANCH-1-4/g10/ChangeLog
===================================================================
--- branches/STABLE-BRANCH-1-4/g10/ChangeLog	2009-03-20 20:33:14 UTC (rev 4962)
+++ branches/STABLE-BRANCH-1-4/g10/ChangeLog	2009-03-20 20:55:47 UTC (rev 4963)
@@ -1,3 +1,11 @@
+2009-03-20  David Shaw  <dshaw at jabberwocky.com>
+
+	* 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  <dshaw at jabberwocky.com>
 
 	* gpgv.c (strusage): Fix name of program in "Syntax" line.

Modified: branches/STABLE-BRANCH-1-4/g10/keyring.c
===================================================================
--- branches/STABLE-BRANCH-1-4/g10/keyring.c	2009-03-20 20:33:14 UTC (rev 4962)
+++ branches/STABLE-BRANCH-1-4/g10/keyring.c	2009-03-20 20:55:47 UTC (rev 4963)
@@ -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 @@
 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 @@
                  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 @@
 #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;
 }