[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[Christian Heimes]

Changes by Christian Heimes :


--
components:  -SSL

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[Christian Heimes]

Changes by Christian Heimes :


--
assignee: christian.heimes -> r.david.murray
stage: needs patch -> patch review
versions: +Python 2.7 -Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[Christian Heimes]

Changes by Christian Heimes :


--
assignee:  -> christian.heimes
components: +SSL

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[gigaplastik]

gigaplastik added the comment:

Here is the updated (3.6.0a4+) patch for the issue with the unit test included.

My unit test only checks for this issue particular artifacts, it does not test 
that .starttls actually works. This is because the current test suite for 
smtplib (Lib/test/test_smtplib.py) does not implement SSL/TLS functionality 
neither in its "debugging server", nor in its "simulated server" and 
implementing it on my own is beyond my expertise. Nevertheless, the provided 
unit test does guarantee .starttls wouldn't break because of the empty ._host 
attribute.
The overall Python test suite also runs successfully with these patches applied.

--
Added file: http://bugs.python.org/file44495/issue25852_v3_with tests.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[Christian Heimes]

Christian Heimes added the comment:

Please provide a more recent patch with a unit test.

--
nosy: +christian.heimes
stage: patch review -> needs patch
versions: +Python 3.7 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[gigaplastik]

gigaplastik added the comment:

Found the same issue independently, but I believe my version of the patch is a 
little more thoughtful. Since the host is allowed to be supplied in 
'hostname:port' format the assignment to ._host should be made _after_ checking 
(and probably parsing) this format.

The reason for this is that ._host is passed to ssl.SSLContext.wrap_socket 
method where it is used for SNI, defined in [1]. According to this RFC, 
"[c]urrently, the only server names supported are DNS hostnames; ... Literal 
IPv4 and IPv6 addresses are not permitted in [HostName]."

Checking if hostname passed is really a DNS name and not an IP address is up to 
ssl library, but here, in .connect method, at least the port number should be 
stripped off.

[1] https://tools.ietf.org/html/rfc4366.html

--
nosy: +gigaplastik
Added file: http://bugs.python.org/file42067/issue25852_v2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[R. David Murray]

Changes by R. David Murray :


--
components: +email
nosy: +barry, r.david.murray
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[SilentGhost]

SilentGhost added the comment:

Here is a usable patch, there doesn't seem to be a test for starttls, but it's 
still would be a good idea to add a test for this issue.

--
nosy: +SilentGhost
versions: +Python 3.6 -Python 3.3
Added file: http://bugs.python.org/file41293/issue25852.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25852] smtplib's SMTP.connect() should store the server name in ._host for .starttls()

[W. Trevor King]

New submission from W. Trevor King:

With the current tip, starttls uses ._host when calling wrap_socket [1], but 
._host is only setup in SMTP.__init__ [2].  Before #22921 [3] starttls would 
ignore ._host when SNI wasn't available locally.  But as far as I can tell, 
starttls has never used _host when connection happens via an explicit connect() 
call.  This leads to errors like [4]:

  >>> smtp = smtplib.SMTP()
  >>> smtp.connect(host="smtp.gmail.com", port=587)
  >>> smtp.ehlo()
  >>> smtp.starttls()
  File "smtp_test.py", line 10, in 
smtp.starttls()
  File "/usr/lib/python3.4/smtplib.py", line 676, in starttls
server_hostname=server_hostname)
  File "/usr/lib/python3.4/ssl.py", line 344, in wrap_socket
_context=self)
  File "/usr/lib/python3.4/ssl.py", line 540, in __init__
self.do_handshake()
  File "/usr/lib/python3.4/ssl.py", line 767, in do_handshake
self._sslobj.do_handshake()
  ssl.SSLError: [SSL: TLSV1_ALERT_DECODE_ERROR] tlsv1 alert decode error 
(_ssl.c:598)

I think a better approach would be to move the ._host set into .connect (patch 
attached).  It would still happen in SMTP(host=…) because [5], but would also 
allow starttls when users use SMTP() and then call connect(host=…) explicitly.

I've formatted the patch with Git, but its simple enough that it should be easy 
to apply in Mercurial.  Still, let me know if I can make applying it easier by 
rerolling the patch.

[1]: https://hg.python.org/cpython/file/323c10701e5d/Lib/smtplib.py#l766
[2]: https://hg.python.org/cpython/file/323c10701e5d/Lib/smtplib.py#l244
[3]: http://bugs.python.org/issue22921
[4]: 
http://stackoverflow.com/questions/23616803/smtplib-smtp-starttls-fails-with-tlsv1-alert-decode-error
[5]: https://hg.python.org/cpython/file/323c10701e5d/Lib/smtplib.py#l251

--
components: Library (Lib)
files: 0001-smtplib-Set-SMTP._host-in-.connect.patch
keywords: patch
messages: 256299
nosy: labrat
priority: normal
severity: normal
status: open
title: smtplib's SMTP.connect() should store the server name in ._host for 
.starttls()
type: behavior
versions: Python 3.3, Python 3.4, Python 3.5
Added file: 
http://bugs.python.org/file41291/0001-smtplib-Set-SMTP._host-in-.connect.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com