wking / rss2email

open-source tool for Windows, Mac OS and UNIX for getting news from RSS feeds in email
http://pypi.python.org/pypi/rss2email/
GNU General Public License v2.0
270 stars 48 forks source link

Use TLSv1 instead of SSLv3 #59

Closed emillon closed 5 years ago

emillon commented 9 years ago

As reported in https://bugs.debian.org/781315, SSLv3 is now disabled in Python3. It is thus necessary to perform the following changes:

wking commented 8 years ago

On Sat, Jul 04, 2015 at 07:31:00AM -0700, Etienne Millon wrote:

As reported in https://bugs.debian.org/781315, SSLv3 is now disabled in Python3.

It looks like it's still there in Python 3.5 [1,2], although it's discouraged 1. Maybe it's just been patched out on Debian? The current default is SSLv23 2, whose compatibility range changed (with OpenSSL v1.0.0?) 3. But instead of coding in a hard default, I think folks that don't care about their SSL version would be best served by using “whatever Python thinks is best” so we don't have to figure that out here ;). Python currently recommends 4 create_default_context (new in 3.4 5). For 3.2, we can't configure the protocol (see the current code). That leaves 3.3, where (assuming a modern OpenSSL), the best choice seems to be SSLv23 2. And also:

$ python3.4

import ssl context = ssl.create_default_context() context.protocol == ssl.PROTOCOL_SSLv23 True

TLSv1 seems like a poor choice, because it is incompatible with TLSv1.2 (the current tip). And SMTP_SSL supports a context argument since Python 3.3 6, so we can drop the “when not using 'smtp-ssl'” caveat. So how about a config setting like:

('smtp-ssl-protocol', ''), # TLS/SSL version to use for SSL connections

and code like:

if ssl or smtp_auth: protocol_name = config.get(section, 'smtp-ssl-protocol') if protocol_name: protocol = getattr(ssl, 'PROTOCOL{}'.format(protocol_name)) context = _ssl.SSLContext(protocol=protocol) else: try: context = _ssl.create_default_context() except AttributeError: # Python 3.3 or earlier context = _ssl.SSLContext(protocol=_ssl.PROTOCOL_SSLv23) if ssl: try: smtp = _smtplib.SMTP_SSL(context=context) except TypeError: # Python 3.2 or earlier smtp = _smtplib.SMTP_SSL() … if config.getboolean(section, 'smtp-auth'): … try: smtp.starttls(context=context) except TypeError:

Python 3.2 or earlier

      smtp.starttls()
jwilk commented 8 years ago

Yes, please use SSLv23 as the default, not TLSv1. SSLv23, contrary to what the name suggests, selects the highest protocol version that both client and server support. OTOH, TLSv1 selects TLSv1.0 only, which happens to be the lowest protocol version that Debian's OpenSSL currently supports.

emillon commented 8 years ago

OK, leaving that to Python is definitely the good thing to do.

I agree with the code snippet you posted. But I'm wondering about providing an upgrade path for users that have SSLv3 in their config on a system where it's unavailable (as in Debian). It think it'd be nicer for such users to fallback to the else case with a warning message.

How would you feel about this change?

Ekleog commented 5 years ago

@emillon I think this change has been superseded by @Yannik's 5a6e3809c6de8e4c55d3b94bd2fe53cde0714c6a and deb8007e8b7c8e45a435d12811f258c147fd9718, which have been merged in https://github.com/rss2email/rss2email/ (new home of rss2email following maintainership changes), feel free to close this PR :)

emillon commented 5 years ago

nice, thanks! I agree that it's better handled by the library itself.