classification
Title: Inform caller of smtplib STARTTLS failures
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.1, Python 3.2, Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: aclover, barry, pitrou, r.david.murray, varun
Priority: normal Keywords: patch

Created on 2014-02-25 18:20 by aclover, last changed 2016-07-01 14:18 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
starttls_doc_fix_20770.patch varun, 2014-03-04 19:54 review
Messages (6)
msg212192 - (view) Author: And Clover (aclover) Date: 2014-02-25 18:20
When an SMTP server responds to the STARTTLS command with an error, the smtplib.SMTP.starttls() method does not raise an exception, as it would if TLS negotiation itself failed. Consequently naïve callers of the function may assume that a TLS connection has actually been established and continue to send sensitive mail through the interface.

In reality starttls() returns a tuple of response code and message from which the fail state can be detected, but this is not documented and no caller code I've met does anything with it.

Either:

1. Treat it as a doc bug for 3.4. The return value should be documented and callers warned that they need to check that value[0]==220 before assuming they have negotiated TLS. Or,

2. starttls() should raise SMTPResponseException for responses other than 220 in a future Python version, especially if moving towards validate-by-default. Possibly only raise an exception if the SSLContext.verify_mode is REQUIRED?
msg212194 - (view) Author: And Clover (aclover) Date: 2014-02-25 18:35
This could potentially be considered a security issue as it would allow a MitM attacker to sabotage the STARTTLS and get the rest of the content in the clear.

I don't personally consider it too serious as I doubt anyone is (a) relying on the security of this for lowly mail and (b) has the rest of the context stuff set up to validate the TLS connection properly anyhow, but there's an argument for sec bug.
msg212200 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-02-25 19:13
I agree that there is an argument for classifying this as a low-impact security bug.  Whether or not it is so classified will affect how we fix it.  I'll email the psrt about it.
msg212201 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-02-25 19:16
It probably isn't a good idea to break the API, but this should certainly be documented.
msg212737 - (view) Author: Varun Sharma (varun) * Date: 2014-03-04 19:54
I think that raising an exception for starttls failure can be avoided, so i have added a patch for documentation which adds the following line to doc string : "If server supports TLS but fails to start it, then it does not raise any exception".
msg269659 - (view) Author: And Clover (aclover) Date: 2016-07-01 11:41
This is CVE-2016-0772 and appears to have been fixed properly with an exception here:

https://hg.python.org/cpython/rev/d590114c2394 (py3)
https://hg.python.org/cpython/rev/b3ce713fb9be (py2)
History
Date User Action Args
2016-07-01 14:18:14r.david.murraysetstage: resolved
2016-07-01 11:41:27acloversetstatus: open -> closed
resolution: fixed
messages: + msg269659
2014-03-04 19:54:10varunsetfiles: + starttls_doc_fix_20770.patch

nosy: + varun
messages: + msg212737

keywords: + patch
2014-02-25 19:24:47barrysetnosy: + barry
2014-02-25 19:16:07pitrousetnosy: + pitrou
messages: + msg212201
2014-02-25 19:13:36r.david.murraysetnosy: + r.david.murray
messages: + msg212200
2014-02-25 18:35:02acloversetmessages: + msg212194
2014-02-25 18:20:19aclovercreate