[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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
Changes by Tim Golden mail@timgolden.me.uk:
---------- versions: -Python 2.6, Python 3.1, Python 3.2
_______________________________________ Python tracker report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
Changes by Antoine Pitrou pitrou@free.fr:
---------- nosy: +christian.heimes, giampaolo.rodola, janssen, pitrou versions: -Python 2.7, Python 3.3, Python 3.5
_______________________________________ Python tracker report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
Changes by Antoine Pitrou pitrou@free.fr:
---------- assignee: docs@python -> components: -Documentation
_______________________________________ Python tracker report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
Changes by Giampaolo Rodola' g.rodola@gmail.com:
---------- nosy: -giampaolo.rodola
_______________________________________ Python tracker report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
Antoine Pitrou added the comment:
So the function cert_time_to_seconds() has to be fixed?
Yes!
----------
_______________________________________ Python tracker report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
gudge added the comment:
Akira, I will fix it. I will put in the patch in the same bug.
----------
_______________________________________ Python tracker report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
Antoine Pitrou added the comment:
Thanks for the updated patch, Akira! I'm gonna take a look right now.
----------
_______________________________________ Python tracker report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
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 report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
akira added the comment:
Antoine, thank you for reviewing. I appreciate the patience.
----------
_______________________________________ Python tracker report@bugs.python.org http://bugs.python.org/issue19940 _______________________________________
-
akira -
Antoine Pitrou
-
Giampaolo Rodola'
-
gudge
-
Roundup Robot
-
Tim Golden