Ticket #5449 regression closed fixed

Opened 17 months ago

Last modified 16 months ago

new SSL implementation gives AttributeError when using an invalid key file

Reported by: wulczer Owned by: exarkun
Priority: normal Milestone: Twisted-12.0
Component: core Keywords:
Cc: Branch: branches/early-context-error-5449
Author: exarkun Launchpad Bug:

Description

The attached snippet tries to open an SSL connection using /etc/passwd as the key file.

In Twisted 11.0.0 this gave a traceback with an OpenSSL.SSL.Error, in Twisted 11.1.0 it also tracebacks with an AttributeError.

Attachments

ssl.py Download (0.8 KB) - added by wulczer 17 months ago.
snippet demonstrating the change in behaviour

Change History

Changed 17 months ago by wulczer

snippet demonstrating the change in behaviour

1

Changed 17 months ago by wulczer

  • summary changed from new SSL implementation gives AttributeError when using an invalid key file results to new SSL implementation gives AttributeError when using an invalid key file

2

Changed 17 months ago by itamar

  • owner set to itamar

3

Changed 16 months ago by itamar

  • milestone set to Twisted-12.0

4

Changed 16 months ago by therve

Here's the output: 

Unhandled Error
Traceback (most recent call last):
  File "/home/therve/Projects/Twisted/trunk/twisted/python/log.py", line 84, in callWithLogger
    return callWithContext({"system": lp}, func, *args, **kw)
  File "/home/therve/Projects/Twisted/trunk/twisted/python/log.py", line 69, in callWithContext
    return context.call({ILogContext: newCtx}, func, *args, **kw)
  File "/home/therve/Projects/Twisted/trunk/twisted/python/context.py", line 118, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/home/therve/Projects/Twisted/trunk/twisted/python/context.py", line 81, in callWithContext
    return func(*args,**kw)
--- <exception caught here> ---
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/posixbase.py", line 591, in _doReadOrWrite
    why = selectable.doWrite()
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/tcp.py", line 419, in doConnect
    self._connectDone()
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/tcp.py", line 427, in _connectDone
    self.protocol.makeConnection(self)
  File "/home/therve/Projects/Twisted/trunk/twisted/protocols/tls.py", line 293, in makeConnection
    tlsContext = self.factory._contextFactory.getContext()
  File "ssl.py", line 11, in getContext
    ctx.use_certificate_file('/etc/passwd')
OpenSSL.SSL.Error: [('PEM routines', 'PEM_read_bio', 'no start line'), ('SSL routines', 'SSL_CTX_use_certificate_file', 'PEM lib')]
Unhandled Error
Traceback (most recent call last):
  File "ssl.py", line 38, in <module>
    reactor.run()
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/base.py", line 1169, in run
    self.mainLoop()
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/base.py", line 1181, in mainLoop
    self.doIteration(t)
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/pollreactor.py", line 167, in doPoll
    log.callWithLogger(selectable, _drdw, selectable, fd, event)
--- <exception caught here> ---
  File "/home/therve/Projects/Twisted/trunk/twisted/python/log.py", line 84, in callWithLogger
    return callWithContext({"system": lp}, func, *args, **kw)
  File "/home/therve/Projects/Twisted/trunk/twisted/python/log.py", line 69, in callWithContext
    return context.call({ILogContext: newCtx}, func, *args, **kw)
  File "/home/therve/Projects/Twisted/trunk/twisted/python/context.py", line 118, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/home/therve/Projects/Twisted/trunk/twisted/python/context.py", line 81, in callWithContext
    return func(*args,**kw)
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/posixbase.py", line 599, in _doReadOrWrite
    self._disconnectSelectable(selectable, why, inRead)
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/posixbase.py", line 266, in _disconnectSelectable
    selectable.connectionLost(failure.Failure(why))
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/tcp.py", line 433, in connectionLost
    Connection.connectionLost(self, reason)
  File "/home/therve/Projects/Twisted/trunk/twisted/internet/tcp.py", line 277, in connectionLost
    protocol.connectionLost(reason)
  File "/home/therve/Projects/Twisted/trunk/twisted/protocols/tls.py", line 455, in connectionLost
    self._tlsConnection.bio_shutdown()
  File "/home/therve/Projects/Twisted/trunk/twisted/protocols/policies.py", line 112, in __getattr__
    return getattr(self.transport, name)
exceptions.AttributeError: 'NoneType' object has no attribute '_tlsConnection'
TIMEOUT

5

Changed 16 months ago by exarkun

  • branch set to branches/early-context-error-5449
  • branch_author set to exarkun

(In [33454]) Branching to 'early-context-error-5449'

6

Changed 16 months ago by exarkun

(In [33455]) Make IReactorSSL.connectSSL raise a synchronous exception if the context factory passed to it is broken

refs #5449

7

Changed 16 months ago by exarkun

  • owner itamar deleted
  • keywords review added

The behavior in 11.0.0 is: 

Unhandled Error
Traceback (most recent call last):
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/python/log.py", line 84, in callWithLogger
    return callWithContext({"system": lp}, func, *args, **kw)
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/python/log.py", line 69, in callWithContext
    return context.call({ILogContext: newCtx}, func, *args, **kw)
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/python/context.py", line 118, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/python/context.py", line 81, in callWithContext
    return func(*args,**kw)
--- <exception caught here> ---
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/internet/selectreactor.py", line 146, in _doReadOrWrite
    why = getattr(selectable, method)()
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/internet/tcp.py", line 674, in doConnect
    self._connectDone()
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/internet/ssl.py", line 149, in _connectDone
    self.startTLS(self.ctxFactory)
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/internet/tcp.py", line 571, in startTLS
    if Connection.startTLS(self, ctx, client):
  File "/home/exarkun/Projects/Twisted/branches/twisted-11.0.0/twisted/internet/tcp.py", line 402, in startTLS
    self.socket = SSL.Connection(ctx.getContext(), self.socket)
  File "ssl.py", line 11, in getContext
    ctx.use_certificate_file('/etc/passwd')
OpenSSL.SSL.Error: [('PEM routines', 'PEM_read_bio', 'no start line'), ('SSL routines', 'SSL_CTX_use_certificate_file', 'PEM lib')]
TIMEOUT

However, that sucks. It's quite unclear in what state the TCP connection is left, and it's a very difficult error for an application to handle, since it's merely logged as unhandled, not passed to any application code.

I don't think this behavior was ever considered, it merely occurred by accident. Better behaviors would be:

  1. Raise the exception from connectSSL
  2. Pass the exception to the factory's clientConnectionFailed.

The latter of these two is rather tempting, however the former is much simpler to implement and more closely mirrors the server-side behavior.

Hence, the behavior in the branch is now: 

$ python ssl.py 
Traceback (most recent call last):
  File "ssl.py", line 36, in <module>
    reactor.connectSSL('twistedmatrix.com', 443, Factory(), CtxFactory())
  File "/home/exarkun/Projects/Twisted/branches/early-context-error-5449/twisted/internet/posixbase.py", line 452, in connectSSL
    tlsFactory = tls.TLSMemoryBIOFactory(contextFactory, True, factory)
  File "/home/exarkun/Projects/Twisted/branches/early-context-error-5449/twisted/protocols/tls.py", line 589, in __init__
    contextFactory.getContext()
  File "ssl.py", line 11, in getContext
    ctx.use_certificate_file('/etc/passwd')
OpenSSL.SSL.Error: [('PEM routines', 'PEM_read_bio', 'no start line'), ('SSL routines', 'SSL_CTX_use_certificate_file', 'PEM lib')]

I expect it would be possible to return exactly to the 11.0.0 (and earlier - apparently back as far as 8.0.0, where I stopped checking) behavior. I'm not sure that's desirable, though.

 Build results

8

Changed 16 months ago by itamar

  • keywords review removed
  • owner set to exarkun

Throwing immediately seems reasonable, given all the relevant APIs are synchronous. And it's a more clever solution than I would've come up with.

Remaining issues:

  1. Equivalent test for startTLS. Looking at the code, I assume it will just work. Could be done in separate ticket.
  2. listenSSL presumably has the same issue. Doing this in separate ticket would also be fine.
  3. News file.

Given fixes/new tickets for above, feel free to merge.

9

Changed 16 months ago by exarkun

  • status changed from new to closed
  • resolution set to fixed

(In [33468]) Merge early-context-error-5449

Author: exarkun Reviewer: itamar Fixes: #5449

Change the users of application-supplied SSL context factories to invoke the getContext method early and allow errors to propagate. This improves the behavior in the case of broken context factories which cannot produce a context object by making the error happen closer to where the broken object is given to Twisted.

Note: See TracTickets for help on using tickets.