Fix CRL nextUpdate handling. (#1169)

* Fix CRL nextUpdate handling.

When setting the nextUpdate field of a CRL, this code grabbed the
nextUpdate ASN1_TIME field from the CRL and set its time. But nextUpdate
is optional in a CRL so that field is usually NULL. But OpenSSL's
ASN1_TIME_set_string succeeds when the destination argument is NULL, so
it was silently a no-op.

Given that, the call in a test to set the nextUpdate field suddenly
starts working and sets the time to 2018, thus causing the CRL to be
considered expired and breaking the test. So this change also changes
the expiry year far into the future.

Additionally, the other CRL and Revoked setters violate const in the
API.

Fixes #1168.

* Replace self-check with an assert for coverage

* Update src/OpenSSL/crypto.py

Co-authored-by: Alex Gaynor <alex.gaynor@gmail.com>

Co-authored-by: Alex Gaynor <alex.gaynor@gmail.com>

2 files changed, 36 insertions, 10 deletions

diff --git a/src/OpenSSL/crypto.py b/src/OpenSSL/crypto.py
index 3224103..17fd522 100644
--- a/src/OpenSSL/crypto.py
+++ b/src/OpenSSL/crypto.py
@@ -168,12 +168,34 @@ def _set_asn1_time(boundary: Any, when: bytes) -> None:
     """
     if not isinstance(when, bytes):
         raise TypeError("when must be a byte string")
+    # ASN1_TIME_set_string validates the string without writing anything
+    # when the destination is NULL.
+    _openssl_assert(boundary != _ffi.NULL)
 
     set_result = _lib.ASN1_TIME_set_string(boundary, when)
     if set_result == 0:
         raise ValueError("Invalid string")
 
 
+def _new_asn1_time(when: bytes) -> Any:
+    """
+    Behaves like _set_asn1_time but returns a new ASN1_TIME object.
+
+    @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.
+    """
+    ret = _lib.ASN1_TIME_new()
+    _openssl_assert(ret != _ffi.NULL)
+    ret = _ffi.gc(ret, _lib.ASN1_TIME_free)
+    _set_asn1_time(ret, when)
+    return ret
+
+
 def _get_asn1_time(timestamp: Any) -> Optional[bytes]:
     """
     Retrieve the time value of an ASN1 time object.
@@ -2283,8 +2305,11 @@ class Revoked:
             as ASN.1 TIME.
         :return: ``None``
         """
-        dt = _lib.X509_REVOKED_get0_revocationDate(self._revoked)
-        return _set_asn1_time(dt, when)
+        revocationDate = _new_asn1_time(when)
+        ret = _lib.X509_REVOKED_set_revocationDate(
+            self._revoked, revocationDate
+        )
+        _openssl_assert(ret == 1)
 
     def get_rev_date(self) -> Optional[bytes]:
         """
@@ -2406,11 +2431,6 @@ class CRL:
         """
         _openssl_assert(_lib.X509_CRL_set_version(self._crl, version) != 0)
 
-    def _set_boundary_time(
-        self, which: Callable[..., Any], when: bytes
-    ) -> None:
-        return _set_asn1_time(which(self._crl), when)
-
     def set_lastUpdate(self, when: bytes) -> None:
         """
         Set when the CRL was last updated.
@@ -2424,7 +2444,9 @@ class CRL:
         :param bytes when: A timestamp string.
         :return: ``None``
         """
-        return self._set_boundary_time(_lib.X509_CRL_get0_lastUpdate, when)
+        lastUpdate = _new_asn1_time(when)
+        ret = _lib.X509_CRL_set1_lastUpdate(self._crl, lastUpdate)
+        _openssl_assert(ret == 1)
 
     def set_nextUpdate(self, when: bytes) -> None:
         """
@@ -2439,7 +2461,9 @@ class CRL:
         :param bytes when: A timestamp string.
         :return: ``None``
         """
-        return self._set_boundary_time(_lib.X509_CRL_get0_nextUpdate, when)
+        nextUpdate = _new_asn1_time(when)
+        ret = _lib.X509_CRL_set1_nextUpdate(self._crl, nextUpdate)
+        _openssl_assert(ret == 1)
 
     def sign(self, issuer_cert: X509, issuer_key: PKey, digest: bytes) -> None:
         """
diff --git a/tests/test_crypto.py b/tests/test_crypto.py
index 88756f0..44bbd0f 100644
--- a/tests/test_crypto.py
+++ b/tests/test_crypto.py
@@ -3850,7 +3850,9 @@ class TestCRL:
             crl.add_revoked(revoked)
         crl.set_version(1)
         crl.set_lastUpdate(b"20140601000000Z")
-        crl.set_nextUpdate(b"20180601000000Z")
+        # The year 5000 is far into the future so that this CRL isn't
+        # considered to have expired.
+        crl.set_nextUpdate(b"50000601000000Z")
         crl.sign(issuer_cert, issuer_key, digest=b"sha512")
         return crl