pyopenssl icon indicating copy to clipboard operation
pyopenssl copied to clipboard

CRL.set_nextUpdate doesn't set specified date

Open mdebb opened this issue 5 years ago • 5 comments

Added two unit tests for CRL.set_lastUpdate and CRL.set_nextUpdate. While the test works for set_lastUpdate, if fails on multiple levels for set_nextUpdate. The probable cause is the nextupdate field being optional (lastupdate is required), and initialized to null.

The tests are in this branch: https://github.com/mdebb/pyopenssl/commits/nextupdate

Output of py.test -k test_crl_next_update:

================================================================================================================================== FAILURES ===================================================================================================================================
________________________________________________________________________________________________________________________ TestCRL.test_crl_next_update _________________________________________________________________________________________________________________________

self = <tests.test_crypto.TestCRL object at 0x7f71ea46b390>

    def test_crl_next_update(self):
        """
            `set_nextUpdate` in _make_test_crl should set the correct date
            """
        root_crl = self._make_test_crl(
            self.root_cert, self.root_key, certs=[self.intermediate_cert])
        cryptograpy_crl = root_crl.to_cryptography()
>       assert cryptograpy_crl.next_update == datetime(2018, 6, 1, 0, 0)

tests/test_crypto.py:3482: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../../.local/lib/python2.7/site-packages/cryptography/hazmat/backends/openssl/x509.py:286: in next_update
    self._backend.openssl_assert(nu != self._backend._ffi.NULL)
../../../.local/lib/python2.7/site-packages/cryptography/hazmat/backends/openssl/backend.py:105: in openssl_assert
    return binding._openssl_assert(self._lib, ok)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

lib = <module 'lib' (built-in)>, ok = False

    def _openssl_assert(lib, ok):
        if not ok:
            errors = _consume_errors(lib)
            errors_with_text = []
            for err in errors:
                buf = ffi.new("char[]", 256)
                lib.ERR_error_string_n(err.code, buf, len(buf))
                err_text_reason = ffi.string(buf)
    
                errors_with_text.append(
                    _OpenSSLErrorWithText(
                        err.code, err.lib, err.func, err.reason, err_text_reason
                    )
                )
    
            raise InternalError(
                "Unknown OpenSSL error. This error is commonly encountered when "
                "another library is not cleaning up the OpenSSL error stack. If "
                "you are using cryptography with another library that uses "
                "OpenSSL try disabling it before reporting a bug. Otherwise "
                "please file an issue at https://github.com/pyca/cryptography/"
                "issues with information on how to reproduce "
                "this. ({0!r})".format(errors_with_text),
>               errors_with_text
            )
E           InternalError: Unknown OpenSSL error. This error is commonly encountered when another library is not cleaning up the OpenSSL error stack. If you are using cryptography with another library that uses OpenSSL try disabling it before reporting a bug. Otherwise please file an issue at https://github.com/pyca/cryptography/issues with information on how to reproduce this. ([])

../../../.local/lib/python2.7/site-packages/cryptography/hazmat/bindings/openssl/binding.py:76: InternalError
============================================================================================================================ 499 tests deselected =============================================================================================================================

mdebb avatar Oct 01 '18 09:10 mdebb

This does a conversion to a cryptography CRL which then causes the error. Could you file this over on the https://github.com/pyca/cryptography repo?

reaperhulk avatar Oct 04 '18 00:10 reaperhulk

I think the error occurs before converting the CRL, as you can see failed assertions while using existing tests on another branch. It was not checked before explicitly, so it could remain hidden.

https://github.com/mdebb/pyopenssl/commit/155e0e14345925d08ae2bb738bcca7366869a064

============================================================================= FAILURES ==============================================================================
_________________________________________________________________ TestCRL.test_verify_with_revoked __________________________________________________________________

self = <tests.test_crypto.TestCRL object at 0x7fe4c45da750>

    def test_verify_with_revoked(self):
        """
            `verify_certificate` raises error when an intermediate certificate is
            revoked.
            """
        store = X509Store()
        store.add_cert(self.root_cert)
        store.add_cert(self.intermediate_cert)
        root_crl = self._make_test_crl(
>           self.root_cert, self.root_key, certs=[self.intermediate_cert])

tests/test_crypto.py:3421: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_crypto.py:3408: in _make_test_crl
    crl.set_nextUpdate(b'20180601000000Z')
../../../git/pyopenssl/src/OpenSSL/crypto.py:2260: in set_nextUpdate
    return self._set_boundary_time(_lib.X509_CRL_get_nextUpdate, when)
../../../git/pyopenssl/src/OpenSSL/crypto.py:2230: in _set_boundary_time
    return _set_asn1_time(which(self._crl), when)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

boundary = <cdata 'struct asn1_string_st *' NULL>, when = '20180601000000Z'

    def _set_asn1_time(boundary, when):
        """
        The the time value of an ASN1 time object.
    
        @param boundary: An ASN1_TIME pointer (or an object safely
            castable to that type) which will have its value set.
        @param when: A string representation of the desired time value.
    
        @raise TypeError: If C{when} is not a L{bytes} string.
        @raise ValueError: If C{when} does not represent a time in the required
            format.
        @raise RuntimeError: If the time value cannot be set for some other
            (unspecified) reason.
        """
        if not isinstance(when, bytes):
            raise TypeError("when must be a byte string")
    
        set_result = _lib.ASN1_TIME_set_string(boundary, when)
        if set_result == 0:
            raise ValueError("Invalid string")
    
>       assert(boundary != _ffi.NULL)
E       AssertionError

../../../git/pyopenssl/src/OpenSSL/crypto.py:163: AssertionError

...

mdebb avatar Oct 18 '18 14:10 mdebb

Looks like more than one bug here. One is that pyca/cryptography assumes nextUpdate will always be present, when it is not required (so https://github.com/pyca/cryptography/blob/master/src/cryptography/hazmat/backends/openssl/x509.py#L296 is incorrect) and another is that https://github.com/pyca/pyopenssl/blob/master/tests/test_crypto.py#L3408 isn't actually setting a nextUpdate.

reaperhulk avatar Oct 19 '18 01:10 reaperhulk

It's more than 3 years since the last comment. I wonder if there's some solutions for this issue in 2022🧐? Thx a lot.

youremailaddress avatar Apr 07 '22 07:04 youremailaddress

I did some digging, and I don't fully understand it but I DO have a workaround.

crl.export() takes a "days" argument, and it properly sets the "nextUpdate", and is (arguably) what you want to use to write out the CRL. If you do NOT want to use crl.export() to write it, you can at least use it to set the nextUpdate and then the nextUpdate field is properly set, so you could use it (say if you wanted to crypto.dump_crl().

Example code:

crl = crypto.CRL()

revoked = crypto.Revoked()
revoked.set_serial(b'1')
revoked.set_rev_date(b'20220618000000Z')
revoked.set_reason(b'keyCompromise')
crl.add_revoked(revoked)

crl_pem = crl.export(ca_cert, ca_key, crypto.FILETYPE_PEM, crl)
print(crl_pem.decode('ascii'))
crl_text = crl.export(ca_cert, ca_key, crypto.FILETYPE_TEXT, crl)
print(crl_text.decode('ascii'))

The code in crl.export() which sets the nextUpdate is:

from OpenSSL._util import lib as _lib
days = 100
sometime = _lib.ASN1_TIME_new()
_lib.X509_gmtime_adj(sometime, days * 24 * 60 * 60)
_lib.X509_CRL_set1_nextUpdate(crl._crl, sometime)

if for some reason you didn't want to call crl.export(). You'd be directly accessing internal functions though, not recomended, but it seems to work.

linsomniac avatar Jun 18 '22 16:06 linsomniac