classification
Title: ssl.SSLObject method getpeercert() is buggy, do_handshake() is strange
Type: behavior Stage: resolved
Components: SSL Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Greg Stark, benjamin.peterson, christian.heimes, iritkatriel, njs
Priority: normal Keywords:

Created on 2017-01-20 12:13 by Greg Stark, last changed 2020-09-19 14:09 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
sslbugs.py Greg Stark, 2017-01-20 20:58 A short python program that illustrates the issues reported
Pull Requests
URL Status Linked Edit
PR 1769 merged christian.heimes, 2017-05-23 19:48
PR 1778 merged christian.heimes, 2017-05-23 23:11
PR 1779 closed christian.heimes, 2017-05-23 23:14
Messages (13)
msg285903 - (view) Author: Greg Stark (Greg Stark) Date: 2017-01-20 12:13
In my experiments with the relatively new class SSLObject from the ssl module I've noticed the following behavior(s) which I think can be described as bugs.

The getpeercert() method raises a ValueError exception "handshake not done" even after the handshake has successfully completed. If, however, I call the do_handshake() method *after* the handshake completes, then getpeercert() correctly runs and returns the peer's certificate. So now let's focus on do_handshake(). This method is basically undocumented, which I thought was ok because what it does should be obvious. It does seem to initiate a handshake if it's the first method call after the SSLObject is created. If called afterward, it doesn't outwardly appear to do anything, but as mentioned previously it does magically make the getpeercert() method start working.
msg285905 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-01-20 12:29
Hi Grek,

can you provide a script to reproduce the problem, please?
msg285906 - (view) Author: Greg Stark (Greg Stark) Date: 2017-01-20 12:31
Christian,

I will gladly do so a little later today. Thanks for your quick response.

--greg

On Fri, Jan 20, 2017 at 7:29 AM, Christian Heimes <report@bugs.python.org>
wrote:

>
> Christian Heimes added the comment:
>
> Hi Grek,
>
> can you provide a script to reproduce the problem, please?
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue29334>
> _______________________________________
>
msg285922 - (view) Author: Greg Stark (Greg Stark) Date: 2017-01-20 20:58
adding script the illustrates the bug.
msg294235 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2017-05-23 07:43
Oddly, I expected to run into this with my code using SSLObject in trio [1], but if I connect to python.org:443 and then 'await trio_ssl_stream.do_handshake(); trio_ssl_stream.getpeercert()' it works just fine ... even though when I run the sslbugs.py script I get the same weird results Greg reports. As far as I can tell the logic is identical. So I guess this might potentially be useful to narrow this down :-).

Test code that works:

@trio.run
async def main():
    import trio
    sock = trio.socket.socket()
    addr = await sock.resolve_remote_address(("python.org", 443))
    await sock.connect(addr)
    s = trio.SocketStream(sock)
    client = trio.ssl.SSLStream(
        s, trio.ssl.create_default_context(), server_hostname="python.org")
    await client.do_handshake()
    print(client.getpeercert())

[1] Currently in https://github.com/python-trio/trio/pull/107, eventually will be at https://github.com/python-trio/trio/blob/master/trio/ssl.py
msg294236 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-05-23 07:54
The issue with getpeercert() is a side-effect of your issue #30141. The peer certificate is cached in do_handshake.
msg294291 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-05-23 23:02
New changeset 66dc33b6822be93f85d84d24d3f9159ff568fbbb by Christian Heimes in branch 'master':
bpo-29334: Fix ssl.getpeercert for auto-handshake (#1769)
https://github.com/python/cpython/commit/66dc33b6822be93f85d84d24d3f9159ff568fbbb
msg295798 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-06-12 16:01
Also needs backport to 2.7 for #22559
msg297461 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017-07-01 01:08
Is anything holding this up for merging into 3.6 and/or 3.5?
msg301370 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-09-05 20:43
New changeset 63b3f2b19cc96801c3b8619e4cf8aa9028e7a33c by Christian Heimes in branch '3.6':
[3.6] bpo-29334: Fix ssl.getpeercert for auto-handshake (GH-1769) (#1778)
https://github.com/python/cpython/commit/63b3f2b19cc96801c3b8619e4cf8aa9028e7a33c
msg312886 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-02-26 08:27
The fix hasn't been ported to 2.7 yet.
msg312887 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-02-26 08:27
The fix hasn't been ported to 2.7 yet.
msg377160 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-09-19 11:35
Backport to 2.7 is no longer relevant, so I think this issue can be closed.
History
Date User Action Args
2020-09-19 14:09:52benjamin.petersonsetstatus: open -> closed
resolution: fixed
stage: backport needed -> resolved
2020-09-19 11:35:27iritkatrielsetnosy: + iritkatriel
messages: + msg377160
2018-02-26 20:22:20ned.deilysetnosy: + benjamin.peterson, - ned.deily
2018-02-26 08:27:45christian.heimessetstatus: open
assignee: christian.heimes ->
messages: + msg312887
2018-02-26 08:27:31christian.heimessetstatus: open -> (no value)

messages: + msg312886
versions: - Python 3.5, Python 3.6, Python 3.7
2017-09-05 20:43:07christian.heimessetmessages: + msg301370
2017-07-01 01:08:37ned.deilysetnosy: + ned.deily
messages: + msg297461
2017-06-12 16:01:56christian.heimessetmessages: + msg295798
versions: + Python 2.7
2017-06-12 06:58:06Mariattasetstage: backport needed
versions: + Python 3.6, Python 3.7
2017-05-23 23:14:59christian.heimessetpull_requests: + pull_request1861
2017-05-23 23:11:16christian.heimessetpull_requests: + pull_request1860
2017-05-23 23:02:04christian.heimessetmessages: + msg294291
2017-05-23 19:48:52christian.heimessetpull_requests: + pull_request1851
2017-05-23 07:54:00christian.heimessetmessages: + msg294236
2017-05-23 07:43:31njssetnosy: + njs
messages: + msg294235
2017-01-20 20:58:35Greg Starksetfiles: + sslbugs.py

messages: + msg285922
2017-01-20 12:31:34Greg Starksetmessages: + msg285906
2017-01-20 12:29:39christian.heimessetmessages: + msg285905
2017-01-20 12:13:47Greg Starkcreate