[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 29Mar2019 1218, Christian Heimes wrote:
> On 28/03/2019 23.35, Steve Dower wrote:
>> The ``importlib.util.open_for_import()`` function is a drop-in
>> replacement?for ``open(str(pathlike), 'rb')``. Its default behaviour is
>> to open a file for raw, binary access. To change the behaviour a new
>> handler should be set. Handler functions only accept ``str`` arguments.
>> The C API ``PyImport_OpenForImport`` function assumes UTF-8 encoding.
> [...]
>> All import and execution functionality involving code from a file will
>> be changed to use ``open_for_import()`` unconditionally. It is important
>> to note that calls to ``compile()``, ``exec()`` and ``eval()`` do not go
>> through this function - an audit hook that includes the code from these
>> calls is the best opportunity to validate code that is read from the
>> file. Given the current decoupling between import and execution in
>> Python, most imported code will go through both ``open_for_import()``
>> and the log hook for ``compile``, and so care should be taken to avoid
>> repeating verification steps.
>> There is no Python API provided for changing the open hook. To modify
>> import behavior from Python code, use the existing functionality
>> provided by ``importlib``.
> I think that the import hook needs to be extended. It only works for
> simple Python files or pyc files. There are at least two other important
> scenarios: zipimport and shared libraries.
> For example how does the importhook work in regarding of alternative
> importers like zipimport? What does the import hook 'see' for an import
> from a zipfile?

Yes, good point. I think opening the zip file with open_for_import() is 
the right place to do it, as this operation relates to opening the file 
on disk rather than files within it.

> Shared libraries are trickier. libc doesn't define a way to dlopen()
> from a file descriptor. dlopen() takes a file name, but a file name
> leaves the audit hook open to a TOCTOU attack.

For Windows, at least, the operating system can run its own validation 
on native modules (if you're using a feature like DeviceGuard, for 
example), so the hook likely isn't necessary for those purposes. I 
believe some configurations of Linux allow this as well?

But there's likely no better option here than a combination of good ACLs 
and checking by filename, which at least lets you whitelist the files 
you know you want to allow. Similarly for the zip file - if you trust a 
particular file and trust your ACLs, checking by filename is fine. That 
said, specific audit events for "I'm about to open this zip/dlopen this 
file for import" are very easy to add. (The PEP proposes many examples, 
but is not trying to be exhaustive. If accepted, we should feel free to 
add new events as we identify places where they matter.)

Aside: an important aspect of this per-file approach to execution is 
that the idea is generally to *enable* the files you trust, rather than 
disable the files that are bad. So the detection routines are typically 
"does this match a known hash" or "is this in a secure location", which 
for a carefully deployed system are already known values, rather than 
trying to figure out whether a file might do a bad thing. If you can't 
validate the files in your deployment match the ones you thought you 
were deploying, you are so far from needing this that it doesn't even 
matter, but most of the deployments I work with are *at least* this well