> On Apr 9, 2021, at 4:07 PM, Richard van der Hoff <rich...@matrix.org> wrote:
>
> Hi folks,
>
> I've been investigating https://github.com/matrix-org/synapse/issues/9566,
> which amounts to: "when there is a TLS error connecting to the SMTP server,
> the resultant exception is unreadable".
>
> I think I've traced the problem to the fact that
> SMTPSenderFactory.clientConnectionFailed is being called with an unhelpful
> ConnectionAborted rather than anything more descriptive.
>
> I've then reproduced that with a simpler test case: see
> https://gist.github.com/richvdh/909761ff5dab23f0873eeddd7936a740. As you can
> see, the output is: "Factory lost connection. Reason: Connection was aborted
> locally using ITCPTransport.abortConnection."
>
> This seems to be thanks to TLSMemoryBIOProtocol.failVerification, which
> stashes the error and calls abortConnection():
> https://github.com/twisted/twisted/blob/trunk/src/twisted/protocols/tls.py#L427.
>
> At this point I'm struggling. Is the SMTP code holding the Factory wrong? Or
> is it reasonable to expect the verification error to propagate into
> clientConnectionFailed - in which case, how could this work?
>
> Thanks for your thoughts!
Hi Richard,
Sorry for the delayed response here.
This is a bug in Twisted, and I think it boils down to this line:
https://github.com/twisted/twisted/blob/3c868ac11786eef7ea269caa3056f00854128957/src/twisted/protocols/tls.py#L391
<https://github.com/twisted/twisted/blob/3c868ac11786eef7ea269caa3056f00854128957/src/twisted/protocols/tls.py#L391>
The code as written here appears to be expecting this sequence:
failVerification is called with a reason containing a helpful OpenSSL
verification error
we save that reason as `self._reason` for reporting later, calling
abortConnection()
since the connection got aborted, we expect our underlying transport to call
loseConnection on us
we will then get a falsey reason [?!?!] and as such we will use self._reason
instead
Assumption 4 is nonsense and has never been true within Twisted as far as I
know; connectionLost always gets a Failure, Failures are never falsey, so we
will never use self._reason. To fix this we need to actually honor
self._reason under the conditions we expect, i.e. it's a ConnectionAborted
https://github.com/twisted/twisted/blob/3c868ac11786eef7ea269caa3056f00854128957/src/twisted/internet/error.py#L209
<https://github.com/twisted/twisted/blob/3c868ac11786eef7ea269caa3056f00854128957/src/twisted/internet/error.py#L209>
Does this make sense? (Will you be able to file / fix this bug?)
Thanks for the report,
-g
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
