[Python-Dev] Proposal for a new function "open_noinherit" to avoid problems with subprocesses and security risks

[Henning von Bargen]

> Martin v. Löwis wrote:
> Exactly. My proposal is still to provide an API to toggle the
> flag after the handle was created.

OK, here is an API that I tested on Windows and for sockets only.
Perhaps someone can test it on Non-Windows (Linux, for example?)

I think the best place for it would be as a new method "set_noinherit"
for file and socket objects or as a new function in the os module
(thus the implementation should probably be rewritten at the C level).

Note that for file objects, the code would need an additional call to
win32file._get_osfhandle, because .fileno() returns the Windows handle
only for sockets, but not for files.

The code below uses thes Mark Hammond's win32all library.

import os

if os.name == "nt":

    import win32api, win32con
    def set_noinherit(socket, noinherit=True):
        """
        Mark a socket as non-inheritable to child processes.

        This should be called right after socket creation if you want
        to prevent the socket from being inherited to child processes.

        Notes:

        Unintentional socket or file inheritance is a security risk and
        can cause errors like "permission denied", "adress already in use" 
etc.
        in programs that start subprocesses, particularly in multi-threaded
        programs. These errors tend to occur seemingly randomly and are hard
        to reproduce (race condition!) and even harder to debug.

        Thus it is good practice to call this function as soon as possible
        after opening a file or socket that you doesn't need to be inherited
        to subprocesses.
        Note that in a multi-threaded environment, it is still possible that
        another thread starts a subprocess after you created a file/socket,
        but before you call set_noinherit.

        Note that for sockets, the new socket returned from accept() will be
        inheritable even if the listener socket was not; so you should call
        set_noinherit for the new socket as well.

        Availability: Posix, Windows
        """

        flags = 0
        if noinherit:
            flags = flags | win32con.HANDLE_FLAG_INHERIT
        win32api.SetHandleInformation(socket.fileno(), 
win32con.HANDLE_FLAG_INHERIT, flags)

else:

    import fcntl
    def set_noinherit(socket, noinherit=True):
        """
        ... documentation copied from the nt case ...
        """

        fd = socket.fileno()
        flags = fcntl.fcntl(fd, fcntl.F_GETFD) & ~fncl.FD_CLOEXEC
        if noinherit:
            flags = flags | fcntl.FD_CLOEXEC)
        fcntl.fcntl(fd, fcntl.F_SETFD, flags)


>
>> Martin, you mentioned that for sockets, inheritance is not a problem
>> unless accept(), recv() or select() is called in the child process
>> (as far as I understood it).
>
> I did not say "no problems". I said "there is no ambiguity whereto
> direct the data if the child processes don't perform accept/recv".
>
>> * http://mail.python.org/pipermail/python-list/2003-November/236043.html
>>    "socket's strange behavior with subprocesses"
>>    Funny: Even notepad.exe is used there as an example child process...
>
> Sure: the system will not shutdown the connection as long as the handle
> is still open in the subprocess (as the subprocess *might* send more
> data - which it won't).
>
> I think the problem could be avoided by the parent process explicitly
> performing shutdown(2), but I'm uncertain as I have never actively used
> shutdown().
>
>> * http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4202949
>>   Java has switched to non-inheritable sockets as well.
>
> Not surprisingly - they don't support fork(). If they would,
> they could not have made that change. The bug report is the
> same issue: clients will be able to connect as long as the
> listen backlog fills. Then they will be turned down, as notepad
> will never perform accept.

I think I more or less understand what happens.

The fact remains, that a subprocess takes influence on the behaviour
of server and client programs in an unwanted (and for many people:
unexpected) manner.

Regarding the influence of socket inheritance for server apps,
I performed some tests on Windows. If a subprocess is started
after the parent called accept(), then a client can still
connect() and send() to the socket - even if the parent process
has closed it and exited meanwhile.
This works until the queue is full (whose size was specified in
listen()). THEN the client will get (10061, 'Connection refused');
as you already explained.

And client and server will have the socket in CLOSE_WAIT resp.
FIN_WAIT2 status. However I doubt that this is actually a problem
as long as the server continues accept()ing.
But it means the client and server OS have to manage the socket
until the subprocess exits - even though neither client nor server
need the socket anymore.

One might argue that it is not actually a big problem, since the
subprocess will exit sooner or later.
This is more or less true for Linux, etc.

However, sometimes a subprocess might crash or hang.
Now what happens if the server program is closed and then started
again? On Linux, no problem (more or less). When the server program
is closed, the subprocess will be killed by the OS (I think), and
the socket is released (perhaps with a few minutes delay).

On Windows the situation is worse.

Subprocess hanging:
When the server program is closed, the subprocess will NOT be killed
by the OS ("officially" there isn't a parent-child relationship for
processes). It will continue hanging.
When the server program is restarted, it will run into an
"address alread in use" error.

Subprocess crashing:
Unfortunately, on a default Windows installation, this will be very
much the same like a hanging subprocess:
Dr. Watson or whichever debugger comes to debug the crashed program.
As long as nobody clicks on the messagebox popup, the crashed program
will not be freed and all handles will be kept open.
Of course, on a server computer, usually there is nobody watching the
desktop...
Note: It is possible to work around this by installing a different
debugger (which usually includes hacking the registry).

These problems can be avoided by calling set_noinherit for the lisener
socket as well as for the new socket returned by accept() ASAP.

>
>> * 
>> http://mail.python.org/pipermail/python-bugs-list/2006-April/032974.html
>>    python-Bugs-1469163 SimpleXMLRPCServer doesn't work anymore on Windows
>>    (see also Bug 1222790).
>
> I don't understand how this is relevant. This is about CLO_EXEC not
> being available on Windows, and has nothing to do with socket
> inheritance.

The original bug was about problems due to unwanted handle inheritance
with SimpleXMLRPCServer. The bug fix was to set CLO_EXEC. The fix
didn't work for Windows of course. A correct bug fix would to use
the "set_noinherit" function above.

>
> [I'm getting bored trying to explain the other cases as well]
>

OK, YOU do understand the issue and know what's going on under the hood.

I understand as well - at least now. It cost be several weeks of
frustrating debugging, changing code to avoid the built-in "open",
reading library source code, searching the internet and cursing...
(and I'm not a beginner, as well as others who mentioned they had
similar problems in this list).

Note that the solution REQUIRED avoiding and/or hacking the standard
library. As mentioned in previous posts, in a multi-threaded program,
the correct solution for files is to use the ftools.open on Windows
and a correct solution is not possible for sockets and on Non-Windows
due to possible race-conditions.

Using set_noinherit will reduce the risk as best as possible.

For Python 2.6 I propose to add the set_noinherit method to file
and socket objects.

For Python 3000 I propose that by default files and sockets should
be created non-inheritable (though it will not work perfectly for
mult-threaded programs on Non-Windows - see the doc in the code).

A server that needs handles to be inherited can then still call
set_noinherit(False).
Typical uses would include SocketServer.ForkingMixIn and the
3 standard handles for subprocess/os popen.

If this seems reasonable and I can help in implementing this,
please let me know.

The change would prevent other developers from making the same
frustrating experiences as I did.

Regards,
Henning