[Python-Dev] PEP 578: Python Runtime Audit Hooks

[Christian Heimes]

Sorry, I forgot to reply.

Do you think it would make sense to split the PEP into two PEPs? The
auditing hook and import opener hook are related, but distinct
improvements. The auditing part looks solid and ready now. The import
opener may need some more refinement. I would also like to get feedback
from some Linux Kernel security engineers first.

On 01/04/2019 18.31, Steve Dower wrote:
> On 31Mar2019 0538, Christian Heimes wrote:
>> I don't like the fact that the PEP requires users to learn and use an
>> additional layer to handle native code. Although we cannot provide a
>> fully secure hook for native code, we could at least try to provide a
>> best effort hook and document the limitations. A bit more information
>> would make the verified open function more useful, too.
> 
> So instead they need to learn a significantly more complicated API? :)
> (I was very happy to be able to say "it's the same as open(p, 'rb')").
> 
>> PyObject *PyImport_OpenForExecution(
>>      const char *path,
>>      const char *intent,
>>      int flags,
>>      PyObject *context
>> )
>>
>> - Path is an absolute (!) file path. The PEP doesn't specify if the file
>> name is relative or absolute. IMO it should be always absolute.
> 
> Yeah, this is fair enough. I'll add it as a requirement.
> 
>> - The new intent argument lets the caller pass information how it
>> intents to use the file, e.g. pythoncode, zipimport, nativecode (for
>> loading a shared library/DLL), ctypes, ... This allows the verify hook
>> to react on the intent and provide different verifications for e.g.
>> Python code and native modules.
> 
> I had an intent argument at one point and the feedback I got (from teams
> who wanted to implement it) is that they wouldn't trust it anyway :)
> 
> In each case there should be associated audit events for tracking the
> intent (and interrupting at that point if it doesn't like the intended
> action), but for the simple case of "let me open this specific file" it
> doesn't really add much. And it almost certainly shouldn't impact
> decision making.

There is no need to trust the intent flag that much. I would like to
have a way to further narrow down the scope for an open call. This would
allow the caller to tell the hook "I want to open something that should
be a shared library suitable for ctypes". It would allow tighter control.

Audit events are useful and powerful. But I don't want to put too much
burden on the auditing framwork. I prefer to have checks that prevent
operations rather than allow operations and audit them.

>> - The flags argument is for additional flags, e.g. return an opened file
>> or None, open the file in text or binary mode, ...
> 
> This just makes it harder for the hook implementer - now you have to
> allow encoding/errors arguments and probably more. And as mentioned
> above, there should be an audit event showing the intent before this
> call, and a hook can reject it at that point (rather than verify without
> actually returning the verified content).

I retract this part of my proposal. With O_MAYEXEC it's better to always
open the file, but then use the file's FD to retrieve the actual file
name for dlopen(). That approach allows the Kernel to verify DAC
permissions, prevents memfd_create() hacks through readlink, and
simplifies the hook.

* Linux: readlink("/proc/self/fd/%i")
* macOS: fcntl F_GETPATH
* Windows: GetFileInformationByHandleEx

>> - Context is an optional Python object from the caller's context. For
>> the import system, it could be the loader instance.
> 
> I think the audit event covers this, unless you have some way of using
> this context in mind that I can't think of?

To be honest I don't have a good use case yet. I just like the idea to
have a way to pass some custom thing into an API and now who called an
API. You seem to like it, too. Your hook has a void *userData, but it's
not passed into the Python function. :)

int PyImport_SetOpenForImportHook(hook_func handler, void *userData)

Christian