#9740 defect closed fixed (fixed)

twisted.mail.smtp.ESMTPSender uses a hardcoded TLSv1 method

Reported by: schwukas Owned by: Craig Rodrigues
Priority: normal Milestone:
Component: mail Keywords:
Cc: Martin Milata Branch:
Author:

Description

Hi there,

due to an issue in Matrix's Synapse https://github.com/matrix-org/synapse/issues/6211 I noticed that there appears to be a hardcoded TLS version 1 in the smtp module (line 2038) https://github.com/twisted/twisted/blob/4ffbe9f6851dbe7e9172f55905f264ecf50da3a6/src/twisted/mail/smtp.py#L2038.

From the internet/ssl.py module the TLS versions should be the more modern TLSv2 and v3 https://github.com/twisted/twisted/blob/4ffbe9f6851dbe7e9172f55905f264ecf50da3a6/src/twisted/internet/ssl.py#L149

This hardcoded bit seems to be an old left-over from a commit in 2004: https://github.com/twisted/twisted/commit/18e0e1cd7a401de83ab492dd5b1ca8c4ee3865a8#diff-ca1c3dcdb24881af2c8a3f0c0f28bd2eR1452

Removing or commenting the line in the smtp module results in working email sending through synapse.

I feel like this should be a minor fix but since I'm not familiar with the code base I'd rather have someone with more experience look over it.

Thanks so much Lukas

Change History (7)

comment:1 Changed 23 months ago by Jean-Paul Calderone

Summary: twisted uses a hardcoded TLSv1 methodtwisted.mail.smtp.ESMTPSender uses a hardcoded TLSv1 method

comment:2 Changed 22 months ago by hawkowl

Owner: set to hawkowl

comment:3 Changed 20 months ago by Martin Milata

Cc: Martin Milata added
Keywords: review added
Owner: hawkowl deleted

comment:4 Changed 20 months ago by Glyph

Keywords: ssl tls mail smtp synapse review removed

Review is on github, on the PR. Thanks again for your contribution, and sorry for the sorry state of twisted.mail's transport security; this dates back to a completely different epoch of internet time, when even using TLS at all for mail was unusual.

comment:5 Changed 20 months ago by Martin Milata

Keywords: review added

Thanks for the review Glyph, I tried to address your comments and pushed a new commit to the branch, please take a look!

comment:6 Changed 19 months ago by Craig Rodrigues

Keywords: review removed
Owner: set to Craig Rodrigues

comment:7 Changed 19 months ago by Craig Rodrigues <rodrigc@…>

Resolution: fixed
Status: newclosed

In d329445d:

Error: Processor CommitTicketReference failed
 does not appear to be a Git repository. See the log for more information.
Note: See TracTickets for help on using tickets.