Add back cert warnings for cases which are not covered by 7527175

Signed-off-by: Jean Snyman <git@jsnyman.com>

keylime/cert_utils.py

@@ -42,7 +42,8 @@ def x509_der_cert(der_cert_data: bytes) -> Certificate:
     """
     try:
         return x509.load_der_x509_certificate(data=der_cert_data, backend=default_backend())
-    except Exception:
+    except Exception as err:
+        logger.warning("Failed to parse DER data with python-cryptography: %s", err)
         pyasn1_cert = decoder.decode(der_cert_data, asn1Spec=rfc2459.Certificate())[0]
         return x509.load_der_x509_certificate(data=encoder.encode(pyasn1_cert), backend=default_backend())
 
@@ -55,7 +56,8 @@ def x509_pem_cert(pem_cert_data: str) -> Certificate:
     """
     try:
         return x509.load_pem_x509_certificate(data=pem_cert_data.encode("utf-8"), backend=default_backend())
-    except Exception:
+    except Exception as err:
+        logger.warning("Failed to parse PEM data with python-cryptography: %s", err)
         # Let's read the DER bytes from the base-64 PEM.
         der_data = pem.readPemFromFile(io.StringIO(pem_cert_data))
         # Now we can load it as we do in x509_der_cert().

keylime/models/base/types/certificate.py

@@ -1,13 +1,17 @@
 import base64
 import binascii
+import io
 from typing import Optional, TypeAlias, Union
 
 import cryptography.x509
 from cryptography.hazmat.primitives.serialization import Encoding
-from pyasn1.error import PyAsn1Error, SubstrateUnderrunError
+from pyasn1.codec.der import decoder as pyasn1_decoder
+from pyasn1.codec.der import encoder as pyasn1_encoder
+from pyasn1.error import PyAsn1Error
+from pyasn1_modules import pem as pyasn1_pem
+from pyasn1_modules import rfc2459 as pyasn1_rfc2459
 from sqlalchemy.types import Text
 
-from keylime import cert_utils
 from keylime.models.base.type import ModelType
 
 
@@ -79,6 +83,72 @@ def _schema(self):
     def __init__(self) -> None:
         super().__init__(Text)
 
+    def _load_der_cert(self, der_cert_data: bytes) -> cryptography.x509.Certificate:
+        """Loads a binary x509 certificate encoded using ASN.1 DER as a ``cryptography.x509.Certificate`` object. This
+        method does not require strict adherence to ASN.1 DER thereby making it possible to accept certificates which do
+        not follow every detail of the spec (this is the case for a number of TPM certs) [1,2].
+
+        It achieves this by first using the strict parser provided by python-cryptography. If that fails, it decodes the
+        certificate and re-encodes it using the more-forgiving pyasn1 library. The re-encoded certificate is then
+        re-parsed by python-cryptography.
+
+        This method is equivalent to the ``cert_utils.x509_der_cert`` function but does not produce a warning when the
+        backup parser is used, allowing this condition to be optionally detected and handled by the model where
+        relevant. This is part of the fix for issue 1559 [3].
+
+        Note: This method is marked as protected as ``self.cast(...)`` should be called from outside the class instead.
+
+        [1] https://github.com/keylime/keylime/issues/944
+        [2] https://github.com/pyca/cryptography/issues/7189
+        [3] https://github.com/keylime/keylime/issues/1559
+
+        :param der_cert_data: the DER bytes of the certificate
+
+        :raises: :class:`SubstrateUnderrunError`: cert could not be deserialized even using the fallback pyasn1 parser
+
+        :returns: A ``cryptography.x509.Certificate`` object
+        """
+
+        try:
+            return cryptography.x509.load_der_x509_certificate(der_cert_data)
+        except Exception:
+            pyasn1_cert = pyasn1_decoder.decode(der_cert_data, asn1Spec=pyasn1_rfc2459.Certificate())[0]
+            return cryptography.x509.load_der_x509_certificate(pyasn1_encoder.encode(pyasn1_cert))
+
+    def _load_pem_cert(self, pem_cert_data: str) -> cryptography.x509.Certificate:
+        """Loads a text x509 certificate encoded using PEM (Base64ed DER with header and footer) as a
+        ``cryptography.x509.Certificate`` object. This method does not require strict adherence to ASN.1 DER thereby
+        making it possible to accept certificates which do not follow every detail of the spec (this is the case for
+        a number of TPM certs) [1,2].
+
+        It achieves this by first using the strict parser provided by python-cryptography. If that fails, it decodes the
+        certificate and re-encodes it using the more-forgiving pyasn1 library. The re-encoded certificate is then
+        re-parsed by python-cryptography.
+
+        This method is equivalent to the ``cert_utils.x509_der_cert`` function but does not produce a warning when the
+        backup parser is used, allowing this condition to be optionally detected and handled by the model where
+        relevant. This is part of the fix for issue 1559 [3].
+
+        Note: This method is marked as protected as ``self.cast(...)`` should be called from outside the class instead.
+
+        [1] https://github.com/keylime/keylime/issues/944
+        [2] https://github.com/pyca/cryptography/issues/7189
+        [3] https://github.com/keylime/keylime/issues/1559
+
+        :param der_cert_data: the DER bytes of the certificate
+
+        :raises: :class:`SubstrateUnderrunError`: cert could not be deserialized even using the fallback pyasn1 parser
+
+        :returns: A ``cryptography.x509.Certificate`` object
+        """
+
+        try:
+            return cryptography.x509.load_pem_x509_certificate(pem_cert_data.encode("utf-8"))
+        except Exception:
+            der_data = pyasn1_pem.readPemFromFile(io.StringIO(pem_cert_data))
+            pyasn1_cert = pyasn1_decoder.decode(der_data, asn1Spec=pyasn1_rfc2459.Certificate())[0]
+            return cryptography.x509.load_der_x509_certificate(pyasn1_encoder.encode(pyasn1_cert))
+
     def infer_encoding(self, value: IncomingValue) -> Optional[str]:
         """Tries to infer the certificate encoding from the given value based on the data type and other surface-level
         checks. Whatever the encoding inferred, it is not guaranteed that the value is a valid certificate which will
@@ -112,9 +182,10 @@ def asn1_compliant(self, value: IncomingValue) -> Optional[bool]:
 
         Note: ``self.cast(...)`` and related methods in this class will not necessarily fail if this method returns
         ``False``. They will first attempt to re-encode the certificate using a more forgiving ASN.1 library, as there
-        are many certificates "in the wild" which are not strictly compliant.
+        are many certificates "in the wild" which are not strictly compliant [1, 2].
 
-        See https://github.com/keylime/keylime/issues/944 and https://github.com/pyca/cryptography/issues/7189
+        [1] https://github.com/keylime/keylime/issues/944
+        [2] https://github.com/pyca/cryptography/issues/7189
 
         :param value: The value in DER, Base64(DER), or PEM format (or an already deserialized certificate object)
 
@@ -143,7 +214,7 @@ def asn1_compliant(self, value: IncomingValue) -> Optional[bool]:
 
     def cast(self, value: IncomingValue) -> Optional[cryptography.x509.Certificate]:
         """Tries to interpret the given value as an X.509 certificate and convert it to a
-        `cryptography.x509.Certificate` object. Values which do not require conversion are returned unchanged.
+        ``cryptography.x509.Certificate`` object. Values which do not require conversion are returned unchanged.
 
         :param value: The value to convert (may be in DER, Base64(DER), or PEM format)
 
@@ -161,23 +232,23 @@ def cast(self, value: IncomingValue) -> Optional[cryptography.x509.Certificate]:
                 return value  # type: ignore[reportReturnType, return-value]
             case "der":
                 try:
-                    return cert_utils.x509_der_cert(value)  # type: ignore[reportArgumentType, arg-type]
-                except (binascii.Error, PyAsn1Error, SubstrateUnderrunError) as err:
+                    return self._load_der_cert(value)  # type: ignore[reportArgumentType, arg-type]
+                except PyAsn1Error as err:
                     raise ValueError(
                         f"value cast to certificate appears DER encoded but cannot be deserialized as such: {value!r}"
                     ) from err
             case "pem":
                 try:
-                    return cert_utils.x509_pem_cert(value)  # type: ignore[reportArgumentType, arg-type]
-                except (PyAsn1Error, SubstrateUnderrunError) as err:
+                    return self._load_pem_cert(value)  # type: ignore[reportArgumentType, arg-type]
+                except PyAsn1Error as err:
                     raise ValueError(
                         f"value cast to certificate appears PEM encoded but cannot be deserialized as such: "
                         f"'{str(value)}'"
                     ) from err
             case "base64":
                 try:
-                    return cert_utils.x509_der_cert(base64.b64decode(value, validate=True))  # type: ignore[reportArgumentType, arg-type]
-                except (binascii.Error, PyAsn1Error, SubstrateUnderrunError) as err:
+                    return self._load_der_cert(base64.b64decode(value, validate=True))  # type: ignore[reportArgumentType, arg-type]
+                except (binascii.Error, PyAsn1Error) as err:
                     raise ValueError(
                         f"value cast to certificate appears Base64 encoded but cannot be deserialized as such: "
                         f"'{str(value)}'"