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

Standard open() does not allow to specify file permissions. #73400

Closed
socketpair mannequin opened this issue Jan 9, 2017 · 18 comments
Closed

Standard open() does not allow to specify file permissions. #73400

socketpair mannequin opened this issue Jan 9, 2017 · 18 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@socketpair
Copy link
Mannequin

socketpair mannequin commented Jan 9, 2017

BPO 29214
Nosy @brettcannon, @vstinner, @giampaolo, @tiran, @socketpair, @serhiy-storchaka

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-01-11.07:55:12.715>
created_at = <Date 2017-01-09.14:17:01.056>
labels = ['3.7', 'type-feature', 'library']
title = 'Standard open() does not allow to specify file permissions.'
updated_at = <Date 2017-07-27.01:32:42.993>
user = 'https://github.com/socketpair'

bugs.python.org fields:

activity = <Date 2017-07-27.01:32:42.993>
actor = 'giampaolo.rodola'
assignee = 'none'
closed = True
closed_date = <Date 2017-01-11.07:55:12.715>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2017-01-09.14:17:01.056>
creator = 'socketpair'
dependencies = []
files = []
hgrepos = []
issue_num = 29214
keywords = []
message_count = 18.0
messages = ['285044', '285046', '285047', '285054', '285061', '285063', '285068', '285069', '285070', '285073', '285077', '285144', '285155', '285158', '285197', '285198', '285199', '285209']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'vstinner', 'giampaolo.rodola', 'christian.heimes', 'socketpair', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue29214'
versions = ['Python 3.7']

@socketpair
Copy link
Mannequin Author

socketpair mannequin commented Jan 9, 2017

  1. Syscall open() allows that.
  2. This is important to prevent race-conditions between creating a file with default permissions and calling fchmod().

@socketpair socketpair mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error 3.7 (EOL) end of life labels Jan 9, 2017
@vstinner
Copy link
Member

vstinner commented Jan 9, 2017

You can actually specify permission:

fd = os.open("document.txt", os.O_WRONLY | os.O_CREAT, 0o777)
fp = open(fd, "wb")
with fp:
   ...

You can also use the dir_fd parameter for even better security ;-)

Is your request for the standard open() function, rather than os.open()?

Note: io.FileIO(), the underlying object when calling the Python open(), calls the C open() function with 0o666 for the permission mode.

@tiran
Copy link
Member

tiran commented Jan 9, 2017

I would be nice to have a public API to set permissions of files for io.open(), too.

Note to your note: the actual permission is influenced by the process' global umask. If you are concerned about security, you should set the umask to 0o077 early.

@socketpair
Copy link
Mannequin Author

socketpair mannequin commented Jan 9, 2017

  1. Yes, I'm reporting problem about standard open(), not os.open().
  2. Yes, I know about umask. But unfortunatelly, umask affects whole process, including all threads, so it is not possible to force specific permissions without of race conditions with other threds.
  3. Calling C's open() with 0666 MUST be documented (with mentioning of umask, which also affects these bits). This is MUST because man 2 open says that there is no default for mode argument, and it must be specified when O_CREAT / O_TMPFILE is specified.

@serhiy-storchaka
Copy link
Member

io.open() is high-level function. It handles buffering, encoding, newline translating. It is even higher-level than C's fopen().

Syscall open() is low-level. Python os.open() is an interface to this low-level feature.

There is a connection between low and high level -- io.open() accepts a file descriptor returned by os.open(). You also can provide the opener argument.

I don't think io.open() needs the support of mode and dir_fd arguments and all possible O_* flags. They are low-level features.

@socketpair
Copy link
Mannequin Author

socketpair mannequin commented Jan 9, 2017

Permissions -- are very important thing. As I think, such high-level functions should not hide that important functionality from end-users.

Also, it is not difficult to make a patch (as I think).

@brettcannon
Copy link
Member

I agree with Serhiy that I don't think this is necessary for io.open() when os.open() supports this. But this really can't be discussed further until someone provides a clear design proposal on how to make this work with io.open().

@brettcannon brettcannon added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jan 9, 2017
@socketpair
Copy link
Mannequin Author

socketpair mannequin commented Jan 9, 2017

I expect this:

open(...., perms=0o644)

Why not?

@socketpair
Copy link
Mannequin Author

socketpair mannequin commented Jan 9, 2017

so, io.open() should just pass that value down to implementation. implementation should use this value instead of hard-coded 0o666.

@serhiy-storchaka
Copy link
Member

open(..., opener=partial(os.open, mode=0o644))

Not every combination of functions deserves a new parameter of a builtin.

@socketpair
Copy link
Mannequin Author

socketpair mannequin commented Jan 9, 2017

Such construction is not so easy. Especially for beginners. Not everyone even uses context managers to work with files. They will try to use os.chmod(). More clever will use os.fchmod(fileobj.fileno()). And in rare case someone asks about race condition between threads. So, not implementing that is error prone, and also forces users to make hidden bugs.

Why not to follow the principle of Least Surprise ?

In you way, we did not have to implement a lot of things, that we have now. Like tempfile, shutil and so on. They may be implemented using complex combination of simplier items. Answer: these things make Python easy, secure and (insert good words here by yourself).

Why are you afraid to add one very simple thing ?

Next, what if I want to use, say, 'r+b', in that case I should remember combination of O_XXXXX constants like O_TRUNC|O_CREAT|O_APPEND and so on.. It's great that Python allows such things! really great! but how many people want to write that in their code?

(Sorry for long text, but I really can not understand)

@brettcannon
Copy link
Member

The point Serhiy is trying to make is that not everyone needs or cares about setting specific file permissions. Python's built-in open() function has not supported this for 26 years and so there's obviously not that much of a need if this has not consistently come up as a shortcoming.

Two, you say to "just pass" something like "0o644" as the mode. OK, but what does that even mean? As a Linux user you may know thanks to chmod usage, but a Windows user or someone who doesn't use a Linux shell won't have any idea what that octal constant represents. So how do you propose to help people set the proper mode? If you say "use os.O_CREAT" then you just made open() have functionality depend on the os module which no other built-in directly does for their API.

In other words you might call this simple, but I call it an advanced feature that's supported by os.fdopen(os.open()) already. So when I say a "clear design" I mean addressing the fact that it's non-obvious for beginners on how to use and something not everyone needs.

@vstinner
Copy link
Member

I don't understand the problem with umask(). It's standard and affect all
code even C extension calling directly or indirectly open(). It is more
secure to use umask() than setting mode on a few Python open() calls no?

For example, Python creates .pyc files. You cannot (easily) modify the
open() call to set mode, whereas it is affected by umask().

If you call umask() at startup, it affects all threads, there is no race
condition like open()+fchmod().

@tiran
Copy link
Member

tiran commented Jan 10, 2017

Victor, you are correct. That was exactly my point.

The most secure way is to tighten security and set umask to 0o077. It's basically a white list or locked down approach. With umask 0o077 all subsequent files, directories and other resources will be created without any permission for group and others. This is even true for operations that create a Unix socket.

You have to change the permission of files to a more permissive mode explicitly. Any mistake is easy to spot (access denied) and not catastrophic.

By the way fchmod() isn't necessarily the optimal way to change permission by file descriptor. The behavior of fchmod() isn't well defined for socket files. On Linux fchmod() of a Unix socket file does not alter the permission bits of the socket device file.

@socketpair
Copy link
Mannequin Author

socketpair mannequin commented Jan 11, 2017

@Haypo, suppose, one thread wants file with permissions A, and other thread wants permissions B. If they will "set" them through umask(), race condition may occur.

@serhiy-storchaka
Copy link
Member

If you needs non-default permission for particular file, you can use one of available ways mentioned above.

I think this issue can be closed. The original issues are resolved:

  1. os.open() (corresponding to syscall open()) allows this.
  2. There are other ways to create Python file with specific permissions (pass fd or use opener).

@vstinner
Copy link
Member

@Haypo, suppose, one thread wants file with permissions A, and other thread wants permissions B. If they will "set" them through umask(), race condition may occur.

Ok, let's say thta you want file "x" with permission A=0o777 and file "y" with permission B=0b700.

You start with os.umask(0o077), as suggested by Christian.

>>> import os
>>> os.umask(0o077)
2
>>> x=open("x", "w")
>>> y=open("y", "w")
>>> os.system("ls -l x y")
-rw-------. 1 haypo haypo 0 11 janv. 08:48 x
-rw-------. 1 haypo haypo 0 11 janv. 08:48 y
0
>>> os.fchmod(x.fileno(), 0o777)
>>> os.system("ls -l x y")
-rwxrwxrwx. 1 haypo haypo 0 11 janv. 08:48 x
-rw-------. 1 haypo haypo 0 11 janv. 08:48 y
0

This is a time window where the file "x" has the permission -rw------- instead of the expected -rwxrwxrwx. Ok, it's expected. But it's not an easy since initial permissions are are more strict that expected permisions.

The race condition only occurs if you work the other way, start with os.umask(0777) (default umask) and then change permissions of the file "x" to stricter. In this specific case, you *already* have two ways to do that in Python 3 (no change needed):

  fp = open("x", opener=partial(os.open, mode=0o700))
or:
  fd = os.open("x", os.O_WRONLY | os.O_CREAT, 0o700)
  fp = open(fd, "wb")

--

About documenting the default mode: the problem is that I'm not sure that *all* functions in Python, especially when using third party libraries, use the same default mode. Morever, the mode on Windows has a very different meaning. Permission per group is handled by ACLs, not with the integer "mode":
https://msdn.microsoft.com/en-us/library/z0kc8e3z.aspx

On Windows, pmode of _open() only contains read and write permissions of the file, for everyone.

For all these reasons, I close the issue.

@tiran
Copy link
Member

tiran commented Jan 11, 2017

I like the partial opener trick. Can we have the examples in the public documentation of open(), too?

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants