Set close-on-exec flag by default in SocketServer
Hi, The SocketServer class creates a socket to listen on clients, and a new socket per client (only for stream server like TCPServer, not for UDPServer). Until recently (2011-05-24, issue #5715), the listening socket was not closed after fork for the ForkingMixIn flavor. This caused two issues: it's a security leak, and it causes "address already in use" error if the server is restarted (see the first message of #12107 for an example with Django). Our implementation of the XMLRPC server uses fcntl() + FD_CLOEXEC to fix this issue since long time ago (2005-12-04, #1222790). IMO it would be safer to always enable close-on-exec flag on all sockets (listening+clients) *by default*. According to Charles-François Natali, it will break applications. Ruby made a similar brave choice one year ago (2011-10-22), not only on sockets but on all file descriptors: http://bugs.ruby-lang.org/issues/5041 Developers of Ruby servers relying on inheriting sockets in child processes have to explicitly disable close-on-exec flag in their server. Unicorn has been fixed for example. My question is: would you accept to break backward compatibility (in Python 3.4) to fix a potential security vulnerability? If not, an alternative is to add an option, disabled by default, to enable (or disable) explicitly close-on-exec in Python 3.4, and wait for 3.5 to enable the option by default. So applications might disable the flag explicitly in Python 3.4. I wrote a patch attached to the issue #12107 which adds a flag to enable or disable close-on-exec, I chose to enable the flag by default: http://bugs.python.org/issue12107 -- I'm not sure that close-on-exec flag must be set on the listening socket *and* on the client sockets. What do you think? -- It would be nice to be able to set close-on-exec flag directly using socket.socket() constructor. For example, an optional cloexec keyword-only argument can be added to socket.socket(family, type, *, cloexec=False). Victor
On Wed, Jan 9, 2013 at 4:48 AM, Victor Stinner <victor.stinner@gmail.com> wrote:
My question is: would you accept to break backward compatibility (in Python 3.4) to fix a potential security vulnerability?
If not, an alternative is to add an option, disabled by default, to enable (or disable) explicitly close-on-exec in Python 3.4, and wait for 3.5 to enable the option by default. So applications might disable the flag explicitly in Python 3.4.
If the end goal is indeed going to close-on-exec ON by default, then I think having it 3.4 itself is a good idea. OFF for one release just gives the framework developers who use SocketServer some additional time. Usually, I have realized that framework devs try our release candidates and see if they see any potential changes to be done. If they realize this change in their testing, it would be good for both parties. So, my vote. +1 for making that in 3.4 Thank you, Senthil
My question is: would you accept to break backward compatibility (in Python 3.4) to fix a potential security vulnerability?
Although obvious, the security implications are not restricted to sockets (yes, it's a contrived example): """ # cat test_inherit.py import fcntl import os import pwd import sys f = open("/tmp/passwd", 'w+') #fcntl.fcntl(f.fileno(), fcntl.F_SETFD, fcntl.FD_CLOEXEC) if os.fork() == 0: os.setuid(pwd.getpwnam('nobody').pw_uid) os.execv(sys.executable, ['python', '-c', 'import os; os.write(3, "owned")']) else: os.waitpid(-1, 0) f.seek(0) print(f.read()) f.close() # python test_inherit.py owned """
I'm not sure that close-on-exec flag must be set on the listening socket *and* on the client sockets. What do you think?
In the listening socket is inherited, it can lead to EADDRINUSE, or the child process "hijacking" new connections (by accept()ing on the same socket). As for the client sockets, there's at least one reason to set them close-on-exec: if a second forked process inherits the first process' client socket, even when the first client closes its file descriptor (and exits), the socket won't be closed until the the second process exits too: so one long-running child process can delay other child processes connection shutdown for arbitrarily long.
2013/1/9 Charles-François Natali <cf.natali@gmail.com>:
My question is: would you accept to break backward compatibility (in Python 3.4) to fix a potential security vulnerability?
Although obvious, the security implications are not restricted to sockets (yes, it's a contrived example): ... f = open("/tmp/passwd", 'w+') if os.fork() == 0: ...
Do you mean that we should provide a way to enable close-on-exec flag by default on all file descriptors? Adding an "e" flag to open() mode is not compatible with this requirement: we would need the opposite flag to disable explicitly close-on-exec. A better API is maybe to add a "cloexec" argument to open(), socket.socket(), pipe(), etc. It's easier to choose the default value of this argument: * False: backward compatible * True: backward incompatible * value retrieved from a global option, ex: sys.getdefaultcloexec() So a simple call to "sys.setdefaultcloexec(True)" would make your program make secure and less error-prone, globally, without having to break backward compatibility. Example without setting cloexec globally: f = open("/tmp/passwd", "w+", cloexec=True) s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, cloexec=True) rfd, wfd = os.pipe(cloexec=True) -- "os.pipe(cloexec=True)" doesn't respect 1-to-1 "rule" of OS functions exposed in the Python module "os", because it it may use pipe2() with O_CLOEXEC internally, whereas os.pipe2() does exist... ... But other Python functions of the os module use a different C function depending on the option. Examples: - os.open() uses open(), or openat() if dir_fd is set. - os.utime() uses utimensat(), utime(), utimes(), futimens(), futimes(), futimesat(), ... Victor
2013/1/10 Victor Stinner <victor.stinner@gmail.com>:
A better API is maybe to add a "cloexec" argument to open(), ...
I realized that setting close-on-exec flag is a non trivial problem. There are many ways to set it depending on the function, on the OS, and on the OS version. There is also the difficult question of the "default value". I started to work on a PEP: https://bitbucket.org/haypo/misc/src/tip/python/pep_cloexec.rst Contact me if you would like to contribute. Victor
The SocketServer class creates a socket to listen on clients, and a new socket per client (only for stream server like TCPServer, not for UDPServer).
Until recently (2011-05-24, issue #5715), the listening socket was not closed after fork for the ForkingMixIn flavor. This caused two issues: it's a security leak, and it causes "address already in use" error if the server is restarted (see the first message of #12107 for an example with Django).
Note that the server socket is actually still not closed in the child process: one this gets fixed, setting FD_CLOEXEC will not be useful anymore (but it would be an extra security it it could be done atomically, especially against race conditions in multi-threaded applications). (Same thing for the client socket, which is actually already closed in the parent process). As for the backward compatibility issue, here's a thought: subprocess was changed in 3.2 to close all FDs > 2 in the child process by default. AFAICT, we didn't get a single report complaining about this behavior change. OTOH, we did get numerous bug reports due to FDs inherited by subprocesses before that change. (I know that Python >= 3.2 is less widespread than its predecessors, but still).
Hello, Le Wed, 9 Jan 2013 13:48:49 +0100, Victor Stinner <victor.stinner@gmail.com> a écrit :
Until recently (2011-05-24, issue #5715), the listening socket was not closed after fork for the ForkingMixIn flavor. This caused two issues: it's a security leak, and it causes "address already in use" error if the server is restarted (see the first message of #12107 for an example with Django).
[...]
I wrote a patch attached to the issue #12107 which adds a flag to enable or disable close-on-exec, I chose to enable the flag by default: http://bugs.python.org/issue12107
So, I read your e-mail again and I'm wondering if you're making a logic error, or if I'm misunderstanding something: 1. first you're talking about duplicate file or socket objects after *fork()* (which is an issue I agree is quite annoying) 2. the solution you're proposing doesn't close the file descriptors after fork() but after *exec()*. Basically the solution doesn't address the problem. Many fork() calls aren't followed by an exec() call (multiprocessing comes to mind). On the other hand, the one widespread user of exec() after fork() in the stdlib, namely subprocess, *already* closes file descriptors by default, so the exec() issue doesn't really exist anymore for us (or is at least quite exotic). Regards Antoine.
So, I read your e-mail again and I'm wondering if you're making a logic error, or if I'm misunderstanding something:
1. first you're talking about duplicate file or socket objects after *fork()* (which is an issue I agree is quite annoying)
2. the solution you're proposing doesn't close the file descriptors after fork() but after *exec()*.
Basically the solution doesn't address the problem. Many fork() calls aren't followed by an exec() call (multiprocessing comes to mind).
Yes. In this specific case, the proper solution is to close the server socket right after fork() in the child process. We can't do anything about file descriptors inherited upon fork() (and shouldn't do anything of course, except on an individual basis like this socket server example). On the other hand, setting file descriptors close-on-exec has the advantage of avoiding file descriptor inheritance to spawned (fork()+exec()) child processes, which, in 99% of cases, don't need them (apart from stdin/stdout/stderr). Not only can this cause subtle bugs (socket/file not being closed when the parent closes the file descriptor, deadlocks, there are several such examples in the bug tracker), but also a security issue, because contrarily to a fork()ed process which runs code controlled by the library/user, after exec() you might be running arbitrary code. Let's take the example of CGIHTTPServer: """ # Child try: try: os.setuid(nobody) except os.error: pass os.dup2(self.rfile.fileno(), 0) os.dup2(self.wfile.fileno(), 1) os.execve(scriptfile, args, env) """ The code tries to execute a CGI script as user "nobody" to minimize privilege, but if the current process has an sensitive file opened, the file descriptor will be leaked to the CGI script, which can do anything with it. In short, close-on-exec can solve a whole class of problems (but does not really apply to this specific case).
On the other hand, the one widespread user of exec() after fork() in the stdlib, namely subprocess, *already* closes file descriptors by default, so the exec() issue doesn't really exist anymore for us (or is at least quite exotic).
See the above example. There can be valid reasons to use fork()+exec() instead of subprocess. Disclaimer: I'm not saying we should be changing all FDs to close-on-exec by default like Ruby did, I'm just saying that there's a real problem.
Le Thu, 10 Jan 2013 11:35:29 +0100, Charles-François Natali <neologix@free.fr> a écrit :
So, I read your e-mail again and I'm wondering if you're making a logic error, or if I'm misunderstanding something:
1. first you're talking about duplicate file or socket objects after *fork()* (which is an issue I agree is quite annoying)
2. the solution you're proposing doesn't close the file descriptors after fork() but after *exec()*.
Basically the solution doesn't address the problem. Many fork() calls aren't followed by an exec() call (multiprocessing comes to mind).
Yes. In this specific case, the proper solution is to close the server socket right after fork() in the child process.
We can't do anything about file descriptors inherited upon fork() (and shouldn't do anything of course, except on an individual basis like this socket server example).
Having an official afterfork facility would still help. I currently rely on multiprocessing.util.register_after_fork(), even though it's an undocumented API (and, of course, it works only for those child processes launched by multiprocessing, not other fork() uses). Regards Antoine.
2. the solution you're proposing doesn't close the file descriptors after fork() but after *exec()*.
Exact. My PEP cannot solve all issues :-) I will update it to make it more explicit.
Having an official afterfork facility would still help.
Yes. IMO it is a different topic: you can also call exec() without calling fork() first ;-) Victor 2013/1/10 Antoine Pitrou <solipsis@pitrou.net>:
Le Thu, 10 Jan 2013 11:35:29 +0100, Charles-François Natali <neologix@free.fr> a écrit :
So, I read your e-mail again and I'm wondering if you're making a logic error, or if I'm misunderstanding something:
1. first you're talking about duplicate file or socket objects after *fork()* (which is an issue I agree is quite annoying)
2. the solution you're proposing doesn't close the file descriptors after fork() but after *exec()*.
Basically the solution doesn't address the problem. Many fork() calls aren't followed by an exec() call (multiprocessing comes to mind).
Yes. In this specific case, the proper solution is to close the server socket right after fork() in the child process.
We can't do anything about file descriptors inherited upon fork() (and shouldn't do anything of course, except on an individual basis like this socket server example).
Having an official afterfork facility would still help. I currently rely on multiprocessing.util.register_after_fork(), even though it's an undocumented API (and, of course, it works only for those child processes launched by multiprocessing, not other fork() uses).
Regards
Antoine.
_______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/victor.stinner%40gmail.com
Am 10.01.2013 11:48, schrieb Antoine Pitrou:
Having an official afterfork facility would still help. I currently rely on multiprocessing.util.register_after_fork(), even though it's an undocumented API (and, of course, it works only for those child processes launched by multiprocessing, not other fork() uses).
The time machine strikes again: http://bugs.python.org/issue16500
2013/1/10 Charles-François Natali <neologix@free.fr>:
Disclaimer: I'm not saying we should be changing all FDs to close-on-exec by default like Ruby did, I'm just saying that there's a real problem.
I changed my mind, the PEP does not propose to change the *default* behaviour (don't set close-on-exec by default). But the PEP proposes to *add a function* to change the default behaviour. Application developers taking care of security can set close-on-exec by default, but they will have maybe to fix bugs (add cloexec=False argument, call os.set_cloexec(fd, True)) because something may expect an inheried file descriptor. IMO it is a smoother transition than forcing all developers to fix their application which may make developers think that Python 3.4 is buggy, and slow down the transition from Python 3.3 to 3.4. For Ruby: the change will only be effective in the next major version of Ruby, which may be the expected "Ruby 2.0". Victor
Le Thu, 10 Jan 2013 12:59:02 +0100, Victor Stinner <victor.stinner@gmail.com> a écrit :
2013/1/10 Charles-François Natali <neologix@free.fr>:
Disclaimer: I'm not saying we should be changing all FDs to close-on-exec by default like Ruby did, I'm just saying that there's a real problem.
I changed my mind, the PEP does not propose to change the *default* behaviour (don't set close-on-exec by default).
But the PEP proposes to *add a function* to change the default behaviour. Application developers taking care of security can set close-on-exec by default, but they will have maybe to fix bugs (add cloexec=False argument, call os.set_cloexec(fd, True)) because something may expect an inheried file descriptor.
Do you have an example of what that "something" may be? Apart from standard streams, I can't think of any inherited file descriptor an external program would want to rely on. In other words, I think close-on-exec by default is probably a reasonable decision. Regards Antoine.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 01/10/2013 07:52 AM, Antoine Pitrou wrote:
Le Thu, 10 Jan 2013 12:59:02 +0100, Victor Stinner <victor.stinner@gmail.com> a écrit :
2013/1/10 Charles-François Natali <neologix@free.fr>:
Disclaimer: I'm not saying we should be changing all FDs to close-on-exec by default like Ruby did, I'm just saying that there's a real problem.
I changed my mind, the PEP does not propose to change the *default* behaviour (don't set close-on-exec by default).
But the PEP proposes to *add a function* to change the default behaviour. Application developers taking care of security can set close-on-exec by default, but they will have maybe to fix bugs (add cloexec=False argument, call os.set_cloexec(fd, True)) because something may expect an inheried file descriptor.
Do you have an example of what that "something" may be? Apart from standard streams, I can't think of any inherited file descriptor an external program would want to rely on.
In other words, I think close-on-exec by default is probably a reasonable decision.
Why would we wander away from POSIX semantics here? There are good reasons not to close open descriptors (the 'pipe()' syscall, for instance), and there is no POSIXy way to ask for them *not* to be closed. Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iEYEARECAAYFAlDu/qsACgkQ+gerLs4ltQ5zxACgu9+OcQx9iMgm9YosUhausoIo orwAoK5NNzGDkC0MKwR8oRDCYTz3zNl4 =TPKH -----END PGP SIGNATURE-----
On Thu, 10 Jan 2013 12:47:23 -0500 Tres Seaver <tseaver@palladion.com> wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 01/10/2013 07:52 AM, Antoine Pitrou wrote:
Le Thu, 10 Jan 2013 12:59:02 +0100, Victor Stinner <victor.stinner@gmail.com> a écrit :
2013/1/10 Charles-François Natali <neologix@free.fr>:
Disclaimer: I'm not saying we should be changing all FDs to close-on-exec by default like Ruby did, I'm just saying that there's a real problem.
I changed my mind, the PEP does not propose to change the *default* behaviour (don't set close-on-exec by default).
But the PEP proposes to *add a function* to change the default behaviour. Application developers taking care of security can set close-on-exec by default, but they will have maybe to fix bugs (add cloexec=False argument, call os.set_cloexec(fd, True)) because something may expect an inheried file descriptor.
Do you have an example of what that "something" may be? Apart from standard streams, I can't think of any inherited file descriptor an external program would want to rely on.
In other words, I think close-on-exec by default is probably a reasonable decision.
Why would we wander away from POSIX semantics here? There are good reasons not to close open descriptors (the 'pipe()' syscall, for instance), and there is no POSIXy way to ask for them *not* to be closed.
Because Python is not POSIX. (and POSIX did mistakes anyway) Regards Antoine.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 01/10/2013 01:30 PM, Antoine Pitrou wrote:
On Thu, 10 Jan 2013 12:47:23 -0500 Tres Seaver <tseaver@palladion.com> wrote:
Why would we wander away from POSIX semantics here? There are good reasons not to close open descriptors (the 'pipe()' syscall, for instance), and there is no POSIXy way to ask for them *not* to be closed.
Because Python is not POSIX. (and POSIX did mistakes anyway)
Anybody using systems programming using fork() should expect to be in POSIX land, and would be surprised not to have the semantics it implies. Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iEYEARECAAYFAlDvE4IACgkQ+gerLs4ltQ4gwwCZAbj6cPhTWahGam7bbr8KgcY4 IY8An1yH6YOsSvVoV38l4Ka2pcJUFzDA =gbOD -----END PGP SIGNATURE-----
2013/1/10 Tres Seaver <tseaver@palladion.com>:
Why would we wander away from POSIX semantics here? There are good reasons not to close open descriptors (the 'pipe()' syscall, for instance), and there is no POSIXy way to ask for them *not* to be closed.
There are different methods to disable the close-on-exec flag. See os.set_cloexec() function of my PEP draft: https://bitbucket.org/haypo/misc/src/tip/python/pep_cloexec.rst Victor
2013/1/10 Antoine Pitrou <solipsis@pitrou.net>:
I changed my mind, the PEP does not propose to change the *default* behaviour (don't set close-on-exec by default).
But the PEP proposes to *add a function* to change the default behaviour. Application developers taking care of security can set close-on-exec by default, but they will have maybe to fix bugs (add cloexec=False argument, call os.set_cloexec(fd, True)) because something may expect an inheried file descriptor.
Do you have an example of what that "something" may be? Apart from standard streams, I can't think of any inherited file descriptor an external program would want to rely on.
In other words, I think close-on-exec by default is probably a reasonable decision.
Network servers like inetd or apache MPM (prefork) uses a process listening on a socket, and then fork to execute a request in a child process. I don't know how it works exactly, but I guess that the child process need a socket from the parent to send the answer to the client. If the socket is closed on execute (ex: Apache with CGI), it does not work :-) Example with CGIHTTPRequestHandler.run_cgi(), self.connection is the socket coming from accept(): self.rfile = self.connection.makefile('rb', self.rbufsize) self.wfile = self.connection.makefile('wb', self.wbufsize) ... try: os.setuid(nobody) except OSError: pass os.dup2(self.rfile.fileno(), 0) os.dup2(self.wfile.fileno(), 1) os.execve(scriptfile, args, env) We are talking about *one* specific socket. So if close-on-exec flag is set on all file descriptors, it should be easy to only unset it on one specific file descriptor. For example, the fix for StreamRequestHandler.setup() would be simple, just add cloexec=True to dup2: os.dup2(self.rfile.fileno(), 0, cloexec=False) os.dup2(self.wfile.fileno(), 1, cloexec=False) Such servers is not the common case, but more the exception. So setting close-on-exec by default makes sense. I don't know if there are other use cases using fork()+exec() and *inheriting* one or more file descriptors. -- On Windows, the CGI server uses a classic PIPE to get the output of the subprocess and then copy data from the pipe into the socket. Victor
Network servers like inetd or apache MPM (prefork) uses a process listening on a socket, and then fork to execute a request in a child process. I don't know how it works exactly, but I guess that the child process need a socket from the parent to send the answer to the client. If the socket is closed on execute (ex: Apache with CGI), it does not work :-)
Yes, but the above (setting close-on-exec by default) would *not* apply to stdin, stdout and stderr. inetd servers use dup(socket, 0); dup(socket, 1, dup(socket, 2) before forking, so it would still work.
Example with CGIHTTPRequestHandler.run_cgi(), self.connection is the socket coming from accept():
self.rfile = self.connection.makefile('rb', self.rbufsize) self.wfile = self.connection.makefile('wb', self.wbufsize) ... try: os.setuid(nobody) except OSError: pass os.dup2(self.rfile.fileno(), 0) os.dup2(self.wfile.fileno(), 1) os.execve(scriptfile, args, env)
Same thing here. And the same thing holds for shell-type pipelines: you're always using stdin, stdout or stderr.
Do you have an example of what that "something" may be? Apart from standard streams, I can't think of any inherited file descriptor an external program would want to rely on.
Indeed, it should be really rare. There are far more programs that are bitten by FD inheritance upon exec than programs relying on it, and whereas failures and security issues in the first category are hard to debug and unpredictable (especially in a multi-threaded program), a program relying on a FD that would be closed will fail immediately with EBADF, and so could be updated quickly and easily.
In other words, I think close-on-exec by default is probably a reasonable decision.
close-on-exec should probably have been the default in Unix, and is a much saner option. The only question is whether we're willing to take the risk of breaking - admittedly a handful - of applications to avoid a whole class of difficult to debug and potential security issues. Note that if we do choose to set all file descriptors close-on-exec by default, there are several questions open: - This would hold for open(), Socket() and other high-level file-descriptor wrappers. Should it be enabled also for low-level syscall wrappers like os.open(), os.pipe(), etc? - On platforms that don't support atomic close-on-exec (e.g. open() with O_CLOEXEC, socket() with SOCK_CLOEXEC, pipe2(), etc), this would require extra fcntl()/ioctl() syscalls. The cost is probably negligible, but we'd have to check the impact on some benchmarks.
Charles-François Natali <cf.natali <at> gmail.com> writes:
Do you have an example of what that "something" may be? Apart from standard streams, I can't think of any inherited file descriptor an external program would want to rely on.
Indeed, it should be really rare.
GnuPG allows communicating with it through arbitrary file descriptors [1]. For example, if you use the command-line argument --passphrase-fd n then it will attempt to read the passphrase from file descriptor n. If n is 0, it reads from stdin, as one would expect, but you can use other values for the fd. There are several other such command line options. How would that sit with the current proposal? I maintain a wrapper, python-gnupg, which communicates with the GnuPG process through subprocess. Although there is no in-built use of these parameters, users are allowed to pass additional parameters to GnuPG, and they might use these esoteric GnuPG options. Regards, Vinay Sajip [1] http://www.gnupg.org/documentation/manuals/gnupg-devel/GPG-Esoteric-Options....
How would that sit with the current proposal? I maintain a wrapper, python-gnupg, which communicates with the GnuPG process through subprocess. Although there is no in-built use of these parameters, users are allowed to pass additional parameters to GnuPG, and they might use these esoteric GnuPG options.
Since you use subprocess, file descriptor passing to gnupg doesn't work since Subproces.Popen changed close_fds default to True (in 3.2) (which was the right decision, IMO).
Charles-François Natali <cf.natali <at> gmail.com> writes:
How would that sit with the current proposal? I maintain a wrapper, python-gnupg, which communicates with the GnuPG process through subprocess. Although there is no in-built use of these parameters, users are allowed to pass additional parameters to GnuPG, and they might use these esoteric GnuPG options.
Since you use subprocess, file descriptor passing to gnupg doesn't work since Subproces.Popen changed close_fds default to True (in 3.2) (which was the right decision, IMO).
That could always be overcome by passing close_fds=False explicitly to subprocess from my code, though, right? I'm not doing that now, but then I'm not using the esoteric options in python-gnupg code, either. My point was that the GnuPG usage looked like an example where fds other than 0, 1 and 2 might be used by design in not-uncommonly-used programs. From a discussion I had with Barry Warsaw a while ago, I seem to remember that there was other software which relied on these features. See [1] for details. Regards, Vinay Sajip [1] https://code.google.com/p/python-gnupg/issues/detail?id=46
That could always be overcome by passing close_fds=False explicitly to subprocess from my code, though, right? I'm not doing that now, but then I'm not using the esoteric options in python-gnupg code, either.
You could do that, or better explicitly support this option, and only specify this file descriptor in subprocess.Popen keep_fds argument.
My point was that the GnuPG usage looked like an example where fds other than 0, 1 and 2 might be used by design in not-uncommonly-used programs. From a discussion I had with Barry Warsaw a while ago, I seem to remember that there was other software which relied on these features. See [1] for details.
Yes, it might be used. But I maintain that it should be really rare, and even if it's not, since the "official" way to launch subprocesses is through the subprocess module, FDs > 2 are already closed by default since Python 3.2. And the failure will be immediate and obvious (EBADF). Note that I admit I may be completely wrong, that's why I suggested to Victor to bring this up on python-dev to gather as much feedback as possible. Something saying like "we never ever break backward compatibility intentionally, even in corner cases" or "this would break POSIX semantics" would be enough (but OTOH, the subprocess change did break those hypothetical rules). Another pet peeve of mine is the non-handling of EINTR by low-level syscall wrappers, which results in code like this spread all over the stdlib and user code: while True: try: return syscall(...) except OSError as e: if e.errno =!= errno.EINTR: raise (and if it's select()/poll()/etc, the code doesn't update the timeout in 90% of cases). It gets a little better since the Exception hierarchy rework (InterruptedException), but's still a nuisance.
On Fri, Jan 11, 2013 at 5:30 PM, Charles-François Natali < cf.natali@gmail.com> wrote:
That could always be overcome by passing close_fds=False explicitly to subprocess from my code, though, right? I'm not doing that now, but then I'm not using the esoteric options in python-gnupg code, either.
You could do that, or better explicitly support this option, and only specify this file descriptor in subprocess.Popen keep_fds argument.
My point was that the GnuPG usage looked like an example where fds other than 0, 1 and 2 might be used by design in not-uncommonly-used programs. From a discussion I had with Barry Warsaw a while ago, I seem to remember that there was other software which relied on these features. See [1] for details.
Yes, it might be used. But I maintain that it should be really rare, and even if it's not, since the "official" way to launch subprocesses is through the subprocess module, FDs > 2 are already closed by default since Python 3.2. And the failure will be immediate and obvious (EBADF).
I suppose I should chime in. I think the pattern of "open some file descriptors, fork, exec" and pass the file descriptors to a child process it a very useful thing, and certainly something I use in my code. Although to be honest, it is usually a C program doing it. I do this for two reason: 1) I can have a relatively small program that runs at a rather high privilege level (e.g: root), which acquires protected resources (e.g: binds to port 80, opens some key files that only a restricted user has access to), and then drops privileges, forks (or just execs) and passes those resources to a larger, less trusted program. 2) I have a monitoring program that opens a socket, and then passes the socket to a child process (via fork, exec). If that program crashes for some reason, the monitoring program can respawn a new child and pass it the exact same socket. Now I'm not saying these are necessarily common use cases, but I think changing the default behaviour of file-descriptors to be close on exec would violate principle of least surprise for anyone familiar with and expecting UNIX semantics. If you are using fork/exec directly then I think you want these semantics. That being said, I have no problem with subprocess closing file-descriptors before exec()-ing as I don't think a developer would necessarily expect the UNIX semantics on that interface. Python is not UNIX, but I think if you are directly using the POSIX interfaces they should work (more or less) the same way the would if you were writing a C program. (Some of us still use Python to prototype things that will later be converted to C!). Cheers, Benno
On 1/11/2013 2:25 AM, Ben Leslie wrote:
Python is not UNIX, but I think if you are directly using the POSIX interfaces they should work (more or less) the same way the would if you were writing a C program. (Some of us still use Python to prototype things that will later be converted to C!).
I believe it has been the intent that if xxx is s syscall, then os.xxx should be a thin wrapper with the same defaults. -- Terry Jan Reedy
On Jan 11, 2013, at 06:25 PM, Ben Leslie wrote:
Python is not UNIX, but I think if you are directly using the POSIX interfaces they should work (more or less) the same way the would if you were writing a C program. (Some of us still use Python to prototype things that will later be converted to C!).
+1 Or put it another way: Python isn't POSIX but it exposes POSIX APIs, they should act like POSIX APIs. It's perfectly fine to provide alternative APIs that have different behavior. I think it's exactly analogous to Python exposing Windows or OS X specific APIs. -Barry
2013/1/11 Ben Leslie <benno@benno.id.au>:
Python is not UNIX, but I think if you are directly using the POSIX interfaces they should work (more or less) the same way the would if you were writing a C program. (Some of us still use Python to prototype things that will later be converted to C!).
I completed the PEP to list advantages and drawbacks of the different alternatives. I moved the PEP to the PEP repository because I now consider it complete enough (for a first draft!): http://www.python.org/dev/peps/pep-0433/ I see 3 major alternatives: (a) add cloexec=False argument to some functions http://www.python.org/dev/peps/pep-0433/#proposition Drawbacks: http://www.python.org/dev/peps/pep-0433/#scope (b) always set close-on-exec flag and add cloexec=True argument to some functions (to be able to disable it) http://www.python.org/dev/peps/pep-0433/#always-set-close-on-exec-flag (c) add cloexec argument to some functions and a sys.setdefaultcloexec() function to change the default value of cloexec (will be False at startup) http://www.python.org/dev/peps/pep-0433/#add-a-function-to-set-close-on-exec... If you care about backward compatibility, (a) is the safest option. If you care about bugs and security, (b) is the best option. For example, (a) does not fix os.urandom(). (c) tries to be a compromise, but has its own drawbacks. Victor
On Thu, Jan 10, 2013 at 09:57:30PM +0000, Vinay Sajip wrote:
My point was that the GnuPG usage looked like an example where fds other than 0, 1 and 2 might be used by design in not-uncommonly-used programs. From a discussion I had with Barry Warsaw a while ago, I seem to remember that there was other software which relied on these features. See [1] for details.
From the "dpkg" man page:
--command-fd n Accept a series of commands on input file descriptor n. Note: additional options set on the command line, and through this file descriptor, are not reset for subsequent commands executed during the same run. -- Ross Lagerwall
2013/1/10 Charles-François Natali <cf.natali@gmail.com>:
Network servers like inetd or apache MPM (prefork) uses a process listening on a socket, and then fork to execute a request in a child process. I don't know how it works exactly, but I guess that the child process need a socket from the parent to send the answer to the client. If the socket is closed on execute (ex: Apache with CGI), it does not work :-)
Yes, but the above (setting close-on-exec by default) would *not* apply to stdin, stdout and stderr. inetd servers use dup(socket, 0); dup(socket, 1, dup(socket, 2) before forking, so it would still work.
Do you mean that file descriptors 0, 1 and 2 must be handled differently? A program can start without stdout (fd 1) nor stderr (fd 2), even if it's not the most common case :-) For example, should "os.dup2(fd, 1)" set the close-on-exec flag on the file descriptor 1?
There are far more programs that are bitten by FD inheritance upon exec than programs relying on it, and whereas failures and security issues in the first category are hard to debug and unpredictable (especially in a multi-threaded program), a program relying on a FD that would be closed will fail immediately with EBADF, and so could be updated quickly and easily.
In other words, I think close-on-exec by default is probably a reasonable decision.
close-on-exec should probably have been the default in Unix, and is a much saner option.
I will update the PEP to at least mention this option (enable close-on-exec by default).
Note that if we do choose to set all file descriptors close-on-exec by default, there are several questions open: - This would hold for open(), Socket() and other high-level file-descriptor wrappers. Should it be enabled also for low-level syscall wrappers like os.open(), os.pipe(), etc?
os.pipe() has no higher level API, and it would be convinient to be able to set the close-on-exec flag, at least for the subprocess module For os.open(), the question is more difficult. I proposed to add an cloexec keyword argument to os.open(). os.open(..., cloexec=False) would not do anything special, whereas os.open(..., cloexec=True) would use the best method to set the close-on-exec flag, including adding O_CLOEXEC or O_NOINHERIT to flags.
- On platforms that don't support atomic close-on-exec (e.g. open() with O_CLOEXEC, socket() with SOCK_CLOEXEC, pipe2(), etc), this would require extra fcntl()/ioctl() syscalls. The cost is probably negligible, but we'd have to check the impact on some benchmarks.
Sure. We are working since a long time trying to reduce the number of syscalls at startup. I proposed to use ioctl(fd, FIOCLEX) which only adds one syscall, instead of two for fcntl(fd, F_SETFD, new_flags) (old flags must be read to be future-proof, as adviced in the glibc documentation). On Windows, Linux 2.6.23+ and FreeBSD 8.3+, no additional syscall is needed: O_NOINHERIT and O_CLOEXEC are enough (Linux just needs one additional syscall to ensure that O_CLOEXEC is supported). So the worst case is when fcntl() is the only available option: 2 additional syscalls are required per creation of new file descriptors. If setting close-on-exec has a significant cost, the configurable default value (sys.setdefaultcloexec()) should be considered more seriously. -- While we are talking about enable close-on-exec by default and the number of syscalls: I would like to know how a new file descriptor can be created without close-on-exec flag. What would be the API? Most existing API are designed to *set* the flag, but how should define that the flag must be unset? An option is to add an new "cloexec" keyword argument which would be True by default, and so set the argument to False to not set the flag. Example: os.pipe(cloexec=False). I dislike this option because developers may write os.pipe(cloexec=True) which would be useless and may be confusing if the flag is True by default. Victor
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 01/10/2013 02:55 PM, Charles-François Natali wrote:
Indeed, it should be really rare.
*Lots* of applications make use of POSIX semantics for fork() / exec().
There are far more programs that are bitten by FD inheritance upon exec than programs relying on it, and whereas failures and security issues in the first category are hard to debug and unpredictable (especially in a multi-threaded program),
Can someone please point to a writeop of the security issues involved? Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iEYEARECAAYFAlDwMtsACgkQ+gerLs4ltQ7e7QCfdc1yxE+1iF6Gf/O9reOxStIG V8YAoKs79z5DHA3dyTDI+9yWkSSW0VD6 =9FVK -----END PGP SIGNATURE-----
*Lots* of applications make use of POSIX semantics for fork() / exec().
This doesn't mean much. We're talking about inheritance of FDs > 2 upon exec, which is a very limited subset of "POSIX semantics for fork() / exec()". I personally think that there's been enough feedback to show that we should stick with the default POSIX behavior, however broken it is...
Can someone please point to a writeop of the security issues involved?
I've posted sample codes earlier in this thread, but here's a writeup by Ulrich Drepper: http://udrepper.livejournal.com/20407.html
participants (12)
-
Antoine Pitrou -
Barry Warsaw -
Ben Leslie -
Charles-François Natali -
Charles-François Natali -
Christian Heimes -
Ross Lagerwall -
Senthil Kumaran -
Terry Reedy -
Tres Seaver -
Victor Stinner -
Vinay Sajip