Skip to content

Using X509Store's set_time function raises an ValueError exception on Windows #798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pckbls opened this issue Oct 10, 2018 · 2 comments · Fixed by #907
Closed

Using X509Store's set_time function raises an ValueError exception on Windows #798

pckbls opened this issue Oct 10, 2018 · 2 comments · Fixed by #907

Comments

@pckbls
Copy link

pckbls commented Oct 10, 2018

Hi there,

when trying to use X509Store's set_time() function I get an exception on my Windows machine:

self = <OpenSSL.crypto.X509Store object at 0x0000000004B9EEF0>
vfy_time = datetime.datetime(2018, 10, 10, 15, 47, 49, 836)
 
    def set_time(self, vfy_time):
        """
            Set the time against which the certificates are verified.
 
            Normally the current time is used.
 
            .. note::
 
              For example, you can determine if a certificate was valid at a given
              time.
 
            .. versionadded:: 17.0.0
 
            :param datetime vfy_time: The verification time to set on this store.
            :return: ``None`` if the verification time was successfully set.
            """
        param = _lib.X509_VERIFY_PARAM_new()
        param = _ffi.gc(param, _lib.X509_VERIFY_PARAM_free)
 
>       _lib.X509_VERIFY_PARAM_set_time(param, int(vfy_time.strftime('%s')))
E       ValueError: Invalid format string
 
C:\Program Files\Python36\lib\site-packages\OpenSSL\crypto.py:1671: ValueError

According to this Stackoverflow post, using strftime('%s') on a Python datetime.datetime object is a rather bad idea: https://stackoverflow.com/a/41811725

The %s directive also is not included in Python's strftime() documentation, however for some reason it still works on Unix based systems: https://docs.python.org/3.7/library/datetime.html#strftime-strptime-behavior

Best regards,
Patrick

@reaperhulk
Copy link
Member

Thanks for the report. set_time takes a datetime (when the other APIs in pyOpenSSL require you to provide a pre-formatted string that can be embedded in an ASN.1 time). That's a bit unfortunate, but to stay API compatible the fix here is probably to change this to calendar.timegm(vfy_time.timetuple()).

I'm not sure how strftime('%s') handles datetimes with time zones though... Anybody know off the top of their head? Is the proposed replacement going to change behavior?

jalberdi004 pushed a commit to jalberdi004/pyopenssl that referenced this issue Apr 6, 2020
…%s' option and getting the date in a more robust way
@orosam
Copy link
Contributor

orosam commented Oct 12, 2020

Actually, I don't know if it is possible to recreate exactly the strftime("%s") behavior, or if it is even desired.
Consider what I just found, while trying to write some tests:

>>> from datetime import datetime
>>> from os import putenv
>>> putenv("TZ", "CET")
>>> dt1=datetime(2020, 10, 25, 1, 59, 59)  # 1 second before the summer-winter fold
>>> dt2=datetime(2020, 10, 25, 2, 00, 00)  # first second in the summer-winter fold
>>> dt3=datetime(2020, 10, 25, 3, 00, 00)  # first second after the summer-winter fold
>>> dt1.strftime("%s")
'1603583999'
>>> dt2.strftime("%s")
'1603584000'
>>> dt3.strftime("%s")
'1603591200'
>>> dt2.strftime("%s")
'1603587600'  # Yes, it's dt2 again, but the timestamp depends on the previous call to strftime...

reaperhulk pushed a commit that referenced this issue Oct 18, 2020
* Fixing issue #798, thanks to @reaperhulk; removing undocumented '%s' option and getting the date in a more robust way

Co-authored-by: Joseba Alberdi <[email protected]>
Co-authored-by: Alex Gaynor <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants