agent: Sanitize permissions of the private key directory.

* agent/gpg-agent.c (create_private_keys_directory): Set permissions.
* common/sysutils.c (modestr_to_mode): New function.
(gnupg_mkdir): Use new function.
(gnupg_chmod): New function.
* common/sysutils.h (gnupg_chmod): New prototype.
* tests/migrations/from-classic.test: Test migration with existing
directory.

GnuPG-bug-id: 2312
Signed-off-by: Justus Winter <justus@g10code.com>

4 files changed, 69 insertions, 24 deletions

diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
index 8aab2b951..a87052a92 100644
--- a/agent/gpg-agent.c
+++ b/agent/gpg-agent.c
@@ -1908,9 +1908,13 @@ create_private_keys_directory (const char *home)
       else if (!opt.quiet)
         log_info (_("directory '%s' created\n"), fname);
     }
+  if (gnupg_chmod (fname, "-rwx"))
+    log_error (_("can't set permissions of '%s': %s\n"),
+               fname, strerror (errno));
   xfree (fname);
 }
 
+
 /* Create the directory only if the supplied directory name is the
    same as the default one.  This way we avoid to create arbitrary
    directories when a non-default home directory is used.  To cope
diff --git a/common/sysutils.c b/common/sysutils.c
index ccd35422c..18625d0ff 100644
--- a/common/sysutils.c
+++ b/common/sysutils.c
@@ -554,6 +554,40 @@ gnupg_remove (const char *fname)
 }
 
 
+#ifndef HAVE_W32_SYSTEM
+static mode_t
+modestr_to_mode (const char *modestr)
+{
+  mode_t mode = 0;
+
+  if (modestr && *modestr)
+    {
+      modestr++;
+      if (*modestr && *modestr++ == 'r')
+        mode |= S_IRUSR;
+      if (*modestr && *modestr++ == 'w')
+        mode |= S_IWUSR;
+      if (*modestr && *modestr++ == 'x')
+        mode |= S_IXUSR;
+      if (*modestr && *modestr++ == 'r')
+        mode |= S_IRGRP;
+      if (*modestr && *modestr++ == 'w')
+        mode |= S_IWGRP;
+      if (*modestr && *modestr++ == 'x')
+        mode |= S_IXGRP;
+      if (*modestr && *modestr++ == 'r')
+        mode |= S_IROTH;
+      if (*modestr && *modestr++ == 'w')
+        mode |= S_IWOTH;
+      if (*modestr && *modestr++ == 'x')
+        mode |= S_IXOTH;
+    }
+
+  return mode;
+}
+#endif
+
+
 /* A wrapper around mkdir which takes a string for the mode argument.
    This makes it easier to handle the mode argument which is not
    defined on all systems.  The format of the modestring is
@@ -589,31 +623,22 @@ gnupg_mkdir (const char *name, const char *modestr)
      because this sets ERRNO.  */
   return mkdir (name);
 #else
-  mode_t mode = 0;
+  return mkdir (name, modestr_to_mode (modestr));
+#endif
+}
 
-  if (modestr && *modestr)
-    {
-      modestr++;
-      if (*modestr && *modestr++ == 'r')
-        mode |= S_IRUSR;
-      if (*modestr && *modestr++ == 'w')
-        mode |= S_IWUSR;
-      if (*modestr && *modestr++ == 'x')
-        mode |= S_IXUSR;
-      if (*modestr && *modestr++ == 'r')
-        mode |= S_IRGRP;
-      if (*modestr && *modestr++ == 'w')
-        mode |= S_IWGRP;
-      if (*modestr && *modestr++ == 'x')
-        mode |= S_IXGRP;
-      if (*modestr && *modestr++ == 'r')
-        mode |= S_IROTH;
-      if (*modestr && *modestr++ == 'w')
-        mode |= S_IWOTH;
-      if (*modestr && *modestr++ == 'x')
-        mode |= S_IXOTH;
-    }
-  return mkdir (name, mode);
+
+/* A wrapper around mkdir which takes a string for the mode argument.
+   This makes it easier to handle the mode argument which is not
+   defined on all systems.  The format of the modestring is the same
+   as for gnupg_mkdir.  */
+int
+gnupg_chmod (const char *name, const char *modestr)
+{
+#ifdef HAVE_W32_SYSTEM
+  return 0;
+#else
+  return chmod (name, modestr_to_mode (modestr));
 #endif
 }
 
diff --git a/common/sysutils.h b/common/sysutils.h
index 95276dea6..ba66ce616 100644
--- a/common/sysutils.h
+++ b/common/sysutils.h
@@ -61,6 +61,7 @@ void gnupg_reopen_std (const char *pgmname);
 void gnupg_allow_set_foregound_window (pid_t pid);
 int  gnupg_remove (const char *fname);
 int  gnupg_mkdir (const char *name, const char *modestr);
+int gnupg_chmod (const char *name, const char *modestr);
 char *gnupg_mkdtemp (char *template);
 int  gnupg_setenv (const char *name, const char *value, int overwrite);
 int  gnupg_unsetenv (const char *name);
diff --git a/tests/migrations/from-classic.test b/tests/migrations/from-classic.test
index 4ee3b616f..a61a5c36f 100755
--- a/tests/migrations/from-classic.test
+++ b/tests/migrations/from-classic.test
@@ -50,3 +50,18 @@ assert_migrated()
 setup_home
 trigger_migration
 assert_migrated
+
+# Test with an existing private-keys-v1.d.
+setup_home
+mkdir "$GNUPGHOME/private-keys-v1.d"
+trigger_migration
+assert_migrated
+
+# Test with an existing private-keys-v1.d with weird permissions.
+setup_home
+mkdir "$GNUPGHOME/private-keys-v1.d"
+chmod 0 "$GNUPGHOME/private-keys-v1.d"
+trigger_migration
+assert_migrated
+
+# XXX Check a case where the migration fails.