[issue19940] ssl.cert_time_to_seconds() returns wrong results if local timezone is not UTC
New submission from akira:
cert_time_to_seconds() uses `time.mktime()` [1] to convert utc time tuple to seconds since epoch. `mktime()` works with local time. It should use `calendar.timegm()` analog instead.
Example from the docs [2] is seven hours off (it shows utc offset of the local timezone of the person who created it):
>>> import ssl
>>> ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT")
1178694000.0
It should be `1178668800`:
>>> from datetime import datetime
>>> datetime.utcfromtimestamp(1178668800)
datetime.datetime(2007, 5, 9, 0, 0)
>>> import time
>>> time.gmtime(1178668800)
time.struct_time(tm_year=2007, tm_mon=5, tm_mday=9, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=2, tm_yday=129, tm_isdst=0)
And `calendar.timegm` returns correct result:
>>> calendar.timegm(time.strptime("May 9 00:00:00 2007 GMT", "%b %d %H:%M:%S %Y GMT"))
1178668800
[1]: http://hg.python.org/cpython/file/96a68e369d13/Lib/ssl.py#l849
[2]: http://hg.python.org/cpython/file/96a68e369d13/Doc/library/ssl.rst#l359
----------
assignee: docs@python
components: Documentation, Library (Lib)
messages: 205774
nosy: akira, docs@python
priority: normal
severity: normal
status: open
title: ssl.cert_time_to_seconds() returns wrong results if local timezone is not UTC
type: behavior
versions: Python 2.6, Python 2.7, Python 3.1, Python 3.2, Python 3.3, Python 3.4, Python 3.5
_______________________________________
Python tracker
Changes by Tim Golden
gudge added the comment:
Will work on this.
Please assign the issue to me.
Instructions before proceeding by Tim Golden(python mailing list):
Having just glanced at that issue, I would point out that there's been a
lot of development around the ssl module for the 3.4 release, so you
definitely want to confirm the issue against the hg tip to ensure it
still applies.
----------
nosy: +gudge
_______________________________________
Python tracker
Changes by Antoine Pitrou
Changes by Antoine Pitrou
Antoine Pitrou added the comment:
Indeed the example in the docs is wrong, and so is the current behaviour.
The example shows "round-tripping" using ssl.cert_time_to_seconds() and then time.ctime(), except that it is bogus as it takes a GMT time and ctime() returns a local time ("""Convert a time expressed in seconds since the epoch to a string representing local time""").
Still, we should only fix it in 3.4, as code written for prior versions may rely on the current (bogus) behaviour.
----------
stage: -> needs patch
_______________________________________
Python tracker
Antoine Pitrou added the comment:
gudge, your contribution is welcome! If you need guidance about how to write a patch, you can read the developer's guide: http://docs.python.org/devguide/
Also you will have to sign a contributor's agreement: http://www.python.org/psf/contrib/
----------
_______________________________________
Python tracker
Changes by Giampaolo Rodola'
akira added the comment:
gudge,
There is also an issue with the current strptime format [1] (`"%b %d %H:%M:%S %Y GMT"`). It is locale-dependent and it may fail if a non-English locale is in effect. I don't know whether I should open a new issue on this or are you going to fix it too.
`cert_time_to_seconds()` is documented [2] to parse notBefore, notAfter fields from a certificate. As far as I can tell they do not depend on current locale. Thus the following code should not fail:
>>> timestr = "May 9 00:00:00 2007 GMT"
>>> import ssl
>>> ssl.cert_time_to_seconds(timestr)
1178661600.0
>>> import locale
>>> locale.setlocale(locale.LC_TIME, 'pl_PL.utf8')
'pl_PL.utf8'
>>> ssl.cert_time_to_seconds(timestr)
Traceback (most recent call last):
...[snip]...
ValueError: time data 'May 9 00:00:00 2007 GMT' does not match format '%b %d %H:%M:%S %Y GMT'
[1]: http://hg.python.org/cpython/file/96a68e369d13/Lib/ssl.py#l849
[2]: http://hg.python.org/cpython/file/96a68e369d13/Doc/library/ssl.rst#l359
----------
_______________________________________
Python tracker
gudge added the comment: 1) Can I get a list of failures. The summary of test results which I compare on my machine. 2) -----------------------------------------------------------------------------------------------------
import ssl ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT") 1178649000.0 from datetime import datetime datetime.utcfromtimestamp(1178668800) datetime.datetime(2007, 5, 9, 0, 0) import time time.gmtime(1178668800) time.struct_time(tm_year=2007, tm_mon=5, tm_mday=9, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=2, tm_yday=129, tm_isdst=0) import calender Traceback (most recent call last): File "<stdin>", line 1, in <module> ImportError: No module named 'calender' import callendar Traceback (most recent call last): File "<stdin>", line 1, in <module> ImportError: No module named 'callendar' import calendar calendar.timegm(time.strptime("May 9 00:00:00 2007 GMT", "%b %d %H:%M:%S %Y GMT")) 1178668800
I am running a VM on windows host machine. In your comment ou have specified:
import ssl >>> ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT") 1178694000.0
It should be `1178668800`:
But I get also get the same answer with the Python build from latest sources?
Therefore I do not get you?
3)
3 tests omitted:
test___all__ test_site test_urllib2net
348 tests OK.
3 tests failed:
test_codecs test_distutils test_ioctl
2 tests altered the execution environment:
test___all__ test_site
33 tests skipped:
test_bz2 test_codecmaps_cn test_codecmaps_hk test_codecmaps_jp
test_codecmaps_kr test_codecmaps_tw test_curses test_dbm_gnu
test_dbm_ndbm test_devpoll test_gzip test_idle test_kqueue
test_lzma test_msilib test_ossaudiodev test_readline test_smtpnet
test_socketserver test_sqlite test_ssl test_startfile test_tcl
test_timeout test_tk test_ttk_guionly test_ttk_textonly
test_urllibnet test_winreg test_winsound test_xmlrpc_net
test_zipfile64 test_zlib
Are these results fine. These results are with no changes.
How can I make all tests (skipped and omiited pass)
What about the 3 tests which failed. Are these known failures?
4)
Now say I have to pull time again to get the latest code. Does it help
to do a make. Or I will have o do configure again.
5) I had posted a query on core-metorship? No answers? Not that I am entitled to.
Thanks
----------
_______________________________________
Python tracker
gudge added the comment:
Sorry I think I did not read msg205774 (1st comment) correctly.
It clearly says:
"cert_time_to_seconds() uses `time.mktime()` [1] to convert utc time tuple to seconds since epoch. `mktime()` works with local time. It should use `calendar.timegm()` analog instead."
So the function cert_time_to_seconds() has to be fixed?
Thanks
----------
_______________________________________
Python tracker
Antoine Pitrou added the comment:
So the function cert_time_to_seconds() has to be fixed?
Yes!
----------
_______________________________________
Python tracker
gudge added the comment: Patch is uploaded. I will also copy paste it. I have created the patch with git. Let me know if it is okay with you. If it is unacceptable I will try and create one for mercury Patch: ------------------------------------------------------------------ diff --combined Doc/library/ssl.rst index a6ce5d6,30cb732..0000000 --- a/Doc/library/ssl.rst +++ b/Doc/library/ssl.rst @@@ -366,7 -366,7 +366,7 @@@ Certificate handlin >>> import ssl >>> ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT") - 1178694000.0 + 1178668800 >>> import time >>> time.ctime(ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT")) 'Wed May 9 00:00:00 2007' diff --combined Lib/ssl.py index f81ef91,052a118..0000000 --- a/Lib/ssl.py +++ b/Lib/ssl.py @@@ -852,8 -852,7 +852,8 @@@ def cert_time_to_seconds(cert_time) a Python time value in seconds past the epoch.""" import time - return time.mktime(time.strptime(cert_time, "%b %d %H:%M:%S %Y GMT")) + import calendar + return calendar.timegm(time.strptime(cert_time, "%b %d %H:%M:%S %Y GMT")) PEM_HEADER = "-----BEGIN CERTIFICATE-----" PEM_FOOTER = "-----END CERTIFICATE-----" ----------------------------------------------------------------- Test Results: 358 tests OK. 1 test failed: test_compileall 1 test altered the execution environment: test___all__ 28 tests skipped: test_codecmaps_cn test_codecmaps_hk test_codecmaps_jp test_codecmaps_kr test_codecmaps_tw test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_idle test_kqueue test_lzma test_msilib test_ossaudiodev test_smtpnet test_socketserver test_sqlite test_startfile test_tcl test_timeout test_tk test_ttk_guionly test_ttk_textonly test_urllibnet test_winreg test_winsound test_xmlrpc_net test_zipfile64 ------------------------------------------------------------------------ Doc changes won't effect the code. The tests would not fail. How would I check if the doc changes are coming up fine in the final version.
import ssl ssl.cert_time_to_seconds("May 9 00:00:00 2007 GMT") 1178668800
I do not have a printer curretly. I will sign the license agreement
in a few days.
----------
Added file: http://bugs.python.org/file33217/patch.txt
_______________________________________
Python tracker
Antoine Pitrou added the comment: Answering to your questions:
I have created the patch with git. Let me know if it is okay with you.
Yes, it's ok. Also, please don't copy / paste it. Uploading is enough.
Doc changes won't effect the code. The tests would not fail. How would I check if the doc changes are coming up fine in the final version.
The devguide has detailed documentation about how to modify and build
the documentation :)
http://docs.python.org/devguide/documenting.html#building-the-documentation
As for the tests:
1. for this issue you should probably concentrate on test_ssl: to run it
in verbose mode, "./python -m test -v test_ssl"
(please read http://docs.python.org/devguide/runtests.html)
2. you will need to add a new test to test_ssl, to check that this bug
is indeed fixed
----------
_______________________________________
Python tracker
akira added the comment:
gudge, have you seen http://bugs.python.org/msg205860 (the locale issue)?
If you can't fix it; say so, I'll open another issue after this issue is fixed.
----------
_______________________________________
Python tracker
gudge added the comment:
Akira, I will fix it. I will put in the patch in the same bug.
----------
_______________________________________
Python tracker
gudge added the comment:
1) I understand I can run a whole test suite as
./python -m test -v test_abc
as mentioned in
http://docs.python.org/devguide/runtests.html
How do I run a particluar test case, like the test I added
test_cert_time_to_seconds
2) I have a added a test case test_cert_time_to_seconds to test_ssl.py.
3) ./python -m test -v test_ssl
is all PASS.
4) I will start my work on http://bugs.python.org/issue19940#msg205860.
5) The patch is attached.
----------
Added file: http://bugs.python.org/file33254/patch.txt
_______________________________________
Python tracker
gudge added the comment:
Can you please provide some hints on how to handle
http://bugs.python.org/issue19940#msg205860.
The value of format_regex
1) Without locale set:
re.compile('(?P<b>jan|feb|mar|apr|may|jun|jul|aug|sep|oct|nov|dec)\\s+(?P<d>3[0-1]|[1-2]\\d|0[1- 9]|[1-9]| [1-9])\\s+(?P<H>2[0-3]|[0-1]\\d|\\d):(?P<M>[0-5]\\d|\\d):(?P<S>6[0-1]|[0-5]\\d|\\d)\\s +(?P<Y>\\d\\d\\d\\d, re.IGNORECASE)
2) With locale set:
re.compile('(?P<b>sty|lut|mar|kwi|maj|cze|lip|sie|wrz|pa\\ΕΊ|lis|gru)\\s+(?P<d>3[0-1]|[1-2]\\d|0[ 1-9]|[1-9]| [1-9])\\s+(?P<H>2[0-3]|[0-1]\\d|\\d):(?P<M>[0-5]\\d|\\d):(?P<S>6[0-1]|[0-5]\\d|\\d)\ \s+(?P<Y>\\d\\d\\d\, re.IGNORECASE)
The value of months are different.
Thanks
----------
_______________________________________
Python tracker
akira added the comment:
The point of the locale issue is that "notBefore", "notAfter" strings do not change if your locale changes. You don't need a new regex for each locale.
I've attached ssl_cert_time_seconds.py file that contains example cert_time_to_seconds(cert_time) implementation that fixes both the timezone and the locale issues.
----------
Added file: http://bugs.python.org/file34197/ssl_cert_time_seconds.py
_______________________________________
Python tracker
Antoine Pitrou added the comment:
Akira, do you want to write a proper patch with tests? If you are interested, you can take a look at http://docs.python.org/devguide/
You'll also have to sign a contributor's agreement at http://www.python.org/psf/contrib/contrib-form/
----------
_______________________________________
Python tracker
akira added the comment:
Antoine, I've signed the agreement. I've added ssl_cert_time_toseconds.patch with code, tests, and documention updates.
----------
keywords: +patch
Added file: http://bugs.python.org/file34201/ssl_cert_time_to_seconds.patch
_______________________________________
Python tracker
Antoine Pitrou added the comment:
Akira, thanks. I have posted a review; if you haven't received the e-mail notification, you can still access it at http://bugs.python.org/review/19940/#ps11142
----------
stage: needs patch -> patch review
versions: +Python 3.5 -Python 3.4
_______________________________________
Python tracker
akira added the comment:
Antoine, I haven't received the e-mail notification.
I've replied to the comments on Rietveld.
Here's the updated patch with the corresponding changes.
----------
Added file: http://bugs.python.org/file34594/ssl_cert_time_to_seconds-ps3.patch
_______________________________________
Python tracker
akira added the comment:
Here's a new patch with a simplified ssl.cert_time_to_seconds()
implementation that brings strptime() back.
The behaviour is changed:
- accept both %e and %d strftime formats for days as strptime-based implementation did before
- return an integer instead of a float (input date has not fractions of a second)
I've added more tests.
Please, review.
----------
Added file: http://bugs.python.org/file35050/ssl_cert_time_to_seconds-462470859e57.patch
_______________________________________
Python tracker
akira added the comment:
Replace IndexError with ValueError in the patch because tuple.index raises ValueError.
----------
Added file: http://bugs.python.org/file35051/ssl_cert_time_to_seconds-ps5.patch
_______________________________________
Python tracker
akira added the comment:
I've updated the patch:
- fixed the code example in the documentation to use int instead of
float result
- removed assertion on the int returned type (float won't lose precision
for the practical dates but guaranteeing an integer would be nice)
- reworded the scary comment
- removed tests that test the tests
Ready for review.
----------
Added file: http://bugs.python.org/file35075/ssl_cert_time_to_seconds-ps6.patch
_______________________________________
Python tracker
Antoine Pitrou added the comment:
Thanks for the updated patch, Akira! I'm gonna take a look right now.
----------
_______________________________________
Python tracker
Roundup Robot added the comment:
New changeset 7191c37238d5 by Antoine Pitrou in branch 'default':
Issue #19940: ssl.cert_time_to_seconds() now interprets the given time string in the UTC timezone (as specified in RFC 5280), not the local timezone.
http://hg.python.org/cpython/rev/7191c37238d5
----------
nosy: +python-dev
_______________________________________
Python tracker
Antoine Pitrou added the comment:
I've committed the patch. Thank you very much for contributing!
----------
resolution: -> fixed
stage: patch review -> resolved
status: open -> closed
_______________________________________
Python tracker
akira added the comment:
Antoine, thank you for reviewing. I appreciate the patience.
----------
_______________________________________
Python tracker
participants (6)
-
akira
-
Antoine Pitrou
-
Giampaolo Rodola'
-
gudge
-
Roundup Robot
-
Tim Golden