Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable TLS certificate validation by default for SMTP/IMAP/FTP/POP/NNTP protocols #91826

Open
The-Compiler opened this issue Apr 22, 2022 · 25 comments
Labels
topic-email topic-SSL type-feature A feature request or enhancement type-security A security issue

Comments

@The-Compiler
Copy link
Contributor

The-Compiler commented Apr 22, 2022

Feature or enhancement

I was surprised that Python does not verify hostnames by default for the stdlib modules for SMTP, IMAP, FTP, POP and NNTP. I believe the "insecure by default" behavior is no longer appropriate even for those protocol (at least SMTP and IMAP, I'm not too familiar with the rest - but even there, I suppose if an user asks for a secure connection, it should be secure).

In PEP 476 (Enabling certificate verification by default for stdlib http clients, 2014), certificate verification was enabled by default for HTTPS, with the rationale that:

The failure to do these checks means that anyone with a privileged network position is able to trivially execute a man in the middle attack against a Python application using either of these HTTP clients, and change traffic at will.

and

The “S” in “HTTPS” stands for secure. When Python’s users type “HTTPS” they are expecting a secure connection, and Python should adhere to a reasonable standard of care in delivering this. Currently we are failing at this, and in doing so, APIs which appear simple are misleading users.

When asked, many Python users state that they were not aware that Python failed to perform these validations, and are shocked.

and

The failure of various applications to note Python’s negligence in this matter is a source of regular CVE assignment

That PEP only improved that situation for HTTPS, stating that:

This PEP only proposes requiring this level of validation for HTTP clients, not for other protocols such as SMTP.

This is because while a high percentage of HTTPS servers have correct certificates, as a result of the validation performed by browsers, for other protocols self-signed or otherwise incorrect certificates are far more common. Note that for SMTP at least, this appears to be changing and should be reviewed for a potential similar PEP in the future:

https://www.facebook.com/notes/protect-the-graph/the-current-state-of-smtp-starttls-deployment/1453015901605223
https://www.facebook.com/notes/protect-the-graph/massive-growth-in-smtp-starttls-deployment/1491049534468526

Unfortunately, it seems to be very difficult to find more recent data about how many SMTP/IMAP/... servers in the wild present an invalid certificate - all I could find is that according to Google, adoption of outgoing encryption in general went up from ~75% when the PEP was written to ~90% now, and inbound encryption went up from ~57% to ~87%.

However, there seems to be a strong consensus to treat this kind of thing as a vulnerability in clients, even as early as 2007. Some examples:

  • CWE - CWE-297: Improper Validation of Certificate with Host Mismatch (4.6)
  • CVE - CVE-2007-5770: The (1) Net::ftptls, (2) Net::telnets, (3) Net::imap, (4) Net::pop, and (5) Net::smtp libraries in Ruby 1.8.5 and 1.8.6 do not verify that the commonName (CN) field in a server certificate matches the domain name in a request sent over SSL, which makes it easier for remote attackers to intercept SSL transmissions via a man-in-the-middle attack or spoofed web site, different components than CVE-2007-5162.
  • NVD - CVE-2009-3766: mutt_ssl.c in mutt 1.5.16 and other versions before 1.5.19, when OpenSSL is used, does not verify the domain name in the subject's Common Name (CN) field of an X.509 certificate, which allows man-in-the-middle attackers to spoof SSL servers via an arbitrary valid certificate.
  • CVE - CVE-2011-4318: Dovecot 2.0.x before 2.0.16, when ssl or starttls is enabled and hostname is used to define the proxy destination, does not verify that the server hostname matches a domain name in the subject's Common Name (CN) of the X.509 certificate, which allows man-in-the-middle attackers to spoof SSL servers via a valid certificate for a different hostname.
  • CVE - CVE-2011-1429: Mutt does not verify that the smtps server hostname matches the domain name of the subject of an X.509 certificate, which allows man-in-the-middle attackers to spoof an SSL SMTP server via an arbitrary certificate, a different vulnerability than CVE-2009-3766.
  • CVE - CVE-2012-2993: Microsoft Windows Phone 7 does not verify the domain name in the subject's Common Name (CN) field of an X.509 certificate, which allows man-in-the-middle attackers to spoof an SSL server for the (1) POP3, (2) IMAP, or (3) SMTP protocol via an arbitrary valid certificate.
  • CVE - CVE-2013-0308: The imap-send command in GIT before 1.8.1.4 does not verify that the server hostname matches a domain name in the subject's Common Name (CN) or subjectAltName field of the X.509 certificate, which allows man-in-the-middle attackers to spoof SSL servers via an arbitrary valid certificate.
  • CVE - CVE-2014-7273 / CVE - CVE-2014-7274: The IMAP-over-SSL implementation in getmail 4.0.0 through 4.43.0 does not verify X.509 certificates from SSL servers, which allows man-in-the-middle attackers to spoof IMAP servers and obtain sensitive information via a crafted certificate.

and more recently:

  • CVE - CVE-2020-1758: A flaw was found in Keycloak [...], where it does not perform the TLS hostname verification while sending emails using the SMTP server. [...]
  • CVE - CVE-2020-9488: Improper validation of certificate with host mismatch in Apache Log4j SMTP appender. [...]
  • CVE - CVE-2020-13163: em-imap 0.5 uses the library eventmachine in an insecure way [...]. The hostname in a TLS server certificate is not verified.
  • NVD - CVE-2020-15047: MSA/SMTP.cpp in Trojita [...] ignores certificate-verification errors, which allows man-in-the-middle attackers to spoof SMTP servers.
  • No enforcement of STARTTLS. OfflineIMAP/offlineimap#669
  • NVD - CVE-2021-44549: Apache Sling Commons Messaging Mail provides a simple layer on top of JavaMail/Jakarta Mail for OSGi to send mails via SMTPS. To reduce the risk of "man in the middle" attacks additional server identity checks must be performed when accessing mail servers. For compatibility reasons these additional checks are disabled by default in JavaMail/Jakarta Mail. The SimpleMailService in Apache Sling Commons Messaging Mail 1.0 lacks an option to enable these checks for the shared mail session. A user could enable these checks nevertheless by accessing the session via the message created by SimpleMessageBuilder and setting the property mail.smtps.ssl.checkserveridentity to true. Apache Sling Commons Messaging Mail 2.0 adds support for enabling server identity checks and these checks are enabled by default. (quoted in full as probably similar backwards compat concerns like in Python)

Previous discussion

Linked PRs

@The-Compiler The-Compiler added the type-feature A feature request or enhancement label Apr 22, 2022
@The-Compiler The-Compiler changed the title Enable hostname verification by default for new protocols Apr 22, 2022
@The-Compiler
Copy link
Contributor Author

The-Compiler commented Apr 22, 2022

Another data point: Based on some searches (IMAP, SMTP), almost nobody seems to pass a context or certificate. This includes various high-profle projects such as:

Are all those projects aware that they are vulnerable to MitM attacks for their mails? I somewhat doubt it. But the problem is so widespread I don't even know where to start with reporting this to affected projects.

@kousu
Copy link

kousu commented Apr 24, 2022

This is a really widespread and misunderstood problem. It reminds me of Operation ORCHESTRA (a satire) where defaults are so opaque so that they intentionally undermine security.

Thank you for your extensive research @The-Compiler. To me, it's clear that this is a problem best fixed in python, because it's a reasonable thing for clients to assume that enabling SSL makes them secure. We shouldn't need such extensive research to convince anyone to do something about it. But if that's what it takes, thank you for laying it all out. I hope it moves the needle.

@kousu
Copy link

kousu commented Apr 24, 2022

Here's a workaround to be run before your app's main() that should make these libraries secure-by-default; I also found the XML-RPC protocol is vulnerable:

import ssl
ssl_context = ssl.create_default_context()
assert ssl_context.check_hostname is True

from functools import partial, partialmethod
import ftplib, imaplib, nntplib, poplib, smtplib, xmlrpc.client
ftplib.FTP_TLS = partial(ftplib.FTP_TLS, context=ssl_context)
imaplib.IMAP4_SSL = partial(imaplib.IMAP4_SSL, ssl_context=ssl_context)
imaplib.IMAP4.starttls = partialmethod(imaplib.IMAP4.starttls, ssl_context=ssl_context)
nntplib.NNTP_SSL = partial(nntplib.NNTP_SSL, ssl_context=ssl_context)  # note: deprecated library
nntplib.NNTP.starttls = partialmethod(nntplib.NNTP.starttls, context=ssl_context)  # ditto
poplib.POP3_SSL = partial(poplib.POP3_SSL, context=ssl_context)
poplib.POP3.stls = partialmethod(poplib.POP3.stls, context=ssl_context)
smtplib.SMTP_SSL = partial(smtplib.SMTP_SSL, context=ssl_context)
smtplib.SMTP.starttls = partialmethod(smtplib.SMTP.starttls, context=ssl_context)
xmlrpc.client.ServerProxy = partial(xmlrpc.client.ServerProxy, context=ssl_context)
del ssl_context, ssl, partial, partialmethod
@tiran tiran changed the title Enable TLS hostname verification by default for SMTP/IMAP/FTP/POP/NNTP protocols Apr 24, 2022
@tiran
Copy link
Member

tiran commented Apr 24, 2022

Correction: Python does neither verify certificate nor hostname for SMTP, IMAP, and so on. Applications are subject to man-in-the-middle attacks for these protocols unless they pass an explicit context.

>>> import ssl
>>> ctx = ssl._create_stdlib_context()
>>> ctx.verify_mode
<VerifyMode.CERT_NONE: 0>
>>> ctx.check_hostname
False

The recommended workaround is:

import ssl

ssl._create_stdlib_context = ssl.create_default_context

or

import ssl

def verified_stdlib_context(protocol=None, *, cert_reqs=ssl.CERT_REQUIRED, check_hostname=True, **kwargs):
    return ssl._create_unverified_context(protocol, cert_reqs=cert_reqs, check_hostname=check_hostname, **kwargs)

ssl._create_stdlib_context = verified_stdlib_context

This enables cert validation and hostname verification for all stdlib modules.

I'll discuss the matter with the other core devs at PyCon next week.

@tiran tiran added type-security A security issue topic-SSL labels Apr 24, 2022
tiran added a commit to tiran/cpython that referenced this issue Apr 24, 2022
@vstinner
Copy link
Member

If this change breaks an application, what is the easiest way to opt-out and get back the old behavior: don't check anything?

ssl._create_unverified_context() is a private function, I would prefer to not advice users to use it.

SMTP/IMAP/FTP/POP/NNTP

I'm not sure that we can treat all protocols the same way. I suggest to look for statistics on TLS usage of each protocol, especially look if x509 certs are usually checked for these protocols.

For example, I expect that SMTP and IMAP check x509 certs, since these protocols are widely used, security matters and spam is a major issue.

I see FTP and NNTP as legacy protocols, I expect them to be used on cheap hardware with weak security.

For POP3, I have no idea.

Anyway, for me the most important is to properly document how to opt-out to keep access to servers which use invalid x509 certs (self signed, outdated, etc.)

@tiran
Copy link
Member

tiran commented Apr 25, 2022

I'm strongly against doing yet another incomplete switch to TLS cert validation. If we are going to switch, then the entire stdlib should verify certificates correctly.

The easiest and best way to solve broken applications is to get proper TLS certificates for your servers. Right now your application would be broken anyway. The change would turn the silent error into a loud error.

@vstinner
Copy link
Member

Another question is if we do the change only in Python 3.11 (or 3.12), or also in all supported Python versions (currently: 3.7, 3.8, 3.9, 3.10): https://devguide.python.org/#status-of-python-branches

I would prefer to only do the switch in the most recent Python version, and maybe provide an option to opt-in in previous Python versions.

@vstinner
Copy link
Member

Since this change is going to impact many users, maybe a PEP would be helpful to properly announce the change and communicate the rationale to (impacted) users.

@tiran
Copy link
Member

tiran commented Apr 25, 2022

I only plan to address the issue in 3.11. Applications can opt-in already by passing a SSLContext object.

@The-Compiler
Copy link
Contributor Author

The-Compiler commented Apr 25, 2022

I see FTP and NNTP as legacy protocols, I expect them to be used on cheap hardware with weak security.

That hardware would then typically use plain FTP and NNTP (i.e. without TLS), no? If someone explicitly opts in to using the secure variant (i.e. FTPS, NNTPS), given that usage is somewhat exotic¹, I think it's fine to expect them to have a proper certificate.

¹ Note that FTPS (FTP with TLS) != the probably more commonly used SFTP (SSH file transfer protocol)

Since this change is going to impact many users, maybe a PEP would be helpful to properly announce the change and communicate the rationale to (impacted) users.

I can try writing one if nobody beats me to it, but I have a lot on my plate with my own projects currently, so I'm not sure if I can get around to it anytime soon. If someone else wants to write one, feel free to use the information from my posts above.

I only plan to address the issue in 3.11. Applications can opt-in already by passing a SSLContext object.

Fair. As evident above, very few projects are apparently aware of this, though. Perhaps the other versions should at least have a big visible warning in the docs that the default behaviour is only marginally better than no encryption at all? E.g. the smtplib docs only seem to mention this in passing for SMTP_SSL (and not for starttls() at all):

Please read Security considerations for best practices.

which then mentions:

For client use, if you don’t have any special requirements for your security policy, it is highly recommended that you use the create_default_context() function to create your SSL context. It will load the system’s trusted CA certificates, enable certificate validation and hostname checking, and try to choose reasonably secure protocol and cipher settings.

yet, evidently, not enough people actually seem to be reading that (or just not reading docs at all...).

@tiran
Copy link
Member

tiran commented Apr 25, 2022

Feature freeze for 3.11 is in 11 days and I will be traveling around PyCon most of the time. I won't have time to write a PEP.

@The-Compiler
Copy link
Contributor Author

Would it help to get this into 3.11 still if I had a quick PEP cooked up (based on my opening post here) ~tomorrow or so?

@vstinner
Copy link
Member

In terms of development cycle, I would prefer to make this change at the beginning of the 3.12 development cycle, to have longer time to test it.

Is there any urgency to change the default? The latest major change (PEP 476) was in 2014: 8 years ago.

@The-Compiler
Copy link
Contributor Author

Given the impact the current defaults seem to have on applications using Python, I see this as a security issue rather than a new feature (the only problem being that it's backwards incompatible). It seems to me that postponing this by a year (?) results in a year more of insecure application code, which is bad even if things were that way for 8 years now. Sure, best practices around TLS security changed in those 8 years, but nowadays, it really leaves a sour aftertaste (and insecure downstream code).

@kousu
Copy link

kousu commented Apr 25, 2022

The recommended workaround is:

import ssl

ssl._create_stdlib_context = ssl.create_default_context

Thank you, this is a much better workaround than my attempt. You can see why I didn't notice it, thought, seeing as it involves a private function.

import ssl

def verified_stdlib_context(protocol=None, *, cert_reqs=ssl.CERT_REQUIRED, check_hostname=True, **kwargs):
    return ssl._create_unverified_context(protocol, cert_reqs=cert_reqs, check_hostname=check_hostname, **kwargs)

ssl._create_stdlib_context = verified_stdlib_context

This enables cert validation and hostname verification for all stdlib modules.

I'm confused, why does using _create_unverified_context verify certs? Is its name misleading? Or is this second workaround supposed to do opt in to the old unverified behaviour?

I'll discuss the matter with the other core devs at PyCon next week.

Thank you for taking this to heart :)

@The-Compiler
Copy link
Contributor Author

Thanks to some help by @mzollin I was able to gather some data for public IMAP/SMTP servers using shodan:

IMAP

  • imap port:143,993 => 5,584,984
  • imap port:143,993 has_ssl:true => 5,455,433
  • imap port:143,993 ssl.cert.expired:true => 705,124
  • imap port:143,993 ssl.chain_count:1 => 1,662,113 (probably self-signed)

Thus, around 98% encrypted, but of those, 13% expired and 30% probably self-signed.

SMTP

  • port:25,465,587,2525 => 13,488,598
  • port:25,465,587,2525 has_ssl:true => 7,583,151
  • port:25,465,587,2525 ssl.cert.expired:true => 893,792
  • port:25,465,587,2525 ssl.chain_count:1 => 2,683,701

So only 56% encrypted, of those, 12% expired and 35% probably self-signed.

HTTP

  • port:443,80 => 179,699,326
  • port:443,80 has_ssl:true => 53,077,390
  • port:443,80 ssl.cert.expired:true => 4,999,790
  • port:443,80 ssl.chain_count:1 => 12,644,539

So around 30% encrypted (huh?), and of those, 9% expired and 24% probably self-signed.


Based on the history of CVEs above, I still believe it would be good for Python to verify those by default, however. I've now also contacted the projects listed earlier, and at least Electrum has already pushed a fix: spesmilo/electrum@cac4b6f

@The-Compiler
Copy link
Contributor Author

Odoo has replied with a valuable perspective on this from the POV of someone who'd rather not have that change. Here is it quoted in full, with permission:

Let me try to give you the Odoo point of view, for the sake of adding
context to the Python proposal.

For us, this change would not only be backwards-incompatible, but also
quite risky. If it became the default, we may even need to change it back
to opt-in.
As you can imagine, our users configure all kinds of outgoing mail servers,
in various contexts (prod, staging, test). The SMTP landscape is incredibly
messy, and many small businesses still rely on custom SMTP deployments or
low-quality third-party ones. Maintaining a valid TLS certificate chain is
quite difficult, especially nowadays with short-lived certs.
So activating the TLS verification is guaranteed to break a good number of
real life deployments, and not just the test ones.

You could say that forcing users to notice the problem and upgrade their
setup is good, so we could make this the default in the next Odoo version.
But the reality may be more nuanced.
Some users may enable STARTTLS to follow recommendations, but don't have
the resources or skills to maintain a valid cert. Not only does it require
a frequently-updated TLS cert, it also requires the cert to match the
server hostname. This is not as trivial as it seems in the world of virtual
hosting, where hosting providers offer aliases for custom domains (e.g.
smtp.mydomain.org), but serve a generic TLS cert on it (ssl0.ovh.net).
Many users are more likely to abandon TLS and switch to plaintext, if it
turns out to be too complicated to set up and maintain.

Now you may say that they'd be better off using plaintext than experiencing
a false sense of security. That's not really true in practice, it's
still a net loss in terms of security: their SMTP credentials would now be exposed in
plaintext, along with all their communications. And it does not require a
MITM attacker, a simple packet dump on unencrypted free wifi is enough to
capture that.
This is generally true for all TLS channels, including HTTPS: having an
invalid certificate is still better than cleartext communication
.

Browsers usually solve this problem by allowing users to bypass the
verification process interactively
if they know what they are doing.
Unfortunately Odoo users aren't generally in control of the SMTP parameters
when they're using the system, so they wouldn't be allowed to make an
exception dynamically. Blocking their messages because the TLS certificate
has just expired would simply cause a denial of service, with no graceful
degradation
.

So if we want to enable TLS verification, I suspect we'll either have to
make it opt-in, or to implement some automatic detection with an easy
exception mechanism, during setup.
I have created a backlog task to discuss it internally (task-2861790).

Odoo's case is perhaps unusual though, due to a large proportion of legacy,
mixed deployment environments ;-)

@Neustradamus
Copy link

To follow this ticket

@vstinner
Copy link
Member

vstinner commented Nov 9, 2023

Changing the default is always complicated. If someone wants to change the default, I suggest to propose a migration plan. Example:

  • Add opt-in and opt-in options to enable/disable validation.
  • Document well the doc: explain how to test it (with the opt-in option), explain how to fix the issue (opt-out to be prepared for the change).
  • Later (in 1 or 2 Python versions), change the default.

The migration can be different for each protocol depending on the feedback, on how it goes.

@vstinner
Copy link
Member

vstinner commented Nov 9, 2023

IMAP: Thus, around 98% encrypted, but of those, 13% expired and 30% probably self-signed.

Preventing users to access their IMAP server because of security can be dismissive and people may stick to an old Python version or use a different programming language, if there is no good doc and no easy way to opt-out from cert validation.

@The-Compiler
Copy link
Contributor Author

@Neustradamus Note you can just click the "Subscribe" button in the sidebar of an issue:

image

Contrary to a comment, this won't send notifications to thousands of people.

@vstinner I definitely agree that it's a double-edged sword and it's not as easy as "just change a single value". But I think a loud failure without security impact is probably still better than a silent failure with security impact, all things considered. Would be interesting to see what other programming languages do by default, in fact.

@floyd-fuh
Copy link

We tried to get this problem fixed in some projects that were doing it wrong. Apart from that, mainly repeating in our blog post what @The-Compiler already said here: https://www.pentagrid.ch/en/blog/python-mail-libraries-certificate-verification/

@vstinner
Copy link
Member

If someone wants to lead an initiative to change the default, I suggest writing down a PEP, I can be your sponsor for that.

@nitram2342
Copy link

@vstinner Okay. I never wrote a PEP before, but why not give it a try: python/peps#3537

@vstinner
Copy link
Member

@vstinner Okay. I never wrote a PEP before, but why not give it a try: python/peps#3537

I see many names in this issue. It would be nice if it could be a collaborative work.

@The-Compiler: You created the issue, do you want to be part of this adventure?

Smattr added a commit to Smattr/needtoknow that referenced this issue Feb 2, 2024
The Python docs say:¹

  _ssl_context_ is a `ssl.SSLContext` object which allows bundling SSL
  configuration options, certificates and private keys into a single
  (potentially long-lived) structure. Please read Security considerations
  for best practices.
  …
  For client use, if you don’t have any special requirements for your security
  policy, it is highly recommended that you use the `create_default_context()`
  function to create your SSL context. It will load the system’s trusted CA
  certificates, enable certificate validation and hostname checking, and try to
  choose reasonably secure protocol and cipher settings.
  …
  By contrast, if you create the SSL context by calling the `SSLContext`
  constructor yourself, it will not have certificate validation nor hostname
  checking enabled by default.

While this is clear, it is counter-intuitive behaviour of which I was unaware.
I only learned of this through an oss-sec posting.² This issue seems to have a
long history and we are not the only software affected by it.³

¹ https://docs.python.org/3/library/imaplib.html#imaplib.IMAP4_SSL
² https://www.openwall.com/lists/oss-security/2024/02/01/4
³ python/cpython#91826,
  https://peps.python.org/pep-0476/,
  python/cpython#91875,
  https://www.pentagrid.ch/en/blog/python-mail-libraries-certificate-verification/,
  python/peps#3537
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-email topic-SSL type-feature A feature request or enhancement type-security A security issue
8 participants