Re: [Twisted-Python] Security Advisory: bash remote code execution

[Matt Haggard]

>
> Any web server which is serving traffic over a CGI or CGI-like interface
> (including WSGI) should upgrade its version of Bash immediately.
>

I feel ignorant, but I'm confused about how WSGI is affected (and have
failed to exploit my WSGI app).  AFAICT from reading the code, Twisted's
WSGIResource doesn't invoke a shell.  I see that it has an `environ`
attribute that gets filled with user-provided information, but I don't see
how that makes it into a shell's environment.

We'll patch bash anyway.

Thanks,

Matt
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] Security Advisory: bash remote code execution

[Glyph Lefkowitz]

On Sep 25, 2014, at 8:09 AM, Matt Haggard  wrote:

> >
> > Any web server which is serving traffic over a CGI or CGI-like interface
> > (including WSGI) should upgrade its version of Bash immediately.
> >
> 
> I feel ignorant, but I'm confused about how WSGI is affected (and have failed 
> to exploit my WSGI app).  AFAICT from reading the code, Twisted's 
> WSGIResource doesn't invoke a shell.  I see that it has an `environ` 
> attribute that gets filled with user-provided information, but I don't see 
> how that makes it into a shell's environment.

As Alex's post said, this vulnerability does not affect Twisted directly.

The point is that most people deploying web services are doing so in a UNIX 
environment, and in so doing they are probably invoking scripts of various 
kinds, or executables which may have been replaced with wrapper shell-scripts.  
It's hard to audit for environment variables containing attacker-controlled 
data, and this is the sort of thing we've all been trained to expect is safe, 
if they're variables in our own "namespace", so it's possible that any number 
of 3rd-party tools you are using with Twisted are vulnerable in surprising ways.

So everybody should just upgrade :).

-glyph

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

[Twisted-Python] INCOMPATIBLE CHANGE: twisted.python.threadpool

[Glyph Lefkowitz]

Hi all,

(For those of you wondering about the subject, note that this message is sent 
in compliance with This email sent in compliance with 
.)

I've been trying to improve the reliability of the buildbots.  (On that note, 
by the way, several builders are failing and more will be soon as security 
updates roll out, it would be great if someone could review 
 before the whole fleet turns red.)

In so doing I (re-)discovered this bug: 
.  At first I just wanted to delete 
an intermittently failing test which appears to be nonsense.  However, after 
some prodding from exarkun, I investigated and discovered that this very 
poorly-written test case does in fact illustrate a real problem with our 
current threadpool implementation which can result in deadlocks, hangs on exit, 
and other unpleasantness.  In my use of Twisted I have encountered several 
"huh, this can't happen, maybe cosmic rays or something" bugs which might have 
been caused by this, so I would very much like to fix it.

One reason that our threadpool implementation is problematic is that it has a 
somewhat complex internal design, lots of weird little features (context 
preservation, completion callbacks, workload estimation) all built at the same 
layer with no composition.  I tried to figure out a way to salvage its current 
architecture and I could not reasonably do so.

I have prototyped a completely new threadpool implementation 
("twisted.threads") in a spike.  It is definitely not ready for review, but I 
would definitely appreciate design-level feedback on the code right now.  You 
can see the implementation here: 

 (or here: 
 if 
that's more your speed)  It's less code than the existing implementation and 
provides more useful features at the same time.  It might, in fact, provide 
some of the threading primitives that we would need for a reactor-per-thread 
implementation.

However, there is a very ugly implementation detail that prevents us from 
marching onwards to a glorious multithreading future: 
twisted.internet.interfaces.IReactorThreads.getThreadPool.  The nominally 
"public" interface provided by its documented return type, the concrete 
(old-style!) class twisted.python.threadpool.ThreadPool, is ridiculously 
over-broad.  Here are the features (attributes and methods) in its "public" 
interface:

callInThread
callInThreadWithCallback
adjustPoolsize
start
stop
q
min
max
joined
started
name
waiters
threads
working
workers
start
startAWorker
stopAWorker
dumpStats
__getstate__
__setstate__
threadFactory
currentThread

Here's what I would like its public interface to be, more or less what it's 
intended to be, based on the ways it's documented and used in Twisted and in 
the various projects I can see using it:

callInThread
callInThreadWithCallback
adjustPoolsize
start
stop

It would not be too much effort to also preserve, for legacy purposes:

min
max
joined
started
name
dumpStats

However, I would REALLY like to delete these implementation details:

workers
threads
working
startAWorker
q
__getstate__
__setstate__
threadFactory
currentThread

Deleting these implementation details straight up would also make it such that 
anyone who inherited from the public class ThreadPool would not be able to 
override any of these things (oh my goodness, why would you do that, I really 
hope nobody has done that).

The three ways I could proceed, in order of my own preference, are:

Put the new implementation with entirely new interfaces into twisted.threads, 
put a compatibility shim into twisted.python.threadpool.ThreadPool that wraps 
those objects, provides my more minimal interface proposed above, and deletes 
90% of its tests.  Change IReactorThreads.getThreadPool to return a documented 
interface considerably more restricted than 

Compatibility implications: twisted.internet.interfaces.IReactorThreads's 
contract would be relaxed.  Implementors of IReactorThreads would not see an 
impact unless they were talking to deleted parts of ThreadPool internally.  
ThreadPool's clients would have several methods removed, and subclassing would 
effectively not be possible any more (a bunch of overridable hooks would have 
been removed, and no replacements would be provided). However, I prefer this 
option because there are an extremely restrictive set of use-cases (basically: 
only logging, any behavioral changes would have been broken) where clients 
could have made legitimate use of these attributes or overriding any of these 
functions.

Delete all of twisted.python.threadpool's test coverage, deprecate the entire 
module, and delete it in the next release (14.1+1).  This has the advantage of 
making 

Re: [Twisted-Python] INCOMPATIBLE CHANGE: twisted.python.threadpool

[weykent]

On Sep 25, 2014, at 3:31 PM, Glyph Lefkowitz  wrote:

> So, does anyone out there have any code which makes use of the aforementioned 
> bad attributes of ThreadPool, whose applications would break if I removed 
> them?

Yes. Specifically, I am maintaining this AMP responder method:

@FetchThreadPoolStats.responder
def fetchThreadPoolStats(self):
pool = self.factory.threadPool
return dict(
threadsWaiting=len(pool.waiters),
threadsWorking=len(pool.working),
workerQueueSize=pool.q.qsize(),
)

These statistics are chunked into graphite and displayed as a nice little 
graph. They’ve been quite useful on some occasions: sometimes all of the 
threads in a thread pool for a WSGI application got blocked, and the symptoms 
were that all of the requests coming in were timing out at the proxy in front 
of twisted. We were able to quickly tell that the problem was the WSGI thread 
pool because the threadsWorking count was at its limit and the workerQueueSize 
was skyrocketing.

> If so, how can I make you feel as bad about yourselves for using it as I feel 
> bad about myself for writing it? ;-)

I don’t really see this as an abuse of the public interface. If possible, I’d 
like to see this diagnostic information preserved in the new interface, as I 
consider it invaluable information as long as we’re using twisted as a WSGI 
runner.

I will say that it is certainly possible for me to emulate these attributes, 
but that would require touching implementation details of WSGIResource. I’m not 
sure which is worse.

~weykent


___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python