classification
Title: rewrite CGIHTTPRequestHandler to always use subprocess
Type: behavior Stage: patch review
Components: Versions: Python 3.4
process
Status: open Resolution:
Dependencies: 10496 Superseder:
Assigned To: Nosy List: neologix, orsenthil, pitrou, v+python
Priority: normal Keywords: needs review, patch

Created on 2013-01-12 14:14 by neologix, last changed 2013-08-02 06:56 by neologix.

Files
File name Uploaded Description Edit
cgi_subprocess.diff neologix, 2013-01-12 14:14 review
cgi_subprocess-1.diff neologix, 2013-02-24 09:47 review
Messages (3)
msg179797 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-01-12 14:14
On Unix, CGIHTTPRequestHandler.run_cgi() uses the following code to run a CGI script:
"""
            pid = os.fork()
            [...]            
            # Child
            try:
                try:
                    os.setuid(nobody)
                except OSError:
                    pass
                os.dup2(self.rfile.fileno(), 0)
                os.dup2(self.wfile.fileno(), 1)
                os.execve(scriptfile, args, env)
"""

It's basically reimplementing subprocess.Popen, with a potential securiy issue: open file descriptors are not closed before exec, which means that the CGI script - which is run as 'nobody' on Unix to reduce its priviledges - can inherit open sockets or files (unless they're close-on-exec)...

The attached patch rewrites run_cgi() to use subprocess on all platorms.
I'm not at all familiar with CGI, so I don't guarantee it's correct, but  the regression test test_httpservers passes on Linux.

It leads to cleaner and safer code, so if someone with some httpsever/CGI background could review it, it would be great.
msg188359 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-05-04 12:44
The latest version of the patch passes on Linux, OpenIndiana and Windows.

Note that I did apply the select()-hack on all platforms (not only Windows), because if I understood #427345 correctly, it's really there to bypass a non-standard IE behavior (which appends trailing '\r\n'), and doesn't depend on the platform.
msg194158 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2013-08-02 06:56
Marking #10496 as a dependency (since it could prevent Python from running  with low level privileges).
History
Date User Action Args
2013-08-02 06:56:00neologixsetdependencies: + Python startup should not require passwd entry
messages: + msg194158
2013-05-04 12:44:17neologixsetmessages: + msg188359
2013-02-24 09:47:20neologixsetfiles: + cgi_subprocess-1.diff
2013-02-23 22:37:54neologixsetnosy: + pitrou
2013-01-19 11:58:50neologixsetnosy: + v+python
2013-01-13 14:47:07neologixsetnosy: + orsenthil
2013-01-12 14:14:43neologixcreate