Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't implement chown or similar ops #5

Open
tvierling opened this issue Apr 26, 2012 · 9 comments
Open

Don't implement chown or similar ops #5

tvierling opened this issue Apr 26, 2012 · 9 comments

Comments

@tvierling
Copy link

(It's unfortunate that github doesn't provide a generic messaging system. This is kinda a response to comments currently in the code.)

It would violate least surprise to make operations that don't have a direct analogue, such as chown, to return an error of EPERM. This is what real filesystems do when they cannot implement an operation properly.

I started to include "don't implement chmod" here, but there's an argument to be made that chmod could be partly supported -- group access corresponding to an Apps domain, for instance, and other access to the world. But that still could be confusing as it doesn't exactly map to POSIX semantics, and AFAIK, it's inherited on folders.

Finally, link(). Docs supported the ability for a file to show up in more than one folder (label in Docs's case). I don't know if this ability is preserved in the Drive API, but it's been removed from the web interface.

(Feel free to use this issue as a discussion point for similar concerns.)

@jcline
Copy link
Owner

jcline commented Apr 26, 2012

It would violate least surprise to make operations that don't have a direct analogue, such as chown, to return an error of EPERM. This is what real filesystems do when they cannot implement an operation properly.

I tend to agree.

I started to include "don't implement chmod" here, but there's an argument to be made that chmod could be partly supported -- group access corresponding to an Apps domain, for instance, and other access to the world. But that still could be confusing as it doesn't exactly map to POSIX semantics, and AFAIK, it's inherited on folders.

I was thinking that it may make sense to, when mounted with -o allow others, have some level of file permissions to restrict local access, since -o uid and -o gid are for the entire mount. I don't think my in-source comment was entirely clear about this.

Finally, link(). Docs supported the ability for a file to show up in more than one folder (label in Docs's case). I don't know if this ability is preserved in the Drive API, but it's been removed from the web interface.

I actually haven't read too much of the API yet, since I'm working toward getting basic authentication done at the moment.

I would love to get more input on stuff like this.

@Marlinc
Copy link

Marlinc commented Apr 27, 2012

How about documents that you can read but not write to? For example a file that a friend shared read-only. It could be useful if you used permissions for that so that applications know they can't write to a specific file but can read.

@tvierling
Copy link
Author

@Marlinc , good point - this is something that can be reflected in the stat call, which is about reading permissions rather than writing them. (My notes about chown, chmod, etc. are about changing permissions, which is a lot more complicated.)

Indeed, files and documents that are not writable by the logged in user should have all modes 4 or 5 rather than 6 or 7.

Which of course brings the question to whether files should be marked executable. The way vfat/ntfs filesystems handle this is to make all files executable, which would be an OK place to start.

@Marlinc
Copy link

Marlinc commented Apr 27, 2012

I think all files should be executable because there is no way in Google Drive itself to give such rights. It would be pretty disappointing if it didn't allow executing files.

@emmaly
Copy link

emmaly commented May 9, 2012

Files should not be shown as executable because it's a security hole to have arbitrary executable files when they aren't expected to be executable.

Is there a way to store meta info in the file to flag a file as being executable? I know that in S3 you can store meta info by storing it via the HTTP PUT headers.

@jcline
Copy link
Owner

jcline commented May 9, 2012

Is there a way to store meta info in the file to flag a file as being executable? I know that in S3 you can store meta info by storing it via the HTTP PUT headers.

From what I have read so far, there is no metadata field like that.

Files should not be shown as executable because it's a security hole to have arbitrary executable files when they aren't expected to be executable.

I suppose they could default to -x, but chmod could let you, locally, set a file to +x.

@tvierling
Copy link
Author

Executability by a permission flag, on a filesystem that doesn't have full ACLs to begin with, isn't inherently a security hole - it's just a flag as to whether a binary can be run directly. Scripts, for instance, can still be executed if one has read permission simply by running the appropriate interpreter and passing it the script. There is some utility in having direct binary files executable, but probably not for a first pass, as this would be mostly useful with a significant caching layer added somewhere in the middle.

In general though, I really dislike the idea of allowing local, unsaved modifications to metadata. If an operation cannot be saved to the backing store, it should either be disallowed (most fs's use EPERM for this), or silently ignored (say, if chmod allows changing some bits that actually have effect, but others have no effect).

@emmaly
Copy link

emmaly commented May 9, 2012

If you shove something that looks like a script into a text file and the user didn't expect it to be executable, that is a problem. The reason that's a problem is that if that file is accessible on your Linux computer and you double-click that file, expecting it to open in a text editor, it may very well execute instead. Yes, you can shoot yourself in the foot by doing something like "bash $filename" but that's the user's fault. But letting files be directly executable is a danger to the user, especially if some external system (like some external vendor's app) has access to the user's Google Drive storage, then they can start inserting (intentionally or not) code that would be executable.

Do this:

1: Create a text file called ~/test.xyz
2: chmod 755 ~/test.xyz
3: Edit the ~/test.xyz file to contain "#!/usr/bin/sensible-browser" (change the path to some existing graphical program for testing if that one doesn't exist on your system)
4: Go find this file in your graphical file browser
5: Double-click the file.

What happened? Did it send it to a text editor or did it run the script? If it ran the script, it should have opened your default browser and shoved the file into the window. Of course that's not useful, but if you changed it to a real script, it would have executed instead.

If it didn't run anything and that path does actually exist, then you got lucky or your file browser doesn't trust executable files. In Gnome it probably asked you if you want to open it or run it. In Xfce it just runs it straight away.

This is not safe. It's not safe because if someone that doesn't like you wants to run an arbitrary script, they just have to figure out how to get a file into your Google Drive. It can be a group-collaborative system that acts like Dropbox or Google Documents. It can be a security hole in some GD-compatible app that has access to your GD account. It could be any number of things. But setting the flag to executable by default permits this. If it is not set that way by default, then this is a non-issue.

My opinion is that the GD mount should have the noexec flag and that if you want something to execute that you should have to copy it outside of the account. Or permit paranoid folks like myself to set that flag via config file. Or permit knowledgeable folks to disable that flag via config file. But this should not permit executable-by-default.

jcline pushed a commit that referenced this issue May 10, 2012
@jcline
Copy link
Owner

jcline commented May 10, 2012

1ddeaed defaults things to -x for when file reading actually starts working in the hopefully near future, assuming I didn't screw up the change.

Edit: It appears as though this change worked correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants