[gvfs/wip/oholy/sftp-ownership] sftp: Fix file ownership when replacing



commit 196c46b2f9fb69dd41ee88e6804db6521f2c02f9
Author: Ondrej Holy <oholy redhat com>
Date:   Fri Dec 13 14:12:56 2019 +0100

    sftp: Fix file ownership when replacing
    
    User and group are reset when replacing even though
    G_FILE_CREATE_REPLACE_DESTINATION is not set. Although the backend
    calls SSH_FXP_OPEN with SSH_FILEXFER_ATTR_UIDGID attribute, the OpenSSH
    server ignores this attribute in this operation. Let's fix this by the
    separate SSH_FXP_FSETSTAT call.
    
    Fixes: https://gitlab.gnome.org/GNOME/gvfs/issues/418

 daemon/gvfsbackendsftp.c | 184 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 152 insertions(+), 32 deletions(-)
---
diff --git a/daemon/gvfsbackendsftp.c b/daemon/gvfsbackendsftp.c
index ccaddee7..cde9cd85 100644
--- a/daemon/gvfsbackendsftp.c
+++ b/daemon/gvfsbackendsftp.c
@@ -3601,6 +3601,144 @@ replace_truncate_original (GVfsBackendSftp *backend,
                                  job, NULL);
 }
 
+static void
+close_temp_file (GVfsBackendSftp *backend,
+                 SftpHandle *handle,
+                 GVfsJob *job)
+{
+  GDataOutputStream *command;
+
+  command = new_command_stream (backend,
+                                SSH_FXP_CLOSE);
+  put_data_buffer (command, handle->raw_handle);
+  queue_command_stream_and_free (&backend->command_connection, command,
+                                 NULL,
+                                 G_VFS_JOB (job), NULL);
+}
+
+static void
+replace_create_temp_fsetstat_reply (GVfsBackendSftp *backend,
+                                    int reply_type,
+                                    GDataInputStream *reply,
+                                    guint32 len,
+                                    GVfsJob *job,
+                                    gpointer user_data)
+{
+  GVfsJobOpenForWrite *op_job = G_VFS_JOB_OPEN_FOR_WRITE (job);
+  ReplaceData *data = job->backend_data;
+  SftpHandle *handle = user_data;
+  GError *error = NULL;
+
+  if (reply_type == SSH_FXP_STATUS)
+    error_from_status (job, reply, -1, -1, &error);
+  else
+    g_vfs_job_failed (job, G_IO_ERROR, G_IO_ERROR_FAILED,
+                      _("Invalid reply received"));
+
+  if (error != NULL)
+    {
+      if (data->set_ownership &&
+          error->code == G_IO_ERROR_PERMISSION_DENIED)
+        {
+          g_error_free (error);
+
+          close_temp_file (backend, handle, job);
+          delete_temp_file (backend, handle, job);
+          sftp_handle_free (handle);
+
+          /* This was probably due to the fact that the ownership could not be
+             set properly. In this case we change our strategy altogether and
+             simply open/truncate the original file. This is not as secure
+             as the atomit tempfile/move approach, but at least ownership
+             doesn't change */
+          if (!op_job->make_backup)
+            replace_truncate_original (backend, job);
+          else
+            {
+              /* We only do this if make_backup is FALSE, as this version breaks
+                 the backup code. Would like to handle the backup case too by
+                 backing up before truncating, but for now error out... */
+              g_vfs_job_failed (job, G_IO_ERROR,
+                                G_IO_ERROR_CANT_CREATE_BACKUP,
+                                _("Backups not supported"));
+            }
+        }
+      else
+        {
+          g_vfs_job_failed_from_error (job, error);
+          g_error_free (error);
+        }
+      return;
+    }
+
+  g_vfs_job_open_for_write_set_handle (op_job, handle);
+  g_vfs_job_open_for_write_set_can_seek (op_job, TRUE);
+  g_vfs_job_open_for_write_set_can_truncate (op_job, TRUE);
+  g_vfs_job_succeeded (job);
+}
+
+static void
+replace_create_temp_fstat_reply (GVfsBackendSftp *backend,
+                                 int reply_type,
+                                 GDataInputStream *reply,
+                                 guint32 len,
+                                 GVfsJob *job,
+                                 gpointer user_data)
+{
+  GVfsJobOpenForWrite *op_job = G_VFS_JOB_OPEN_FOR_WRITE (job);
+  ReplaceData *data = job->backend_data;
+  SftpHandle *handle = user_data;
+  GFileInfo *info;
+  guint32 uid = -1;
+  guint32 gid = -1;
+
+  if (reply_type == SSH_FXP_STATUS ||
+      reply_type != SSH_FXP_ATTRS)
+    {
+      close_temp_file (backend, handle, job);
+      delete_temp_file (backend, handle, job);
+      sftp_handle_free (handle);
+
+      if (reply_type == SSH_FXP_STATUS)
+        result_from_status (job, reply, -1, -1);
+      else
+        g_vfs_job_failed (job, G_IO_ERROR, G_IO_ERROR_FAILED,
+                          _("Invalid reply received"));
+      return;
+    }
+
+  info = g_file_info_new ();
+  parse_attributes (backend, info, NULL, reply, NULL);
+
+  if (g_file_info_has_attribute (info, G_FILE_ATTRIBUTE_UNIX_UID) &&
+      g_file_info_has_attribute (info, G_FILE_ATTRIBUTE_UNIX_GID))
+    {
+      uid = g_file_info_get_attribute_uint32 (info, G_FILE_ATTRIBUTE_UNIX_UID);
+      gid = g_file_info_get_attribute_uint32 (info, G_FILE_ATTRIBUTE_UNIX_GID);
+    }
+
+  g_object_unref (info);
+
+  if (uid != data->uid || gid != data->gid)
+    {
+      GDataOutputStream *command;
+
+      command = new_command_stream (backend, SSH_FXP_FSETSTAT);
+      put_data_buffer (command, handle->raw_handle);
+      g_data_output_stream_put_uint32 (command, SSH_FILEXFER_ATTR_UIDGID, NULL, NULL);
+      g_data_output_stream_put_uint32 (command, data->uid, NULL, NULL);
+      g_data_output_stream_put_uint32 (command, data->gid, NULL, NULL);
+      queue_command_stream_and_free (&backend->command_connection, command,
+                                     replace_create_temp_fsetstat_reply, job, handle);
+      return;
+    }
+
+  g_vfs_job_open_for_write_set_handle (op_job, handle);
+  g_vfs_job_open_for_write_set_can_seek (op_job, TRUE);
+  g_vfs_job_open_for_write_set_can_truncate (op_job, TRUE);
+  g_vfs_job_succeeded (job);
+}
+
 static void
 replace_create_temp_reply (GVfsBackendSftp *backend,
                            int reply_type,
@@ -3632,28 +3770,6 @@ replace_create_temp_reply (GVfsBackendSftp *backend,
 
           replace_create_temp (backend, op_job);
         }
-      else if (data->set_ownership &&
-              error->code == G_IO_ERROR_PERMISSION_DENIED)
-        {
-          g_error_free (error);
-         
-          /* This was probably due to the fact that the ownership could not be
-             set properly. In this case we change our strategy altogether and
-             simply open/truncate the original file. This is not as secure
-             as the atomit tempfile/move approach, but at least ownership 
-             doesn't change */
-         if (!op_job->make_backup)
-           replace_truncate_original (backend, job);
-         else
-           {
-             /* We only do this if make_backup is FALSE, as this version breaks
-                the backup code. Would like to handle the backup case too by
-                backing up before truncating, but for now error out... */
-             g_vfs_job_failed (job, G_IO_ERROR,
-                               G_IO_ERROR_CANT_CREATE_BACKUP,
-                               _("Backups not supported"));
-           }
-        }
       else
         {
           g_vfs_job_failed_from_error (job, error);
@@ -3675,7 +3791,18 @@ replace_create_temp_reply (GVfsBackendSftp *backend,
   handle->permissions = data->permissions;
   handle->set_permissions = data->set_permissions;
   handle->make_backup = op_job->make_backup;
-  
+
+  if (data->set_ownership)
+    {
+      GDataOutputStream *command;
+
+      command = new_command_stream (backend, SSH_FXP_FSTAT);
+      put_data_buffer (command, handle->raw_handle);
+      queue_command_stream_and_free (&backend->command_connection, command,
+                                     replace_create_temp_fstat_reply, job, handle);
+      return;
+    }
+
   g_vfs_job_open_for_write_set_handle (op_job, handle);
   g_vfs_job_open_for_write_set_can_seek (op_job, TRUE);
   g_vfs_job_open_for_write_set_can_truncate (op_job, TRUE);
@@ -3716,15 +3843,8 @@ replace_create_temp (GVfsBackendSftp *backend,
                                 SSH_FXP_OPEN);
   put_string (command, data->tempname);
   g_data_output_stream_put_uint32 (command, SSH_FXF_WRITE|SSH_FXF_CREAT|SSH_FXF_EXCL,  NULL, NULL); /* open 
flags */
-  g_data_output_stream_put_uint32 (command, (data->set_permissions ? SSH_FILEXFER_ATTR_PERMISSIONS : 0) |
-                                            (data->set_ownership ? SSH_FILEXFER_ATTR_UIDGID : 0), NULL, 
NULL); /* Attr flags */
-  
-  if (data->set_ownership)
-  {
-    g_data_output_stream_put_uint32 (command, data->uid, NULL, NULL);
-    g_data_output_stream_put_uint32 (command, data->gid, NULL, NULL);
-  }
-  
+  g_data_output_stream_put_uint32 (command, (data->set_permissions ? SSH_FILEXFER_ATTR_PERMISSIONS : 0), 
NULL, NULL);
+
   if (data->set_permissions)
     g_data_output_stream_put_uint32 (command, data->permissions, NULL, NULL);
   queue_command_stream_and_free (&op_backend->command_connection, command,


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]