classification
Title: Document SSLSocket.getpeercert always returns None without do_handshake
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, docs@python, dsuch, giampaolo.rodola, janssen, pitrou, python-dev
Priority: normal Keywords:

Created on 2013-09-25 23:33 by dsuch, last changed 2013-09-29 17:55 by pitrou. This issue is now closed.

Messages (7)
msg198425 - (view) Author: Dariusz Suchojad (dsuch) Date: 2013-09-25 23:33
Hello,

I'd like to suggest adding a simple note to SSLSocket.getpeercert stating that it will always return None if do_handshake has never been called.

This is not the default behaviour, by default SSLSocket.__init__'s do_handshake_on_connect is True so .getpeercert nicely returns a cert (assuming the usual caveats - the other side offers a certificate and cert_reqs is not CERT_NONE).

However, I've just been debugging a someone else's server and I spent some time figuring out why client certificates weren't available - turned out this was because do_handshake was never called (PySSL_SSLdo_handshake in _ssl.c).

Adding a single-sentence line will certainly be very helpful.

Many thanks!
msg198431 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-09-26 08:45
To be honest I'd rather have it raise an exception in that case. None isn't helpful as it could mean other things.
msg198436 - (view) Author: Dariusz Suchojad (dsuch) Date: 2013-09-26 10:45
> None isn't helpful as it could mean other things.

This is another story but yes, it's true. API-wise, None should be returned in one situation only - we're on server side, ca_certs is non-CERT_NONE, do_handshake has been called yet there is no client certificate. And no other checks should be applied.

But the current behavior of returning None is documented and people depend on it so straightening it out would break backward compatibility - it's up to you to decide. I wouldn't mind it personally. 

But as far as this ticket goes - I'm on 2.7 and it's set in stone so for 2.7 - can you please change copy only? If you decide that for 3.x an exception will be raised then such a caveat would be included in 2.7 docs as well.

Thanks again.
msg198441 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-09-26 12:40
> But the current behavior of returning None is documented and people
> depend on it so straightening it out would break backward
> compatibility - it's up to you to decide. I wouldn't mind it
> personally.

I'm not sure people depend on getpeercert() returning None before the
handshake is done, or perhaps by accident?

> But as far as this ticket goes - I'm on 2.7 and it's set in stone so
> for 2.7 - can you please change copy only? If you decide that for
> 3.x an exception will be raised then such a caveat would be included
> in 2.7 docs as well.

Yes, if we change behaviour it will only be in 3.4.
msg198443 - (view) Author: Dariusz Suchojad (dsuch) Date: 2013-09-26 13:20
> I'm not sure people depend on getpeercert() returning None before the
> handshake is done, or perhaps by accident?

Ah, no, I meant that people may depend on the documented behaviour of .getpeercert's returning an empty dict (which I mixed up with returning None) if the certificate was not validated.

That this dictionary's contents depends on the validation is a bit quirky but it's documented so changing that one would surely break existing code.
msg198615 - (view) Author: Roundup Robot (python-dev) Date: 2013-09-29 17:50
New changeset ddcdf7f7eac8 by Antoine Pitrou in branch 'default':
Issue #19095: SSLSocket.getpeercert() now raises ValueError when the SSL handshake hasn't been done.
http://hg.python.org/cpython/rev/ddcdf7f7eac8
msg198617 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-09-29 17:55
Change committed in 3.4. Thanks for reporting!
History
Date User Action Args
2013-09-29 17:55:16pitrousetstatus: open -> closed
resolution: fixed
messages: + msg198617

stage: needs patch -> resolved
2013-09-29 17:50:59python-devsetnosy: + python-dev
messages: + msg198615
2013-09-29 17:29:29pitrousetassignee: docs@python ->
stage: needs patch
type: enhancement -> behavior
components: + Library (Lib), - Documentation
versions: + Python 3.4
2013-09-26 13:20:50dsuchsetmessages: + msg198443
2013-09-26 12:40:23pitrousetmessages: + msg198441
2013-09-26 10:45:12dsuchsetmessages: + msg198436
2013-09-26 08:45:59pitrousetnosy: + janssen, pitrou, giampaolo.rodola, christian.heimes
messages: + msg198431
2013-09-25 23:33:02dsuchcreate