classification
Title: ssl.cert_time_to_seconds() returns wrong results if local timezone is not UTC
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: akira, christian.heimes, docs@python, gudge, janssen, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2013-12-10 07:29 by akira, last changed 2014-04-28 22:46 by akira. This issue is now closed.

Files
File name Uploaded Description Edit
patch.txt gudge, 2013-12-19 18:38 Patch
patch.txt gudge, 2013-12-22 18:05 Patch with the combined test case and code chages. This overrides the previous patch patch.txt. review
ssl_cert_time_seconds.py akira, 2014-02-23 10:10 example cert_time_to_seconds(cert_time) implementation
ssl_cert_time_to_seconds.patch akira, 2014-02-23 18:54 Fixed code (timezone and locale issues), added tests, updated documention review
ssl_cert_time_to_seconds-ps3.patch akira, 2014-03-23 21:51 changes following the review review
ssl_cert_time_to_seconds-462470859e57.patch akira, 2014-04-27 09:30 simplify implementation review
ssl_cert_time_to_seconds-ps5.patch akira, 2014-04-27 09:49 review
ssl_cert_time_to_seconds-ps6.patch akira, 2014-04-28 13:35 fix issues pointed out in the review review
Messages (26)
msg205774 - (view) Author: Akira Li (akira) * Date: 2013-12-10 07:29
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
msg205803 - (view) Author: gudge (gudge) Date: 2013-12-10 12:10
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.
msg205814 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-10 13:46
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.
msg205815 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-10 13:48
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/
msg205860 - (view) Author: Akira Li (akira) * Date: 2013-12-10 21:37
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
msg206616 - (view) Author: gudge (gudge) Date: 2013-12-19 15:08
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
msg206617 - (view) Author: gudge (gudge) Date: 2013-12-19 15:20
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
msg206620 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-19 15:23
> So the function cert_time_to_seconds() has to be fixed?

Yes!
msg206629 - (view) Author: gudge (gudge) Date: 2013-12-19 18:38
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.
msg206635 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-19 19:33
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
msg206666 - (view) Author: Akira Li (akira) * Date: 2013-12-20 06:48
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.
msg206810 - (view) Author: gudge (gudge) Date: 2013-12-22 07:31
Akira, I will fix it. I will put in the patch in the same bug.
msg206825 - (view) Author: gudge (gudge) Date: 2013-12-22 18:05
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.
msg207083 - (view) Author: gudge (gudge) Date: 2013-12-29 18:29
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
msg211988 - (view) Author: Akira Li (akira) * Date: 2014-02-23 10:10
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.
msg211992 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-02-23 12:08
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/
msg212022 - (view) Author: Akira Li (akira) * Date: 2014-02-23 18:54
Antoine, I've signed the agreement. I've added ssl_cert_time_toseconds.patch with code, tests, and documention updates.
msg212380 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-02-27 19:46
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
msg214637 - (view) Author: Akira Li (akira) * Date: 2014-03-23 21:51
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.
msg217243 - (view) Author: Akira Li (akira) * Date: 2014-04-27 09:30
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.
msg217245 - (view) Author: Akira Li (akira) * Date: 2014-04-27 09:49
Replace IndexError with ValueError in the patch because tuple.index raises ValueError.
msg217373 - (view) Author: Akira Li (akira) * Date: 2014-04-28 13:35
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.
msg217392 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-28 18:49
Thanks for the updated patch, Akira! I'm gonna take a look right now.
msg217394 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-28 18:57
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
msg217395 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-28 18:58
I've committed the patch. Thank you very much for contributing!
msg217435 - (view) Author: Akira Li (akira) * Date: 2014-04-28 22:46
Antoine, thank you for reviewing. I appreciate the patience.
History
Date User Action Args
2014-04-28 22:46:14akirasetmessages: + msg217435
2014-04-28 18:58:17pitrousetstatus: open -> closed
resolution: fixed
messages: + msg217395

stage: patch review -> resolved
2014-04-28 18:57:46python-devsetnosy: + python-dev
messages: + msg217394
2014-04-28 18:49:35pitrousetmessages: + msg217392
2014-04-28 13:35:43akirasetfiles: + ssl_cert_time_to_seconds-ps6.patch

messages: + msg217373
2014-04-27 09:49:47akirasetfiles: + ssl_cert_time_to_seconds-ps5.patch

messages: + msg217245
2014-04-27 09:30:49akirasetfiles: + ssl_cert_time_to_seconds-462470859e57.patch

messages: + msg217243
2014-03-23 21:51:24akirasetfiles: + ssl_cert_time_to_seconds-ps3.patch

messages: + msg214637
2014-02-27 19:46:15pitrousetstage: needs patch -> patch review
messages: + msg212380
versions: + Python 3.5, - Python 3.4
2014-02-23 18:54:20akirasetfiles: + ssl_cert_time_to_seconds.patch
keywords: + patch
messages: + msg212022
2014-02-23 12:08:53pitrousetmessages: + msg211992
2014-02-23 10:10:11akirasetfiles: + ssl_cert_time_seconds.py

messages: + msg211988
2013-12-29 18:29:54gudgesetmessages: + msg207083
2013-12-22 18:05:11gudgesetfiles: + patch.txt

messages: + msg206825
2013-12-22 07:31:39gudgesetmessages: + msg206810
2013-12-20 06:48:26akirasetmessages: + msg206666
2013-12-19 19:33:29pitrousetmessages: + msg206635
2013-12-19 18:38:12gudgesetfiles: + patch.txt

messages: + msg206629
2013-12-19 15:23:42pitrousetmessages: + msg206620
2013-12-19 15:20:31gudgesetmessages: + msg206617
2013-12-19 15:08:07gudgesetmessages: + msg206616
2013-12-10 21:37:28akirasetmessages: + msg205860
2013-12-10 20:04:07giampaolo.rodolasetnosy: - giampaolo.rodola
2013-12-10 13:48:13pitrousetmessages: + msg205815
2013-12-10 13:46:54pitrousetmessages: + msg205814
stage: needs patch
2013-12-10 13:43:08pitrousetassignee: docs@python ->
components: - Documentation
2013-12-10 13:42:55pitrousetnosy: + janssen, pitrou, giampaolo.rodola, christian.heimes

versions: - Python 2.7, Python 3.3, Python 3.5
2013-12-10 12:10:46gudgesetnosy: + gudge
messages: + msg205803
2013-12-10 12:02:03tim.goldensetversions: - Python 2.6, Python 3.1, Python 3.2
2013-12-10 07:29:12akiracreate