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

[Henning von Bargen]

"""
My very personal opinion:
After a sleepness night, it seems to me that this is not a Python problem
(or any other programming language at all).
It looks more like an OS design problem (on MS Windows as well as
on Linux etc). In an ideal world, when a program asks the OS to start
a child process, it should have to explicitly list the handles that should
be inherited.
"""

> Martin v. Löwis wrote:

>> As you can see, at the C level, basically "fopen" is "open" with a
>> little code around it to parse flags etc. It's the same kind of hackish 
>> code.
>
> "little code" is quite an understatement. In Microsoft's C library
> (which we would have to emulate), the argument parsing of fopen is
> 120 lines of code. In addition, that code changes across compiler
> versions (where VS 2005 adds additional error checking).

Hmm. Wow!

>
>> Anyway, I don't want to argue about the implementation of a patch.
>> The fact is that until now the python programmer does not have an
>> easy, platform-independent option to open files non-inheritable.
>> As you mentioned yourself, the only way to work around it
>> in a platform-independent manner, IS VERY HACKISH.
>> So, shouldn't this hackish-ness better be hidden in the library
>> instead of leaving it as an execise to the common programmer?
>
> Putting it into the library is fine. However, we need to find
> an implementation strategy that meets the user's needs, and
> is still maintainable.
>
> Python 3 will offer a clean solution, deviating entirely from
> stdio.

Let me point out that stdio is not the problem.
The problem is handle inheritance.
So I don't see how this is going to be solve in Python 3 just by
not using stdio.
Inheritance has to be taken into account regardless of how it is
implemented on the C level.
And to open a file non-inheritable should be possible in an easy
and platform-independent way for the average python programmer.

> For 2.x, we need to find a better solution than the one you proposed.

Stephen, perhaps you can describe the workaround you used?
Maybe it is better than mine.
Or anyone else?

>
>> I think it is well worth the effort to keep this trouble away from
>> the Python programmers if possible.
>
> I don't argue about efforts - I argue about your proposed solution.
>
>> Apart from shutil.copyfile, other examples of using open that can cause
>> trouble are in socket.py (tell me any good reason why socket handles
>> should be inherited to child processes) and even in logging.py.
>
> On Unix, it is *very* common to inherit socket handles to child
> processes. The parent process opens the socket, and the child
> processes perform accept(3). This allows many processes to
> serve requests on the same port. In Python,
> SocketServer.Forking*Server rely on this precise capability.

Ahh, I see.
Maybe this is why my HTTP Server sometimes seems to not
react when a subprocess is running...
If more than one process has a handle for the same socket,
how does the OS know which process should react?

>
>>> Sure - and in turn, open is implemented on CreateFile.
>>> However, I don't think I would like to see an fopen
>>> implementation in Python. Python 3 will drop stdio entirely;
>>> for 2.x, I'd be cautious to change things because that
>>> may break other things in an unexpected manner.
>>
>> Yeah, if you think it should not be included in 2.x,
>> then the handle inheritance problem should at least be considered
>> in the PEPs [(3116, "New I/O"), (337, "Logging Usage in the Standard
>> Modules")]
>
> I didn't say that a solution shouldn't be included in 2.x.
> I said *your* solution shouldn't be. In 3.x, your solution
> won't apply, sine Python won't be using stdio (so
> fdopen becomes irrelevant).

See above - please take it into account for Python 3 then.

>
>>>> Each handle has a security attribute that specifies whether the
>>>> handle should be inherited or not - but this has to be specified
>>>> when creating the handle (in the Windows CreateFile API internally).
>>>
>>> Not necessarily. You can turn on the flag later, through
>>> SetHandleInformation.
>>
>> So do you think that a working "close_fds" could be implemented
>> for Windows as well?
>
> No. close_fds should have the semantics of only closing the handles
> for that subprocess. SetHandleInformation applies to the parent
> process, and *all* subprocesses. So this is different from close_fds.

Yes - that's why I doubt that could work.
And according to http://support.microsoft.com/kb/190351/en-us
in order to capture stdout and stderr of the child process,
one has to specify bInheritHandle=TRUE in CreateProcess,
with the net effect that you can only choose if
either ALL handles (if not explicitly specified otherwise during
handle creation) should be inherited or none of them.

>
>> Explicitly turning off the inheritance flag for all child handles except
>> stdin, stdout and stderr in subprocess / popen (the equivalent to
>> what close_fds does for Posix) - that's what I call hackish.
>
> I didn't propose that, and it wouldn't be the equivalent. In POSIX,
> the closing occurs in the child process. This is not possible on
> Windows, as there is no fork().

OK - I agree it is not possible. But "avoiding handle inheritance"
is what one wants to achieve when specifying close_fds, I think.

>> And I doubt that it is possible at all, for two reasons:
>> - you have to KNOW all the handles.
>> - due to the different process creation in Windows (there's no fork),
>>  you had to set the inheritance flags afterwards
>> - all this is not thread-safe.
>
> All true, and I did not suggest to integrate SetHandleInformation
> into subprocess. I *ONLY* claimed that you can change the flag
> after the file was opened.
>
> With that API, it would be possible to provide cross-platform
> access to the close-on-exec flag. Applications interested in setting
> it could then set it right after opening the file.

YES - that's exactly why I proposed an open_noinherit function.
It is a simple solution for a common problem - such a function,
documented in the library, would tell developers that they have to
be aware of the problem and that a solution exists (though the
implementation is more or less hackish due to platform-specific
code).

>
>> Apart from the fact that this is not possible on MS Windows, it won't
>> solve the problem!
>> (Because then I couldn't use all those standard modules that use open
>> *without* FD_CLOEXEC).
>>
>> The fact is that the combination ("multi-threading", "subprocess
>> creation", "standard modules")
>> simply *does not work* flawlessly and produces errors that are hard to
>> understand.
>> And probably most progammers are not even aware of the problem.
>> That's the main reason why I posted here.
>
> I don't see how your proposed change solves that. If there was
> an "n" flag, then the modules in the standard library that open
> files still won't use it.

That's why I said the standard library should be reviewed
for unintentionally handle inheritance by the use of open.
Note this is a security risk as well, see
http://msdn.microsoft.com/msdnmag/issues/0300/security/

Regards,
Henning