After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 623161 - file ACL are not preserved when saving file in gedit
file ACL are not preserved when saving file in gedit
Status: RESOLVED DUPLICATE of bug 656052
Product: glib
Classification: Platform
Component: gio
unspecified
Other Windows
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2010-06-29 18:27 UTC by leniviy
Modified: 2017-10-05 12:56 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description leniviy 2010-06-29 18:27:46 UTC
+++ This bug was initially created as a clone of Bug #613586 +++

+++ This bug was initially created as a clone of Bug #484882 +++

Saving a file in gedit 2.30.1 for windows actually swaps files which drops NTFS ACLs.
Someone assumed this could be gio bug.

**************************
# stat -c "%a %i" test.txt
664 4785074604276252
# # edit test.txt in gedit
# stat -c "%a %i" test.txt
700 3659174697712230
#
**************************
Comment 1 Tor Lillqvist 2010-06-30 07:20:42 UTC
I guess this is mostly a WONTFIX. The maintainers are not going to add code to GLib to take ACLs into consideration. You just shouldn't use gedit to edit files that have special individually crafted ACLs. Sorry.

This "stat" tool you are running, is that the GNU one? Are you running it on Linux? On an actual NTFS volume, which has been mounted on Linux? Or over SMB (which of course brings in more layers of complexity)? Are you sure NTFS ACLs are translated correctly to POSIX/Linux ACLs? But anyway, I don't see any mention in the stat(1) man page that it would even handle POSIX ACLs, and you just use %a, which looks at the POSIX rwxrwxrwx bits, the st_mode of struct stat ("access rights", as stat(1) calls it).

It seems you are misusing the term "ACL" I hope you realize that *real* ACLs, which is how file protection on Windows and NTFS is done, are totally different from POSIX's single access right integer.

Any mapping from an arbitrary Windows ACL to a POSIX single "access right" integer is going to be just a guesstimate. If you want to display the true ACL of a file on a NTFS volume, you need to use a tool that actually understands ACLs. Are you aware how mind-bogglingly complex the ACL issue even is? (This complexity, of course, is the main reason why I say WONTFIX would be the way to resolve this.)

If you can come up with a clean patch, that might be accepted, though. (But then, there will of course be no guarantee that such code would be actively maintained by people who didn't write it and don't like it;)
Comment 2 leniviy 2010-06-30 08:14:43 UTC
(In reply to comment #1)
Hi. I have some clarifications.

> The maintainers are not going to add code to GLib to take ACLs into consideration
I'm not asking for that. Just don't delete the existing file when saving and the ACLs will be preserved by windows.
On Linux gio doesn't replace the file, on windows it does. 

> This "stat" tool you are running, is that the GNU one? Are you running it on
> Linux? 
I use cygwin stat on Windows XP.
Comment 3 Paolo Borelli 2010-06-30 09:07:45 UTC
Tor: on unix the file saving code has two paths if acl or permissions etc can be preserved it takes the save tmp and swap files strategy otherwise it truncates and rewrites the original file. I know nothing about ntsf acls, but I wonder if the same be done on Windows: if we detect that there are acl simply take the slower but reliable strategy.
Comment 4 Tor Lillqvist 2010-06-30 10:34:45 UTC
leniviy: Using Cygwin "stat" (either the command, or the stat() Cygwin "system call") to look at ACLs of files manipulated by non-Cygwin programs (like gedit) is pointless. Cygwin's emulated POSIX semantics, including the emulated protection bits, are just an emulation. They are meaningful only for files and directories that have been created and manipulated only with Cygwin code. Cygwin does some dirtyish non-canonical tricks with the ACL in order to be able to emulate POSIX semantics more closely.

Instead, if you want to see the actual ACL of a file on Windows, simply use the cacls or icacls commands, or some Windows-specific 3rd-party tool. The "Properties" in Windows Explorer can also be used, even if it gives you a somewhat simplified polished GUI view of the ACL.

Paolo: ok, thanks for the hints. Need to take a look at the actual code involved;) You say "if we detect that there are acl", but note that on Windows there is always an ACL. It's not like it would be an exceptional situation (as it is on POSIX, where use of POSIX ACLs is pretty rare, at least for "normal" Linux users).

(In addition, files on NTFS can have the legacy FAT-style "readonly" attribute, and files (and other objects on Windows) also have an owner and a group. Note that the stat() call in the Microsoft C library fabricates the protection bits in the st_mode field of the stat struct basically just looking at the readonly attribute. The ACL is not inspected at all, there is no attempt in the Microsoft C library to generate a "correct" st_mode that would actually reflect the actual permissions the process has on the file. Ditto for chmod() in the Microsoft C library, the only thing it does is set the readonly attribute or not. Cygwin is totally different of course, but Cygwin is really a Unix system that just happens to run on top of Windows.)
Comment 5 Paolo Borelli 2010-06-30 11:06:01 UTC
> Paolo: ok, thanks for the hints. Need to take a look at the actual code
> involved;) You say "if we detect that there are acl", but note that on Windows
> there is always an ACL. It's not like it would be an exceptional situation (as
> it is on POSIX, where use of POSIX ACLs is pretty rare, at least for "normal"
> Linux users).
> 


Actually I lied here, in the unix code (at least in the orginal gedit code on which the gio code was based) we do not check "if there are acl", but if the acl can be restored on the tmp file: if the tmp file has different acls then fallback to the truncate&rewrite path
Comment 6 Tor Lillqvist 2010-06-30 11:13:27 UTC
Paolo, when you say "acl", do you really mean it? I don't see any ACL APIs used in gio. Please, let's use the term ACL only when we really do mean ACL.
Comment 7 Paolo Borelli 2010-06-30 11:17:54 UTC
I mean the stuff handled by libattr which I thought were ACLs... that was in the original gedit code, but maybe it has been lost in gio :/
Comment 8 Tor Lillqvist 2010-06-30 11:36:08 UTC
OK. I see some xattr stuff being called in glocalfileinfo.c.

But the code we are talking about here, that has two alternative backup strategies, is the one in glocalfileoutputstream.c:handle_overwrite_open(), right? Introduced with the below comment:

  /* We use two backup strategies.
   * The first one (which is faster) consist in saving to a
   * tmp file then rename the original file to the backup and the
   * tmp file to the original name. This is fast but doesn't work
   * when the file is a link (hard or symbolic) or when we can't
   * write to the current dir or can't set the permissions on the
   * new file. 
   * The second strategy consist simply in copying the old file
   * to a backup file and rewrite the contents of the file.
   */

That code does not check any extended attributes. It just does:

	  /* Check that we really needed to change something */
	  if (fstat (tmpfd, &tmp_statbuf) != 0 ||
	      original_stat.st_uid != tmp_statbuf.st_uid ||
	      original_stat.st_gid != tmp_statbuf.st_gid ||
	      original_stat.st_mode != tmp_statbuf.st_mode)
	    {
	      close (tmpfd);
	      g_unlink (tmp_filename);
	      g_free (tmp_filename);
	      goto fallback_strategy;
	    }

so is this then actually broken on POSIX, too? If a file has some specifically set extended attribute (does a POSIX ACL fall into this category?), that will be lost when edited and saved by this code, won't it?
Comment 9 Paolo Borelli 2010-06-30 13:09:27 UTC
indeed the code seems gone... not sure if that was on purpose.

This is the old gedit code:

http://git.gnome.org/browse/gedit/tree/gedit/gedit-local-document-saver.c?h=gnome-2-28#n449
Comment 10 leniviy 2010-07-02 13:51:01 UTC
Maybe the fallback strategy should always be used on mingw? The file st_mode is just a stub on this system anyway.
Comment 11 Tor Lillqvist 2010-07-02 14:02:13 UTC
s/mingw/Windows/

Probably, yes. Nobody is using this code for huge files anyway, I assume... so any speed issue is mostly irrelevant.

I wonder if it should be used always on Unix, too... unless the extended attribute check code that used to be in gedit can be resurrected?

Of course, the fallback code path also has problems: if the file being saved has sensitive contents, and is protected by an ACL that is more restrictive than that which the backup file gets by default, then that sensitive information will be more visible in the backup file than it should be.
Comment 12 Philip Withnall 2017-10-05 12:56:36 UTC
Marking as a duplicate of bug #656052, since it’s the same problem, but there’s more discussion of a solution there.

*** This bug has been marked as a duplicate of bug 656052 ***