[issue15452] Eliminate the use of eval() in the logging config implementation

[Vinay Sajip]

Vinay Sajip added the comment:

> As far as your other suggestion goes, don't reinvent crypto badly -
> if you want to provide authentication support in listener(), provide a
> hook that allows the application to decide whether or not to accept
> the configuration before it gets applied.

Well, that's fine. My earlier suggestion keeps the API change to a minimum, but 
I suppose there's no real need to be so minimal.

I suppose the basic approach would be to pass to listen() an optional verify 
callable (defaulting to None) which, if provided, would be called with the 
bytes received over the socket. That allows for e.g. signed or encrypted data. 
The value returned from the verify() call would be processed as the current 
received value is (allowing the verifier to transform the input, e.g. by 
decrypting it).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15452] Eliminate the use of eval() in the logging config implementation

[Nick Coghlan]

Nick Coghlan added the comment:

I know ast.literal_eval() isn't enough - that's why I suggested the addition of 
ast.lookup_eval() in the opening post.

As far as your other suggestion goes, don't reinvent crypto badly - if you want 
to provide authentication support in listener(), provide a hook that allows the 
application to decide whether or not to accept the configuration before it gets 
applied. They can then choose there own authentication mechanism based on their 
own needs, and handle any appropriate security updates. Some will choose a 
simple shared secret, some may choose to require a cryptographic signature, 
some may pass the entire payload in an encrypted form.

However, this isn't an either/or situation - we can, and should, do both (i.e. 
provide a hook that allows the application to preauthenticate the configuration 
before it is applied, as well as replacing the use of eval() with something 
more limited like str.format lookup syntax). The right security mindset is to 
set up defence in depth, not trust one particular layer of defence to handle 
all possible varieties of attack.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15452] Eliminate the use of eval() in the logging config implementation

[Vinay Sajip]

Vinay Sajip  added the comment:

Having reflected on this further, ISTM that limiting the scope of evaluation is 
not the correct answer. For example, a malicious user could still send a bogus 
configuration which, for example, just turns the verbosity of all loggers off, 
or configures a huge number of bogus loggers. This would certainly be allowed 
even by a limited-evaluation scheme if a user legitimately wanted to do so; but 
if a malicious user sends the exact same “legal” configuration, it is still a 
security exploit because the consequences may be undesirable to the victim.

But how is the listener to know whether or not the configuration is coming from 
a legitimate source (a client process controlled by the same user who is 
running the process which uses listen())  or a malicious user (a client process 
controlled by some other user)? The simplest answer would appear to be a shared 
secret: When listen() is called, it is passed a text passphrase, which is also 
known to legitimate clients. When handling a configuration request via the 
socket, the configuration is checked to see if it contains the passphrase. If 
it does, the request is processed; otherwise, it is ignored.

In the fileConfig() input data, the passphrase could be provided via a 
passphrase=secret entry in the [default] section. In the dictConfig() input 
data, the passphrase could be provided against the passphrase key in the dict 
which is passed to dictConfig(). The checking would be done in the request 
handler code before calling fileConfig() or dictConfig(). If the passphrase 
argument to the listen() call is None (the default, preserving the current 
behaviour) no passphrase checking would be done.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15452] Eliminate the use of eval() in the logging config implementation

[Vinay Sajip]

Vinay Sajip  added the comment:

Initial evaluation indicates that ast.literal_eval doesn't cut the mustard: it 
doesn't do any name lookups, so you can't for example successfully evaluate 
something like 'handlers.WatchedFileHandler' or even 'FileHandler'. 

However, a limited evaluator which goes further than ast.literal_eval will 
probably work. One such is shown in this Gist:

https://gist.github.com/3182304

It supports a reasonable subset of Python expressions and also could be useful 
in other contexts than logging configuration.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15452] Eliminate the use of eval() in the logging config implementation

[Vinay Sajip]

Vinay Sajip  added the comment:

It's not actually the PEP 391 implementation - dictConfig() - that uses eval(). 
Rather, it's the older fileConfig() API which was part of the original logging 
package when added to Python 2.3. The use of eval() by fileConfig() was 
documented at that time, IIRC.

I have no problem in principle with updating fileConfig() - which uses eval() 
in just one private function - to use ast.literal_eval(), but it may break 
existing, innocuous code which can't be handled by ast.literal_eval().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15452] Eliminate the use of eval() in the logging config implementation

[Arfrever Frehtes Taifersar Arahesis]

Changes by Arfrever Frehtes Taifersar Arahesis :


--
nosy: +Arfrever

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue15452] Eliminate the use of eval() in the logging config implementation

[Nick Coghlan]

New submission from Nick Coghlan :

The current implementation of PEP 391 relies on eval, which is substantially 
more permissive than the expected syntax described in the spec. This means the 
listen() feature provides an attack vector for injection of untrusted code.

While the documentation has been updated with a cautionary note to this effect, 
longer term, the use of eval() should be replaced with:

1. ast.literal_eval()
2. refactoring the str.format attribute and item lookup code into something 
suitable for reuse in other contexts (perhaps exposed via the ast module as 
"ast.lookup_eval()")

--
components: Library (Lib)
messages: 166448
nosy: ncoghlan, vinay.sajip
priority: normal
severity: normal
stage: needs patch
status: open
title: Eliminate the use of eval() in the logging config implementation
type: security
versions: Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com