Add a function to "take ownership" of a filesystem

We already allow taking ownership when creating a filesystem (basically running chown on the filesystem root after creating it) but it could be useful to have this functionality for existing filesystems too because right now it isn't possible to write to devices with filesystems created using different tools and it isn't user friendly to run chmod/chown manually.
storaged-project · Jul 28, 2017 · 25688a3 · 25688a3
1 parent 067a248
commit 25688a3
Show file tree

Hide file tree

Showing 8 changed files with 474 additions and 89 deletions.
diff --git a/data/org.freedesktop.UDisks2.policy.in b/data/org.freedesktop.UDisks2.policy.in
@@ -69,6 +69,19 @@
     </defaults>
   </action>
 
+  <!-- ###################################################################### -->
+  <!-- Taking ownership of filesystems -->
+
+  <action id="org.freedesktop.udisks2.filesystem-take-ownership">
+    <_description>Take ownership of a filesystem</_description>
+    <_message>Authentication is required to take ownership of a filesystem.</_message>
+    <defaults>
+      <allow_any>auth_admin</allow_any>
+      <allow_inactive>auth_admin</allow_inactive>
+      <allow_active>auth_admin_keep</allow_active>
+    </defaults>
+  </action>
+
   <!-- ###################################################################### -->
   <!-- Unlocking encrypted devices -->
 

diff --git a/data/org.freedesktop.UDisks2.xml b/data/org.freedesktop.UDisks2.xml
@@ -1836,6 +1836,19 @@
       <arg name="options" direction="in" type="a{sv}"/>
       <arg name="repaired" direction="out" type="b"/>
     </method>
+
+    <!-- TakeOwnership:
+         @options: Options (in addition to <link linkend="udisks-std-options">standard options</link>) includes <parameter>recursive</parameter> (of type 'b').
+         @since: 2.7.2
+
+         Changes ownership of the filesystem to the UID and GID of the calling
+         user.
+
+         Filesystems that don't support ownership result in an error.
+    -->
+    <method name="TakeOwnership">
+      <arg name="options" direction="in" type="a{sv}"/>
+    </method>
   </interface>
 
   <!-- ********************************************************************** -->

diff --git a/src/Makefile.am b/src/Makefile.am
@@ -57,6 +57,7 @@ libudisks_daemon_la_SOURCES =                                                  \
 	udiskslinuxpartition.h         udiskslinuxpartition.c                  \
 	udiskslinuxpartitiontable.h    udiskslinuxpartitiontable.c             \
 	udiskslinuxfilesystem.h        udiskslinuxfilesystem.c                 \
+	udiskslinuxfilesystemhelpers.h udiskslinuxfilesystemhelpers.c          \
 	udiskslinuxencrypted.h         udiskslinuxencrypted.c                  \
 	udiskslinuxencryptedhelpers.h udiskslinuxencryptedhelpers.c            \
 	udiskslinuxswapspace.h         udiskslinuxswapspace.c                  \

diff --git a/src/tests/dbus-tests/test_80_filesystem.py b/src/tests/dbus-tests/test_80_filesystem.py
@@ -21,6 +21,8 @@ class UdisksFSTestCase(udiskstestcase.UdisksTestCase):
     _can_label = False
     _can_mount = False
 
+    username = 'udisks_test_user'
+
     def _clean_format(self, disk):
         d = dbus.Dictionary(signature='sv')
         d['erase'] = True
@@ -29,6 +31,26 @@ def _clean_format(self, disk):
     def _unmount(self, disk_path):
         self.run_command('umount %s' % disk_path)
 
+    def _add_user(self, username):
+        ret, out = self.run_command('useradd -M -p "" %s' % username)
+        if ret != 0:
+            self.fail('Failed to create user %s: %s' % (username, out))
+
+        ret, uid = self.run_command('id -u %s' % username)
+        if ret != 0:
+            self.fail('Failed to get UID for user %s' % username)
+
+        ret, gid = self.run_command('id -g %s' % username)
+        if ret != 0:
+            self.fail('Failed to get GID for user %s.' % username)
+
+        return (uid, gid)
+
+    def _remove_user(self, username):
+        ret, out = self.run_command('userdel %s' % username)
+        if ret != 0:
+            self.fail('Failed to remove user user %s: %s' % (username, out))
+
     @classmethod
     def command_exists(cls, command):
         ret, _out = cls.run_command('type %s' % command)
@@ -262,6 +284,66 @@ class Ext4TestCase(Ext2TestCase):
     def _invalid_label(self, disk):
         pass
 
+    def test_take_ownership(self):
+        if not self._can_create:
+            self.skipTest('Cannot create %s filesystem' % self._fs_name)
+
+        if not self._can_mount:
+            self.skipTest('Cannot mount %s filesystem' % self._fs_name)
+
+        disk = self.get_object('/block_devices/' + os.path.basename(self.vdevs[0]))
+        self.assertIsNotNone(disk)
+
+        # create filesystem
+        disk.Format(self._fs_name, self.no_options, dbus_interface=self.iface_prefix + '.Block')
+        self.addCleanup(self._clean_format, disk)
+
+        # create user for our test
+        self.addCleanup(self._remove_user, self.username)
+        uid, gid = self._add_user(self.username)
+
+        # mount the device
+        mnt_path = disk.Mount(self.no_options, dbus_interface=self.iface_prefix + '.Filesystem')
+        self.addCleanup(self._unmount, self.vdevs[0])
+
+        # change owner of the mountpoint to our user
+        os.chown(mnt_path, int(uid), int(gid))
+
+        # now take ownership of the filesystem -- owner should now be root
+        disk.TakeOwnership(self.no_options, dbus_interface=self.iface_prefix + '.Filesystem')
+
+        sys_stat = os.stat(mnt_path)
+        self.assertEqual(sys_stat.st_uid, 0)
+        self.assertEqual(sys_stat.st_gid, 0)
+
+        # change the owner back and create some files and directories there
+        os.chown(mnt_path, int(uid), int(gid))
+
+        dirname = 'udisks_test_dir'
+        fname = 'file.txt'
+
+        os.mknod(os.path.join(mnt_path, fname))
+        os.mkdir(os.path.join(mnt_path, dirname))
+        os.mknod(os.path.join(mnt_path, dirname, fname))
+
+        # now take ownership of the filesystem with recursive option -- owner
+        # of everything should now be root
+        d = dbus.Dictionary(signature='sv')
+        d['recursive'] = True
+        disk.TakeOwnership(d, dbus_interface=self.iface_prefix + '.Filesystem')
+
+        sys_stat = os.stat(mnt_path)
+        self.assertEqual(sys_stat.st_uid, 0)
+        self.assertEqual(sys_stat.st_gid, 0)
+
+        sys_stat = os.stat(os.path.join(mnt_path, dirname))
+        self.assertEqual(sys_stat.st_uid, 0)
+        self.assertEqual(sys_stat.st_gid, 0)
+
+        sys_stat = os.stat(os.path.join(mnt_path, dirname, fname))
+        self.assertEqual(sys_stat.st_uid, 0)
+        self.assertEqual(sys_stat.st_gid, 0)
+
 
 class XFSTestCase(UdisksFSTestCase):
     _fs_name = 'xfs'
@@ -282,34 +364,12 @@ class VFATTestCase(UdisksFSTestCase):
     _can_label = True and UdisksFSTestCase.command_exists('fatlabel')
     _can_mount = True
 
-    username = 'udisks_mount_test'
-
     def _invalid_label(self, disk):
         label = 'a' * 12  # at most 11 characters
         msg = 'org.freedesktop.UDisks2.Error.Failed: Error setting label'
         with six.assertRaisesRegex(self, dbus.exceptions.DBusException, msg):
             disk.SetLabel(label, self.no_options, dbus_interface=self.iface_prefix + '.Filesystem')
 
-    def _add_user(self):
-        ret, out = self.run_command('useradd -M -p "" %s' % self.username)
-        if ret != 0:
-            self.fail('Failed to create user %s: %s' % (self.username, out))
-
-        ret, uid = self.run_command('id -u %s' % self.username)
-        if ret != 0:
-            self.fail('Failed to get UID for user %s' % self.username)
-
-        ret, gid = self.run_command('id -g %s' % self.username)
-        if ret != 0:
-            self.fail('Failed to get GID for user %s.' % self.username)
-
-        return (uid, gid)
-
-    def _remove_user(self):
-        ret, out = self.run_command('userdel %s' % self.username)
-        if ret != 0:
-            self.fail('Failed to remove user user %s: %s' % (self.username, out))
-
     def _set_user_mountable(self, disk):
         # create a tempdir
         tmp = tempfile.mkdtemp()
@@ -472,8 +532,8 @@ def test_mount_fstab_user(self):
         self.addCleanup(self._clean_format, disk)
 
         # create user for our test
-        self.addCleanup(self._remove_user)
-        uid, gid = self._add_user()
+        self.addCleanup(self._remove_user, self.username)
+        uid, gid = self._add_user(self.username)
 
         # add the disk to fstab
         self._set_user_mountable(disk)
@@ -510,8 +570,8 @@ def test_mount_fstab_user_fail(self):
         self.addCleanup(self._clean_format, disk)
 
         # create user for our test
-        self.addCleanup(self._remove_user)
-        uid, gid = self._add_user()
+        self.addCleanup(self._remove_user, self.username)
+        uid, gid = self._add_user(self.username)
 
         # add unmount cleanup now in case something wrong happens in the other process
         self.addCleanup(self._unmount, self.vdevs[0])

diff --git a/src/udiskslinuxblock.c b/src/udiskslinuxblock.c
@@ -63,6 +63,7 @@
 #include "udiskslinuxencrypted.h"
 #include "udiskslinuxencryptedhelpers.h"
 #include "udiskslinuxpartitiontable.h"
+#include "udiskslinuxfilesystemhelpers.h"
 
 /**
  * SECTION:udiskslinuxblock
@@ -3086,70 +3087,12 @@ udisks_linux_block_handle_format (UDisksBlock             *block,
   /* Change overship, if requested and supported */
   if (take_ownership && fs_info->supports_owners)
     {
-      gchar tos_dir[256] = PACKAGE_LOCALSTATE_DIR "/run/udisks2/block-format-tos-XXXXXX";
-
-      if (mkdtemp (tos_dir) == NULL)
-        {
-            handle_format_failure (invocation, g_error_new (UDISKS_ERROR, UDISKS_ERROR_FAILED,
-                                   "Cannot create directory %s: %m", tos_dir));
-          goto out;
-        }
-      if (mount (udisks_block_get_device (block_to_mkfs), tos_dir, type, 0, NULL) != 0)
-        {
-          handle_format_failure (invocation, g_error_new (UDISKS_ERROR, UDISKS_ERROR_FAILED,
-                                 "Cannot mount %s at %s: %m", udisks_block_get_device (block_to_mkfs), tos_dir));
-          if (rmdir (tos_dir) != 0)
-            {
-              udisks_warning ("Error removing directory %s: %m", tos_dir);
-            }
-          goto out;
-        }
-      if (chown (tos_dir, caller_uid, caller_gid) != 0)
-        {
-          handle_format_failure (invocation, g_error_new (UDISKS_ERROR, UDISKS_ERROR_FAILED,
-                                 "Cannot chown %s to uid=%u and gid=%u: %m", tos_dir, caller_uid, caller_gid));
-          if (umount (tos_dir) != 0)
-            {
-              udisks_warning ("Error unmounting directory %s: %m", tos_dir);
-              goto out;
-            }
-          if (rmdir (tos_dir) != 0)
-            {
-              udisks_warning ("Error removing directory %s: %m", tos_dir);
-            }
-          goto out;
-        }
-      if (chmod (tos_dir, 0700) != 0)
+      if (!take_filesystem_ownership (udisks_block_get_device (block_to_mkfs),
+                                      type, caller_uid, caller_gid, FALSE, &error))
         {
-          handle_format_failure (invocation, g_error_new (UDISKS_ERROR, UDISKS_ERROR_FAILED,
-                                 "Cannot chmod %s to mode 0700: %m", tos_dir));
-          if (umount (tos_dir) != 0)
-            {
-              udisks_warning ("Error unmounting directory %s: %m", tos_dir);
-              goto out;
-            }
-          if (rmdir (tos_dir) != 0)
-            {
-              udisks_warning ("Error removing directory %s: %m", tos_dir);
-            }
-          goto out;
-        }
-
-      if (umount (tos_dir) != 0)
-        {
-          handle_format_failure (invocation, g_error_new (UDISKS_ERROR, UDISKS_ERROR_FAILED,
-                                 "Cannot unmount %s: %m", tos_dir));
-          if (rmdir (tos_dir) != 0)
-            {
-              udisks_warning ("Error removing directory %s: %m", tos_dir);
-            }
-          goto out;
-        }
-
-      if (rmdir (tos_dir) != 0)
-        {
-          handle_format_failure (invocation, g_error_new (UDISKS_ERROR, UDISKS_ERROR_FAILED,
-                                 "Cannot remove directory %s: %m", tos_dir));
+          g_prefix_error (&error,
+                          "Failed to take ownership of newly created filesystem: ");
+          handle_format_failure (invocation, error);
           goto out;
         }
     }