classification
Title: smtplib should support SSL contexts
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: asdfasdfasdfasdfasdfasdfasdf, giampaolo.rodola, janssen, jcea, kasun, pitrou, python-dev, r.david.murray, terry.reedy, thomas.scrace
Priority: normal Keywords: patch

Created on 2010-05-24 16:15 by pitrou, last changed 2011-05-18 16:17 by kasun. This issue is now closed.

Files
File name Uploaded Description Edit
smtp_sslcontext.patch kasun, 2011-04-19 20:54 review
smtp_sslcontext_updated.patch kasun, 2011-04-20 06:36 review
smtp_sslcontext_updated2.patch kasun, 2011-04-26 04:16 review
smtp_sslcontext_updated3.patch kasun, 2011-04-27 06:00 review
smtp_sslcontext_test.patch kasun, 2011-05-14 18:11 review
Messages (18)
msg106369 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-24 16:15
3.2 introduces SSL contexts, which allow bundling SSL configuration options, certificates and private keys into a single (potentially long-lived) structure.
http://docs.python.org/dev/py3k/library/ssl.html#ssl.SSLContext

The SMTP_SSL constructor should allow passing an SSL context object instead of a key/cert pair.
msg133463 - (view) Author: Thomas Scrace (thomas.scrace) Date: 2011-04-10 13:47
Is anybody working on this issue? If not, I think it looks like it might be a nice one for me to tackle. I'll go ahead unless there are any objections.
msg133474 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-04-10 17:37
> Is anybody working on this issue? If not, I think it looks like it
> might be a nice one for me to tackle. I'll go ahead unless there are
> any objections.

Nobody is working on it AFAIK. Feel free to give it a try :)
msg134098 - (view) Author: Kasun Herath (kasun) Date: 2011-04-19 20:54
I did a patch that allows smtplib.SMTP_SSL constructor to accept an optional SSLContext
msg134120 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-04-20 02:36
This sentence is awkward, at best.
"Alternative to a private key and a certificate key an optional SSLContext could be specified."

I suggest something like: "SSLcontext, also optional, is an alternative to keyfile and certfile; if it is specified, keyfile and certfile must be None."
msg134121 - (view) Author: david (asdfasdfasdfasdfasdfasdfasdf) Date: 2011-04-20 02:48
It should also explain how the context can be used. 
An example of how to use it to establish a 'secured' connection would be a nice to have.
msg134124 - (view) Author: Kasun Herath (kasun) Date: 2011-04-20 06:36
I'm submitting a new patch with changes suggested by Terry. But I feel an example of how to use a SSLContext inside the Docstring of SMTP_SSL constructor is unnecessary since no smtp specific things are done to the passed SSLContext. Any ideas about this suggestion?
msg134181 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-04-20 20:52
Thanks for the patch. A couple of comments:
- it would be nice to have some tests, but of course there are no tests for SMTP_SSL currently. If you are motivated, perhaps you could investigate into adding some.
- there are some tab characters in your patch. Please only use spaces for indentation (see PEP 8 if you have any doubts); make sure your editor is configured for that :)
- starttls() should probably get a context parameter too. You can take a look at imaplib for an example.
- you need to update the API docs in Doc/library/smtplib.rst

I don't think an example of using a context is necessary. By using proper ReST markup a link will be generated to the SSLContext documentation, and that's probably enough for people to understand.
msg134443 - (view) Author: Kasun Herath (kasun) Date: 2011-04-26 04:16
I did another patch based on feedback given. A test for SMTP_SSL wasn't added; will look into it later. Tab character that were present are removed. Also SSLContext support was added to starttls() but it is slightly different from the starttls() implementation of imaplib. Further feedback would be much appreciated.
msg134479 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-04-26 16:48
For the record, issue11927 reminds me that test_smtpnet actually has a trivial test for SMTP-over-SSL.
msg134481 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-04-26 16:52
So, thanks for the new patch. I tested it with my ISP's server and it works fine!

Two remaining issues though:
- in the SMTP_SSL constructor, you must add the "context" argument at the end of the argument list (after "timeout"), otherwise someone passing "timeout" as a positional argument will find their code breaking
- in the doc, you need a "versionchanged" tag for the "context" argument (in both instances)

Thank you again.
msg134528 - (view) Author: Kasun Herath (kasun) Date: 2011-04-27 06:00
Thanks for the quick review. I'm submitting a new patch with changes suggested.
msg135529 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-05-08 16:06
Thank you, the patch looks good. A simple test could probably be added to test_smtpnet, along the lines of the existing tests, do you want to tackle that?
msg135561 - (view) Author: Kasun Herath (kasun) Date: 2011-05-09 03:35
Yes, I would like to have a try at it
msg135981 - (view) Author: Kasun Herath (kasun) Date: 2011-05-14 18:11
I added a test to smtpnet and submitting a separate patch for it as my test_smtpnet.py file was old and could have had conflicts.

I didn't use keys and certificates for the SSLContext as those would have to be shipped with the source. Looking forward for reviews and suggestions.
msg136248 - (view) Author: Roundup Robot (python-dev) Date: 2011-05-18 16:04
New changeset 6737de76487b by Antoine Pitrou in branch 'default':
Issue #8809: The SMTP_SSL constructor and SMTP.starttls() now support
http://hg.python.org/cpython/rev/6737de76487b
msg136249 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-05-18 16:05
Thank you Kasun. I did a couple of minor style fixes (whitespace at end of line, and spaces around the '=' assignment sign). I also added a test for starttls(), since it turns out gmail.com supports it on port 25.
msg136250 - (view) Author: Kasun Herath (kasun) Date: 2011-05-18 16:17
Thanks Antoine
History
Date User Action Args
2011-05-18 16:17:41kasunsetmessages: + msg136250
2011-05-18 16:05:43pitrousetstatus: open -> closed
resolution: fixed
messages: + msg136249

stage: patch review -> resolved
2011-05-18 16:04:19python-devsetnosy: + python-dev
messages: + msg136248
2011-05-14 18:11:41kasunsetfiles: + smtp_sslcontext_test.patch

messages: + msg135981
2011-05-09 03:35:55kasunsetmessages: + msg135561
2011-05-08 16:06:16pitrousetmessages: + msg135529
stage: needs patch -> patch review
2011-04-27 06:00:54kasunsetfiles: + smtp_sslcontext_updated3.patch

messages: + msg134528
2011-04-26 16:52:29pitrousetmessages: + msg134481
2011-04-26 16:48:59pitrousetmessages: + msg134479
2011-04-26 04:16:03kasunsetfiles: + smtp_sslcontext_updated2.patch

messages: + msg134443
2011-04-25 01:06:06jceasetnosy: + jcea
2011-04-20 20:52:32pitrousetmessages: + msg134181
2011-04-20 06:36:03kasunsetfiles: + smtp_sslcontext_updated.patch

messages: + msg134124
2011-04-20 02:48:12asdfasdfasdfasdfasdfasdfasdfsetmessages: + msg134121
2011-04-20 02:36:10terry.reedysetnosy: + terry.reedy
messages: + msg134120
2011-04-19 20:54:06kasunsetfiles: + smtp_sslcontext.patch

nosy: + kasun
messages: + msg134098

keywords: + patch
2011-04-13 13:10:32r.david.murraysetnosy: + r.david.murray
2011-04-10 17:37:37pitrousetmessages: + msg133474
2011-04-10 13:47:45thomas.scracesetnosy: + thomas.scrace
messages: + msg133463
2011-04-10 10:22:23asdfasdfasdfasdfasdfasdfasdfsetnosy: + asdfasdfasdfasdfasdfasdfasdf
2011-04-10 09:28:38pitroulinkissue11821 superseder
2011-04-10 09:27:54pitrousettitle: smptlib should support SSL contexts -> smtplib should support SSL contexts
2011-01-07 18:14:57pitrousetstage: needs patch
versions: + Python 3.3, - Python 3.2
2010-05-24 18:16:37giampaolo.rodolasetnosy: + janssen
2010-05-24 16:15:41pitroucreate