diff --git a/SOURCES/0013-fix-malformed-certs-workaround.patch b/SOURCES/0013-fix-malformed-certs-workaround.patch new file mode 100644 index 0000000..05c3f41 --- /dev/null +++ b/SOURCES/0013-fix-malformed-certs-workaround.patch @@ -0,0 +1,1265 @@ +diff --git a/keylime/certificate_wrapper.py b/keylime/certificate_wrapper.py +new file mode 100644 +index 0000000..899a19a +--- /dev/null ++++ b/keylime/certificate_wrapper.py +@@ -0,0 +1,99 @@ ++""" ++X.509 Certificate wrapper that preserves original bytes for malformed certificates. ++ ++This module provides a wrapper around cryptography.x509.Certificate that preserves ++the original certificate bytes when the certificate required pyasn1 re-encoding ++due to ASN.1 DER non-compliance. This ensures signature validity is maintained ++throughout the database lifecycle. ++""" ++ ++import base64 ++from typing import Any, Dict, Optional ++ ++import cryptography.x509 ++from cryptography.hazmat.primitives.serialization import Encoding ++ ++ ++class CertificateWrapper: ++ """ ++ A wrapper around cryptography.x509.Certificate that preserves original bytes ++ when malformed certificates require pyasn1 re-encoding. ++ ++ This class wraps a cryptography.x509.Certificate and adds the ability ++ to store the original certificate bytes when the certificate was malformed ++ and required re-encoding using pyasn1. This ensures that signature validation ++ works correctly even for certificates that don't strictly follow ASN.1 DER. ++ """ ++ ++ def __init__(self, cert: cryptography.x509.Certificate, original_bytes: Optional[bytes] = None): ++ """ ++ Initialize the wrapper certificate. ++ ++ :param cert: The cryptography.x509.Certificate object ++ :param original_bytes: The original DER bytes if certificate was re-encoded, None otherwise ++ """ ++ self._cert = cert ++ self._original_bytes = original_bytes ++ ++ def __getattr__(self, name: str) -> Any: ++ """Delegate attribute access to the wrapped certificate.""" ++ return getattr(self._cert, name) ++ ++ def __setstate__(self, state: Dict[str, Any]) -> None: ++ """Support for pickling.""" ++ self.__dict__.update(state) ++ ++ def __getstate__(self) -> Dict[str, Any]: ++ """Support for pickling.""" ++ return self.__dict__ ++ ++ @property ++ def has_original_bytes(self) -> bool: ++ """Check if this certificate has preserved original bytes.""" ++ return self._original_bytes is not None ++ ++ @property ++ def original_bytes(self) -> Optional[bytes]: ++ """Return the preserved original bytes if available.""" ++ return self._original_bytes ++ ++ def public_bytes(self, encoding: Encoding) -> bytes: ++ """ ++ Return certificate bytes, using original bytes when available. ++ ++ For certificates with preserved original bytes, this method always uses ++ the original DER bytes to maintain signature validity. For PEM encoding, ++ it converts the original DER bytes to PEM format. ++ """ ++ if self.has_original_bytes: ++ if encoding == Encoding.DER: ++ return self._original_bytes # type: ignore[return-value] ++ if encoding == Encoding.PEM: ++ # Convert original DER bytes to PEM format ++ der_b64 = base64.b64encode(self._original_bytes).decode("utf-8") # type: ignore[arg-type] ++ # Split into 64-character lines per PEM specification (RFC 1421) ++ lines = [der_b64[i : i + 64] for i in range(0, len(der_b64), 64)] ++ # Create PEM format with proper headers ++ pem_content = "\n".join(["-----BEGIN CERTIFICATE-----"] + lines + ["-----END CERTIFICATE-----"]) + "\n" ++ return pem_content.encode("utf-8") ++ ++ # For certificates without original bytes, use standard method ++ return self._cert.public_bytes(encoding) ++ ++ # Delegate common certificate methods to maintain full compatibility ++ def __str__(self) -> str: ++ return f"CertificateWrapper(subject={self._cert.subject})" ++ ++ def __repr__(self) -> str: ++ return f"CertificateWrapper(subject={self._cert.subject}, has_original_bytes={self.has_original_bytes})" ++ ++ ++def wrap_certificate(cert: cryptography.x509.Certificate, original_bytes: Optional[bytes] = None) -> CertificateWrapper: ++ """ ++ Factory function to create a wrapped certificate. ++ ++ :param cert: The cryptography.x509.Certificate object ++ :param original_bytes: The original DER bytes if certificate was re-encoded ++ :returns: Wrapped certificate that preserves original bytes ++ """ ++ return CertificateWrapper(cert, original_bytes) +diff --git a/keylime/models/base/types/certificate.py b/keylime/models/base/types/certificate.py +index 0f03169..f6cdd48 100644 +--- a/keylime/models/base/types/certificate.py ++++ b/keylime/models/base/types/certificate.py +@@ -12,6 +12,7 @@ from pyasn1_modules import pem as pyasn1_pem + from pyasn1_modules import rfc2459 as pyasn1_rfc2459 + from sqlalchemy.types import Text + ++from keylime.certificate_wrapper import CertificateWrapper, wrap_certificate + from keylime.models.base.type import ModelType + + +@@ -78,19 +79,20 @@ class Certificate(ModelType): + cert = Certificate().cast("-----BEGIN CERTIFICATE-----\nMIIE...") + """ + +- IncomingValue = Union[cryptography.x509.Certificate, bytes, str, None] ++ IncomingValue = Union[cryptography.x509.Certificate, CertificateWrapper, bytes, str, None] + + 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 ++ def _load_der_cert(self, der_cert_data: bytes) -> CertificateWrapper: ++ """Loads a binary x509 certificate encoded using ASN.1 DER as a ``CertificateWrapper`` 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. ++ re-parsed by python-cryptography. For malformed certificates requiring re-encoding, the original bytes are ++ preserved in the wrapper to maintain signature validity. + + 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 +@@ -106,24 +108,28 @@ class Certificate(ModelType): + + :raises: :class:`SubstrateUnderrunError`: cert could not be deserialized even using the fallback pyasn1 parser + +- :returns: A ``cryptography.x509.Certificate`` object ++ :returns: A ``CertificateWrapper`` object + """ + + try: +- return cryptography.x509.load_der_x509_certificate(der_cert_data) ++ cert = cryptography.x509.load_der_x509_certificate(der_cert_data) ++ return wrap_certificate(cert, None) + 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)) ++ cert = cryptography.x509.load_der_x509_certificate(pyasn1_encoder.encode(pyasn1_cert)) ++ # Preserve the original bytes when re-encoding is necessary ++ return wrap_certificate(cert, der_cert_data) + +- def _load_pem_cert(self, pem_cert_data: str) -> cryptography.x509.Certificate: ++ def _load_pem_cert(self, pem_cert_data: str) -> CertificateWrapper: + """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 ++ ``CertificateWrapper`` 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. ++ re-parsed by python-cryptography. For malformed certificates requiring re-encoding, the original DER bytes are ++ preserved in the wrapper to maintain signature validity. + + 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 +@@ -135,19 +141,24 @@ class Certificate(ModelType): + [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 ++ :param pem_cert_data: the PEM text of the certificate + + :raises: :class:`SubstrateUnderrunError`: cert could not be deserialized even using the fallback pyasn1 parser + +- :returns: A ``cryptography.x509.Certificate`` object ++ :returns: A ``CertificateWrapper`` object + """ + + try: +- return cryptography.x509.load_pem_x509_certificate(pem_cert_data.encode("utf-8")) ++ cert = cryptography.x509.load_pem_x509_certificate(pem_cert_data.encode("utf-8")) ++ return wrap_certificate(cert, None) + 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)) ++ cert = cryptography.x509.load_der_x509_certificate(pyasn1_encoder.encode(pyasn1_cert)) ++ # Only preserve original bytes if we have valid DER data ++ original_bytes = der_data if isinstance(der_data, bytes) and der_data else None ++ # Preserve the original bytes when re-encoding is necessary ++ return wrap_certificate(cert, original_bytes) + + 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 +@@ -159,15 +170,21 @@ class Certificate(ModelType): + :returns: ``"der"`` when the value appears to be DER encoded + :returns: ``"pem"`` when the value appears to be PEM encoded + :returns: ``"base64"`` when the value appears to be Base64(DER) encoded (without PEM headers) ++ :returns: ``"wrapped"`` when the value is already a ``CertificateWrapper`` object + :returns: ``"decoded"`` when the value is already a ``cryptography.x509.Certificate`` object ++ :returns: ``"disabled"`` when the value is the string "disabled" + :returns: ``None`` when the encoding cannot be inferred + """ + # pylint: disable=no-else-return + +- if isinstance(value, cryptography.x509.Certificate): ++ if isinstance(value, CertificateWrapper): ++ return "wrapped" ++ elif isinstance(value, cryptography.x509.Certificate): + return "decoded" + elif isinstance(value, bytes): + return "der" ++ elif isinstance(value, str) and value == "disabled": ++ return "disabled" + elif isinstance(value, str) and value.startswith("-----BEGIN CERTIFICATE-----"): + return "pem" + elif isinstance(value, str): +@@ -190,19 +207,25 @@ class Certificate(ModelType): + :param value: The value in DER, Base64(DER), or PEM format (or an already deserialized certificate object) + + :returns: ``"True"`` if the value can be deserialized by python-cryptography and is ASN.1 DER compliant ++ :returns: ``"True"`` if the value is the string "disabled" (considered compliant as it's a valid field value) + :returns: ``"False"`` if the value cannot be deserialized by python-cryptography + :returns: ``None`` if the value is already a deserialized certificate of type ``cryptography.x509.Certificate`` + """ + + try: + encoding_inf = self.infer_encoding(value) ++ if encoding_inf == "wrapped": ++ # For CertificateWrapper objects, check if they have original bytes (indicating re-encoding was needed) ++ return not value.has_original_bytes # type: ignore[union-attr] + if encoding_inf == "decoded": + return None ++ if encoding_inf == "disabled": ++ return True + + if encoding_inf == "der": + cryptography.x509.load_der_x509_certificate(value) # type: ignore[reportArgumentType, arg-type] + elif encoding_inf == "pem": +- cryptography.x509.load_pem_x509_certificate(value) # type: ignore[reportArgumentType, arg-type] ++ cryptography.x509.load_pem_x509_certificate(value.encode("utf-8")) # type: ignore[reportArgumentType, arg-type, union-attr] + elif encoding_inf == "base64": + der_value = base64.b64decode(value, validate=True) # type: ignore[reportArgumentType, arg-type] + cryptography.x509.load_der_x509_certificate(der_value) +@@ -213,25 +236,27 @@ class Certificate(ModelType): + + return True + +- def cast(self, value: IncomingValue) -> Optional[cryptography.x509.Certificate]: ++ def cast(self, value: IncomingValue) -> Optional[CertificateWrapper]: + """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. ++ ``CertificateWrapper`` 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) + + :raises: :class:`TypeError`: ``value`` is of an unexpected data type + :raises: :class:`ValueError`: ``value`` does not contain data which is interpretable as a certificate + +- :returns: A ``cryptography.x509.Certificate`` object or None if an empty value is given ++ :returns: A ``CertificateWrapper`` object or None if an empty value is given + """ + + if not value: + return None + + encoding_inf = self.infer_encoding(value) ++ if encoding_inf == "wrapped": ++ return value # type: ignore[return-value] + if encoding_inf == "decoded": +- return value # type: ignore[reportReturnType, return-value] +- ++ # Wrap raw cryptography certificate without original bytes ++ return wrap_certificate(value, None) # type: ignore[arg-type] + if encoding_inf == "der": + try: + return self._load_der_cert(value) # type: ignore[reportArgumentType, arg-type] +@@ -271,7 +296,6 @@ class Certificate(ModelType): + if not cert: + return None + +- # Save as Base64-encoded value (without the PEM "BEGIN" and "END" header/footer for efficiency) + return base64.b64encode(cert.public_bytes(Encoding.DER)).decode("utf-8") + + def render(self, value: IncomingValue) -> Optional[str]: +@@ -281,9 +305,8 @@ class Certificate(ModelType): + if not cert: + return None + +- # Render certificate in PEM format + return cert.public_bytes(Encoding.PEM).decode("utf-8") # type: ignore[no-any-return] + + @property + def native_type(self) -> type: +- return cryptography.x509.Certificate ++ return CertificateWrapper +diff --git a/keylime/models/registrar/registrar_agent.py b/keylime/models/registrar/registrar_agent.py +index b232049..680316b 100644 +--- a/keylime/models/registrar/registrar_agent.py ++++ b/keylime/models/registrar/registrar_agent.py +@@ -1,7 +1,6 @@ + import base64 + import hmac + +-import cryptography.x509 + from cryptography.hazmat.primitives.asymmetric import ec, rsa + from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat + +@@ -116,35 +115,35 @@ class RegistrarAgent(PersistableModel): + if not cert_utils.verify_cert(cert, trust_store, cert_type): + self._add_error(cert_field, "must contain a certificate issued by a CA present in the trust store") + +- def _check_cert_compliance(self, cert_field, raw_cert): ++ def _check_cert_compliance(self, cert_field): + new_cert = self.changes.get(cert_field) + old_cert = self.values.get(cert_field) + + # If the certificate field has not been changed, no need to perform check +- if not raw_cert or not new_cert: ++ if not new_cert: ++ return True ++ ++ # If the certificate field is set as "disabled" (for mtls_cert) ++ if new_cert == "disabled": + return True + + # If the new certificate value is the same as the old certificate value, no need to perform check +- if ( +- isinstance(new_cert, cryptography.x509.Certificate) +- and isinstance(old_cert, cryptography.x509.Certificate) +- and new_cert.public_bytes(Encoding.DER) == old_cert.public_bytes(Encoding.DER) +- ): ++ if old_cert and new_cert.public_bytes(Encoding.DER) == old_cert.public_bytes(Encoding.DER): + return True + +- compliant = Certificate().asn1_compliant(raw_cert) ++ compliant = Certificate().asn1_compliant(new_cert) + + if not compliant: + if config.get("registrar", "malformed_cert_action") == "reject": +- self._add_error(cert_field, Certificate().generate_error_msg(raw_cert)) ++ self._add_error(cert_field, Certificate().generate_error_msg(new_cert)) + + return compliant + +- def _check_all_cert_compliance(self, data): ++ def _check_all_cert_compliance(self): + non_compliant_certs = [] + + for field_name in ("ekcert", "iak_cert", "idevid_cert", "mtls_cert"): +- if not self._check_cert_compliance(field_name, data.get(field_name)): ++ if not self._check_cert_compliance(field_name): + non_compliant_certs.append(f"'{field_name}'") + + if not non_compliant_certs: +@@ -291,7 +290,7 @@ class RegistrarAgent(PersistableModel): + # Ensure either an EK or IAK/IDevID is present, depending on configuration + self._check_root_identity_presence() + # Handle certificates which are not fully compliant with ASN.1 DER +- self._check_all_cert_compliance(data) ++ self._check_all_cert_compliance() + + # Basic validation of values + self.validate_required(["aik_tpm"]) +diff --git a/test/test_certificate_modeltype.py b/test/test_certificate_modeltype.py +new file mode 100644 +index 0000000..335ae0f +--- /dev/null ++++ b/test/test_certificate_modeltype.py +@@ -0,0 +1,197 @@ ++""" ++Unit tests for the Certificate ModelType class. ++ ++This module tests the certificate model type functionality including ++encoding inference and ASN.1 compliance checking. ++""" ++ ++import base64 ++import unittest ++ ++import cryptography.x509 ++from cryptography.hazmat.primitives.serialization import Encoding ++ ++from keylime.certificate_wrapper import CertificateWrapper, wrap_certificate ++from keylime.models.base.types.certificate import Certificate ++ ++ ++class TestCertificateModelType(unittest.TestCase): ++ """Test cases for Certificate ModelType class.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ self.cert_type = Certificate() ++ ++ # Compliant certificate for testing (loads fine with python-cryptography) ++ self.compliant_cert_pem = """-----BEGIN CERTIFICATE----- ++MIIClzCCAX+gAwIBAgIBATANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDDARUZXN0 ++MB4XDTI1MDkxMTEyNDU1MVoXDTI2MDkxMTEyNDU1MVowDzENMAsGA1UEAwwEVGVz ++dDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAO2V27HsMnKczHCaLgf9 ++FtxuorvkA5OMkz6KsW1eyryHr0TJ801prLpeNnMZ3U4pqLMqocMc7T2KO6nPZJxO ++7zRzehyo9pBBVO4pUR1QMGoTWuJQbqNieDQ4V9dW67N5wp/UWEkK6CNNd6aXjswb ++dVaDbIfDL8hMX6Lil3+pTysRWGqjRvBGJxS9r/mYRAvbz1JHPjfegSc0uxnUE+qZ ++SrbWa3TN82LX6jw6tKk0Z3CcPJC6QN+ijCxxAoHyLRYUIgZbAKe/FGRbjO0fuW11 ++L7TcE1k3eaC7RkvotIaCOW/RMOkwKu1MbCzFEA2YRYf9covEwdItzI4FE++ZJrsz ++LhUCAwEAaTANBgkqhkiG9w0BAQsFAAOCAQEAeqqJT0LnmAluAjrsCSK/eYYjwjhZ ++aKMi/iBO10zfb+GvT4yqEL5gnuWxJEx4TTcDww1clvOC1EcPUZFaKR3GIBGy0ZgJ ++zGCfg+sC6liyZ+4PSWSJHD2dT5N3IGp4/hPsrhKnVb9fYbRc0Bc5VHeS9QQoSJDH ++f9EbxCcwdErVllRter29OZCb4XnEEbTqLIKRYVrbsu/t4C+vzi0tmKg5HZXf9PMo ++D28zJGsCAr8sKW/iUKObqDOHEn56lk12NTJmJmi+g6rEikk/0czJlRjSGnJQLjUg ++d4wslruibXBsLPtJw2c6vTC2SV2F1PXwy5j1OKU+D6nxaaItQvWADEjcTg== ++-----END CERTIFICATE-----""" ++ ++ # Malformed certificate that requires pyasn1 re-encoding (fails with python-cryptography) ++ self.malformed_cert_b64 = ( ++ "MIIDUjCCAvegAwIBAgILAI5xYHQ14nH5hdYwCgYIKoZIzj0EAwIwVTFTMB8GA1UEAxMYTnV2b3Rv" ++ "biBUUE0gUm9vdCBDQSAyMTExMCUGA1UEChMeTnV2b3RvbiBUZWNobm9sb2d5IENvcnBvcmF0aW9u" ++ "MAkGA1UEBhMCVFcwHhcNMTkwNzIzMTcxNTEzWhcNMzkwNzE5MTcxNTEzWjAAMIIBIjANBgkqhkiG" ++ "9w0BAQEFAAOCAQ8AMIIBCgKCAQEAk8kCj7srY/Zlvm1795fVXdyX44w5qsd1m5VywMDgSOavzPKO" ++ "kgbHgQNx6Ak5+4Q43EJ/5qsaDBv59F8W7K69maUwcMNq1xpuq0V/LiwgJVAtc3CdvlxtwQrn7+Uq" ++ "ieIGf+i8sGxpeUCSmYHJPTHNHqjQnvUtdGoy/+WO0i7WsAvX3k/gHHr4p58a8urjJ1RG2Lk1g48D" ++ "ESwl+D7atQEPWzgjr6vK/s5KpLrn7M+dh97TUbG1510AOWBPP35MtT8IZbqC4hs2Ol16gT1M3a9e" ++ "+GaMZkItLUwV76vKDNEgTZG8M1C9OItA/xwzlfXbPepzpxWb4kzHS4qZoQtl4vBZrQIDAQABo4IB" ++ "NjCCATIwUAYDVR0RAQH/BEYwRKRCMEAxPjAUBgVngQUCARMLaWQ6NEU1NDQzMDAwEAYFZ4EFAgIT" ++ "B05QQ1Q3NXgwFAYFZ4EFAgMTC2lkOjAwMDcwMDAyMAwGA1UdEwEB/wQCMAAwEAYDVR0lBAkwBwYF" ++ "Z4EFCAEwHwYDVR0jBBgwFoAUI/TiKtO+N0pEl3KVSqKDrtdSVy4wDgYDVR0PAQH/BAQDAgUgMCIG" ++ "A1UdCQQbMBkwFwYFZ4EFAhAxDjAMDAMyLjACAQACAgCKMGkGCCsGAQUFBwEBBF0wWzBZBggrBgEF" ++ "BQcwAoZNaHR0cHM6Ly93d3cubnV2b3Rvbi5jb20vc2VjdXJpdHkvTlRDLVRQTS1FSy1DZXJ0L051" ++ "dm90b24gVFBNIFJvb3QgQ0EgMjExMS5jZXIwCgYIKoZIzj0EAwIDSQAwRgIhAPHOFiBDZd0dfml2" ++ "a/KlPFhmX7Ahpd0Wq11ZUW1/ixviAiEAlex8BB5nsR6w8QrANwCxc7fH/YnbjXfMCFiWzeZH7ps=" ++ ) ++ ++ # Load certificates for testing ++ self.compliant_cert = cryptography.x509.load_pem_x509_certificate(self.compliant_cert_pem.encode()) ++ self.malformed_cert_der = base64.b64decode(self.malformed_cert_b64) ++ ++ def test_infer_encoding_wrapped_certificate(self): ++ """Test that CertificateWrapper objects are identified as 'wrapped'.""" ++ wrapped_cert = wrap_certificate(self.compliant_cert, None) ++ encoding = self.cert_type.infer_encoding(wrapped_cert) ++ self.assertEqual(encoding, "wrapped") ++ ++ def test_infer_encoding_raw_certificate(self): ++ """Test that raw cryptography.x509.Certificate objects are identified as 'decoded'.""" ++ encoding = self.cert_type.infer_encoding(self.compliant_cert) ++ self.assertEqual(encoding, "decoded") ++ ++ def test_infer_encoding_der_bytes(self): ++ """Test that DER bytes are identified as 'der'.""" ++ der_bytes = self.compliant_cert.public_bytes(Encoding.DER) ++ encoding = self.cert_type.infer_encoding(der_bytes) ++ self.assertEqual(encoding, "der") ++ ++ def test_infer_encoding_pem_string(self): ++ """Test that PEM strings are identified as 'pem'.""" ++ encoding = self.cert_type.infer_encoding(self.compliant_cert_pem) ++ self.assertEqual(encoding, "pem") ++ ++ def test_infer_encoding_base64_string(self): ++ """Test that Base64 strings are identified as 'base64'.""" ++ encoding = self.cert_type.infer_encoding(self.malformed_cert_b64) ++ self.assertEqual(encoding, "base64") ++ ++ def test_infer_encoding_none_for_invalid(self): ++ """Test that invalid types return None.""" ++ encoding = self.cert_type.infer_encoding(12345) # type: ignore[arg-type] # Testing invalid type ++ self.assertIsNone(encoding) ++ ++ def test_asn1_compliant_wrapped_without_original_bytes(self): ++ """Test that CertificateWrapper without original bytes is ASN.1 compliant.""" ++ wrapped_cert = wrap_certificate(self.compliant_cert, None) ++ compliant = self.cert_type.asn1_compliant(wrapped_cert) ++ self.assertTrue(compliant) ++ ++ def test_asn1_compliant_wrapped_with_original_bytes(self): ++ """Test that CertificateWrapper with original bytes is not ASN.1 compliant.""" ++ wrapped_cert = wrap_certificate(self.compliant_cert, b"fake_original_bytes") ++ compliant = self.cert_type.asn1_compliant(wrapped_cert) ++ self.assertFalse(compliant) ++ ++ def test_asn1_compliant_raw_certificate(self): ++ """Test that raw cryptography.x509.Certificate returns None (already decoded).""" ++ compliant = self.cert_type.asn1_compliant(self.compliant_cert) ++ self.assertIsNone(compliant) ++ ++ def test_asn1_compliant_pem_strings(self): ++ """Test ASN.1 compliance checking on PEM strings.""" ++ # The regular certificate and TPM certificate from test_registrar_db.py are actually ASN.1 compliant ++ # and can be loaded directly by python-cryptography without requiring pyasn1 re-encoding ++ compliant_regular = self.cert_type.asn1_compliant(self.compliant_cert_pem) ++ # Only test one certificate since both are the same type (ASN.1 compliant) ++ ++ # Should be ASN.1 compliant (True) since it loads fine with python-cryptography ++ self.assertTrue(compliant_regular) ++ ++ def test_asn1_compliant_der_and_base64(self): ++ """Test ASN.1 compliance checking on DER and Base64 formats.""" ++ # Test DER bytes - regular certificate should be compliant ++ der_bytes = self.compliant_cert.public_bytes(Encoding.DER) ++ compliant_der = self.cert_type.asn1_compliant(der_bytes) ++ self.assertTrue(compliant_der) ++ ++ # Test Base64 string - regular certificate should be compliant ++ b64_string = base64.b64encode(der_bytes).decode("utf-8") ++ compliant_b64 = self.cert_type.asn1_compliant(b64_string) ++ self.assertTrue(compliant_b64) ++ ++ def test_asn1_compliant_malformed_certificate(self): ++ """Test ASN.1 compliance checking on a truly malformed certificate.""" ++ # Test the malformed certificate that requires pyasn1 re-encoding ++ compliant = self.cert_type.asn1_compliant(self.malformed_cert_b64) ++ self.assertFalse(compliant) # Should be non-compliant since it needs pyasn1 fallback ++ ++ def test_asn1_compliant_invalid_data(self): ++ """Test that invalid certificate data is not ASN.1 compliant.""" ++ compliant = self.cert_type.asn1_compliant("invalid_certificate_data") ++ self.assertFalse(compliant) ++ ++ def test_cast_wrapped_certificate(self): ++ """Test that CertificateWrapper objects are returned unchanged.""" ++ wrapped_cert = wrap_certificate(self.compliant_cert, None) ++ result = self.cert_type.cast(wrapped_cert) ++ self.assertIs(result, wrapped_cert) ++ ++ def test_cast_raw_certificate_to_wrapped(self): ++ """Test that raw certificates are wrapped without original bytes.""" ++ result = self.cert_type.cast(self.compliant_cert) ++ self.assertIsInstance(result, CertificateWrapper) ++ assert result is not None # For type checker ++ self.assertFalse(result.has_original_bytes) ++ ++ def test_cast_pem_strings(self): ++ """Test casting PEM strings to CertificateWrapper.""" ++ # Test regular certificate - should be ASN.1 compliant, no original bytes needed ++ result_regular = self.cert_type.cast(self.compliant_cert_pem) ++ self.assertIsInstance(result_regular, CertificateWrapper) ++ assert result_regular is not None # For type checker ++ self.assertFalse(result_regular.has_original_bytes) ++ ++ # Note: Only testing compliant certificate since we now use one consistent certificate for all compliant scenarios ++ ++ def test_cast_malformed_certificate(self): ++ """Test casting the malformed certificate that requires pyasn1 re-encoding.""" ++ result = self.cert_type.cast(self.malformed_cert_b64) ++ self.assertIsInstance(result, CertificateWrapper) ++ assert result is not None # For type checker ++ # Malformed certificate should have original bytes since it needs re-encoding ++ self.assertTrue(result.has_original_bytes) ++ ++ def test_cast_der_bytes(self): ++ """Test casting DER bytes to CertificateWrapper.""" ++ der_bytes = self.compliant_cert.public_bytes(Encoding.DER) ++ result = self.cert_type.cast(der_bytes) ++ self.assertIsInstance(result, CertificateWrapper) ++ ++ def test_cast_none_value(self): ++ """Test that None values return None.""" ++ result = self.cert_type.cast(None) ++ self.assertIsNone(result) ++ ++ def test_cast_empty_string(self): ++ """Test that empty strings return None.""" ++ result = self.cert_type.cast("") ++ self.assertIsNone(result) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +diff --git a/test/test_certificate_wrapper.py b/test/test_certificate_wrapper.py +new file mode 100644 +index 0000000..6b47260 +--- /dev/null ++++ b/test/test_certificate_wrapper.py +@@ -0,0 +1,385 @@ ++""" ++Unit tests for the CertificateWrapper class. ++ ++This module tests the certificate wrapper functionality that preserves original bytes ++for malformed certificates requiring pyasn1 re-encoding. ++""" ++ ++import base64 ++import subprocess ++import tempfile ++import unittest ++from unittest.mock import Mock ++ ++import cryptography.x509 ++from cryptography.hazmat.primitives.serialization import Encoding ++from pyasn1.codec.der import decoder as pyasn1_decoder ++from pyasn1.codec.der import encoder as pyasn1_encoder ++from pyasn1_modules import rfc2459 as pyasn1_rfc2459 ++ ++from keylime.certificate_wrapper import CertificateWrapper, wrap_certificate ++ ++ ++class TestCertificateWrapper(unittest.TestCase): ++ """Test cases for CertificateWrapper class.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ # Malformed certificate (Base64 encoded) that requires pyasn1 re-encoding ++ # This is a real TPM certificate that doesn't strictly follow ASN.1 DER rules ++ self.malformed_cert_b64 = ( ++ "MIIDUjCCAvegAwIBAgILAI5xYHQ14nH5hdYwCgYIKoZIzj0EAwIwVTFTMB8GA1UEAxMYTnV2b3Rv" ++ "biBUUE0gUm9vdCBDQSAyMTExMCUGA1UEChMeTnV2b3RvbiBUZWNobm9sb2d5IENvcnBvcmF0aW9u" ++ "MAkGA1UEBhMCVFcwHhcNMTkwNzIzMTcxNTEzWhcNMzkwNzE5MTcxNTEzWjAAMIIBIjANBgkqhkiG" ++ "9w0BAQEFAAOCAQ8AMIIBCgKCAQEAk8kCj7srY/Zlvm1795fVXdyX44w5qsd1m5VywMDgSOavzPKO" ++ "kgbHgQNx6Ak5+4Q43EJ/5qsaDBv59F8W7K69maUwcMNq1xpuq0V/LiwgJVAtc3CdvlxtwQrn7+Uq" ++ "ieIGf+i8sGxpeUCSmYHJPTHNHqjQnvUtdGoy/+WO0i7WsAvX3k/gHHr4p58a8urjJ1RG2Lk1g48D" ++ "ESwl+D7atQEPWzgjr6vK/s5KpLrn7M+dh97TUbG1510AOWBPP35MtT8IZbqC4hs2Ol16gT1M3a9e" ++ "+GaMZkItLUwV76vKDNEgTZG8M1C9OItA/xwzlfXbPepzpxWb4kzHS4qZoQtl4vBZrQIDAQABo4IB" ++ "NjCCATIwUAYDVR0RAQH/BEYwRKRCMEAxPjAUBgVngQUCARMLaWQ6NEU1NDQzMDAwEAYFZ4EFAgIT" ++ "B05QQ1Q3NXgwFAYFZ4EFAgMTC2lkOjAwMDcwMDAyMAwGA1UdEwEB/wQCMAAwEAYDVR0lBAkwBwYF" ++ "Z4EFCAEwHwYDVR0jBBgwFoAUI/TiKtO+N0pEl3KVSqKDrtdSVy4wDgYDVR0PAQH/BAQDAgUgMCIG" ++ "A1UdCQQbMBkwFwYFZ4EFAhAxDjAMDAMyLjACAQACAgCKMGkGCCsGAQUFBwEBBF0wWzBZBggrBgEF" ++ "BQcwAoZNaHR0cHM6Ly93d3cubnV2b3Rvbi5jb20vc2VjdXJpdHkvTlRDLVRQTS1FSy1DZXJ0L051" ++ "dm90b24gVFBNIFJvb3QgQ0EgMjExMS5jZXIwCgYIKoZIzj0EAwIDSQAwRgIhAPHOFiBDZd0dfml2" ++ "a/KlPFhmX7Ahpd0Wq11ZUW1/ixviAiEAlex8BB5nsR6w8QrANwCxc7fH/YnbjXfMCFiWzeZH7ps=" ++ ) ++ self.malformed_cert_der = base64.b64decode(self.malformed_cert_b64) ++ ++ # Create a mock certificate for testing ++ self.mock_cert = Mock(spec=cryptography.x509.Certificate) ++ self.mock_cert.subject = Mock() ++ self.mock_cert.subject.__str__ = Mock(return_value="CN=Test Certificate") ++ self.mock_cert.public_bytes.return_value = b"mock_der_data" ++ ++ def test_init_without_original_bytes(self): ++ """Test wrapper initialization without original bytes.""" ++ wrapper = CertificateWrapper(self.mock_cert) ++ ++ # Test through public interface ++ self.assertFalse(wrapper.has_original_bytes) ++ self.assertIsNone(wrapper.original_bytes) ++ # Test delegation works ++ self.assertEqual(wrapper.subject, self.mock_cert.subject) ++ ++ def test_init_with_original_bytes(self): ++ """Test wrapper initialization with original bytes.""" ++ original_data = b"original_certificate_data" ++ wrapper = CertificateWrapper(self.mock_cert, original_data) ++ ++ # Test through public interface ++ self.assertTrue(wrapper.has_original_bytes) ++ self.assertEqual(wrapper.original_bytes, original_data) ++ # Test delegation works ++ self.assertEqual(wrapper.subject, self.mock_cert.subject) ++ ++ def test_getattr_delegation(self): ++ """Test that attributes are properly delegated to the wrapped certificate.""" ++ wrapper = CertificateWrapper(self.mock_cert) ++ ++ # Access an attribute that should be delegated ++ result = wrapper.subject ++ self.assertEqual(result, self.mock_cert.subject) ++ ++ def test_public_bytes_der_without_original(self): ++ """Test public_bytes DER encoding without original bytes.""" ++ wrapper = CertificateWrapper(self.mock_cert) ++ ++ result = wrapper.public_bytes(Encoding.DER) ++ ++ self.mock_cert.public_bytes.assert_called_once_with(Encoding.DER) ++ self.assertEqual(result, b"mock_der_data") ++ ++ def test_public_bytes_der_with_original(self): ++ """Test public_bytes DER encoding with original bytes.""" ++ original_data = b"original_certificate_data" ++ wrapper = CertificateWrapper(self.mock_cert, original_data) ++ ++ result = wrapper.public_bytes(Encoding.DER) ++ ++ # Should return original bytes, not call the wrapped certificate ++ self.mock_cert.public_bytes.assert_not_called() ++ self.assertEqual(result, original_data) ++ ++ def test_public_bytes_pem_without_original(self): ++ """Test public_bytes PEM encoding without original bytes.""" ++ self.mock_cert.public_bytes.return_value = b"-----BEGIN CERTIFICATE-----\nMIIB...\n-----END CERTIFICATE-----\n" ++ wrapper = CertificateWrapper(self.mock_cert) ++ ++ result = wrapper.public_bytes(Encoding.PEM) ++ ++ self.mock_cert.public_bytes.assert_called_once_with(Encoding.PEM) ++ self.assertEqual(result, b"-----BEGIN CERTIFICATE-----\nMIIB...\n-----END CERTIFICATE-----\n") ++ ++ def test_public_bytes_pem_with_original(self): ++ """Test public_bytes PEM encoding with original bytes.""" ++ original_data = self.malformed_cert_der ++ wrapper = CertificateWrapper(self.mock_cert, original_data) ++ ++ result = wrapper.public_bytes(Encoding.PEM) ++ ++ # Should not call the wrapped certificate's method ++ self.mock_cert.public_bytes.assert_not_called() ++ ++ # Result should be PEM format derived from original bytes ++ self.assertIsInstance(result, bytes) ++ result_str = result.decode("utf-8") ++ self.assertTrue(result_str.startswith("-----BEGIN CERTIFICATE-----")) ++ self.assertTrue(result_str.endswith("-----END CERTIFICATE-----\n")) ++ ++ # Verify that the PEM content can be converted back to the original DER ++ pem_lines = result_str.strip().split("\n") ++ pem_content = "".join(pem_lines[1:-1]) # Remove headers and join ++ recovered_der = base64.b64decode(pem_content) ++ self.assertEqual(recovered_der, original_data) ++ ++ def test_pem_line_length_compliance(self): ++ """Test that PEM output follows RFC 1421 line length requirements (64 chars).""" ++ original_data = self.malformed_cert_der ++ wrapper = CertificateWrapper(self.mock_cert, original_data) ++ ++ result = wrapper.public_bytes(Encoding.PEM) ++ result_str = result.decode("utf-8") ++ ++ lines = result_str.strip().split("\n") ++ # Check that content lines (excluding headers) are max 64 chars ++ for line in lines[1:-1]: # Skip header and footer ++ self.assertLessEqual(len(line), 64) ++ ++ def test_str_representation(self): ++ """Test string representation of the wrapper.""" ++ wrapper = CertificateWrapper(self.mock_cert) ++ ++ result = str(wrapper) ++ ++ expected = f"CertificateWrapper(subject={self.mock_cert.subject})" ++ self.assertEqual(result, expected) ++ ++ def test_repr_representation_without_original(self): ++ """Test repr representation without original bytes.""" ++ wrapper = CertificateWrapper(self.mock_cert) ++ ++ result = repr(wrapper) ++ ++ expected = f"CertificateWrapper(subject={self.mock_cert.subject}, has_original_bytes=False)" ++ self.assertEqual(result, expected) ++ ++ def test_repr_representation_with_original(self): ++ """Test repr representation with original bytes.""" ++ original_data = b"original_data" ++ wrapper = CertificateWrapper(self.mock_cert, original_data) ++ ++ result = repr(wrapper) ++ ++ expected = f"CertificateWrapper(subject={self.mock_cert.subject}, has_original_bytes=True)" ++ self.assertEqual(result, expected) ++ ++ def test_pickling_support(self): ++ """Test that the wrapper supports pickling operations.""" ++ original_data = b"test_data" ++ wrapper = CertificateWrapper(self.mock_cert, original_data) ++ ++ # Test getstate ++ state = wrapper.__getstate__() ++ self.assertIsInstance(state, dict) ++ self.assertIn("_cert", state) ++ self.assertIn("_original_bytes", state) ++ ++ # Test setstate ++ new_wrapper = CertificateWrapper(Mock(), None) ++ new_wrapper.__setstate__(state) ++ # Verify state was restored correctly through public interface ++ self.assertTrue(new_wrapper.has_original_bytes) ++ self.assertEqual(new_wrapper.original_bytes, original_data) ++ ++ def test_wrap_certificate_function_without_original(self): ++ """Test the wrap_certificate factory function without original bytes.""" ++ wrapper = wrap_certificate(self.mock_cert) ++ ++ self.assertIsInstance(wrapper, CertificateWrapper) ++ self.assertFalse(wrapper.has_original_bytes) ++ self.assertIsNone(wrapper.original_bytes) ++ ++ def test_wrap_certificate_function_with_original(self): ++ """Test the wrap_certificate factory function with original bytes.""" ++ original_data = b"original_certificate_data" ++ wrapper = wrap_certificate(self.mock_cert, original_data) ++ ++ self.assertIsInstance(wrapper, CertificateWrapper) ++ self.assertTrue(wrapper.has_original_bytes) ++ self.assertEqual(wrapper.original_bytes, original_data) ++ ++ def test_real_malformed_certificate_handling(self): ++ """Test with a real malformed certificate that requires pyasn1 re-encoding.""" ++ # This test simulates the scenario where a malformed certificate is processed ++ ++ # Mock the scenario where cryptography fails but pyasn1 succeeds ++ mock_reencoded_cert = Mock(spec=cryptography.x509.Certificate) ++ mock_reencoded_cert.subject = Mock() ++ mock_reencoded_cert.subject.__str__ = Mock(return_value="CN=Nuvoton TPM") ++ ++ # Create wrapper as if it came from the certificate loading process ++ wrapper = wrap_certificate(mock_reencoded_cert, self.malformed_cert_der) ++ ++ # Test that original bytes are preserved ++ self.assertTrue(wrapper.has_original_bytes) ++ self.assertEqual(wrapper.original_bytes, self.malformed_cert_der) ++ ++ # Test DER output uses original bytes ++ der_output = wrapper.public_bytes(Encoding.DER) ++ self.assertEqual(der_output, self.malformed_cert_der) ++ ++ # Test PEM output is derived from original bytes ++ pem_output = wrapper.public_bytes(Encoding.PEM) ++ self.assertIsInstance(pem_output, bytes) ++ ++ # Verify PEM can be converted back to original DER ++ pem_str = pem_output.decode("utf-8") ++ lines = pem_str.strip().split("\n") ++ content = "".join(lines[1:-1]) ++ recovered_der = base64.b64decode(content) ++ self.assertEqual(recovered_der, self.malformed_cert_der) ++ ++ def test_unsupported_encoding_fallback(self): ++ """Test that unsupported encoding types fall back to wrapped certificate.""" ++ # Create a custom encoding that's not DER or PEM ++ custom_encoding = Mock() ++ custom_encoding.name = "CUSTOM" ++ ++ original_data = b"original_data" ++ wrapper = CertificateWrapper(self.mock_cert, original_data) ++ ++ # Should fall back to wrapped certificate for unknown encoding ++ wrapper.public_bytes(custom_encoding) ++ self.mock_cert.public_bytes.assert_called_once_with(custom_encoding) ++ ++ def test_malformed_certificate_cryptography_failure_and_verification(self): ++ """ ++ Comprehensive test demonstrating that the malformed certificate: ++ 1. Fails to load with python-cryptography ++ 2. Can be verified with OpenSSL ++ 3. Is successfully handled by our wrapper after pyasn1 re-encoding ++ """ ++ # Test 1: Demonstrate that python-cryptography fails to load the malformed certificate ++ with self.assertRaises(Exception) as context: ++ cryptography.x509.load_der_x509_certificate(self.malformed_cert_der) ++ ++ # The specific exception type may vary, but it should fail ++ self.assertIsInstance(context.exception, Exception) ++ ++ # Test 2: Demonstrate that pyasn1 can handle the malformed certificate ++ try: ++ # Decode and re-encode using pyasn1 (simulating what the Certificate type does) ++ pyasn1_cert = pyasn1_decoder.decode(self.malformed_cert_der, asn1Spec=pyasn1_rfc2459.Certificate())[0] ++ reencoded_der = pyasn1_encoder.encode(pyasn1_cert) ++ ++ # Now cryptography should be able to load the re-encoded certificate ++ reencoded_cert = cryptography.x509.load_der_x509_certificate(reencoded_der) ++ self.assertIsNotNone(reencoded_cert) ++ ++ except Exception as e: ++ self.fail(f"pyasn1 should handle the malformed certificate, but got: {e}") ++ ++ # Test 3: Verify that our wrapper preserves the original bytes correctly ++ wrapper = wrap_certificate(reencoded_cert, self.malformed_cert_der) ++ ++ # The wrapper should preserve original bytes ++ self.assertTrue(wrapper.has_original_bytes) ++ self.assertEqual(wrapper.original_bytes, self.malformed_cert_der) ++ ++ # DER output should use original bytes ++ der_output = wrapper.public_bytes(Encoding.DER) ++ self.assertEqual(der_output, self.malformed_cert_der) ++ ++ # PEM output should be derived from original bytes ++ pem_output = wrapper.public_bytes(Encoding.PEM) ++ pem_str = pem_output.decode("utf-8") ++ ++ # Verify PEM format is correct ++ self.assertTrue(pem_str.startswith("-----BEGIN CERTIFICATE-----")) ++ self.assertTrue(pem_str.endswith("-----END CERTIFICATE-----\n")) ++ ++ # Test 4: Demonstrate OpenSSL can verify the certificate structure ++ # (Even without the root CA, OpenSSL should be able to parse the certificate) ++ try: ++ with tempfile.NamedTemporaryFile(mode="wb", suffix=".der", delete=False) as temp_file: ++ temp_file.write(self.malformed_cert_der) ++ temp_file.flush() ++ ++ # Use OpenSSL to parse the certificate (should succeed) ++ result = subprocess.run( ++ ["openssl", "x509", "-in", temp_file.name, "-inform", "DER", "-text", "-noout"], ++ capture_output=True, ++ text=True, ++ check=False, ++ ) ++ ++ # OpenSSL should successfully parse the certificate ++ self.assertEqual(result.returncode, 0) ++ self.assertIn("Nuvoton TPM Root CA 2111", result.stdout) ++ self.assertIn("Certificate:", result.stdout) ++ ++ except (subprocess.CalledProcessError, FileNotFoundError) as e: ++ # Skip if OpenSSL is not available, but don't fail the test ++ self.skipTest(f"OpenSSL not available for verification test: {e}") ++ ++ # Test 5: Verify certificate details are accessible through wrapper ++ # The subject should be empty (as shown in the OpenSSL output) ++ self.assertEqual(len(reencoded_cert.subject), 0) ++ ++ # The issuer should contain Nuvoton information ++ issuer_attrs = {} ++ for attr in reencoded_cert.issuer: ++ # Use dotted string representation to avoid accessing private _name ++ oid_name = attr.oid.dotted_string ++ if oid_name == "2.5.4.3": # Common Name OID ++ issuer_attrs["commonName"] = attr.value ++ self.assertIn("commonName", issuer_attrs) ++ self.assertEqual(issuer_attrs["commonName"], "Nuvoton TPM Root CA 2111") ++ ++ # Test 6: Demonstrate that even re-encoded certificates may have parsing issues ++ # This shows why preserving original bytes is crucial ++ try: ++ # Try to access extensions - this may fail due to malformed ASN.1 ++ extensions = list(reencoded_cert.extensions) ++ # If it succeeds, verify it has the expected Subject Alternative Name ++ # Subject Alternative Name OID is 2.5.29.17 ++ has_subject_alt_name = any(ext.oid.dotted_string == "2.5.29.17" for ext in extensions) ++ self.assertTrue(has_subject_alt_name, "EK certificate should have Subject Alternative Name extension") ++ except (ValueError, Exception) as e: ++ # This is actually expected for malformed certificates! ++ # Even after pyasn1 re-encoding, some parsing issues may remain ++ self.assertIn("parsing asn1", str(e).lower(), f"Expected ASN.1 parsing error, got: {e}") ++ # This demonstrates why our wrapper preserves original bytes - ++ # they maintain signature validity even when parsing has issues ++ ++ def test_certificate_chain_verification_simulation(self): ++ """ ++ Test that simulates certificate chain verification where original bytes matter. ++ This demonstrates why preserving original bytes is crucial for signature validation. ++ """ ++ # Create a wrapper with the malformed certificate ++ mock_reencoded_cert = Mock(spec=cryptography.x509.Certificate) ++ mock_reencoded_cert.subject = Mock() ++ mock_reencoded_cert.public_key.return_value = Mock() ++ ++ wrapper = wrap_certificate(mock_reencoded_cert, self.malformed_cert_der) ++ ++ # Simulate signature verification scenario ++ # In real verification, the signature is computed over the exact DER bytes ++ original_bytes_for_verification = wrapper.public_bytes(Encoding.DER) ++ ++ # Should get the original malformed bytes (preserving signature validity) ++ self.assertEqual(original_bytes_for_verification, self.malformed_cert_der) ++ ++ # If we didn't preserve original bytes, we'd get re-encoded bytes which would ++ # invalidate the signature even though the certificate content is the same ++ mock_reencoded_cert.public_bytes.return_value = b"reencoded_different_bytes" ++ ++ # Verify that using the wrapper gets original bytes, not re-encoded bytes ++ self.assertNotEqual(original_bytes_for_verification, b"reencoded_different_bytes") ++ self.assertEqual(original_bytes_for_verification, self.malformed_cert_der) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +diff --git a/test/test_registrar_agent_cert_compliance.py b/test/test_registrar_agent_cert_compliance.py +new file mode 100644 +index 0000000..ede9b9f +--- /dev/null ++++ b/test/test_registrar_agent_cert_compliance.py +@@ -0,0 +1,289 @@ ++""" ++Integration tests for RegistrarAgent certificate compliance functionality. ++ ++This module tests the simplified certificate compliance checking methods ++to ensure they work correctly with the new CertificateWrapper-based approach. ++""" ++ ++import types ++import unittest ++from unittest.mock import Mock, patch ++ ++import cryptography.x509 ++ ++from keylime.certificate_wrapper import wrap_certificate ++from keylime.models.base.types.certificate import Certificate ++from keylime.models.registrar.registrar_agent import RegistrarAgent ++ ++ ++class TestRegistrarAgentCertCompliance(unittest.TestCase): ++ """Test cases for RegistrarAgent certificate compliance methods.""" ++ ++ # pylint: disable=protected-access,not-callable # Testing protected methods and dynamic method binding ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ # Create a test certificate ++ self.valid_cert_pem = """-----BEGIN CERTIFICATE----- ++MIIEnzCCA4egAwIBAgIEMV64bDANBgkqhkiG9w0BAQUFADBtMQswCQYDVQQGEwJE ++RTEQMA4GA1UECBMHQmF2YXJpYTEhMB8GA1UEChMYSW5maW5lb24gVGVjaG5vbG9n ++aWVzIEFHMQwwCgYDVQQLEwNBSU0xGzAZBgNVBAMTEklGWCBUUE0gRUsgUm9vdCBD ++QTAeFw0wNTEwMjAxMzQ3NDNaFw0yNTEwMjAxMzQ3NDNaMHcxCzAJBgNVBAYTAkRF ++MQ8wDQYDVQQIEwZTYXhvbnkxITAfBgNVBAoTGEluZmluZW9uIFRlY2hub2xvZ2ll ++cyBBRzEMMAoGA1UECxMDQUlNMSYwJAYDVQQDEx1JRlggVFBNIEVLIEludGVybWVk ++aWF0ZSBDQSAwMTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALftPhYN ++t4rE+JnU/XOPICbOBLvfo6iA7nuq7zf4DzsAWBdsZEdFJQfaK331ihG3IpQnlQ2i ++YtDim289265f0J4OkPFpKeFU27CsfozVaNUm6UR/uzwA8ncxFc3iZLRMRNLru/Al ++VG053ULVDQMVx2iwwbBSAYO9pGiGbk1iMmuZaSErMdb9v0KRUyZM7yABiyDlM3cz ++UQX5vLWV0uWqxdGoHwNva5u3ynP9UxPTZWHZOHE6+14rMzpobs6Ww2RR8BgF96rh ++4rRAZEl8BXhwiQq4STvUXkfvdpWH4lzsGcDDtrB6Nt3KvVNvsKz+b07Dk+Xzt+EH ++NTf3Byk2HlvX+scCAwEAAaOCATswggE3MB0GA1UdDgQWBBQ4k8292HPEIzMV4bE7 ++qWoNI8wQxzAOBgNVHQ8BAf8EBAMCAgQwEgYDVR0TAQH/BAgwBgEB/wIBADBYBgNV ++HSABAf8ETjBMMEoGC2CGSAGG+EUBBy8BMDswOQYIKwYBBQUHAgEWLWh0dHA6Ly93 ++d3cudmVyaXNpZ24uY29tL3JlcG9zaXRvcnkvaW5kZXguaHRtbDCBlwYDVR0jBIGP ++MIGMgBRW65FEhWPWcrOu1EWWC/eUDlRCpqFxpG8wbTELMAkGA1UEBhMCREUxEDAO ++BgNVBAgTB0JhdmFyaWExITAfBgNVBAoTGEluZmluZW9uIFRlY2hub2xvZ2llcyBB ++RzEMMAoGA1UECxMDQUlNMRswGQYDVQQDExJJRlggVFBNIEVLIFJvb3QgQ0GCAQMw ++DQYJKoZIhvcNAQEFBQADggEBABJ1+Ap3rNlxZ0FW0aIgdzktbNHlvXWNxFdYIBbM ++OKjmbOos0Y4O60eKPu259XmMItCUmtbzF3oKYXq6ybARUT2Lm+JsseMF5VgikSlU ++BJALqpKVjwAds81OtmnIQe2LSu4xcTSavpsL4f52cUAu/maMhtSgN9mq5roYptq9 ++DnSSDZrX4uYiMPl//rBaNDBflhJ727j8xo9CCohF3yQUoQm7coUgbRMzyO64yMIO ++3fhb+Vuc7sNwrMOz3VJN14C3JMoGgXy0c57IP/kD5zGRvljKEvrRC2I147+fPeLS ++DueRMS6lblvRKiZgmGAg7YaKOkOaEmVDMQ+fTo2Po7hI5wc= ++-----END CERTIFICATE-----""" ++ ++ self.valid_cert = cryptography.x509.load_pem_x509_certificate(self.valid_cert_pem.encode()) ++ ++ # Malformed certificate that actually requires pyasn1 re-encoding ++ self.malformed_cert_b64 = ( ++ "MIIDUjCCAvegAwIBAgILAI5xYHQ14nH5hdYwCgYIKoZIzj0EAwIwVTFTMB8GA1UEAxMYTnV2b3Rv" ++ "biBUUE0gUm9vdCBDQSAyMTExMCUGA1UEChMeTnV2b3RvbiBUZWNobm9sb2d5IENvcnBvcmF0aW9u" ++ "MAkGA1UEBhMCVFcwHhcNMTkwNzIzMTcxNTEzWhcNMzkwNzE5MTcxNTEzWjAAMIIBIjANBgkqhkiG" ++ "9w0BAQEFAAOCAQ8AMIIBCgKCAQEAk8kCj7srY/Zlvm1795fVXdyX44w5qsd1m5VywMDgSOavzPKO" ++ "kgbHgQNx6Ak5+4Q43EJ/5qsaDBv59F8W7K69maUwcMNq1xpuq0V/LiwgJVAtc3CdvlxtwQrn7+Uq" ++ "ieIGf+i8sGxpeUCSmYHJPTHNHqjQnvUtdGoy/+WO0i7WsAvX3k/gHHr4p58a8urjJ1RG2Lk1g48D" ++ "ESwl+D7atQEPWzgjr6vK/s5KpLrn7M+dh97TUbG1510AOWBPP35MtT8IZbqC4hs2Ol16gT1M3a9e" ++ "+GaMZkItLUwV76vKDNEgTZG8M1C9OItA/xwzlfXbPepzpxWb4kzHS4qZoQtl4vBZrQIDAQABo4IB" ++ "NjCCATIwUAYDVR0RAQH/BEYwRKRCMEAxPjAUBgVngQUCARMLaWQ6NEU1NDQzMDAwEAYFZ4EFAgIT" ++ "B05QQ1Q3NXgwFAYFZ4EFAgMTC2lkOjAwMDcwMDAyMAwGA1UdEwEB/wQCMAAwEAYDVR0lBAkwBwYF" ++ "Z4EFCAEwHwYDVR0jBBgwFoAUI/TiKtO+N0pEl3KVSqKDrtdSVy4wDgYDVR0PAQH/BAQDAgUgMCIG" ++ "A1UdCQQbMBkwFwYFZ4EFAhAxDjAMDAMyLjACAQACAgCKMGkGCCsGAQUFBwEBBF0wWzBZBggrBgEF" ++ "BQcwAoZNaHR0cHM6Ly93d3cubnV2b3Rvbi5jb20vc2VjdXJpdHkvTlRDLVRQTS1FSy1DZXJ0L051" ++ "dm90b24gVFBNIFJvb3QgQ0EgMjExMS5jZXIwCgYIKoZIzj0EAwIDSQAwRgIhAPHOFiBDZd0dfml2" ++ "a/KlPFhmX7Ahpd0Wq11ZUW1/ixviAiEAlex8BB5nsR6w8QrANwCxc7fH/YnbjXfMCFiWzeZH7ps=" ++ ) ++ ++ # Create wrapped certificates for testing using Certificate type to ensure proper behavior ++ cert_type = Certificate() ++ ++ # Create compliant certificate (no original bytes needed) ++ self.compliant_wrapped_cert = wrap_certificate(self.valid_cert, None) ++ ++ # Create non-compliant certificate using the malformed cert data ++ self.non_compliant_wrapped_cert = cert_type.cast(self.malformed_cert_b64) ++ ++ def create_mock_registrar_agent(self): ++ """Create a mock RegistrarAgent with necessary attributes.""" ++ agent = Mock() ++ agent.changes = {} ++ agent.values = {} ++ agent._add_error = Mock() ++ ++ # Bind the actual methods to the mock instance ++ agent._check_cert_compliance = types.MethodType(RegistrarAgent._check_cert_compliance, agent) ++ agent._check_all_cert_compliance = types.MethodType(RegistrarAgent._check_all_cert_compliance, agent) ++ ++ return agent ++ ++ def test_check_cert_compliance_no_new_cert(self): ++ """Test _check_cert_compliance when no new certificate is provided.""" ++ agent = self.create_mock_registrar_agent() ++ agent.changes = {} # No new certificate ++ ++ result = agent._check_cert_compliance("ekcert") ++ self.assertTrue(result) ++ agent._add_error.assert_not_called() ++ ++ def test_check_cert_compliance_same_cert(self): ++ """Test _check_cert_compliance when new cert is same as old cert.""" ++ agent = self.create_mock_registrar_agent() ++ agent.changes = {"ekcert": self.compliant_wrapped_cert} ++ agent.values = {"ekcert": self.compliant_wrapped_cert} ++ ++ result = agent._check_cert_compliance("ekcert") ++ self.assertTrue(result) ++ agent._add_error.assert_not_called() ++ ++ def test_check_cert_compliance_different_cert_same_der(self): ++ """Test _check_cert_compliance when certificates have same DER bytes.""" ++ agent = self.create_mock_registrar_agent() ++ # Create two different wrapper objects but with same underlying certificate ++ cert1 = wrap_certificate(self.valid_cert, None) ++ cert2 = wrap_certificate(self.valid_cert, None) ++ ++ agent.changes = {"ekcert": cert1} ++ agent.values = {"ekcert": cert2} ++ ++ result = agent._check_cert_compliance("ekcert") ++ self.assertTrue(result) ++ agent._add_error.assert_not_called() ++ ++ @patch("keylime.config.get") ++ def test_check_cert_compliance_compliant_cert(self, mock_config): ++ """Test _check_cert_compliance with ASN.1 compliant certificate.""" ++ mock_config.return_value = "warn" # Default action ++ ++ agent = self.create_mock_registrar_agent() ++ agent.changes = {"ekcert": self.compliant_wrapped_cert} ++ agent.values = {} # No old certificate ++ ++ result = agent._check_cert_compliance("ekcert") ++ self.assertTrue(result) ++ agent._add_error.assert_not_called() ++ ++ @patch("keylime.config.get") ++ def test_check_cert_compliance_non_compliant_cert_warn(self, mock_config): ++ """Test _check_cert_compliance with non-compliant certificate (warn mode).""" ++ mock_config.return_value = "warn" # Warn action ++ ++ agent = self.create_mock_registrar_agent() ++ agent.changes = {"ekcert": self.non_compliant_wrapped_cert} ++ agent.values = {} # No old certificate ++ ++ result = agent._check_cert_compliance("ekcert") ++ self.assertFalse(result) ++ agent._add_error.assert_not_called() # Should not add error in warn mode ++ ++ @patch("keylime.config.get") ++ def test_check_cert_compliance_non_compliant_cert_reject(self, mock_config): ++ """Test _check_cert_compliance with non-compliant certificate (reject mode).""" ++ mock_config.return_value = "reject" # Reject action ++ ++ agent = self.create_mock_registrar_agent() ++ agent.changes = {"ekcert": self.non_compliant_wrapped_cert} ++ agent.values = {} # No old certificate ++ ++ result = agent._check_cert_compliance("ekcert") ++ self.assertFalse(result) ++ agent._add_error.assert_called_once() # Should add error in reject mode ++ ++ @patch("keylime.config.get") ++ def test_check_all_cert_compliance_no_non_compliant(self, mock_config): ++ """Test _check_all_cert_compliance when all certificates are compliant.""" ++ mock_config.return_value = "warn" ++ ++ agent = self.create_mock_registrar_agent() ++ agent.changes = { ++ "ekcert": self.compliant_wrapped_cert, ++ "iak_cert": self.compliant_wrapped_cert, ++ } ++ agent.values = {} ++ ++ # Should not raise any exceptions or log warnings ++ with patch("keylime.models.registrar.registrar_agent.logger") as mock_logger: ++ agent._check_all_cert_compliance() ++ mock_logger.warning.assert_not_called() ++ mock_logger.error.assert_not_called() ++ ++ @patch("keylime.config.get") ++ def test_check_all_cert_compliance_with_non_compliant_warn(self, mock_config): ++ """Test _check_all_cert_compliance with non-compliant certificates (warn mode).""" ++ mock_config.return_value = "warn" ++ ++ agent = self.create_mock_registrar_agent() ++ agent.changes = { ++ "ekcert": self.non_compliant_wrapped_cert, ++ "iak_cert": self.compliant_wrapped_cert, ++ "idevid_cert": self.non_compliant_wrapped_cert, ++ } ++ agent.values = {} ++ ++ with patch("keylime.models.registrar.registrar_agent.logger") as mock_logger: ++ agent._check_all_cert_compliance() ++ # Should log warning for non-compliant certificates ++ mock_logger.warning.assert_called_once() ++ format_string = mock_logger.warning.call_args[0][0] ++ cert_names = mock_logger.warning.call_args[0][1] ++ self.assertIn("Certificate(s) %s may not conform", format_string) ++ self.assertEqual("'ekcert' and 'idevid_cert'", cert_names) ++ ++ @patch("keylime.config.get") ++ def test_check_all_cert_compliance_with_non_compliant_reject(self, mock_config): ++ """Test _check_all_cert_compliance with non-compliant certificates (reject mode).""" ++ mock_config.return_value = "reject" ++ ++ agent = self.create_mock_registrar_agent() ++ agent.changes = { ++ "ekcert": self.non_compliant_wrapped_cert, ++ "mtls_cert": self.non_compliant_wrapped_cert, ++ } ++ agent.values = {} ++ ++ with patch("keylime.models.registrar.registrar_agent.logger") as mock_logger: ++ agent._check_all_cert_compliance() ++ # Should log error for non-compliant certificates ++ mock_logger.error.assert_called_once() ++ format_string = mock_logger.error.call_args[0][0] ++ cert_names = mock_logger.error.call_args[0][1] ++ self.assertIn("Certificate(s) %s may not conform", format_string) ++ self.assertIn("were rejected due to config", format_string) ++ self.assertEqual("'ekcert' and 'mtls_cert'", cert_names) ++ ++ @patch("keylime.config.get") ++ def test_check_all_cert_compliance_ignore_mode(self, mock_config): ++ """Test _check_all_cert_compliance with ignore mode.""" ++ mock_config.return_value = "ignore" ++ ++ agent = self.create_mock_registrar_agent() ++ agent.changes = { ++ "ekcert": self.non_compliant_wrapped_cert, ++ "iak_cert": self.non_compliant_wrapped_cert, ++ } ++ agent.values = {} ++ ++ with patch("keylime.models.registrar.registrar_agent.logger") as mock_logger: ++ agent._check_all_cert_compliance() ++ # Should not log anything in ignore mode ++ mock_logger.warning.assert_not_called() ++ mock_logger.error.assert_not_called() ++ ++ def test_check_all_cert_compliance_single_non_compliant(self): ++ """Test _check_all_cert_compliance message formatting for single certificate.""" ++ agent = self.create_mock_registrar_agent() ++ agent.changes = {"ekcert": self.non_compliant_wrapped_cert} ++ agent.values = {} ++ ++ with patch("keylime.config.get", return_value="warn"): ++ with patch("keylime.models.registrar.registrar_agent.logger") as mock_logger: ++ agent._check_all_cert_compliance() ++ # Should format message correctly for single certificate ++ format_string = mock_logger.warning.call_args[0][0] ++ cert_names = mock_logger.warning.call_args[0][1] ++ self.assertIn("Certificate(s) %s may not conform", format_string) ++ self.assertEqual("'ekcert'", cert_names) ++ self.assertNotIn(" and", cert_names) # Should not have "and" for single cert ++ ++ def test_field_names_coverage(self): ++ """Test that all expected certificate field names are checked.""" ++ agent = self.create_mock_registrar_agent() ++ agent.changes = { ++ "ekcert": self.non_compliant_wrapped_cert, ++ "iak_cert": self.non_compliant_wrapped_cert, ++ "idevid_cert": self.non_compliant_wrapped_cert, ++ "mtls_cert": self.non_compliant_wrapped_cert, ++ } ++ agent.values = {} ++ ++ with patch("keylime.config.get", return_value="warn"): ++ with patch("keylime.models.registrar.registrar_agent.logger") as mock_logger: ++ agent._check_all_cert_compliance() ++ # Should check all four certificate fields ++ format_string = mock_logger.warning.call_args[0][0] ++ cert_names = mock_logger.warning.call_args[0][1] ++ self.assertIn("Certificate(s) %s may not conform", format_string) ++ expected_names = "'ekcert', 'iak_cert', 'idevid_cert' and 'mtls_cert'" ++ self.assertEqual(expected_names, cert_names) ++ ++ ++if __name__ == "__main__": ++ unittest.main() diff --git a/SOURCES/0014-Add-shared-memory-infrastructure-for-multiprocess-co.patch b/SOURCES/0014-Add-shared-memory-infrastructure-for-multiprocess-co.patch new file mode 100644 index 0000000..1a0b7c0 --- /dev/null +++ b/SOURCES/0014-Add-shared-memory-infrastructure-for-multiprocess-co.patch @@ -0,0 +1,1107 @@ +From 61a8bfb4acc28e33f5b25c54ede38fb981cdb0b4 Mon Sep 17 00:00:00 2001 +From: Sergio Correia +Date: Tue, 9 Dec 2025 11:11:43 +0000 +Subject: [PATCH 14/15] Add shared memory infrastructure for multiprocess + communication + +Backport of upstream https://github.com/keylime/keylime/pull/1817/commits/1024e19d + +Signed-off-by: Sergio Correia +--- + keylime-selinux-42.1.2/keylime.te | 2 + + keylime/cloud_verifier_tornado.py | 89 ++--- + keylime/cmd/verifier.py | 6 + + keylime/config.py | 87 +++++ + keylime/shared_data.py | 513 +++++++++++++++++++++++++ + keylime/tpm/tpm_main.py | 17 +- + keylime/web/base/default_controller.py | 6 + + test/test_shared_data.py | 199 ++++++++++ + 8 files changed, 868 insertions(+), 51 deletions(-) + create mode 100644 keylime/shared_data.py + create mode 100644 test/test_shared_data.py + +diff --git a/keylime-selinux-42.1.2/keylime.te b/keylime-selinux-42.1.2/keylime.te +index 2c6a59e..8b8a615 100644 +--- a/keylime-selinux-42.1.2/keylime.te ++++ b/keylime-selinux-42.1.2/keylime.te +@@ -77,6 +77,8 @@ optional_policy(` + allow keylime_server_t self:key { create read setattr view write }; + allow keylime_server_t self:netlink_route_socket { create_stream_socket_perms nlmsg_read }; + allow keylime_server_t self:udp_socket create_stream_socket_perms; ++allow keylime_server_t keylime_tmp_t:sock_file { create write }; ++allow keylime_server_t self:unix_stream_socket connectto; + + fs_dontaudit_search_cgroup_dirs(keylime_server_t) + +diff --git a/keylime/cloud_verifier_tornado.py b/keylime/cloud_verifier_tornado.py +index 89aa703..67ba8af 100644 +--- a/keylime/cloud_verifier_tornado.py ++++ b/keylime/cloud_verifier_tornado.py +@@ -6,8 +6,8 @@ import signal + import sys + import traceback + from concurrent.futures import ThreadPoolExecutor +-from multiprocessing import Process + from contextlib import contextmanager ++from multiprocessing import Process + from typing import Any, Dict, Iterator, List, Optional, Tuple, Union, cast + + import tornado.httpserver +@@ -27,6 +27,7 @@ from keylime import ( + json, + keylime_logging, + revocation_notifier, ++ shared_data, + signing, + tornado_requests, + web_util, +@@ -43,7 +44,6 @@ from keylime.mba import mba + + logger = keylime_logging.init_logging("verifier") + +-GLOBAL_POLICY_CACHE: Dict[str, Dict[str, str]] = {} + + set_severity_config(config.getlist("verifier", "severity_labels"), config.getlist("verifier", "severity_policy")) + +@@ -140,44 +140,41 @@ def _from_db_obj(agent_db_obj: VerfierMain) -> Dict[str, Any]: + return agent_dict + + +-def verifier_read_policy_from_cache(ima_policy_data: Dict[str, str]) -> str: +- checksum = ima_policy_data.get("checksum", "") +- name = ima_policy_data.get("name", "empty") +- agent_id = ima_policy_data.get("agent_id", "") ++def verifier_read_policy_from_cache(stored_agent: VerfierMain) -> str: ++ checksum = "" ++ name = "empty" ++ agent_id = str(stored_agent.agent_id) + +- if not agent_id: +- return "" ++ # Initialize agent policy cache if it doesn't exist ++ shared_data.initialize_agent_policy_cache(agent_id) + +- if agent_id not in GLOBAL_POLICY_CACHE: +- GLOBAL_POLICY_CACHE[agent_id] = {} +- GLOBAL_POLICY_CACHE[agent_id][""] = "" ++ if stored_agent.ima_policy: ++ checksum = str(stored_agent.ima_policy.checksum) ++ name = stored_agent.ima_policy.name + +- if checksum not in GLOBAL_POLICY_CACHE[agent_id]: +- if len(GLOBAL_POLICY_CACHE[agent_id]) > 1: +- # Perform a cleanup of the contents, IMA policy checksum changed +- logger.debug( +- "Cleaning up policy cache for policy named %s, with checksum %s, used by agent %s", +- name, +- checksum, +- agent_id, +- ) ++ # Check if policy is already cached ++ cached_policy = shared_data.get_cached_policy(agent_id, checksum) ++ if cached_policy is not None: ++ return cached_policy + +- GLOBAL_POLICY_CACHE[agent_id] = {} +- GLOBAL_POLICY_CACHE[agent_id][""] = "" ++ # Policy not cached, need to clean up and load from database ++ shared_data.cleanup_agent_policy_cache(agent_id, checksum) + +- logger.debug( +- "IMA policy named %s, with checksum %s, used by agent %s is not present on policy cache on this verifier, performing SQLAlchemy load", +- name, +- checksum, +- agent_id, +- ) ++ logger.debug( ++ "IMA policy named %s, with checksum %s, used by agent %s is not present on policy cache on this verifier, performing SQLAlchemy load", ++ name, ++ checksum, ++ agent_id, ++ ) + +- # Get the large ima_policy content - it's already loaded in ima_policy_data +- ima_policy = ima_policy_data.get("ima_policy", "") +- assert isinstance(ima_policy, str) +- GLOBAL_POLICY_CACHE[agent_id][checksum] = ima_policy ++ # Actually contacts the database and load the (large) ima_policy column for "allowlists" table ++ ima_policy = stored_agent.ima_policy.ima_policy ++ assert isinstance(ima_policy, str) + +- return GLOBAL_POLICY_CACHE[agent_id][checksum] ++ # Cache the policy for future use ++ shared_data.cache_policy(agent_id, checksum, ima_policy) ++ ++ return ima_policy + + + def verifier_db_delete_agent(session: Session, agent_id: str) -> None: +@@ -475,12 +472,11 @@ class AgentsHandler(BaseHandler): + return + + # Cleanup the cache when the agent is deleted. Do it early. +- if agent_id in GLOBAL_POLICY_CACHE: +- del GLOBAL_POLICY_CACHE[agent_id] +- logger.debug( +- "Cleaned up policy cache from all entries used by agent %s", +- agent_id, +- ) ++ shared_data.clear_agent_policy_cache(agent_id) ++ logger.debug( ++ "Cleaned up policy cache from all entries used by agent %s", ++ agent_id, ++ ) + + op_state = agent.operational_state + if op_state in (states.SAVED, states.FAILED, states.TERMINATED, states.TENANT_FAILED, states.INVALID_QUOTE): +@@ -1763,7 +1759,6 @@ async def process_agent( + stored_agent = None + + # First database operation - read agent data and extract all needed data within session context +- ima_policy_data = {} + mb_policy_data = None + with session_context() as session: + try: +@@ -1779,15 +1774,6 @@ async def process_agent( + .first() + ) + +- # Extract IMA policy data within session context to avoid DetachedInstanceError +- if stored_agent and stored_agent.ima_policy: +- ima_policy_data = { +- "checksum": str(stored_agent.ima_policy.checksum), +- "name": stored_agent.ima_policy.name, +- "agent_id": str(stored_agent.agent_id), +- "ima_policy": stored_agent.ima_policy.ima_policy, # Extract the large content too +- } +- + # Extract MB policy data within session context + if stored_agent and stored_agent.mb_policy: + mb_policy_data = stored_agent.mb_policy.mb_policy +@@ -1869,7 +1855,10 @@ async def process_agent( + logger.error("SQLAlchemy Error for agent ID %s: %s", agent["agent_id"], e) + + # Load agent's IMA policy +- runtime_policy = verifier_read_policy_from_cache(ima_policy_data) ++ if stored_agent: ++ runtime_policy = verifier_read_policy_from_cache(stored_agent) ++ else: ++ runtime_policy = "" + + # Get agent's measured boot policy + mb_policy = mb_policy_data +diff --git a/keylime/cmd/verifier.py b/keylime/cmd/verifier.py +index f3e1a86..1f9f4e5 100644 +--- a/keylime/cmd/verifier.py ++++ b/keylime/cmd/verifier.py +@@ -1,6 +1,7 @@ + from keylime import cloud_verifier_tornado, config, keylime_logging + from keylime.common.migrations import apply + from keylime.mba import mba ++from keylime.shared_data import initialize_shared_memory + + logger = keylime_logging.init_logging("verifier") + +@@ -10,6 +11,11 @@ def main() -> None: + if config.has_option("verifier", "auto_migrate_db") and config.getboolean("verifier", "auto_migrate_db"): + apply("cloud_verifier") + ++ # Initialize shared memory BEFORE creating server instance ++ # This MUST happen before verifier instantiation and worker forking ++ logger.info("Initializing shared memory manager in main process before server creation") ++ initialize_shared_memory() ++ + # Explicitly load and initialize measured boot components + mba.load_imports() + cloud_verifier_tornado.main() +diff --git a/keylime/config.py b/keylime/config.py +index e7ac634..b5cd546 100644 +--- a/keylime/config.py ++++ b/keylime/config.py +@@ -114,6 +114,85 @@ if "KEYLIME_LOGGING_CONFIG" in os.environ: + _config: Optional[Dict[str, RawConfigParser]] = None + + ++def _check_file_permissions(component: str, file_path: str) -> bool: ++ """Check if a config file has correct permissions and is readable. ++ ++ Args: ++ component: The component name (e.g., 'verifier', 'agent') ++ file_path: Path to the config file ++ ++ Returns: ++ True if file is readable, False otherwise ++ """ ++ if not os.path.exists(file_path): ++ return False ++ ++ if not os.access(file_path, os.R_OK): ++ import grp # pylint: disable=import-outside-toplevel ++ import pwd # pylint: disable=import-outside-toplevel ++ import stat # pylint: disable=import-outside-toplevel ++ ++ try: ++ file_stat = os.stat(file_path) ++ owner = pwd.getpwuid(file_stat.st_uid).pw_name ++ group = grp.getgrgid(file_stat.st_gid).gr_name ++ mode = stat.filemode(file_stat.st_mode) ++ except Exception: ++ owner = group = mode = "unknown" ++ ++ base_logger.error( # pylint: disable=logging-not-lazy ++ "=" * 80 ++ + "\n" ++ + "CRITICAL CONFIG ERROR: Config file %s exists but is not readable!\n" ++ + "File permissions: %s (owner: %s, group: %s)\n" ++ + "The keylime_%s service needs read access to this file.\n" ++ + "Fix with: chown keylime:keylime %s && chmod 440 %s\n" ++ + "=" * 80, ++ file_path, ++ mode, ++ owner, ++ group, ++ component, ++ file_path, ++ file_path, ++ ) ++ return False ++ ++ return True ++ ++ ++def _validate_config_files(component: str, file_paths: List[str], files_read: List[str]) -> None: ++ """Validate that config files were successfully parsed. ++ ++ Args: ++ component: The component name (e.g., 'verifier', 'agent') ++ file_paths: List of file paths that were attempted to be read ++ files_read: List of files that ConfigParser successfully read ++ """ ++ for file_path in file_paths: ++ # Check file permissions first ++ if not _check_file_permissions(component, file_path): ++ continue ++ ++ if file_path not in files_read: ++ base_logger.error( # pylint: disable=logging-not-lazy ++ "=" * 80 ++ + "\n" ++ + "CRITICAL CONFIG ERROR: Config file %s exists but failed to parse!\n" ++ + "This usually indicates duplicate keys within the same file.\n" ++ + "Common issues:\n" ++ + " - Same option appears multiple times in the same [%s] section\n" ++ + " - Empty values (key = ) conflicting with defined values\n" ++ + " - Invalid INI file syntax\n" ++ + "Please check the file for duplicate entries.\n" ++ + "You can validate the file with: python3 -c \"import configparser; c = configparser.RawConfigParser(); print(c.read('%s'))\"\n" ++ + "=" * 80, ++ file_path, ++ component, ++ file_path, ++ ) ++ ++ + def get_config(component: str) -> RawConfigParser: + """Find the configuration file to use for the given component and apply the + overrides defined by configuration snippets. +@@ -216,6 +295,10 @@ def get_config(component: str) -> RawConfigParser: + + # Validate that at least one config file is present + config_file = _config[component].read(c) ++ ++ # Validate the config file was parsed successfully ++ _validate_config_files(component, [c], config_file) ++ + if config_file: + base_logger.info("Reading configuration from %s", config_file) + +@@ -230,6 +313,10 @@ def get_config(component: str) -> RawConfigParser: + [os.path.join(d, f) for f in os.listdir(d) if f and os.path.isfile(os.path.join(d, f))] + ) + applied_snippets = _config[component].read(snippets) ++ ++ # Validate all snippet files were parsed successfully ++ _validate_config_files(component, snippets, applied_snippets) ++ + if applied_snippets: + base_logger.info("Applied configuration snippets from %s", d) + +diff --git a/keylime/shared_data.py b/keylime/shared_data.py +new file mode 100644 +index 0000000..23a3d81 +--- /dev/null ++++ b/keylime/shared_data.py +@@ -0,0 +1,513 @@ ++"""Shared memory management for keylime multiprocess applications. ++ ++This module provides thread-safe shared data management between processes ++using multiprocessing.Manager(). ++""" ++ ++import atexit ++import multiprocessing as mp ++import threading ++import time ++from typing import Any, Dict, List, Optional ++ ++from keylime import keylime_logging ++ ++logger = keylime_logging.init_logging("shared_data") ++ ++ ++class FlatDictView: ++ """A dictionary-like view over a flat key-value store. ++ ++ This class provides dict-like access to a subset of keys in a flat store, ++ identified by a namespace prefix. This avoids the nested DictProxy issues. ++ ++ Example: ++ store = manager.dict() # Flat store ++ view = FlatDictView(store, lock, "sessions") ++ view["123"] = "data" # Stores as "dict:sessions:123" in flat store ++ val = view["123"] # Retrieves from "dict:sessions:123" ++ """ ++ ++ def __init__(self, store: Any, lock: Any, namespace: str) -> None: ++ self._store = store ++ self._lock = lock ++ self._namespace = namespace ++ ++ def _make_key(self, key: Any) -> str: ++ """Convert user key to internal flat key with namespace prefix.""" ++ return f"dict:{self._namespace}:{key}" ++ ++ def __getitem__(self, key: Any) -> Any: ++ with self._lock: ++ return self._store[self._make_key(key)] ++ ++ def __setitem__(self, key: Any, value: Any) -> None: ++ flat_key = self._make_key(key) ++ with self._lock: ++ self._store[flat_key] = value ++ ++ def __delitem__(self, key: Any) -> None: ++ flat_key = self._make_key(key) ++ with self._lock: ++ del self._store[flat_key] ++ ++ def __contains__(self, key: Any) -> bool: ++ return self._make_key(key) in self._store ++ ++ def get(self, key: Any, default: Any = None) -> Any: ++ with self._lock: ++ return self._store.get(self._make_key(key), default) ++ ++ def keys(self) -> List[Any]: ++ """Return keys in this namespace.""" ++ prefix = f"dict:{self._namespace}:" ++ all_store_keys = list(self._store.keys()) ++ matching_keys = [k[len(prefix) :] for k in all_store_keys if k.startswith(prefix)] ++ return matching_keys ++ ++ def values(self) -> List[Any]: ++ """Return values in this namespace.""" ++ prefix = f"dict:{self._namespace}:" ++ with self._lock: ++ return [v for k, v in self._store.items() if k.startswith(prefix)] ++ ++ def items(self) -> List[tuple[Any, Any]]: ++ """Return (key, value) pairs in this namespace.""" ++ prefix = f"dict:{self._namespace}:" ++ with self._lock: ++ result = [(k[len(prefix) :], v) for k, v in self._store.items() if k.startswith(prefix)] ++ return result ++ ++ def __len__(self) -> int: ++ """Return number of items in this namespace.""" ++ return len(self.keys()) ++ ++ def __repr__(self) -> str: ++ return f"FlatDictView({self._namespace}, {len(self)} items)" ++ ++ ++class SharedDataManager: ++ """Thread-safe shared data manager for multiprocess applications. ++ ++ This class uses multiprocessing.Manager() to create proxy objects that can ++ be safely accessed from multiple processes. All data stored must be pickleable. ++ ++ Example: ++ manager = SharedDataManager() ++ ++ # Store simple data ++ manager.set_data("config_value", "some_config") ++ value = manager.get_data("config_value") ++ ++ # Work with shared dictionaries ++ agent_cache = manager.get_or_create_dict("agent_cache") ++ agent_cache["agent_123"] = {"last_seen": time.time()} ++ ++ # Work with shared lists ++ event_log = manager.get_or_create_list("events") ++ event_log.append({"type": "attestation", "agent": "agent_123"}) ++ """ ++ ++ def __init__(self) -> None: ++ """Initialize the shared data manager. ++ ++ This must be called before any process forking occurs to ensure ++ all child processes inherit access to the shared data. ++ """ ++ logger.debug("Initializing SharedDataManager") ++ ++ # Use explicit context to ensure fork compatibility ++ # The Manager must be started BEFORE any fork() calls ++ ctx = mp.get_context("fork") ++ self._manager = ctx.Manager() ++ ++ # CRITICAL FIX: Use a SINGLE flat dict instead of nested dicts ++ # Nested DictProxy objects have synchronization issues ++ # We'll use key prefixes like "dict:auth_sessions:session_id" instead ++ self._store = self._manager.dict() # Single flat store for all data ++ self._lock = self._manager.Lock() ++ self._initialized_at = time.time() ++ ++ # Register handler to reinitialize manager connection after fork ++ # This is needed because Manager uses network connections that don't survive fork ++ try: ++ import os # pylint: disable=import-outside-toplevel ++ ++ self._parent_pid = os.getpid() ++ logger.debug("SharedDataManager initialized in process %d", self._parent_pid) ++ except Exception as e: ++ logger.warning("Could not register PID tracking: %s", e) ++ ++ # Ensure cleanup on exit ++ atexit.register(self.cleanup) ++ ++ logger.info("SharedDataManager initialized successfully") ++ ++ def set_data(self, key: str, value: Any) -> None: ++ """Store arbitrary pickleable data by key. ++ ++ Args: ++ key: Unique identifier for the data ++ value: Any pickleable Python object ++ ++ Raises: ++ TypeError: If value is not pickleable ++ """ ++ with self._lock: ++ try: ++ self._store[key] = value ++ logger.debug("Stored data for key: %s", key) ++ except Exception as e: ++ logger.error("Failed to store data for key '%s': %s", key, e) ++ raise ++ ++ def get_data(self, key: str, default: Any = None) -> Any: ++ """Retrieve data by key. ++ ++ Args: ++ key: The key to retrieve ++ default: Value to return if key doesn't exist ++ ++ Returns: ++ The stored value or default if key doesn't exist ++ """ ++ with self._lock: ++ value = self._store.get(key, default) ++ logger.debug("Retrieved data for key: %s (found: %s)", key, value is not default) ++ return value ++ ++ def get_or_create_dict(self, key: str) -> Dict[str, Any]: ++ """Get or create a shared dictionary. ++ ++ Args: ++ key: Unique identifier for the dictionary ++ ++ Returns: ++ A shared dictionary-like object that syncs across processes ++ ++ Note: ++ Returns a FlatDictView that uses key prefixes in the flat store ++ instead of actual nested dicts, to avoid DictProxy nesting issues. ++ """ ++ # Mark that this namespace exists ++ namespace_key = f"__namespace__{key}" ++ if namespace_key not in self._store: ++ with self._lock: ++ self._store[namespace_key] = True ++ ++ # Return a view that operates on the flat store with key prefix ++ return FlatDictView(self._store, self._lock, key) # type: ignore[return-value,no-untyped-call] ++ ++ def get_or_create_list(self, key: str) -> List[Any]: ++ """Get or create a shared list. ++ ++ Args: ++ key: Unique identifier for the list ++ ++ Returns: ++ A shared list (proxy object) that syncs across processes ++ """ ++ with self._lock: ++ if key not in self._store: ++ self._store[key] = self._manager.list() ++ logger.debug("Created new shared list for key: %s", key) ++ else: ++ logger.debug("Retrieved existing shared list for key: %s", key) ++ return self._store[key] # type: ignore[no-any-return] ++ ++ def delete_data(self, key: str) -> bool: ++ """Delete data by key. ++ ++ Args: ++ key: The key to delete ++ ++ Returns: ++ True if the key existed and was deleted, False otherwise ++ """ ++ with self._lock: ++ if key in self._store: ++ del self._store[key] ++ logger.debug("Deleted data for key: %s", key) ++ return True ++ logger.debug("Key not found for deletion: %s", key) ++ return False ++ ++ def has_key(self, key: str) -> bool: ++ """Check if a key exists. ++ ++ Args: ++ key: The key to check ++ ++ Returns: ++ True if key exists, False otherwise ++ """ ++ with self._lock: ++ return key in self._store ++ ++ def get_keys(self) -> List[str]: ++ """Get all stored keys. ++ ++ Returns: ++ List of all keys in the store ++ """ ++ with self._lock: ++ return list(self._store.keys()) ++ ++ def clear_all(self) -> None: ++ """Clear all stored data. Use with caution!""" ++ with self._lock: ++ key_count = len(self._store) ++ self._store.clear() ++ logger.warning("Cleared all shared data (%d keys)", key_count) ++ ++ def get_stats(self) -> Dict[str, Any]: ++ """Get statistics about stored data. ++ ++ Returns: ++ Dictionary containing storage statistics ++ """ ++ with self._lock: ++ return { ++ "total_keys": len(self._store), ++ "initialized_at": self._initialized_at, ++ "uptime_seconds": time.time() - self._initialized_at, ++ } ++ ++ def cleanup(self) -> None: ++ """Cleanup shared resources. ++ ++ This is automatically called on exit but can be called manually ++ for explicit cleanup. ++ """ ++ if hasattr(self, "_manager"): ++ logger.debug("Shutting down SharedDataManager") ++ try: ++ self._manager.shutdown() ++ logger.info("SharedDataManager shutdown complete") ++ except Exception as e: ++ logger.error("Error during SharedDataManager shutdown: %s", e) ++ ++ def __repr__(self) -> str: ++ stats = self.get_stats() ++ return f"SharedDataManager(keys={stats['total_keys']}, " f"uptime={stats['uptime_seconds']:.1f}s)" ++ ++ @property ++ def manager(self) -> Any: # type: ignore[misc] ++ """Access to the underlying multiprocessing Manager for advanced usage.""" ++ return self._manager ++ ++ ++# Global shared memory manager instance ++_global_shared_manager: Optional[SharedDataManager] = None ++_manager_lock = threading.Lock() ++ ++ ++def initialize_shared_memory() -> SharedDataManager: ++ """Initialize the global shared memory manager. ++ ++ This function MUST be called before any process forking occurs to ensure ++ all child processes share the same manager instance. ++ ++ For tornado/multiprocess servers, call this before starting workers. ++ ++ Returns: ++ SharedDataManager: The global shared memory manager instance ++ ++ Raises: ++ RuntimeError: If called after manager is already initialized ++ """ ++ global _global_shared_manager ++ ++ with _manager_lock: ++ if _global_shared_manager is not None: ++ logger.warning("Shared memory manager already initialized, returning existing instance") ++ return _global_shared_manager ++ ++ logger.info("Initializing global shared memory manager") ++ _global_shared_manager = SharedDataManager() ++ logger.info("Global shared memory manager initialized") ++ ++ return _global_shared_manager ++ ++ ++def get_shared_memory() -> SharedDataManager: ++ """Get the global shared memory manager instance. ++ ++ This function returns a singleton SharedDataManager that can be used ++ throughout keylime for caching and inter-process communication. ++ ++ The manager is automatically initialized on first access and cleaned up ++ on process exit. ++ ++ IMPORTANT: In multiprocess applications (like tornado with workers), ++ you MUST call initialize_shared_memory() BEFORE forking workers. ++ Otherwise each worker will get its own separate manager. ++ ++ Returns: ++ SharedDataManager: The global shared memory manager instance ++ """ ++ global _global_shared_manager ++ ++ if _global_shared_manager is None: ++ with _manager_lock: ++ if _global_shared_manager is None: ++ logger.info("Initializing global shared memory manager") ++ _global_shared_manager = SharedDataManager() # type: ignore[no-untyped-call] ++ logger.info("Global shared memory manager initialized") ++ ++ return _global_shared_manager ++ ++ ++def cleanup_global_shared_memory() -> None: ++ """Cleanup the global shared memory manager. ++ ++ This is automatically called on exit but can be called manually. ++ """ ++ global _global_shared_manager ++ ++ if _global_shared_manager is not None: ++ logger.info("Cleaning up global shared memory manager") ++ _global_shared_manager.cleanup() ++ _global_shared_manager = None ++ ++ ++# Convenience functions for common keylime patterns ++ ++ ++def cache_policy(agent_id: str, checksum: str, policy: str) -> None: ++ """Cache a policy in shared memory. ++ ++ Args: ++ agent_id: The agent identifier ++ checksum: The policy checksum ++ policy: The policy content to cache ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ ++ if agent_id not in policy_cache: ++ policy_cache[agent_id] = manager.manager.dict() # type: ignore[attr-defined] ++ ++ policy_cache[agent_id][checksum] = policy ++ logger.debug("Cached policy for agent %s with checksum %s", agent_id, checksum) ++ ++ ++def get_cached_policy(agent_id: str, checksum: str) -> Optional[str]: ++ """Retrieve cached policy. ++ ++ Args: ++ agent_id: The agent identifier ++ checksum: The policy checksum ++ ++ Returns: ++ The cached policy content or None if not found ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ agent_policies = policy_cache.get(agent_id, {}) ++ ++ result = agent_policies.get(checksum) ++ if result: ++ logger.debug("Found cached policy for agent %s with checksum %s", agent_id, checksum) ++ else: ++ logger.debug("No cached policy found for agent %s with checksum %s", agent_id, checksum) ++ ++ return result # type: ignore[no-any-return] ++ ++ ++def clear_agent_policy_cache(agent_id: str) -> None: ++ """Clear all cached policies for an agent. ++ ++ Args: ++ agent_id: The agent identifier ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ ++ if agent_id in policy_cache: ++ del policy_cache[agent_id] ++ logger.debug("Cleared policy cache for agent %s", agent_id) ++ ++ ++def cleanup_agent_policy_cache(agent_id: str, keep_checksum: str = "") -> None: ++ """Clean up agent policy cache, keeping only the specified checksum. ++ ++ This mimics the cleanup behavior from GLOBAL_POLICY_CACHE where when ++ a new policy checksum is encountered, old cached policies are removed. ++ ++ Args: ++ agent_id: The agent identifier ++ keep_checksum: The checksum to keep in the cache (empty string by default) ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ ++ if agent_id in policy_cache and len(policy_cache[agent_id]) > 1: ++ # Keep only the empty entry and the specified checksum ++ old_policies = dict(policy_cache[agent_id]) ++ policy_cache[agent_id] = manager.manager.dict() ++ ++ # Always keep the empty entry ++ policy_cache[agent_id][""] = old_policies.get("", "") ++ ++ # Keep the specified checksum if it exists and is not empty ++ if keep_checksum and keep_checksum in old_policies: ++ policy_cache[agent_id][keep_checksum] = old_policies[keep_checksum] ++ ++ logger.debug("Cleaned up policy cache for agent %s, keeping checksum %s", agent_id, keep_checksum) ++ ++ ++def initialize_agent_policy_cache(agent_id: str) -> Dict[str, Any]: ++ """Initialize policy cache for an agent if it doesn't exist. ++ ++ Args: ++ agent_id: The agent identifier ++ ++ Returns: ++ The agent's policy cache dictionary ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ ++ if agent_id not in policy_cache: ++ policy_cache[agent_id] = manager.manager.dict() # type: ignore[attr-defined] ++ policy_cache[agent_id][""] = "" ++ logger.debug("Initialized policy cache for agent %s", agent_id) ++ ++ return policy_cache[agent_id] # type: ignore[no-any-return] ++ ++ ++def get_agent_cache(agent_id: str) -> Dict[str, Any]: ++ """Get shared cache for a specific agent. ++ ++ Args: ++ agent_id: The agent identifier ++ ++ Returns: ++ A shared dictionary for caching agent-specific data ++ """ ++ manager = get_shared_memory() ++ return manager.get_or_create_dict(f"agent_cache:{agent_id}") ++ ++ ++def get_verification_queue(agent_id: str) -> List[Any]: ++ """Get verification queue for batching database operations. ++ ++ Args: ++ agent_id: The agent identifier ++ ++ Returns: ++ A shared list for queuing verification operations ++ """ ++ manager = get_shared_memory() ++ return manager.get_or_create_list(f"verification_queue:{agent_id}") ++ ++ ++def get_shared_stats() -> Dict[str, Any]: ++ """Get statistics about shared memory usage. ++ ++ Returns: ++ Dictionary containing storage statistics ++ """ ++ manager = get_shared_memory() ++ return manager.get_stats() +diff --git a/keylime/tpm/tpm_main.py b/keylime/tpm/tpm_main.py +index 6f2e89f..9b54fc3 100644 +--- a/keylime/tpm/tpm_main.py ++++ b/keylime/tpm/tpm_main.py +@@ -10,7 +10,7 @@ from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey + + from keylime import cert_utils, config, json, keylime_logging + from keylime.agentstates import AgentAttestState, TPMClockInfo +-from keylime.common.algorithms import Hash ++from keylime.common.algorithms import Hash, Sign + from keylime.failure import Component, Failure + from keylime.ima import ima + from keylime.ima.file_signatures import ImaKeyrings +@@ -50,6 +50,21 @@ class Tpm: + + return (keyblob, key) + ++ # Mapping from keylime.common.algorithms enums to TPM algorithm constants ++ # Used for validating that TPM attestations use expected cryptographic algorithms ++ HASH_ALG_TO_TPM = { ++ Hash.SHA1: tpm2_objects.TPM_ALG_SHA1, ++ Hash.SHA256: tpm2_objects.TPM_ALG_SHA256, ++ Hash.SHA384: tpm2_objects.TPM_ALG_SHA384, ++ Hash.SHA512: tpm2_objects.TPM_ALG_SHA512, ++ } ++ ++ SIGN_ALG_TO_TPM = { ++ Sign.RSASSA: tpm2_objects.TPM_ALG_RSASSA, ++ Sign.RSAPSS: tpm2_objects.TPM_ALG_RSAPSS, ++ Sign.ECDSA: tpm2_objects.TPM_ALG_ECDSA, ++ } ++ + @staticmethod + def verify_aik_with_iak(uuid: str, aik_tpm: bytes, iak_tpm: bytes, iak_attest: bytes, iak_sign: bytes) -> bool: + attest_body = iak_attest.split(b"\x00$")[1] +diff --git a/keylime/web/base/default_controller.py b/keylime/web/base/default_controller.py +index 971ed06..ba0782e 100644 +--- a/keylime/web/base/default_controller.py ++++ b/keylime/web/base/default_controller.py +@@ -19,6 +19,12 @@ class DefaultController(Controller): + self.send_response(400, "Bad Request") + + def malformed_params(self, **_params: Any) -> None: ++ import traceback # pylint: disable=import-outside-toplevel ++ ++ from keylime import keylime_logging # pylint: disable=import-outside-toplevel ++ ++ logger = keylime_logging.init_logging("web") ++ logger.error("Malformed params error. Traceback: %s", traceback.format_exc()) + self.send_response(400, "Malformed Request Parameter") + + def action_dispatch_error(self, **_param: Any) -> None: +diff --git a/test/test_shared_data.py b/test/test_shared_data.py +new file mode 100644 +index 0000000..8de7e64 +--- /dev/null ++++ b/test/test_shared_data.py +@@ -0,0 +1,199 @@ ++"""Unit tests for shared memory infrastructure.""" ++ ++import unittest ++ ++from keylime.shared_data import ( ++ SharedDataManager, ++ cache_policy, ++ cleanup_agent_policy_cache, ++ cleanup_global_shared_memory, ++ clear_agent_policy_cache, ++ get_cached_policy, ++ get_shared_memory, ++ initialize_agent_policy_cache, ++) ++ ++ ++class TestSharedDataManager(unittest.TestCase): ++ """Test cases for SharedDataManager class.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ self.manager = SharedDataManager() ++ ++ def tearDown(self): ++ """Clean up after tests.""" ++ if self.manager: ++ self.manager.cleanup() ++ ++ def test_set_and_get_data(self): ++ """Test basic set and get operations.""" ++ self.manager.set_data("test_key", "test_value") ++ result = self.manager.get_data("test_key") ++ self.assertEqual(result, "test_value") ++ ++ def test_get_nonexistent_data(self): ++ """Test getting data that doesn't exist returns None.""" ++ result = self.manager.get_data("nonexistent_key") ++ self.assertIsNone(result) ++ ++ def test_get_data_with_default(self): ++ """Test getting data with default value.""" ++ result = self.manager.get_data("nonexistent_key", default="default_value") ++ self.assertEqual(result, "default_value") ++ ++ def test_delete_data(self): ++ """Test deleting data.""" ++ self.manager.set_data("test_key", "test_value") ++ result = self.manager.delete_data("test_key") ++ self.assertTrue(result) ++ ++ # Verify it's actually deleted ++ self.assertIsNone(self.manager.get_data("test_key")) ++ ++ def test_delete_nonexistent_data(self): ++ """Test deleting data that doesn't exist returns False.""" ++ result = self.manager.delete_data("nonexistent_key") ++ self.assertFalse(result) ++ ++ def test_has_key(self): ++ """Test checking if key exists.""" ++ self.manager.set_data("test_key", "test_value") ++ self.assertTrue(self.manager.has_key("test_key")) ++ self.assertFalse(self.manager.has_key("nonexistent_key")) ++ ++ def test_get_or_create_dict(self): ++ """Test getting or creating a shared dictionary.""" ++ shared_dict = self.manager.get_or_create_dict("test_dict") ++ shared_dict["key1"] = "value1" ++ shared_dict["key2"] = "value2" ++ ++ # Retrieve the same dict ++ retrieved_dict = self.manager.get_or_create_dict("test_dict") ++ self.assertEqual(retrieved_dict["key1"], "value1") ++ self.assertEqual(retrieved_dict["key2"], "value2") ++ ++ def test_get_or_create_list(self): ++ """Test getting or creating a shared list.""" ++ shared_list = self.manager.get_or_create_list("test_list") ++ shared_list.append("item1") ++ shared_list.append("item2") ++ ++ # Retrieve the same list ++ retrieved_list = self.manager.get_or_create_list("test_list") ++ self.assertEqual(len(retrieved_list), 2) ++ self.assertEqual(retrieved_list[0], "item1") ++ self.assertEqual(retrieved_list[1], "item2") ++ ++ def test_get_stats(self): ++ """Test getting manager statistics.""" ++ self.manager.set_data("key1", "value1") ++ self.manager.set_data("key2", "value2") ++ ++ stats = self.manager.get_stats() ++ self.assertIn("total_keys", stats) ++ self.assertIn("uptime_seconds", stats) ++ self.assertEqual(stats["total_keys"], 2) ++ self.assertGreaterEqual(stats["uptime_seconds"], 0) ++ ++ ++class TestPolicyCacheFunctions(unittest.TestCase): ++ """Test cases for policy cache functions.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ # Get the global shared memory manager ++ self.manager = get_shared_memory() ++ ++ def tearDown(self): ++ """Clean up after tests.""" ++ # Clean up global shared memory ++ cleanup_global_shared_memory() ++ ++ def test_initialize_agent_policy_cache(self): ++ """Test initializing agent policy cache.""" ++ agent_id = "test_agent_123" ++ initialize_agent_policy_cache(agent_id) ++ ++ # Verify the cache was initialized ++ policy_cache = self.manager.get_or_create_dict("policy_cache") ++ self.assertIn(agent_id, policy_cache) ++ ++ def test_cache_and_get_policy(self): ++ """Test caching and retrieving a policy.""" ++ agent_id = "test_agent_123" ++ checksum = "abc123def456" ++ policy_content = '{"policy": "test_policy_content"}' ++ ++ # Initialize and cache policy ++ initialize_agent_policy_cache(agent_id) ++ cache_policy(agent_id, checksum, policy_content) ++ ++ # Retrieve cached policy ++ cached = get_cached_policy(agent_id, checksum) ++ self.assertEqual(cached, policy_content) ++ ++ def test_get_nonexistent_cached_policy(self): ++ """Test getting a policy that hasn't been cached.""" ++ agent_id = "test_agent_123" ++ checksum = "nonexistent_checksum" ++ ++ initialize_agent_policy_cache(agent_id) ++ cached = get_cached_policy(agent_id, checksum) ++ self.assertIsNone(cached) ++ ++ def test_clear_agent_policy_cache(self): ++ """Test clearing an agent's policy cache.""" ++ agent_id = "test_agent_123" ++ checksum = "abc123def456" ++ policy_content = '{"policy": "test_policy_content"}' ++ ++ # Initialize, cache, and then clear ++ initialize_agent_policy_cache(agent_id) ++ cache_policy(agent_id, checksum, policy_content) ++ clear_agent_policy_cache(agent_id) ++ ++ # Verify it's cleared ++ cached = get_cached_policy(agent_id, checksum) ++ self.assertIsNone(cached) ++ ++ def test_cleanup_agent_policy_cache(self): ++ """Test cleaning up old policy checksums.""" ++ agent_id = "test_agent_123" ++ old_checksum = "old_checksum" ++ new_checksum = "new_checksum" ++ policy_content = '{"policy": "test"}' ++ ++ # Initialize and cache multiple policies ++ initialize_agent_policy_cache(agent_id) ++ cache_policy(agent_id, old_checksum, policy_content) ++ cache_policy(agent_id, new_checksum, policy_content) ++ ++ # Cleanup old checksums (keeping only new_checksum) ++ cleanup_agent_policy_cache(agent_id, new_checksum) ++ ++ # Verify old checksum is removed but new one remains ++ self.assertIsNone(get_cached_policy(agent_id, old_checksum)) ++ self.assertEqual(get_cached_policy(agent_id, new_checksum), policy_content) ++ ++ def test_cache_multiple_agents(self): ++ """Test caching policies for multiple agents.""" ++ agent1 = "agent_1" ++ agent2 = "agent_2" ++ checksum = "same_checksum" ++ policy1 = '{"policy": "agent1_policy"}' ++ policy2 = '{"policy": "agent2_policy"}' ++ ++ # Cache policies for different agents ++ initialize_agent_policy_cache(agent1) ++ initialize_agent_policy_cache(agent2) ++ cache_policy(agent1, checksum, policy1) ++ cache_policy(agent2, checksum, policy2) ++ ++ # Verify each agent has its own policy ++ self.assertEqual(get_cached_policy(agent1, checksum), policy1) ++ self.assertEqual(get_cached_policy(agent2, checksum), policy2) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +-- +2.47.3 + diff --git a/SOURCES/0015-Fix-registrar-duplicate-UUID-vulnerability.patch b/SOURCES/0015-Fix-registrar-duplicate-UUID-vulnerability.patch new file mode 100644 index 0000000..2a18765 --- /dev/null +++ b/SOURCES/0015-Fix-registrar-duplicate-UUID-vulnerability.patch @@ -0,0 +1,1188 @@ +From fcf665cf4770c0fceb6cad8bee3533e3b82271c7 Mon Sep 17 00:00:00 2001 +From: Sergio Correia +Date: Tue, 9 Dec 2025 12:12:22 +0000 +Subject: [PATCH 15/15] Fix registrar duplicate UUID vulnerability + +Backport upstream PR#1825 + +Signed-off-by: Sergio Correia +--- + keylime/cmd/registrar.py | 6 + + keylime/models/registrar/registrar_agent.py | 116 +++++ + keylime/shared_data.py | 6 + + keylime/web/registrar/agents_controller.py | 98 +++- + test/test_agents_controller.py | 513 ++++++++++++++++++++ + test/test_registrar_tpm_identity.py | 342 +++++++++++++ + 6 files changed, 1071 insertions(+), 10 deletions(-) + create mode 100644 test/test_agents_controller.py + create mode 100644 test/test_registrar_tpm_identity.py + +diff --git a/keylime/cmd/registrar.py b/keylime/cmd/registrar.py +index 584275a..2e2b25e 100644 +--- a/keylime/cmd/registrar.py ++++ b/keylime/cmd/registrar.py +@@ -5,6 +5,7 @@ import cryptography + from keylime import config, keylime_logging + from keylime.common.migrations import apply + from keylime.models import da_manager, db_manager ++from keylime.shared_data import initialize_shared_memory + from keylime.web import RegistrarServer + + logger = keylime_logging.init_logging("registrar") +@@ -47,6 +48,11 @@ def main() -> None: + # Prepare backend for durable attestation, if configured + da_manager.make_backend("registrar") + ++ # Initialize shared memory for cross-process synchronization ++ # CRITICAL: Must be called before server.start_multi() to ensure all forked ++ # worker processes share the same manager instance for agent registration locks ++ initialize_shared_memory() ++ + # Start HTTP server + server = RegistrarServer() + server.start_multi() +diff --git a/keylime/models/registrar/registrar_agent.py b/keylime/models/registrar/registrar_agent.py +index 680316b..d071029 100644 +--- a/keylime/models/registrar/registrar_agent.py ++++ b/keylime/models/registrar/registrar_agent.py +@@ -65,8 +65,14 @@ class RegistrarAgent(PersistableModel): + def empty(cls): + agent = super().empty() + agent.provider_keys = {} ++ object.__setattr__(agent, "_tpm_identity_violation", False) + return agent + ++ @property ++ def has_tpm_identity_violation(self): ++ """Returns True if a TPM identity violation was detected during validation.""" ++ return getattr(self, "_tpm_identity_violation", False) ++ + def _check_key_against_cert(self, tpm_key_field, cert_field): + # If neither key nor certificate is being updated, no need to check + if tpm_key_field not in self.changes and cert_field not in self.changes: +@@ -139,6 +145,111 @@ class RegistrarAgent(PersistableModel): + + return compliant + ++ def _check_tpm_identity_immutable(self): ++ """ ++ Checks that TPM identity fields are not being changed during re-registration. ++ ++ This prevents an attacker from registering with the same UUID but a different TPM, ++ which would allow them to impersonate the original agent and bypass attestation. ++ ++ Checked fields (EK-based identity only): ++ - ek_tpm: Endorsement Key (primary TPM identity) ++ - ekcert: EK Certificate (binds EK to TPM manufacturer) ++ - aik_tpm: Attestation Key (bound to EK via MakeCredential/ActivateCredential) ++ ++ Note: IAK/IDevID fields are NOT checked and can change on re-registration. ++ ++ This check only applies to existing agents (those loaded from the database). ++ New agents created via RegistrarAgent.empty() have no committed values and are ++ allowed to set identity fields during initial registration. ++ ++ If the agent needs to be registered with a new TPM (e.g., hardware replacement), ++ the old agent record must be explicitly deleted first. ++ """ ++ # Define TPM identity fields that must remain immutable once set ++ # Only checking EK-based identity (ek_tpm, ekcert, aik_tpm) ++ # IAK/IDevID fields (iak_tpm, iak_cert, idevid_tpm, idevid_cert) are not checked ++ identity_fields = ["ek_tpm", "ekcert", "aik_tpm"] ++ ++ # Only check for existing agents (those loaded from database) ++ # New agents created via empty() will have no committed values ++ if not self.committed: ++ return ++ ++ # Track which fields have been changed ++ changed_fields = [] ++ ++ for field_name in identity_fields: ++ # Skip fields that are not being changed in this update ++ if field_name not in self.changes: ++ continue ++ ++ # Get the old (committed/database) and new (proposed) values ++ old_value = self.committed.get(field_name) ++ new_value = self.changes.get(field_name) ++ ++ # Allow setting a previously unset field (e.g., adding EK cert later) ++ if old_value is None: ++ continue ++ ++ # Reject attempts to remove an already-set identity field ++ if new_value is None: ++ changed_fields.append(field_name) ++ continue ++ ++ # Compare values based on field type ++ if field_name == "ekcert": ++ # For certificates, compare the actual certificate bytes ++ # Note: We compare full certificate, not just public key, because: ++ # 1. User requirement: reject if certificate changed even if same public key ++ # 2. Certificate contains more than just key (issuer, validity period, etc.) ++ # 3. Different cert for same key could indicate compromise or unauthorized replacement ++ try: ++ old_cert_bytes = old_value.public_bytes(Encoding.DER) ++ new_cert_bytes = new_value.public_bytes(Encoding.DER) ++ ++ if old_cert_bytes != new_cert_bytes: ++ changed_fields.append(field_name) ++ except Exception: ++ # If we can't extract certificate bytes, treat as changed to be safe ++ changed_fields.append(field_name) ++ else: ++ # For TPM keys (ek_tpm, aik_tpm), compare as binary data ++ # These are Binary(persist_as=String) fields, so they could be bytes or base64 strings ++ try: ++ old_bytes = old_value if isinstance(old_value, bytes) else base64.b64decode(old_value) ++ new_bytes = new_value if isinstance(new_value, bytes) else base64.b64decode(new_value) ++ ++ if old_bytes != new_bytes: ++ changed_fields.append(field_name) ++ except Exception: ++ # If comparison fails (e.g., invalid base64), treat as changed to be safe ++ changed_fields.append(field_name) ++ ++ # If any TPM identity fields were changed, this is a security violation ++ if changed_fields: ++ # Set flag to indicate TPM identity violation occurred ++ object.__setattr__(self, "_tpm_identity_violation", True) ++ ++ # Log security warning for audit trail ++ # Include agent_id and changed fields, but NOT the actual TPM values (sensitive data) ++ logger.warning( ++ "SECURITY: Rejected attempt to re-register agent '%s' with different TPM identity. " ++ "Changed fields: %s. This indicates a potential UUID spoofing attack. " ++ "The existing agent must be deleted before registering with a new TPM. " ++ "If this is unexpected, investigate for compromise.", ++ self.agent_id, ++ ", ".join(changed_fields), ++ ) ++ ++ # Add validation error to prevent registration ++ # Using "agent_id" field for the error because it's the UUID that's being improperly reused ++ self._add_error( ++ "agent_id", ++ f"cannot re-register with different TPM identity. Changed fields: {', '.join(changed_fields)}. " ++ "To register this UUID with a new TPM, delete the existing agent record first.", ++ ) ++ + def _check_all_cert_compliance(self): + non_compliant_certs = [] + +@@ -281,6 +392,11 @@ class RegistrarAgent(PersistableModel): + + ["port", "mtls_cert"], + ) + ++ # SECURITY CHECK: Verify TPM identity is not being changed on re-registration ++ # This must happen after cast_changes() (so we have new values to compare) ++ # but before other validation (so we reject immediately without processing further) ++ self._check_tpm_identity_immutable() ++ + # Log info about received EK or IAK/IDevID + self._log_root_identity() + # Verify EK as valid +diff --git a/keylime/shared_data.py b/keylime/shared_data.py +index 23a3d81..a415496 100644 +--- a/keylime/shared_data.py ++++ b/keylime/shared_data.py +@@ -58,6 +58,12 @@ class FlatDictView: + with self._lock: + return self._store.get(self._make_key(key), default) + ++ def pop(self, key: Any, default: Any = None) -> Any: ++ """Remove and return value for key, or default if key not present.""" ++ flat_key = self._make_key(key) ++ with self._lock: ++ return self._store.pop(flat_key, default) ++ + def keys(self) -> List[Any]: + """Return keys in this namespace.""" + prefix = f"dict:{self._namespace}:" +diff --git a/keylime/web/registrar/agents_controller.py b/keylime/web/registrar/agents_controller.py +index 9be2ef9..f2246de 100644 +--- a/keylime/web/registrar/agents_controller.py ++++ b/keylime/web/registrar/agents_controller.py +@@ -1,5 +1,8 @@ ++from sqlalchemy.exc import IntegrityError ++ + from keylime import keylime_logging + from keylime.models import RegistrarAgent ++from keylime.shared_data import get_shared_memory + from keylime.web.base import Controller + + logger = keylime_logging.init_logging("registrar") +@@ -28,16 +31,91 @@ class AgentsController(Controller): + + # POST /v2[.:minor]/agents/[:agent_id] + def create(self, agent_id, **params): +- agent = RegistrarAgent.get(agent_id) or RegistrarAgent.empty() # type: ignore[no-untyped-call] +- agent.update({"agent_id": agent_id, **params}) +- challenge = agent.produce_ak_challenge() +- +- if not challenge or not agent.changes_valid: +- self.log_model_errors(agent, logger) +- self.respond(400, "Could not register agent with invalid data") +- return +- +- agent.commit_changes() ++ """Register a new agent or re-register an existing agent. ++ ++ For new agents, this: ++ 1. Validates TPM identity (EK/AIK or IAK/IDevID) ++ 2. Generates an AK challenge encrypted with the EK ++ 3. Stores agent record in pending state ++ 4. Returns challenge blob to agent ++ ++ For existing agents (re-registration with same UUID): ++ 1. Verifies TPM identity has not changed (security check) ++ 2. If identity changed: rejects with 403 Forbidden ++ 3. If identity same: allows re-registration (e.g., after agent restart) ++ ++ Security: Re-registration with a different TPM is forbidden to prevent ++ UUID spoofing attacks where an attacker could impersonate a legitimate ++ agent by reusing its UUID. ++ ++ Race condition protection: Uses per-agent locks from SharedDataManager to prevent ++ race conditions between concurrent registration requests for the same agent_id. ++ This ensures the check-validate-commit sequence is atomic. Additionally, database ++ constraint violations (e.g., duplicate UUIDs from concurrent requests) are caught ++ and returned as 403 Forbidden. ++ """ ++ # Get shared memory manager and per-agent lock storage ++ shared_mem = get_shared_memory() ++ agent_locks = shared_mem.get_or_create_dict("agent_registration_locks") ++ ++ # Get or create a lock specific to this agent_id ++ if agent_id not in agent_locks: ++ agent_locks[agent_id] = shared_mem.manager.Lock() ++ ++ agent_lock = agent_locks[agent_id] ++ ++ # CRITICAL SECTION: Acquire lock to make check-validate-commit atomic ++ with agent_lock: ++ # Step 1: Load existing agent or create new one (inside lock) ++ agent = RegistrarAgent.get(agent_id) or RegistrarAgent.empty() # type: ignore[no-untyped-call] ++ ++ # Step 2: Update agent with new data and validate (inside lock) ++ agent.update({"agent_id": agent_id, **params}) ++ ++ # Step 3: Check for TPM identity change security violation ++ # Use explicit flag instead of fragile string matching for security check ++ if not agent.changes_valid and agent.has_tpm_identity_violation: ++ # Log the validation errors (includes security warning) ++ self.log_model_errors(agent, logger) ++ ++ # Return 403 Forbidden ++ # 403 indicates a policy violation, not a malformed request ++ self.respond(403, "Agent re-registration with different TPM identity is forbidden for security reasons") ++ return ++ ++ # Step 4: Generate AK challenge (inside lock) ++ challenge = agent.produce_ak_challenge() ++ ++ # Step 5: Check for any validation errors or challenge generation failure ++ if not challenge or not agent.changes_valid: ++ self.log_model_errors(agent, logger) ++ self.respond(400, "Could not register agent with invalid data") ++ return ++ ++ # Step 6: Commit to database (inside lock) ++ # This ensures no other request can modify the agent between validation and commit ++ try: ++ agent.commit_changes() ++ except IntegrityError as e: ++ # Database constraint violation - most likely duplicate agent_id ++ # This can happen if two requests try to register the same new UUID simultaneously ++ # and both pass validation before either commits (database race condition) ++ logger.warning( ++ "SECURITY: Agent registration failed due to database constraint violation for agent_id '%s'. " ++ "This UUID may already be registered by a concurrent request or the agent already exists. " ++ "Database error: %s", ++ agent_id, ++ str(e), ++ ) ++ self.respond( ++ 403, ++ f"Agent with UUID '{agent_id}' cannot be registered. " ++ "This UUID is already in use or a concurrent registration is in progress.", ++ ) ++ return ++ ++ # Lock released - safe to respond to client ++ # Return challenge blob for agent to decrypt + self.respond(200, "Success", {"blob": challenge}) + + # DELETE /v2[.:minor]/agents/:agent_id/ +diff --git a/test/test_agents_controller.py b/test/test_agents_controller.py +new file mode 100644 +index 0000000..898d8f0 +--- /dev/null ++++ b/test/test_agents_controller.py +@@ -0,0 +1,513 @@ ++"""Unit tests for AgentsController (registrar). ++ ++Tests the registrar's agent registration endpoints, including the ++security fix that prevents UUID spoofing via re-registration with ++a different TPM identity. ++""" ++ ++# type: ignore - Controller methods are dynamically bound ++ ++import unittest ++from typing import cast ++from unittest.mock import MagicMock, patch ++ ++from sqlalchemy.exc import IntegrityError ++ ++from keylime.web.registrar.agents_controller import AgentsController ++ ++ ++class TestAgentsControllerIndex(unittest.TestCase): ++ """Test cases for AgentsController.index().""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ ++ @patch("keylime.models.RegistrarAgent.all_ids") ++ def test_index_success(self, mock_all_ids): ++ """Test successful retrieval of all agent IDs.""" ++ mock_all_ids.return_value = ["agent-1", "agent-2", "agent-3"] ++ ++ self.controller.index() ++ ++ self.controller.respond.assert_called_once_with(200, "Success", {"uuids": ["agent-1", "agent-2", "agent-3"]}) # type: ignore[attr-defined] ++ ++ ++class TestAgentsControllerShow(unittest.TestCase): ++ """Test cases for AgentsController.show().""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.test_agent_id = "test-agent-123" ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_show_not_found(self, mock_get): ++ """Test show with non-existent agent.""" ++ mock_get.return_value = None ++ ++ self.controller.show(self.test_agent_id) ++ ++ self.controller.respond.assert_called_once_with(404, f"Agent with ID '{self.test_agent_id}' not found") # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_show_not_active(self, mock_get): ++ """Test show with inactive agent.""" ++ mock_agent = MagicMock() ++ mock_agent.active = False ++ mock_get.return_value = mock_agent ++ ++ self.controller.show(self.test_agent_id) ++ ++ self.controller.respond.assert_called_once_with( # type: ignore[attr-defined] ++ 404, f"Agent with ID '{self.test_agent_id}' has not been activated" ++ ) ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_show_success(self, mock_get): ++ """Test successful show of active agent.""" ++ mock_agent = MagicMock() ++ mock_agent.active = True ++ mock_agent.render.return_value = {"agent_id": self.test_agent_id, "active": True} ++ mock_get.return_value = mock_agent ++ ++ self.controller.show(self.test_agent_id) ++ ++ self.controller.respond.assert_called_once_with( # type: ignore[attr-defined] ++ 200, "Success", {"agent_id": self.test_agent_id, "active": True} ++ ) ++ ++ ++class TestAgentsControllerCreate(unittest.TestCase): ++ """Test cases for AgentsController.create() - the main registration endpoint.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.controller.log_model_errors = MagicMock() ++ self.test_agent_id = "test-agent-123" ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_new_agent_success(self, mock_get): ++ """Test successful registration of a new agent.""" ++ # Mock that agent doesn't exist yet ++ mock_get.return_value = None ++ ++ # Create mock agent that will be returned by empty() ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = "challenge_blob_data" ++ ++ # Patch RegistrarAgent.empty to return our mock ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify agent was updated with params ++ mock_agent.update.assert_called_once_with({"agent_id": self.test_agent_id, **params}) ++ ++ # Verify challenge was generated ++ mock_agent.produce_ak_challenge.assert_called_once() ++ ++ # Verify agent was saved ++ mock_agent.commit_changes.assert_called_once() ++ ++ # Verify 200 response with challenge ++ self.controller.respond.assert_called_once_with(200, "Success", {"blob": "challenge_blob_data"}) # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_reregistration_same_tpm_identity(self, mock_get): ++ """Test successful re-registration with same TPM identity.""" ++ # Mock existing agent ++ mock_existing_agent = MagicMock() ++ mock_existing_agent.changes_valid = True ++ mock_existing_agent.errors = {} ++ mock_existing_agent.produce_ak_challenge.return_value = "challenge_blob_data" ++ mock_get.return_value = mock_existing_agent ++ ++ params = {"ek_tpm": "same_ek_key", "aik_tpm": "same_aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify agent was updated ++ mock_existing_agent.update.assert_called_once_with({"agent_id": self.test_agent_id, **params}) ++ ++ # Verify challenge was generated ++ mock_existing_agent.produce_ak_challenge.assert_called_once() ++ ++ # Verify agent was saved ++ mock_existing_agent.commit_changes.assert_called_once() ++ ++ # Verify 200 response ++ self.controller.respond.assert_called_once_with(200, "Success", {"blob": "challenge_blob_data"}) # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_reregistration_different_tpm_identity_forbidden(self, mock_get): ++ """Test re-registration with different TPM identity is rejected with 403. ++ ++ This is the key security fix: preventing UUID spoofing by rejecting ++ attempts to re-register an agent with a different TPM identity. ++ """ ++ # Mock existing agent ++ mock_existing_agent = MagicMock() ++ mock_existing_agent.changes_valid = False # Validation failed ++ # Simulate the error added by _check_tpm_identity_immutable ++ mock_existing_agent.errors = { ++ "agent_id": [ ++ "Agent re-registration attempted with different TPM identity (changed fields: ek_tpm). " ++ "This is a security violation - the same agent UUID cannot be reused with a different TPM." ++ ] ++ } ++ mock_get.return_value = mock_existing_agent ++ ++ params = {"ek_tpm": "different_ek_key", "aik_tpm": "same_aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify agent was updated (which triggers validation) ++ mock_existing_agent.update.assert_called_once_with({"agent_id": self.test_agent_id, **params}) ++ ++ # Verify errors were logged ++ self.controller.log_model_errors.assert_called_once() # type: ignore[attr-defined] ++ ++ # Verify 403 Forbidden response (not 400!) ++ self.controller.respond.assert_called_once_with( # type: ignore[attr-defined] ++ 403, "Agent re-registration with different TPM identity is forbidden for security reasons" ++ ) ++ ++ # Verify agent was NOT saved ++ mock_existing_agent.commit_changes.assert_not_called() ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_invalid_data_other_validation_error(self, mock_get): ++ """Test registration with other validation errors returns 400.""" ++ # Mock agent with validation errors (not TPM identity related) ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = False ++ # Error not related to TPM identity ++ mock_agent.errors = {"ek_tpm": ["must be a valid TPM2B_PUBLIC structure"]} ++ mock_agent.has_tpm_identity_violation = False # Not a TPM identity violation ++ mock_agent.produce_ak_challenge.return_value = None ++ mock_get.return_value = None ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "invalid_ek_format"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify errors were logged ++ self.controller.log_model_errors.assert_called_once() # type: ignore[attr-defined] ++ ++ # Verify 400 Bad Request (not 403) ++ self.controller.respond.assert_called_once_with(400, "Could not register agent with invalid data") # type: ignore[attr-defined] ++ ++ # Verify agent was NOT saved ++ mock_agent.commit_changes.assert_not_called() ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_challenge_generation_failure(self, mock_get): ++ """Test registration fails if challenge generation fails.""" ++ # Mock agent where challenge generation fails ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = None # Challenge generation failed ++ mock_get.return_value = None ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify errors were logged ++ self.controller.log_model_errors.assert_called_once() # type: ignore[attr-defined] ++ ++ # Verify 400 response ++ self.controller.respond.assert_called_once_with(400, "Could not register agent with invalid data") # type: ignore[attr-defined] ++ ++ # Verify agent was NOT saved ++ mock_agent.commit_changes.assert_not_called() ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_validation_error_with_agent_id_but_not_tpm_identity(self, mock_get): ++ """Test that agent_id errors unrelated to TPM identity get 400, not 403.""" ++ # Mock agent with agent_id error, but not about TPM identity ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = False ++ mock_agent.errors = {"agent_id": ["must be a valid UUID format"]} # Not about TPM identity ++ mock_agent.has_tpm_identity_violation = False # Not a TPM identity violation ++ mock_agent.produce_ak_challenge.return_value = None ++ mock_get.return_value = None ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify 400 Bad Request (not 403) because it's not a TPM identity violation ++ self.controller.respond.assert_called_once_with(400, "Could not register agent with invalid data") # type: ignore[attr-defined] ++ ++ ++class TestAgentsControllerDelete(unittest.TestCase): ++ """Test cases for AgentsController.delete().""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.test_agent_id = "test-agent-123" ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_delete_not_found(self, mock_get): ++ """Test delete with non-existent agent.""" ++ mock_get.return_value = None ++ ++ self.controller.delete(self.test_agent_id) ++ ++ self.controller.respond.assert_called_once_with(404, f"Agent with ID '{self.test_agent_id}' not found") # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_delete_success(self, mock_get): ++ """Test successful agent deletion.""" ++ mock_agent = MagicMock() ++ mock_get.return_value = mock_agent ++ ++ self.controller.delete(self.test_agent_id) ++ ++ # Verify agent was deleted ++ mock_agent.delete.assert_called_once() ++ ++ # Verify 200 response ++ self.controller.respond.assert_called_once_with(200, "Success") # type: ignore[attr-defined] ++ ++ ++class TestAgentsControllerActivate(unittest.TestCase): ++ """Test cases for AgentsController.activate().""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.test_agent_id = "test-agent-123" ++ self.test_auth_tag = "valid_auth_tag" ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_activate_not_found(self, mock_get): ++ """Test activate with non-existent agent.""" ++ mock_get.return_value = None ++ ++ self.controller.activate(self.test_agent_id, self.test_auth_tag) ++ ++ self.controller.respond.assert_called_once_with(404, f"Agent with ID '{self.test_agent_id}' not found") # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_activate_success(self, mock_get): ++ """Test successful agent activation.""" ++ mock_agent = MagicMock() ++ mock_agent.verify_ak_response.return_value = True # Auth tag is valid ++ mock_get.return_value = mock_agent ++ ++ self.controller.activate(self.test_agent_id, self.test_auth_tag) ++ ++ # Verify auth tag was verified ++ mock_agent.verify_ak_response.assert_called_once_with(self.test_auth_tag) ++ ++ # Verify agent was saved ++ mock_agent.commit_changes.assert_called_once() ++ ++ # Verify 200 response ++ self.controller.respond.assert_called_once_with(200, "Success") # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_activate_invalid_auth_tag(self, mock_get): ++ """Test activation with invalid auth tag.""" ++ mock_agent = MagicMock() ++ mock_agent.verify_ak_response.return_value = False # Auth tag is invalid ++ mock_get.return_value = mock_agent ++ ++ self.controller.activate(self.test_agent_id, self.test_auth_tag) ++ ++ # Verify auth tag was verified ++ mock_agent.verify_ak_response.assert_called_once_with(self.test_auth_tag) ++ ++ # Verify agent was deleted (due to failed activation) ++ mock_agent.delete.assert_called_once() ++ ++ # Verify agent was NOT saved ++ mock_agent.commit_changes.assert_not_called() ++ ++ # Verify 400 response with detailed error message ++ self.controller.respond.assert_called_once() # type: ignore[attr-defined] ++ call_args = self.controller.respond.call_args # type: ignore[attr-defined] ++ self.assertEqual(call_args[0][0], 400) ++ self.assertIn(self.test_auth_tag, call_args[0][1]) ++ self.assertIn(self.test_agent_id, call_args[0][1]) ++ self.assertIn("deleted", call_args[0][1]) ++ ++ ++class TestAgentsControllerConcurrency(unittest.TestCase): ++ """Test cases for concurrent registration TOCTOU race condition protection.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.controller.log_model_errors = MagicMock() ++ self.test_agent_id = "concurrent-test-agent" ++ ++ @patch("keylime.web.registrar.agents_controller.get_shared_memory") ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_concurrent_registration_uses_locking(self, mock_get, mock_shared_mem): ++ """Test that concurrent registration attempts use per-agent locking. ++ ++ This test verifies that the locking mechanism is invoked to prevent ++ TOCTOU race conditions during concurrent registration. ++ """ ++ # Mock shared memory manager with lock support ++ mock_manager = MagicMock() ++ mock_lock = MagicMock() ++ mock_lock.__enter__ = MagicMock(return_value=None) ++ mock_lock.__exit__ = MagicMock(return_value=None) ++ mock_manager.Lock.return_value = mock_lock ++ ++ mock_agent_locks = MagicMock() ++ mock_agent_locks.__contains__ = MagicMock(return_value=False) ++ mock_agent_locks.__setitem__ = MagicMock() ++ mock_agent_locks.__getitem__ = MagicMock(return_value=mock_lock) ++ ++ mock_shared_mem.return_value.get_or_create_dict.return_value = mock_agent_locks ++ mock_shared_mem.return_value.manager = mock_manager ++ ++ # Mock agent that doesn't exist yet (new registration) ++ mock_get.return_value = None ++ ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = "challenge_blob" ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify lock was acquired and released ++ mock_lock.__enter__.assert_called_once() ++ mock_lock.__exit__.assert_called_once() ++ ++ # Verify successful registration ++ mock_agent.commit_changes.assert_called_once() ++ self.controller.respond.assert_called_once_with(200, "Success", {"blob": "challenge_blob"}) # type: ignore[attr-defined] ++ ++ @patch("keylime.web.registrar.agents_controller.get_shared_memory") ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_different_agents_use_different_locks(self, mock_get, mock_shared_mem): ++ """Test that different agent_ids use different locks for parallel registration. ++ ++ This ensures that registrations for different agents don't block each other, ++ only concurrent registrations for the same agent_id are serialized. ++ """ ++ # Mock shared memory manager ++ mock_manager = MagicMock() ++ ++ def mock_lock_factory(): ++ """Return a new lock each time.""" ++ return MagicMock() ++ ++ mock_manager.Lock.side_effect = mock_lock_factory ++ ++ mock_agent_locks = {} ++ ++ def mock_getitem(_self, key): # pylint: disable=unused-argument ++ return mock_agent_locks.get(key) ++ ++ def mock_setitem(_self, key, value): # pylint: disable=unused-argument ++ mock_agent_locks[key] = value ++ ++ def mock_contains(_self, key): # pylint: disable=unused-argument ++ return key in mock_agent_locks ++ ++ mock_locks_dict = MagicMock() ++ mock_locks_dict.__contains__ = mock_contains ++ mock_locks_dict.__setitem__ = mock_setitem ++ mock_locks_dict.__getitem__ = mock_getitem ++ ++ mock_shared_mem.return_value.get_or_create_dict.return_value = mock_locks_dict ++ mock_shared_mem.return_value.manager = mock_manager ++ ++ # Register two different agents ++ mock_get.return_value = None ++ ++ for agent_id in ["agent-a", "agent-b"]: ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = f"challenge_{agent_id}" ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ self.controller.respond = MagicMock() # Reset for each call ++ params = {"ek_tpm": f"ek_{agent_id}", "aik_tpm": f"aik_{agent_id}"} ++ self.controller.create(agent_id, **params) ++ ++ # Verify that two different locks were created (one per agent) ++ self.assertEqual(len(mock_agent_locks), 2) ++ self.assertIn("agent-a", mock_agent_locks) ++ self.assertIn("agent-b", mock_agent_locks) ++ # Verify they are different lock objects ++ self.assertIsNot(mock_agent_locks["agent-a"], mock_agent_locks["agent-b"]) ++ ++ @patch("keylime.web.registrar.agents_controller.get_shared_memory") ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_concurrent_new_registration_database_constraint_violation(self, mock_get, mock_shared_mem): ++ """Test that database constraint violations during concurrent new agent registration return 403. ++ ++ This handles the edge case where two requests both create empty agents for the same UUID, ++ both pass validation, but the second commit fails with IntegrityError due to duplicate ++ primary key. This should return 403 Forbidden, not 500 Internal Server Error. ++ """ ++ # Mock shared memory manager with lock support ++ mock_manager = MagicMock() ++ mock_lock = MagicMock() ++ mock_lock.__enter__ = MagicMock(return_value=None) ++ mock_lock.__exit__ = MagicMock(return_value=None) ++ mock_manager.Lock.return_value = mock_lock ++ ++ mock_agent_locks = MagicMock() ++ mock_agent_locks.__contains__ = MagicMock(return_value=False) ++ mock_agent_locks.__setitem__ = MagicMock() ++ mock_agent_locks.__getitem__ = MagicMock(return_value=mock_lock) ++ ++ mock_shared_mem.return_value.get_or_create_dict.return_value = mock_agent_locks ++ mock_shared_mem.return_value.manager = mock_manager ++ ++ # Mock agent that doesn't exist yet (new registration) ++ mock_get.return_value = None ++ ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = "challenge_blob" ++ ++ # Simulate IntegrityError during commit (duplicate primary key) ++ # IntegrityError(statement, params, orig) where orig is the original exception ++ orig_exception = Exception("UNIQUE constraint failed: registrarmain.agent_id") ++ mock_agent.commit_changes.side_effect = IntegrityError("INSERT INTO registrarmain ...", None, orig_exception) ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify 403 Forbidden response (not 500) ++ self.controller.respond.assert_called_once() # type: ignore[attr-defined] ++ call_args = self.controller.respond.call_args # type: ignore[attr-defined] ++ self.assertEqual(call_args[0][0], 403) ++ self.assertIn(self.test_agent_id, call_args[0][1]) ++ self.assertIn("already in use", call_args[0][1]) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +diff --git a/test/test_registrar_tpm_identity.py b/test/test_registrar_tpm_identity.py +new file mode 100644 +index 0000000..2fc69b2 +--- /dev/null ++++ b/test/test_registrar_tpm_identity.py +@@ -0,0 +1,342 @@ ++""" ++Unit tests for RegistrarAgent TPM identity immutability security check. ++ ++This module tests the _check_tpm_identity_immutable() method which prevents ++UUID spoofing attacks by rejecting re-registration attempts with different TPM identities. ++""" ++ ++import base64 ++import types ++import unittest ++from unittest.mock import Mock ++ ++import cryptography.x509 ++ ++from keylime.certificate_wrapper import wrap_certificate ++from keylime.models.registrar.registrar_agent import RegistrarAgent ++ ++ ++class TestRegistrarAgentTPMIdentity(unittest.TestCase): ++ """Test cases for RegistrarAgent TPM identity immutability.""" ++ ++ # pylint: disable=protected-access # Testing protected methods ++ # pylint: disable=not-callable # False positive: methods bound via types.MethodType are callable ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ # EK certificate (used for testing certificate comparison) ++ self.ek_cert_pem = """-----BEGIN CERTIFICATE----- ++MIIEnzCCA4egAwIBAgIEMV64bDANBgkqhkiG9w0BAQUFADBtMQswCQYDVQQGEwJE ++RTEQMA4GA1UECBMHQmF2YXJpYTEhMB8GA1UEChMYSW5maW5lb24gVGVjaG5vbG9n ++aWVzIEFHMQwwCgYDVQQLEwNBSU0xGzAZBgNVBAMTEklGWCBUUE0gRUsgUm9vdCBD ++QTAeFw0wNTEwMjAxMzQ3NDNaFw0yNTEwMjAxMzQ3NDNaMHcxCzAJBgNVBAYTAkRF ++MQ8wDQYDVQQIEwZTYXhvbnkxITAfBgNVBAoTGEluZmluZW9uIFRlY2hub2xvZ2ll ++cyBBRzEMMAoGA1UECxMDQUlNMSYwJAYDVQQDEx1JRlggVFBNIEVLIEludGVybWVk ++aWF0ZSBDQSAwMTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALftPhYN ++t4rE+JnU/XOPICbOBLvfo6iA7nuq7zf4DzsAWBdsZEdFJQfaK331ihG3IpQnlQ2i ++YtDim289265f0J4OkPFpKeFU27CsfozVaNUm6UR/uzwA8ncxFc3iZLRMRNLru/Al ++VG053ULVDQMVx2iwwbBSAYO9pGiGbk1iMmuZaSErMdb9v0KRUyZM7yABiyDlM3cz ++UQX5vLWV0uWqxdGoHwNva5u3ynP9UxPTZWHZOHE6+14rMzpobs6Ww2RR8BgF96rh ++4rRAZEl8BXhwiQq4STvUXkfvdpWH4lzsGcDDtrB6Nt3KvVNvsKz+b07Dk+Xzt+EH ++NTf3Byk2HlvX+scCAwEAAaOCATswggE3MB0GA1UdDgQWBBQ4k8292HPEIzMV4bE7 ++qWoNI8wQxzAOBgNVHQ8BAf8EBAMCAgQwEgYDVR0TAQH/BAgwBgEB/wIBADBYBgNV ++HSABAf8ETjBMMEoGC2CGSAGG+EUBBy8BMDswOQYIKwYBBQUHAgEWLWh0dHA6Ly93 ++d3cudmVyaXNpZ24uY29tL3JlcG9zaXRvcnkvaW5kZXguaHRtbDCBlwYDVR0jBIGP ++MIGMgBRW65FEhWPWcrOu1EWWC/eUDlRCpqFxpG8wbTELMAkGA1UEBhMCREUxEDAO ++BgNVBAgTB0JhdmFyaWExITAfBgNVBAoTGEluZmluZW9uIFRlY2hub2xvZ2llcyBB ++RzEMMAoGA1UECxMDQUlNMRswGQYDVQQDExJJRlggVFBNIEVLIFJvb3QgQ0GCAQMw ++DQYJKoZIhvcNAQEFBQADggEBABJ1+Ap3rNlxZ0FW0aIgdzktbNHlvXWNxFdYIBbM ++OKjmbOos0Y4O60eKPu259XmMItCUmtbzF3oKYXq6ybARUT2Lm+JsseMF5VgikSlU ++BJALqpKVjwAds81OtmnIQe2LSu4xcTSavpsL4f52cUAu/maMhtSgN9mq5roYptq9 ++DnSSDZrX4uYiMPl//rBaNDBflhJ727j8xo9CCohF3yQUoQm7coUgbRMzyO64yMIO ++3fhb+Vuc7sNwrMOz3VJN14C3JMoGgXy0c57IP/kD5zGRvljKEvrRC2I147+fPeLS ++DueRMS6lblvRKiZgmGAg7YaKOkOaEmVDMQ+fTo2Po7hI5wc= ++-----END CERTIFICATE-----""" ++ ++ # Create wrapped cert from real certificate ++ self.ek_cert = cryptography.x509.load_pem_x509_certificate(self.ek_cert_pem.encode()) ++ self.ek_cert_wrapped = wrap_certificate(self.ek_cert, None) ++ ++ # Create a different cert mock that returns different DER bytes ++ self.different_ek_cert_wrapped = Mock() ++ self.different_ek_cert_wrapped.public_bytes = Mock(return_value=b"DIFFERENT_CERTIFICATE_DER_BYTES_FOR_TESTING") ++ ++ # Sample TPM keys (base64 encoded for simplicity in tests) ++ self.ek_tpm_1 = b"EK_TPM_KEY_NUMBER_ONE_SAMPLE_DATA" ++ self.ek_tpm_2 = b"EK_TPM_KEY_NUMBER_TWO_DIFFERENT_" ++ self.aik_tpm_1 = b"AIK_TPM_KEY_NUMBER_ONE_SAMPLE_DATA" ++ self.aik_tpm_2 = b"AIK_TPM_KEY_NUMBER_TWO_DIFFERENT_" ++ ++ # IAK/IDevID keys for testing that they are not checked ++ self.iak_tpm_1 = b"IAK_TPM_KEY_NUMBER_ONE" ++ self.iak_tpm_2 = b"IAK_TPM_KEY_NUMBER_TWO" ++ ++ def create_mock_registrar_agent(self, agent_id="test-agent-uuid"): ++ """Create a mock RegistrarAgent with necessary attributes.""" ++ agent = Mock() ++ agent.agent_id = agent_id ++ agent.changes = {} ++ agent.values = {} ++ agent.committed = {} ++ agent._add_error = Mock() ++ agent.errors = {} ++ ++ # Bind the actual method to the mock instance ++ agent._check_tpm_identity_immutable = types.MethodType(RegistrarAgent._check_tpm_identity_immutable, agent) ++ ++ return agent ++ ++ def test_new_agent_no_committed_values(self): ++ """Test that new agents (no committed values) are not checked.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = {} # New agent, no previous values ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors for new agents ++ agent._add_error.assert_not_called() ++ ++ def test_reregistration_same_tpm_all_fields_identical(self): ++ """Test re-registration with identical TPM identity passes.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, # Same ++ "ekcert": self.ek_cert_wrapped, # Same ++ "aik_tpm": self.aik_tpm_1, # Same ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors ++ agent._add_error.assert_not_called() ++ ++ def test_reregistration_different_ek_tpm(self): ++ """Test re-registration with different EK TPM is rejected.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_2, # DIFFERENT ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error for agent_id field ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertEqual(call_args[0][0], "agent_id") ++ self.assertIn("different TPM identity", call_args[0][1]) ++ self.assertIn("ek_tpm", call_args[0][1]) ++ ++ def test_reregistration_different_aik_tpm(self): ++ """Test re-registration with different AIK TPM is rejected.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_2, # DIFFERENT ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error for agent_id field ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertEqual(call_args[0][0], "agent_id") ++ self.assertIn("different TPM identity", call_args[0][1]) ++ self.assertIn("aik_tpm", call_args[0][1]) ++ ++ def test_reregistration_different_ekcert(self): ++ """Test re-registration with different EK certificate is rejected.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.different_ek_cert_wrapped, # DIFFERENT ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error for agent_id field ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertEqual(call_args[0][0], "agent_id") ++ self.assertIn("different TPM identity", call_args[0][1]) ++ self.assertIn("ekcert", call_args[0][1]) ++ ++ def test_reregistration_multiple_fields_changed(self): ++ """Test re-registration with multiple fields changed lists all of them.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_2, # DIFFERENT ++ "ekcert": self.different_ek_cert_wrapped, # DIFFERENT ++ "aik_tpm": self.aik_tpm_2, # DIFFERENT ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error listing all changed fields ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertEqual(call_args[0][0], "agent_id") ++ error_message = call_args[0][1] ++ self.assertIn("ek_tpm", error_message) ++ self.assertIn("ekcert", error_message) ++ self.assertIn("aik_tpm", error_message) ++ ++ def test_adding_ekcert_to_existing_agent(self): ++ """Test that adding EK cert to existing agent (without cert) is allowed.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": None, # Previously no cert ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, # NOW adding cert ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors - adding cert is allowed ++ agent._add_error.assert_not_called() ++ ++ def test_removing_ek_tpm_rejected(self): ++ """Test that removing an existing EK TPM is rejected.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": None, # Trying to remove ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertIn("ek_tpm", call_args[0][1]) ++ ++ def test_iak_idevid_changes_not_checked(self): ++ """Test that IAK/IDevID field changes are NOT checked (allowed).""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ "iak_tpm": self.iak_tpm_1, ++ "idevid_tpm": b"IDEVID_OLD", ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, # Same ++ "ekcert": self.ek_cert_wrapped, # Same ++ "aik_tpm": self.aik_tpm_1, # Same ++ "iak_tpm": self.iak_tpm_2, # DIFFERENT - but not checked ++ "idevid_tpm": b"IDEVID_NEW", # DIFFERENT - but not checked ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors - IAK/IDevID are not checked ++ agent._add_error.assert_not_called() ++ ++ def test_only_changed_fields_are_checked(self): ++ """Test that only fields in changes dict are checked.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ # Only updating IP, not touching TPM identity fields ++ agent.changes = { ++ "ip": "192.168.1.100", ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors - no identity fields changed ++ agent._add_error.assert_not_called() ++ ++ def test_base64_encoded_tpm_keys(self): ++ """Test that base64-encoded TPM keys are properly compared.""" ++ agent = self.create_mock_registrar_agent() ++ ++ # Simulate keys stored as base64 strings (as they might be from database) ++ ek_b64 = base64.b64encode(self.ek_tpm_1).decode("utf-8") ++ aik_b64 = base64.b64encode(self.aik_tpm_1).decode("utf-8") ++ ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, # As bytes ++ "aik_tpm": self.aik_tpm_1, # As bytes ++ } ++ agent.changes = { ++ "ek_tpm": ek_b64, # As base64 string ++ "aik_tpm": aik_b64, # As base64 string ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors - should handle both formats ++ agent._add_error.assert_not_called() ++ ++ def test_partial_update_only_one_field(self): ++ """Test updating only one TPM field while others remain unchanged.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ # Only changing AIK in this update ++ agent.changes = { ++ "aik_tpm": self.aik_tpm_2, # DIFFERENT ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error for the changed field ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertIn("aik_tpm", call_args[0][1]) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +-- +2.47.3 + diff --git a/SOURCES/0016-algorithms-add-support-for-specific-ECC-curve-algori.patch b/SOURCES/0016-algorithms-add-support-for-specific-ECC-curve-algori.patch new file mode 100644 index 0000000..81a6398 --- /dev/null +++ b/SOURCES/0016-algorithms-add-support-for-specific-ECC-curve-algori.patch @@ -0,0 +1,382 @@ +From 7a723f0938edf9ccc597507a4230922e9235cf18 Mon Sep 17 00:00:00 2001 +From: Sergio Correia +Date: Wed, 24 Sep 2025 07:20:53 +0100 +Subject: [PATCH 13/18] algorithms: add support for specific ECC curve + algorithms + +Extended the Encrypt enum to support specific ECC curves including: +- ecc192 (P-192) +- ecc224 (P-224) +- ecc256 (P-256) +- ecc384 (P-384) +- ecc521 (P-521) + +This enables Keylime to accept and validate different ECC curves +for TPM attestation operations. + +Also, when agent reports specific algorithm like 'ecc256' but tenant +configuration uses generic 'ecc', the is_accepted function now uses +bidirectional normalization to properly match algorithms. + +Signed-off-by: Sergio Correia +--- + keylime/common/algorithms.py | 28 +++- + test/test_algorithms.py | 275 ++++++++++++++++++++++++++++++++++- + 2 files changed, 301 insertions(+), 2 deletions(-) + +diff --git a/keylime/common/algorithms.py b/keylime/common/algorithms.py +index db12c26..bb22fb6 100644 +--- a/keylime/common/algorithms.py ++++ b/keylime/common/algorithms.py +@@ -9,7 +9,18 @@ def is_accepted(algorithm: str, accepted: List[Any]) -> bool: + @param algorithm: algorithm to be checked + @param accepted: a list of acceptable algorithms + """ +- return algorithm in accepted ++ # Check direct match first. ++ if algorithm in accepted: ++ return True ++ ++ # Check if any accepted algorithm normalizes to the same value as our algorithm ++ # This handles backwards compatibility cases like "ecc" accepting "ecc256". ++ normalized_algorithm = Encrypt.normalize(algorithm) ++ for accepted_alg in accepted: ++ if Encrypt.normalize(str(accepted_alg)) == normalized_algorithm: ++ return True ++ ++ return False + + + class Hash(str, enum.Enum): +@@ -74,11 +85,26 @@ class Hash(str, enum.Enum): + class Encrypt(str, enum.Enum): + RSA = "rsa" + ECC = "ecc" ++ ECC192 = "ecc192" ++ ECC224 = "ecc224" ++ ECC256 = "ecc256" ++ ECC384 = "ecc384" ++ ECC521 = "ecc521" + + @staticmethod + def is_recognized(algorithm: str) -> bool: ++ # Handle aliases to match agent behavior ++ if algorithm == "ecc": ++ algorithm = "ecc256" # Default ECC alias maps to P-256, same as the agent. + return algorithm in list(Encrypt) + ++ @staticmethod ++ def normalize(algorithm: str) -> str: ++ """Normalize algorithm string to handle aliases, matching the agent behavior""" ++ if algorithm == "ecc": ++ return "ecc256" # Default ECC alias maps to P-256. ++ return algorithm ++ + + class Sign(str, enum.Enum): + RSASSA = "rsassa" +diff --git a/test/test_algorithms.py b/test/test_algorithms.py +index b5a29c7..8a31fa9 100644 +--- a/test/test_algorithms.py ++++ b/test/test_algorithms.py +@@ -2,7 +2,7 @@ import os + import tempfile + import unittest + +-from keylime.common.algorithms import Encrypt, Hash, Sign ++from keylime.common.algorithms import Encrypt, Hash, Sign, is_accepted + + + class TestHash(unittest.TestCase): +@@ -117,11 +117,88 @@ class TestEncrypt(unittest.TestCase): + "enc": "ecc", + "valid": True, + }, ++ { ++ "enc": "ecc192", ++ "valid": True, ++ }, ++ { ++ "enc": "ecc224", ++ "valid": True, ++ }, ++ { ++ "enc": "ecc256", ++ "valid": True, ++ }, ++ { ++ "enc": "ecc384", ++ "valid": True, ++ }, ++ { ++ "enc": "ecc521", ++ "valid": True, ++ }, + ] + + for c in test_cases: + self.assertEqual(Encrypt.is_recognized(c["enc"]), c["valid"], msg=f"enc = {c['enc']}") + ++ def test_enum_membership(self): ++ """Test that all ECC curve algorithms are members of the Encrypt enum""" ++ self.assertTrue(Encrypt.RSA in Encrypt) ++ self.assertTrue(Encrypt.ECC in Encrypt) ++ self.assertTrue(Encrypt.ECC192 in Encrypt) ++ self.assertTrue(Encrypt.ECC224 in Encrypt) ++ self.assertTrue(Encrypt.ECC256 in Encrypt) ++ self.assertTrue(Encrypt.ECC384 in Encrypt) ++ self.assertTrue(Encrypt.ECC521 in Encrypt) ++ ++ def test_normalize(self): ++ """Test the normalize method for handling ECC aliases""" ++ test_cases = [ ++ { ++ "input": "ecc", ++ "expected": "ecc256", ++ }, ++ { ++ "input": "ecc192", ++ "expected": "ecc192", ++ }, ++ { ++ "input": "ecc224", ++ "expected": "ecc224", ++ }, ++ { ++ "input": "ecc256", ++ "expected": "ecc256", ++ }, ++ { ++ "input": "ecc384", ++ "expected": "ecc384", ++ }, ++ { ++ "input": "ecc521", ++ "expected": "ecc521", ++ }, ++ { ++ "input": "rsa", ++ "expected": "rsa", ++ }, ++ ] ++ ++ for c in test_cases: ++ self.assertEqual(Encrypt.normalize(c["input"]), c["expected"], msg=f"input = {c['input']}") ++ ++ def test_normalize_ecc_alias_behavior(self): ++ """Test that ECC alias normalization matches agent behavior""" ++ # Test that "ecc" is recognized through alias handling ++ self.assertTrue(Encrypt.is_recognized("ecc")) ++ ++ # Test that normalize converts "ecc" to "ecc256" (P-256) ++ self.assertEqual(Encrypt.normalize("ecc"), "ecc256") ++ ++ # Test that direct ecc256 works ++ self.assertTrue(Encrypt.is_recognized("ecc256")) ++ + + class TestSign(unittest.TestCase): + def test_is_recognized(self): +@@ -158,3 +235,199 @@ class TestSign(unittest.TestCase): + + for c in test_cases: + self.assertEqual(Sign.is_recognized(c["sign"]), c["valid"], msg=f"sign = {c['sign']}") ++ ++ ++class TestIsAccepted(unittest.TestCase): ++ def test_direct_algorithm_matching(self): ++ """Test that direct algorithm matches work correctly""" ++ test_cases = [ ++ { ++ "algorithm": "ecc256", ++ "accepted": ["ecc256"], ++ "expected": True, ++ }, ++ { ++ "algorithm": "rsa", ++ "accepted": ["rsa"], ++ "expected": True, ++ }, ++ { ++ "algorithm": "ecc384", ++ "accepted": ["ecc256", "ecc384"], ++ "expected": True, ++ }, ++ { ++ "algorithm": "ecc521", ++ "accepted": ["ecc256"], ++ "expected": False, ++ }, ++ { ++ "algorithm": "unknown", ++ "accepted": ["rsa", "ecc256"], ++ "expected": False, ++ }, ++ ] ++ ++ for c in test_cases: ++ result = is_accepted(c["algorithm"], c["accepted"]) ++ self.assertEqual(result, c["expected"], msg=f"algorithm='{c['algorithm']}', accepted={c['accepted']}") ++ ++ def test_backwards_compatibility_ecc_normalization(self): ++ """Test backwards compatibility: 'ecc' in accepted list should accept specific ECC algorithms""" ++ test_cases = [ ++ { ++ "algorithm": "ecc256", ++ "accepted": ["ecc"], ++ "expected": True, ++ "desc": "ecc256 should be accepted when 'ecc' is in accepted list", ++ }, ++ { ++ "algorithm": "ecc384", ++ "accepted": ["ecc"], ++ "expected": False, ++ "desc": "ecc384 should NOT be accepted when only 'ecc' is in accepted list (ecc maps to ecc256)", ++ }, ++ { ++ "algorithm": "ecc521", ++ "accepted": ["ecc"], ++ "expected": False, ++ "desc": "ecc521 should NOT be accepted when only 'ecc' is in accepted list", ++ }, ++ { ++ "algorithm": "ecc192", ++ "accepted": ["ecc"], ++ "expected": False, ++ "desc": "ecc192 should NOT be accepted when only 'ecc' is in accepted list", ++ }, ++ ] ++ ++ for c in test_cases: ++ result = is_accepted(c["algorithm"], c["accepted"]) ++ self.assertEqual( ++ result, c["expected"], msg=f"{c['desc']} - algorithm='{c['algorithm']}', accepted={c['accepted']}" ++ ) ++ ++ def test_forward_compatibility_ecc_normalization(self): ++ """Test forward compatibility: specific ECC in accepted list should accept 'ecc' algorithm""" ++ test_cases = [ ++ { ++ "algorithm": "ecc", ++ "accepted": ["ecc256"], ++ "expected": True, ++ "desc": "ecc should be accepted when 'ecc256' is in accepted list (both normalize to ecc256)", ++ }, ++ { ++ "algorithm": "ecc", ++ "accepted": ["ecc384"], ++ "expected": False, ++ "desc": "ecc should NOT be accepted when only 'ecc384' is in accepted list", ++ }, ++ { ++ "algorithm": "ecc", ++ "accepted": ["ecc521"], ++ "expected": False, ++ "desc": "ecc should NOT be accepted when only 'ecc521' is in accepted list", ++ }, ++ ] ++ ++ for c in test_cases: ++ result = is_accepted(c["algorithm"], c["accepted"]) ++ self.assertEqual( ++ result, c["expected"], msg=f"{c['desc']} - algorithm='{c['algorithm']}', accepted={c['accepted']}" ++ ) ++ ++ def test_bidirectional_algorithm_matching(self): ++ """Test bidirectional matching scenarios that happen in real usage""" ++ test_cases = [ ++ { ++ "algorithm": "ecc256", ++ "accepted": ["rsa", "ecc"], ++ "expected": True, ++ "desc": "Agent reports ecc256, tenant config has generic 'ecc'", ++ }, ++ { ++ "algorithm": "ecc", ++ "accepted": ["rsa", "ecc256"], ++ "expected": True, ++ "desc": "Agent reports generic 'ecc', tenant config has specific 'ecc256'", ++ }, ++ { ++ "algorithm": "ecc384", ++ "accepted": ["rsa", "ecc"], ++ "expected": False, ++ "desc": "Agent reports ecc384, tenant has generic 'ecc' (should not match)", ++ }, ++ { ++ "algorithm": "ecc", ++ "accepted": ["rsa", "ecc384"], ++ "expected": False, ++ "desc": "Agent reports generic 'ecc', tenant has ecc384 (should not match)", ++ }, ++ ] ++ ++ for c in test_cases: ++ result = is_accepted(c["algorithm"], c["accepted"]) ++ self.assertEqual( ++ result, c["expected"], msg=f"{c['desc']} - algorithm='{c['algorithm']}', accepted={c['accepted']}" ++ ) ++ ++ def test_mixed_algorithm_types(self): ++ """Test mixing different algorithm types in accepted list""" ++ test_cases = [ ++ { ++ "algorithm": "rsa", ++ "accepted": ["ecc", "rsa"], ++ "expected": True, ++ }, ++ { ++ "algorithm": "ecc256", ++ "accepted": ["rsa", "ecc"], ++ "expected": True, ++ }, ++ { ++ "algorithm": "ecc384", ++ "accepted": ["rsa", "ecc256", "ecc384"], ++ "expected": True, ++ }, ++ { ++ "algorithm": "unknown", ++ "accepted": ["rsa", "ecc", "ecc384"], ++ "expected": False, ++ }, ++ ] ++ ++ for c in test_cases: ++ result = is_accepted(c["algorithm"], c["accepted"]) ++ self.assertEqual(result, c["expected"], msg=f"algorithm='{c['algorithm']}', accepted={c['accepted']}") ++ ++ def test_edge_cases(self): ++ """Test edge cases and boundary conditions""" ++ test_cases = [ ++ {"algorithm": "", "accepted": ["ecc"], "expected": False, "desc": "Empty algorithm string"}, ++ {"algorithm": "ecc256", "accepted": [], "expected": False, "desc": "Empty accepted list"}, ++ {"algorithm": "ecc256", "accepted": [""], "expected": False, "desc": "Accepted list with empty string"}, ++ { ++ "algorithm": "ECC256", ++ "accepted": ["ecc256"], ++ "expected": False, ++ "desc": "Case sensitivity - uppercase should not match", ++ }, ++ { ++ "algorithm": "ecc256", ++ "accepted": ["ecc"], ++ "expected": True, ++ "desc": "ecc256 algorithm should match ecc in accepted list", ++ }, ++ { ++ "algorithm": "ecc", ++ "accepted": ["ecc256"], ++ "expected": True, ++ "desc": "ecc algorithm should match ecc256 in accepted list", ++ }, ++ ] ++ ++ for c in test_cases: ++ result = is_accepted(c["algorithm"], c["accepted"]) ++ self.assertEqual( ++ result, c["expected"], msg=f"{c['desc']} - algorithm='{c['algorithm']}', accepted={c['accepted']}" ++ ) +-- +2.47.3 + diff --git a/SOURCES/0017-algorithms-add-support-for-specific-RSA-algorithms.patch b/SOURCES/0017-algorithms-add-support-for-specific-RSA-algorithms.patch new file mode 100644 index 0000000..b081517 --- /dev/null +++ b/SOURCES/0017-algorithms-add-support-for-specific-RSA-algorithms.patch @@ -0,0 +1,87 @@ +From eecd2f73642f784b19cb1bb9c78c6d0b1e486dda Mon Sep 17 00:00:00 2001 +From: Sergio Correia +Date: Fri, 26 Sep 2025 00:03:49 +0100 +Subject: [PATCH 14/18] algorithms: add support for specific RSA algorithms + +Similar to the previous change for ECC, now we extend the Encrypt enum +to support the following specific RSA algorithms: +- RSA1024 +- RSA2048 +- RSA3072 +- RSA4096 + +Map also 'rsa' to 'rsa2048' for backwards compatibility. + +Signed-off-by: Sergio Correia +--- + keylime/common/algorithms.py | 8 ++++++++ + test/test_algorithms.py | 13 ++++++++++++- + 2 files changed, 20 insertions(+), 1 deletion(-) + +diff --git a/keylime/common/algorithms.py b/keylime/common/algorithms.py +index bb22fb6..32a1ec1 100644 +--- a/keylime/common/algorithms.py ++++ b/keylime/common/algorithms.py +@@ -84,6 +84,10 @@ class Hash(str, enum.Enum): + + class Encrypt(str, enum.Enum): + RSA = "rsa" ++ RSA1024 = "rsa1024" ++ RSA2048 = "rsa2048" ++ RSA3072 = "rsa3072" ++ RSA4096 = "rsa4096" + ECC = "ecc" + ECC192 = "ecc192" + ECC224 = "ecc224" +@@ -96,6 +100,8 @@ class Encrypt(str, enum.Enum): + # Handle aliases to match agent behavior + if algorithm == "ecc": + algorithm = "ecc256" # Default ECC alias maps to P-256, same as the agent. ++ if algorithm == "rsa": ++ algorithm = "rsa2048" # Default RSA alias maps to RSA-2048, same as the agent. + return algorithm in list(Encrypt) + + @staticmethod +@@ -103,6 +109,8 @@ class Encrypt(str, enum.Enum): + """Normalize algorithm string to handle aliases, matching the agent behavior""" + if algorithm == "ecc": + return "ecc256" # Default ECC alias maps to P-256. ++ if algorithm == "rsa": ++ return "rsa2048" # Default RSA alias maps to RSA-2048. + return algorithm + + +diff --git a/test/test_algorithms.py b/test/test_algorithms.py +index 8a31fa9..5542c0f 100644 +--- a/test/test_algorithms.py ++++ b/test/test_algorithms.py +@@ -181,7 +181,7 @@ class TestEncrypt(unittest.TestCase): + }, + { + "input": "rsa", +- "expected": "rsa", ++ "expected": "rsa2048", + }, + ] + +@@ -199,6 +199,17 @@ class TestEncrypt(unittest.TestCase): + # Test that direct ecc256 works + self.assertTrue(Encrypt.is_recognized("ecc256")) + ++ def test_normalize_rsa_alias_behavior(self): ++ """Test that RSA alias normalization matches agent behavior""" ++ # Test that "rsa" is recognized through alias handling ++ self.assertTrue(Encrypt.is_recognized("rsa")) ++ ++ # Test that normalize converts "rsa" to "rsa2048" ++ self.assertEqual(Encrypt.normalize("rsa"), "rsa2048") ++ ++ # Test that direct rsa2048 works ++ self.assertTrue(Encrypt.is_recognized("rsa2048")) ++ + + class TestSign(unittest.TestCase): + def test_is_recognized(self): +-- +2.47.3 + diff --git a/SOURCES/0018-tpm_util-fix-quote-signature-extraction-for-ECDSA.patch b/SOURCES/0018-tpm_util-fix-quote-signature-extraction-for-ECDSA.patch new file mode 100644 index 0000000..9a5480a --- /dev/null +++ b/SOURCES/0018-tpm_util-fix-quote-signature-extraction-for-ECDSA.patch @@ -0,0 +1,43 @@ +From 690a2059be01993f5e7f65a01d994e53b82211e4 Mon Sep 17 00:00:00 2001 +From: Thore Sommer +Date: Mon, 3 Mar 2025 15:44:37 +0100 +Subject: [PATCH 15/18] tpm_util: fix quote signature extraction for ECDSA + +Signed-off-by: Thore Sommer +--- + keylime/tpm/tpm_util.py | 12 +++++++++--- + 1 file changed, 9 insertions(+), 3 deletions(-) + +diff --git a/keylime/tpm/tpm_util.py b/keylime/tpm/tpm_util.py +index cdecd32..25c40e0 100644 +--- a/keylime/tpm/tpm_util.py ++++ b/keylime/tpm/tpm_util.py +@@ -223,9 +223,7 @@ def checkquote( + pcrblob: The state of the PCRs that were quoted; Intel tpm2-tools specific format + exp_hash_alg: The hash that was expected to have been used for quoting + """ +- sig_alg, hash_alg, sig_size = struct.unpack_from(">HHH", sigblob, 0) +- +- (signature,) = struct.unpack_from(f"{sig_size}s", sigblob, 6) ++ sig_alg, hash_alg = struct.unpack_from(">HH", sigblob, 0) + + pubkey = serialization.load_pem_public_key(aikblob, backend=backends.default_backend()) + if not isinstance(pubkey, (RSAPublicKey, EllipticCurvePublicKey)): +@@ -236,6 +234,14 @@ def checkquote( + if isinstance(pubkey, EllipticCurvePublicKey) and sig_alg not in [tpm2_objects.TPM_ALG_ECDSA]: + raise ValueError(f"Unsupported quote signature algorithm '{sig_alg:#x}' for EC keys") + ++ if sig_alg in [tpm2_objects.TPM_ALG_RSASSA]: ++ (sig_size,) = struct.unpack_from(">H", sigblob, 4) ++ (signature,) = struct.unpack_from(f"{sig_size}s", sigblob, 6) ++ elif sig_alg in [tpm2_objects.TPM_ALG_ECDSA]: ++ signature = ecdsa_der_from_tpm(sigblob) ++ else: ++ raise ValueError(f"Unsupported quote signature algorithm '{sig_alg:#x}'") ++ + hashfunc = tpm2_objects.HASH_FUNCS.get(hash_alg) + if not hashfunc: + raise ValueError(f"Unsupported hash with id {hash_alg:#x} in signature blob") +-- +2.47.3 + diff --git a/SOURCES/0019-tpm-fix-ECC-P-521-coordinate-validation.patch b/SOURCES/0019-tpm-fix-ECC-P-521-coordinate-validation.patch new file mode 100644 index 0000000..e0493ea --- /dev/null +++ b/SOURCES/0019-tpm-fix-ECC-P-521-coordinate-validation.patch @@ -0,0 +1,516 @@ +From c0aaf2ad80e2ec714b46ae1ba94678791d58b02d Mon Sep 17 00:00:00 2001 +From: rpm-build +Date: Wed, 4 Feb 2026 01:00:00 +0100 +Subject: [PATCH] tpm: fix ECC P-521 coordinate validation +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +P-521 coordinates can vary from 65-66 bytes due to TPM implementations +padding 521-bit values to byte boundaries or stripping leading zeros. +The previous validation was too strict, rejecting valid coordinates. + +Enhanced validation: +- Accepts P-521 coordinate range 65-66 bytes (520-528 bits) +- Validates against actual NIST prime moduli per SEC1 §2.3.5 and + FIPS 186-4 App D (coordinates must be < field prime p) +- Strict rejection of unknown curves for security + +The enhanced approach prevents false validation of coordinates that are +the correct byte length but exceed the curve's field prime. + +Backported from upstream commit 0219550c3a29db85b202a02b17590260a41a262f + +Signed-off-by: Anderson Toshiyuki Sasaki +--- + keylime/tpm/tpm2_objects.py | 42 +++- + test/test_tpm2_objects.py | 417 ++++++++++++++++++++++++++++++++++++ + 2 files changed, 455 insertions(+), 4 deletions(-) + create mode 100644 test/test_tpm2_objects.py + +diff --git a/keylime/tpm/tpm2_objects.py b/keylime/tpm/tpm2_objects.py +index fcc5bb5..9170628 100644 +--- a/keylime/tpm/tpm2_objects.py ++++ b/keylime/tpm/tpm2_objects.py +@@ -31,6 +31,16 @@ TPM_ECC_NIST_P256 = 0x0003 + TPM_ECC_NIST_P384 = 0x0004 + TPM_ECC_NIST_P521 = 0x0005 + ++# ECC curve prime moduli lookup table (coordinates must be < p) ++# This structure supports NIST curves and can be extended for other curves. ++ECC_CURVE_PRIMES = { ++ TPM_ECC_NIST_P192: 2**192 - 2**64 - 1, # P-192 prime ++ TPM_ECC_NIST_P224: 2**224 - 2**96 + 1, # P-224 prime ++ TPM_ECC_NIST_P256: 2**256 - 2**224 + 2**192 + 2**96 - 1, # P-256 prime ++ TPM_ECC_NIST_P384: 2**384 - 2**128 - 2**96 + 2**32 - 1, # P-384 prime ++ TPM_ECC_NIST_P521: 2**521 - 1, # P-521 prime ++} ++ + TPM_ALG_RSA = 0x0001 + TPM_ALG_ECC = 0x0023 + +@@ -318,10 +328,34 @@ def pubkey_parms_from_tpm2b_public( + if len(rest) != 0: + raise ValueError("Misparsed: more contents after X and Y") + +- if (len(x) * 8) != curve.key_size: +- raise ValueError(f"Misparsed either X or curve: {len(x)}*8 != {curve.key_size}") +- if (len(y) * 8) != curve.key_size: +- raise ValueError(f"Misparsed either Y or curve curve: {len(y)}*8 != {curve.key_size}") ++ # ECC coordinates can vary in byte length due to: ++ # 1. Padding to byte boundaries (most common) ++ # 2. Leading zero stripping in some encodings ++ # Validate both byte length and actual coordinate value. ++ max_bytes = (curve.key_size + 7) // 8 ++ min_bytes = max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes ++ ++ # Get the actual prime modulus for the curve ++ prime_p = ECC_CURVE_PRIMES.get(curve_id) ++ if prime_p is None: ++ raise ValueError(f"Unsupported curve ID {curve_id:#x}: prime modulus not known") ++ ++ for label, coord in (("X", x), ("Y", y)): ++ coord_len = len(coord) ++ if coord_len < min_bytes or coord_len > max_bytes: ++ raise ValueError( ++ f"Misparsed {label} coordinate: got {coord_len} bytes, " ++ f"expected {min_bytes}-{max_bytes} for {curve.key_size}-bit curve" ++ ) ++ ++ coord_int = int.from_bytes(coord, "big") ++ # Coordinates must be reduced modulo the field prime p ++ # (SEC1 §2.3.5, FIPS 186-4 App D). Reject values >= p. ++ if coord_int >= prime_p: ++ raise ValueError( ++ f"{label} coordinate too large: {coord_int.bit_length()} bits, " ++ f"must be < {prime_p.bit_length()}-bit prime modulus" ++ ) + + bx = int.from_bytes(x, byteorder="big") + by = int.from_bytes(y, byteorder="big") +diff --git a/test/test_tpm2_objects.py b/test/test_tpm2_objects.py +new file mode 100644 +index 0000000..9cc6b70 +--- /dev/null ++++ b/test/test_tpm2_objects.py +@@ -0,0 +1,417 @@ ++import struct ++import unittest ++ ++from cryptography.hazmat.primitives.asymmetric import ec ++ ++from keylime.tpm.tpm2_objects import ( ++ ECC_CURVE_PRIMES, ++ TPM_ECC_NIST_P192, ++ TPM_ECC_NIST_P224, ++ TPM_ECC_NIST_P256, ++ TPM_ECC_NIST_P384, ++ TPM_ECC_NIST_P521, ++ _curve_from_curve_id, ++ _pack_in_tpm2b, ++ pubkey_parms_from_tpm2b_public, ++) ++ ++ ++class TestTpm2Objects(unittest.TestCase): ++ def test_p521_coordinate_validation_logic(self): ++ """Test the specific coordinate validation logic for P-521""" ++ curve = _curve_from_curve_id(TPM_ECC_NIST_P521) ++ ++ # Test the updated validation logic ++ max_bytes = (curve.key_size + 7) // 8 # Should be 66 bytes for P-521 ++ min_bytes = max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes # Should be 65 bytes for P-521 ++ ++ self.assertEqual(max_bytes, 66) ++ self.assertEqual(min_bytes, 65) # P-521 is not byte-aligned, so allows 65-66 bytes ++ ++ # Test coordinate sizes that should be accepted (65-66 bytes for P-521) ++ valid_sizes = [65, 66] ++ ++ for size in valid_sizes: ++ # Check that the validation logic would accept this size ++ should_pass = min_bytes <= size <= max_bytes ++ self.assertTrue(should_pass, f"Size {size} bytes should be valid for P-521") ++ ++ # Test coordinate sizes that should be rejected ++ invalid_sizes = [64, 67, 32, 68] ++ ++ for size in invalid_sizes: ++ # This should fail: not in the valid range ++ should_fail = size < min_bytes or size > max_bytes ++ self.assertTrue(should_fail, f"Size {size} bytes should be invalid for P-521") ++ ++ def test_p256_coordinate_validation_logic(self): ++ """Test the coordinate validation logic for P-256 to ensure no regression""" ++ curve = _curve_from_curve_id(TPM_ECC_NIST_P256) ++ ++ max_bytes = (curve.key_size + 7) // 8 # Should be 32 bytes for P-256 ++ min_bytes = ( ++ max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes ++ ) # Should be 32 bytes for P-256 (byte-aligned) ++ ++ self.assertEqual(max_bytes, 32) ++ self.assertEqual(min_bytes, 32) # P-256 is byte-aligned, so only accepts 32 bytes ++ ++ # 32 bytes should be accepted ++ size = 32 ++ should_pass = min_bytes <= size <= max_bytes ++ self.assertTrue(should_pass, f"P-256 should accept {size} bytes") ++ ++ # Other sizes should be rejected ++ invalid_sizes = [31, 33, 64] ++ for size in invalid_sizes: ++ should_fail = size < min_bytes or size > max_bytes ++ self.assertTrue(should_fail, f"P-256 should reject {size} bytes") ++ ++ def test_p384_coordinate_validation_logic(self): ++ """Test the coordinate validation logic for P-384 to ensure no regression""" ++ curve = _curve_from_curve_id(TPM_ECC_NIST_P384) ++ ++ max_bytes = (curve.key_size + 7) // 8 # Should be 48 bytes for P-384 ++ min_bytes = ( ++ max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes ++ ) # Should be 48 bytes for P-384 (byte-aligned) ++ ++ self.assertEqual(max_bytes, 48) ++ self.assertEqual(min_bytes, 48) # P-384 is byte-aligned, so only accepts 48 bytes ++ ++ # 48 bytes should be accepted ++ size = 48 ++ should_pass = min_bytes <= size <= max_bytes ++ self.assertTrue(should_pass, f"P-384 should accept {size} bytes") ++ ++ def test_coordinate_size_calculation(self): ++ """Test that coordinate size calculations are correct for different curves""" ++ # P-256: 256 bits -> (256 + 7) // 8 = 32 bytes ++ curve_p256 = _curve_from_curve_id(TPM_ECC_NIST_P256) ++ expected_p256 = (curve_p256.key_size + 7) // 8 ++ self.assertEqual(expected_p256, 32) ++ self.assertEqual(curve_p256.key_size, 256) ++ ++ # P-384: 384 bits -> (384 + 7) // 8 = 48 bytes ++ curve_p384 = _curve_from_curve_id(TPM_ECC_NIST_P384) ++ expected_p384 = (curve_p384.key_size + 7) // 8 ++ self.assertEqual(expected_p384, 48) ++ self.assertEqual(curve_p384.key_size, 384) ++ ++ # P-521: 521 bits -> (521 + 7) // 8 = 66 bytes ++ curve_p521 = _curve_from_curve_id(TPM_ECC_NIST_P521) ++ expected_p521 = (curve_p521.key_size + 7) // 8 ++ self.assertEqual(expected_p521, 66) ++ self.assertEqual(curve_p521.key_size, 521) ++ ++ def test_p521_specific_fix(self): ++ """Test the specific scenario that was fixed: P-521 with 66-byte coordinates""" ++ curve = _curve_from_curve_id(TPM_ECC_NIST_P521) ++ ++ # The key issue: P-521 has 521 bits ++ self.assertEqual(curve.key_size, 521) ++ ++ # TPMs pad to 66 bytes (528 bits) ++ tpm_padded_size = 66 ++ tpm_padded_bits = tpm_padded_size * 8 ++ self.assertEqual(tpm_padded_bits, 528) ++ ++ # The old validation would reject: (66 * 8) != 521 ++ old_validation_fails = tpm_padded_bits != curve.key_size ++ self.assertTrue(old_validation_fails, "Old validation would incorrectly reject 66-byte coordinates") ++ ++ # The new validation should accept: len(x) == expected_bytes OR (len(x) * 8) == curve.key_size ++ expected_bytes = (curve.key_size + 7) // 8 ++ new_validation_passes = (tpm_padded_size == expected_bytes) or (tpm_padded_bits == curve.key_size) ++ self.assertTrue(new_validation_passes, "New validation should accept 66-byte coordinates") ++ ++ def test_validation_before_and_after_fix(self): ++ """Test that demonstrates the fix by comparing old vs new validation logic""" ++ curve = _curve_from_curve_id(TPM_ECC_NIST_P521) ++ ++ # Test multiple coordinate sizes that P-521 can have ++ test_sizes = [65, 66] # 65 bytes (leading zero stripped), 66 bytes (padded) ++ ++ max_bytes = (curve.key_size + 7) // 8 # 66 bytes ++ min_bytes = max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes # 65 bytes for P-521 ++ ++ for coordinate_size in test_sizes: ++ # Old validation logic (strict bit size match) - would require exactly 65.125 bytes ++ # which is impossible since we can't have fractional bytes ++ ++ # New validation logic (accept range for non-byte-aligned curves) ++ new_logic_passes = min_bytes <= coordinate_size <= max_bytes ++ self.assertTrue(new_logic_passes, f"New logic should accept {coordinate_size}-byte coordinates for P-521") ++ ++ # Verify the calculations ++ self.assertEqual(max_bytes, 66) ++ self.assertEqual(min_bytes, 65) ++ ++ def test_p521_coordinate_range_validation(self): ++ """Test that P-521 accepts coordinates in the range 65-66 bytes (520-528 bits)""" ++ curve = _curve_from_curve_id(TPM_ECC_NIST_P521) ++ ++ # P-521: 521 bits, padded to 66 bytes (528 bits), or 65 bytes with leading zero stripped ++ max_bytes = (curve.key_size + 7) // 8 # 66 bytes ++ min_bytes = max_bytes - 1 # 65 bytes (since 521 % 8 != 0) ++ ++ # Test all valid sizes ++ valid_sizes = [65, 66] ++ for size in valid_sizes: ++ is_valid = min_bytes <= size <= max_bytes ++ self.assertTrue(is_valid, f"P-521 should accept {size} bytes ({size * 8} bits)") ++ ++ # Test invalid sizes ++ invalid_sizes = [64, 67, 68, 32] ++ for size in invalid_sizes: ++ is_invalid = size < min_bytes or size > max_bytes ++ self.assertTrue(is_invalid, f"P-521 should reject {size} bytes ({size * 8} bits)") ++ ++ def test_coordinate_value_validation(self): ++ """Test that coordinate values are validated against actual prime moduli""" ++ # Test P-521 with actual prime ++ # curve_p521 = _curve_from_curve_id(TPM_ECC_NIST_P521) # Not needed for this test ++ p521_prime = ECC_CURVE_PRIMES[TPM_ECC_NIST_P521] ++ ++ # Test valid coordinate value (within range) ++ valid_coord_int = p521_prime - 1 # Largest valid value ++ valid_coord_bytes = valid_coord_int.to_bytes(66, "big") # 66 bytes, padded ++ ++ # Test the validation logic ++ coord_int = int.from_bytes(valid_coord_bytes, "big") ++ is_valid_value = coord_int < p521_prime ++ self.assertTrue(is_valid_value, "Coordinate value should be valid for P-521") ++ ++ # Test invalid coordinate value (>= prime) ++ invalid_coord_int = p521_prime # Equal to prime (invalid) ++ invalid_coord_bytes = invalid_coord_int.to_bytes(66, "big") # 66 bytes, but value too large ++ ++ coord_int = int.from_bytes(invalid_coord_bytes, "big") ++ is_invalid_value = coord_int >= p521_prime ++ self.assertTrue(is_invalid_value, "Coordinate value >= prime should be invalid for P-521") ++ ++ def test_prime_constants_accuracy(self): ++ """Test that our hardcoded prime constants are correct""" ++ # Verify the NIST prime values ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P192], 2**192 - 2**64 - 1) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P224], 2**224 - 2**96 + 1) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P256], 2**256 - 2**224 + 2**192 + 2**96 - 1) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P384], 2**384 - 2**128 - 2**96 + 2**32 - 1) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P521], 2**521 - 1) ++ ++ # Verify they are actually less than 2^m for all curves except P-521 ++ self.assertLess(ECC_CURVE_PRIMES[TPM_ECC_NIST_P192], 2**192) ++ self.assertLess(ECC_CURVE_PRIMES[TPM_ECC_NIST_P224], 2**224) ++ self.assertLess(ECC_CURVE_PRIMES[TPM_ECC_NIST_P256], 2**256) ++ self.assertLess(ECC_CURVE_PRIMES[TPM_ECC_NIST_P384], 2**384) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P521], 2**521 - 1) # P-521 is special case ++ ++ def test_prime_lookup_table(self): ++ """Test that the prime lookup table works correctly""" ++ # Test known curves ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P192], 2**192 - 2**64 - 1) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P224], 2**224 - 2**96 + 1) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P256], 2**256 - 2**224 + 2**192 + 2**96 - 1) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P384], 2**384 - 2**128 - 2**96 + 2**32 - 1) ++ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P521], 2**521 - 1) ++ ++ # Test rejection of unknown curve ++ unknown_curve_id = 0x9999 ++ unknown_prime = ECC_CURVE_PRIMES.get(unknown_curve_id) ++ self.assertIsNone(unknown_prime, "Unknown curves should not be in ECC_CURVE_PRIMES") ++ ++ def test_error_message_formatting(self): ++ """Test that error messages use bit_length() instead of full integers""" ++ # Create a large coordinate value ++ large_value = ECC_CURVE_PRIMES[TPM_ECC_NIST_P521] # This would be hundreds of digits ++ ++ # Verify bit_length() is much more reasonable than the full number ++ bit_length = large_value.bit_length() ++ self.assertEqual(bit_length, 521) # Much more readable than 150+ digit number ++ ++ # The error message should use bit lengths, not full integers ++ expected_msg_pattern = f"coordinate too large: {bit_length} bits" ++ self.assertIn("521 bits", expected_msg_pattern) ++ ++ def test_unknown_curve_rejection(self): ++ """Test that unknown curves are strictly rejected""" ++ # This tests the design decision to be strict rather than use fallbacks ++ unknown_curve_id = 0x9999 ++ ++ # The strict approach: unknown curves should not have fallback behavior ++ # This ensures we only validate curves we explicitly understand ++ result = ECC_CURVE_PRIMES.get(unknown_curve_id) ++ self.assertIsNone(result, "Unknown curves should be explicitly rejected, not given fallback primes") ++ ++ ++class TestEccPublicKeySecurityValidation(unittest.TestCase): ++ """Test that ECC public key validation includes all required security checks: ++ 1. Point is on the curve ++ 2. Point is not zero or infinity ++ 3. Point is not in a small subgroup (not applicable to NIST curves with cofactor=1) ++ """ ++ ++ def create_ecc_tpm2b_public(self, x: int, y: int, curve_id: int = TPM_ECC_NIST_P256) -> bytes: ++ """Helper to create a TPM2B_PUBLIC structure for ECC key with given coordinates""" ++ # Get coordinate size based on curve ++ curve = _curve_from_curve_id(curve_id) ++ coord_bytes = (curve.key_size + 7) // 8 ++ ++ # Convert coordinates to bytes ++ x_bytes = x.to_bytes(coord_bytes, "big") ++ y_bytes = y.to_bytes(coord_bytes, "big") ++ ++ # Build TPMT_PUBLIC structure ++ # alg_type (TPM_ALG_ECC = 0x0023) ++ tpmt = struct.pack(">H", 0x0023) ++ # name_alg (TPM_ALG_SHA256 = 0x000B) ++ tpmt += struct.pack(">H", 0x000B) ++ # object_attributes (4 bytes) ++ tpmt += struct.pack(">I", 0x00040072) ++ # auth_policy (empty TPM2B) ++ tpmt += struct.pack(">H", 0) ++ # symmetric (TPM_ALG_NULL) ++ tpmt += struct.pack(">H", 0x0010) ++ # scheme (TPM_ALG_NULL) ++ tpmt += struct.pack(">H", 0x0010) ++ # curve_id ++ tpmt += struct.pack(">H", curve_id) ++ # kdf_scheme (TPM_ALG_NULL) ++ tpmt += struct.pack(">H", 0x0010) ++ # x coordinate (TPM2B) ++ tpmt += _pack_in_tpm2b(x_bytes) ++ # y coordinate (TPM2B) ++ tpmt += _pack_in_tpm2b(y_bytes) ++ ++ # Wrap in TPM2B_PUBLIC ++ return _pack_in_tpm2b(tpmt) ++ ++ def test_point_on_curve_validation(self): ++ """Test that points not on the curve are rejected (Security Check #1)""" ++ # For P-256, the curve equation is: y² = x³ - 3x + b (mod p) ++ # Choose coordinates that don't satisfy this equation ++ x = 1 ++ y = 1 # (1, 1) is not on the P-256 curve ++ ++ tpm2b_public = self.create_ecc_tpm2b_public(x, y, TPM_ECC_NIST_P256) ++ ++ # The cryptography library should reject this point as not being on the curve ++ with self.assertRaises(ValueError) as cm: ++ pubkey_parms_from_tpm2b_public(tpm2b_public) ++ self.assertIn("invalid ec key", str(cm.exception).lower()) ++ ++ def test_point_at_infinity_validation(self): ++ """Test that the point at infinity (0, 0) is rejected (Security Check #2)""" ++ # The point at infinity should be rejected ++ x = 0 ++ y = 0 ++ ++ tpm2b_public = self.create_ecc_tpm2b_public(x, y, TPM_ECC_NIST_P256) ++ ++ # The cryptography library should reject the point at infinity ++ with self.assertRaises(ValueError) as cm: ++ pubkey_parms_from_tpm2b_public(tpm2b_public) ++ self.assertIn("invalid ec key", str(cm.exception).lower()) ++ ++ def test_valid_point_accepted(self): ++ """Test that a valid point on the curve is accepted""" ++ # Generate a valid key and extract its coordinates ++ private_key = ec.generate_private_key(ec.SECP256R1()) ++ public_key = private_key.public_key() ++ numbers = public_key.public_numbers() ++ ++ # Create TPM2B_PUBLIC with valid coordinates ++ tpm2b_public = self.create_ecc_tpm2b_public(numbers.x, numbers.y, TPM_ECC_NIST_P256) ++ ++ # Should parse successfully ++ parsed_key, _ = pubkey_parms_from_tpm2b_public(tpm2b_public) ++ self.assertIsInstance(parsed_key, ec.EllipticCurvePublicKey) ++ ++ # Verify the coordinates match ++ assert isinstance(parsed_key, ec.EllipticCurvePublicKey) # Type narrowing for pyright ++ parsed_numbers = parsed_key.public_numbers() ++ self.assertEqual(parsed_numbers.x, numbers.x) ++ self.assertEqual(parsed_numbers.y, numbers.y) ++ ++ def test_small_subgroup_not_applicable_to_nist_curves(self): ++ """Test documenting that small subgroup checks are not needed for NIST curves (Security Check #3) ++ ++ NIST P-curves (P-192, P-224, P-256, P-384, P-521) all have cofactor h=1, ++ meaning the entire curve has prime order. There are no small subgroups to check. ++ ++ Curves with cofactor > 1 (like Curve25519 with h=8) require additional validation ++ to ensure the point is not in a small subgroup, but this is not applicable to ++ the NIST curves used by TPMs. ++ """ ++ # This test documents the cofactor=1 property for all supported NIST curves ++ # The cryptography library's point validation is sufficient for these curves ++ ++ test_curves = [ ++ (TPM_ECC_NIST_P192, ec.SECP192R1()), ++ (TPM_ECC_NIST_P224, ec.SECP224R1()), ++ (TPM_ECC_NIST_P256, ec.SECP256R1()), ++ (TPM_ECC_NIST_P384, ec.SECP384R1()), ++ (TPM_ECC_NIST_P521, ec.SECP521R1()), ++ ] ++ ++ for curve_id, curve_obj in test_curves: ++ with self.subTest(curve=curve_obj.name): ++ try: ++ # Generate a valid key for this curve ++ # Note: P-192 may not be supported in newer OpenSSL versions ++ private_key = ec.generate_private_key(curve_obj) ++ except Exception: # pylint: disable=broad-except ++ # Skip this specific curve if not supported by OpenSSL (e.g., P-192) ++ self.skipTest(f"Curve {curve_obj.name} not supported by OpenSSL") ++ ++ public_key = private_key.public_key() ++ numbers = public_key.public_numbers() ++ ++ # Create TPM2B_PUBLIC and verify it parses successfully ++ tpm2b_public = self.create_ecc_tpm2b_public(numbers.x, numbers.y, curve_id) ++ parsed_key, _ = pubkey_parms_from_tpm2b_public(tpm2b_public) ++ ++ # All NIST curves have cofactor = 1, so no small subgroup attacks possible ++ # The point validation by the cryptography library is sufficient ++ self.assertIsInstance(parsed_key, ec.EllipticCurvePublicKey) ++ ++ def test_coordinate_exceeds_field_prime_rejected(self): ++ """Test that coordinates >= field prime are rejected""" ++ # Use a coordinate value that's >= the field prime for P-256 ++ p256_prime = ECC_CURVE_PRIMES[TPM_ECC_NIST_P256] ++ ++ # x coordinate exceeds the field prime ++ x = p256_prime + 1 ++ y = 1 ++ ++ tpm2b_public = self.create_ecc_tpm2b_public(x, y, TPM_ECC_NIST_P256) ++ ++ # Should be rejected during coordinate validation ++ with self.assertRaises(ValueError) as cm: ++ pubkey_parms_from_tpm2b_public(tpm2b_public) ++ # Will fail either at coordinate validation or curve validation ++ self.assertTrue( ++ "coordinate too large" in str(cm.exception).lower() or "invalid ec key" in str(cm.exception).lower() ++ ) ++ ++ def test_p521_point_validation(self): ++ """Test point validation works correctly for P-521 (non-byte-aligned curve)""" ++ # Generate a valid P-521 key ++ private_key = ec.generate_private_key(ec.SECP521R1()) ++ public_key = private_key.public_key() ++ numbers = public_key.public_numbers() ++ ++ # Valid point should be accepted ++ tpm2b_public = self.create_ecc_tpm2b_public(numbers.x, numbers.y, TPM_ECC_NIST_P521) ++ parsed_key, _ = pubkey_parms_from_tpm2b_public(tpm2b_public) ++ self.assertIsInstance(parsed_key, ec.EllipticCurvePublicKey) ++ ++ # Invalid point should be rejected ++ tpm2b_public_invalid = self.create_ecc_tpm2b_public(1, 1, TPM_ECC_NIST_P521) ++ with self.assertRaises(ValueError) as cm: ++ pubkey_parms_from_tpm2b_public(tpm2b_public_invalid) ++ self.assertIn("invalid ec key", str(cm.exception).lower()) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +-- +2.52.0 + diff --git a/SOURCES/0020-tpm-fix-ECC-P-521-credential-activation-with-consist.patch b/SOURCES/0020-tpm-fix-ECC-P-521-credential-activation-with-consist.patch new file mode 100644 index 0000000..176ba07 --- /dev/null +++ b/SOURCES/0020-tpm-fix-ECC-P-521-credential-activation-with-consist.patch @@ -0,0 +1,222 @@ +From a4c32b7a84c93df86284b95e735923beeb18ca94 Mon Sep 17 00:00:00 2001 +From: rpm-build +Date: Wed, 4 Feb 2026 18:58:40 +0100 +Subject: [PATCH] tpm: fix ECC P-521 credential activation with consistent + marshaling + +The TPM credential activation was failing for P-521 curves due to +inconsistent ECC point marshaling in tpms_ecc_point_marshal(). + +The function used bit_length() which varies for P-521 coordinates +(520-521 bits), producing different blob sizes and causing TPM +integrity check failures during ActivateCredential operations. + +Backported from upstream commit 1db525b7abf62e6d2d9450817477fd0911e083d4 + +Signed-off-by: Anderson Toshiyuki Sasaki +--- + keylime/tpm/tpm2_objects.py | 11 +-- + keylime/tpm/tpm_util.py | 7 +- + test/test_tpm2_objects.py | 130 ++++++++++++++++++++++++++++++++++++ + 3 files changed, 142 insertions(+), 6 deletions(-) + +diff --git a/keylime/tpm/tpm2_objects.py b/keylime/tpm/tpm2_objects.py +index 9170628..d33ebaa 100644 +--- a/keylime/tpm/tpm2_objects.py ++++ b/keylime/tpm/tpm2_objects.py +@@ -597,9 +597,12 @@ def unmarshal_tpml_pcr_selection(tpml_pcr_selection: bytes) -> Tuple[Dict[int, i + + def tpms_ecc_point_marshal(public_key: EllipticCurvePublicKey) -> bytes: + pn = public_key.public_numbers() ++ curve = public_key.curve + +- sz = (pn.x.bit_length() + 7) // 8 +- secret = struct.pack(">H", sz) + pn.x.to_bytes(sz, "big") ++ # Use fixed coordinate size based on curve to ensure consistent marshaling ++ # This is critical for P-521 where bit_length() can vary (520-521 bits) ++ # leading to credential activation failures due to inconsistent blob sizes ++ coord_size = (curve.key_size + 7) // 8 + +- sz = (pn.y.bit_length() + 7) // 8 +- return secret + struct.pack(">H", sz) + pn.y.to_bytes(sz, "big") ++ secret = struct.pack(">H", coord_size) + pn.x.to_bytes(coord_size, "big") ++ return secret + struct.pack(">H", coord_size) + pn.y.to_bytes(coord_size, "big") +diff --git a/keylime/tpm/tpm_util.py b/keylime/tpm/tpm_util.py +index 25c40e0..fbbe557 100644 +--- a/keylime/tpm/tpm_util.py ++++ b/keylime/tpm/tpm_util.py +@@ -318,11 +318,14 @@ def crypt_secret_encrypt_ecc(public_key: EllipticCurvePublicKey, hashfunc: hashe + + digest_size = hashfunc.digest_size + ++ # Use fixed coordinate size for consistent marshaling ++ coord_size = (public_key.curve.key_size + 7) // 8 ++ + x = my_public_key.public_numbers().x +- party_x = x.to_bytes((x.bit_length() + 7) >> 3, "big") ++ party_x = x.to_bytes(coord_size, "big") + + x = public_key.public_numbers().x +- party_y = x.to_bytes((x.bit_length() + 7) >> 3, "big") ++ party_y = x.to_bytes(coord_size, "big") + + data = crypt_kdfe(hashfunc, ecc_secret_x, "IDENTITY", party_x, party_y, digest_size << 3) + +diff --git a/test/test_tpm2_objects.py b/test/test_tpm2_objects.py +index 9cc6b70..aa91f8d 100644 +--- a/test/test_tpm2_objects.py ++++ b/test/test_tpm2_objects.py +@@ -1,6 +1,7 @@ + import struct + import unittest + ++from cryptography.hazmat.primitives import hashes + from cryptography.hazmat.primitives.asymmetric import ec + + from keylime.tpm.tpm2_objects import ( +@@ -13,7 +14,9 @@ from keylime.tpm.tpm2_objects import ( + _curve_from_curve_id, + _pack_in_tpm2b, + pubkey_parms_from_tpm2b_public, ++ tpms_ecc_point_marshal, + ) ++from keylime.tpm.tpm_util import crypt_secret_encrypt_ecc + + + class TestTpm2Objects(unittest.TestCase): +@@ -413,5 +416,132 @@ class TestEccPublicKeySecurityValidation(unittest.TestCase): + self.assertIn("invalid ec key", str(cm.exception).lower()) + + ++class TestEccMarshaling(unittest.TestCase): ++ """Test ECC point marshaling consistency fixes""" ++ ++ def test_p521_marshaling_consistency(self): ++ """Test that P-521 marshaling produces consistent blob sizes regardless of coordinate values""" ++ # Generate multiple P-521 keys to test with different coordinate values ++ keys = [] ++ for _ in range(10): ++ private_key = ec.generate_private_key(ec.SECP521R1()) ++ keys.append(private_key.public_key()) ++ ++ # Marshal all keys and check that blob sizes are consistent ++ blob_sizes = [] ++ for key in keys: ++ blob = tpms_ecc_point_marshal(key) ++ blob_sizes.append(len(blob)) ++ ++ # All blobs should be the same size for P-521 ++ self.assertEqual(len(set(blob_sizes)), 1, "All P-521 marshaled blobs should have the same size") ++ ++ # Expected size: 2 bytes (x size) + 66 bytes (x coord) + 2 bytes (y size) + 66 bytes (y coord) = 136 bytes ++ expected_size = 2 + 66 + 2 + 66 ++ self.assertEqual(blob_sizes[0], expected_size, f"P-521 marshaled blob should be {expected_size} bytes") ++ ++ def test_marshaling_coordinate_sizes(self): ++ """Test that marshaled coordinates use fixed sizes based on curve key size""" ++ # Test P-521: 521 bits -> (521 + 7) // 8 = 66 bytes per coordinate ++ p521_key = ec.generate_private_key(ec.SECP521R1()).public_key() ++ p521_blob = tpms_ecc_point_marshal(p521_key) ++ ++ # Parse the blob to check coordinate sizes ++ x_size = struct.unpack(">H", p521_blob[:2])[0] ++ y_size = struct.unpack(">H", p521_blob[2 + x_size : 2 + x_size + 2])[0] ++ ++ self.assertEqual(x_size, 66, "P-521 X coordinate should be 66 bytes") ++ self.assertEqual(y_size, 66, "P-521 Y coordinate should be 66 bytes") ++ ++ # Test P-256: 256 bits -> (256 + 7) // 8 = 32 bytes per coordinate ++ p256_key = ec.generate_private_key(ec.SECP256R1()).public_key() ++ p256_blob = tpms_ecc_point_marshal(p256_key) ++ ++ x_size = struct.unpack(">H", p256_blob[:2])[0] ++ y_size = struct.unpack(">H", p256_blob[2 + x_size : 2 + x_size + 2])[0] ++ ++ self.assertEqual(x_size, 32, "P-256 X coordinate should be 32 bytes") ++ self.assertEqual(y_size, 32, "P-256 Y coordinate should be 32 bytes") ++ ++ def test_p521_credential_activation_consistency(self): ++ """Test the specific issue: P-521 credential activation with consistent marshaling""" ++ # This test verifies the fix for credential activation failures ++ # Generate two P-521 keys with potentially different bit lengths for coordinates ++ key1 = ec.generate_private_key(ec.SECP521R1()).public_key() ++ key2 = ec.generate_private_key(ec.SECP521R1()).public_key() ++ ++ # Marshal both keys ++ blob1 = tpms_ecc_point_marshal(key1) ++ blob2 = tpms_ecc_point_marshal(key2) ++ ++ # The critical fix: both blobs should be the same size regardless of coordinate bit lengths ++ self.assertEqual( ++ len(blob1), len(blob2), "P-521 marshaled blobs must be same size regardless of coordinate bit lengths" ++ ) ++ ++ # Both should use the fixed coordinate size (66 bytes) ++ expected_total_size = 2 + 66 + 2 + 66 # size_x + x + size_y + y ++ self.assertEqual(len(blob1), expected_total_size) ++ self.assertEqual(len(blob2), expected_total_size) ++ ++ def test_marshaling_format_correctness(self): ++ """Test that marshaling follows the correct TPM format: size(2) + coord(n) + size(2) + coord(n)""" ++ key = ec.generate_private_key(ec.SECP521R1()).public_key() ++ blob = tpms_ecc_point_marshal(key) ++ ++ # Parse the blob structure ++ if len(blob) < 4: ++ self.fail("Marshaled blob too short") ++ ++ x_size = struct.unpack(">H", blob[:2])[0] ++ self.assertEqual(x_size, 66, "X coordinate size should be 66 for P-521") ++ ++ if len(blob) < 2 + x_size + 2: ++ self.fail("Marshaled blob missing Y coordinate size") ++ ++ y_size = struct.unpack(">H", blob[2 + x_size : 2 + x_size + 2])[0] ++ self.assertEqual(y_size, 66, "Y coordinate size should be 66 for P-521") ++ ++ # Total size should be: 2 + 66 + 2 + 66 = 136 ++ expected_total = 2 + x_size + 2 + y_size ++ self.assertEqual(len(blob), expected_total, "Total marshaled blob size incorrect") ++ ++ def test_crypt_secret_encrypt_ecc_consistency(self): ++ """Test that crypt_secret_encrypt_ecc produces consistent results with fixed coordinate sizes""" ++ # Generate a P-521 key to test with ++ public_key = ec.generate_private_key(ec.SECP521R1()).public_key() ++ hashfunc = hashes.SHA256() ++ ++ # Call the function multiple times and check consistency ++ results = [] ++ for _ in range(5): ++ data, point = crypt_secret_encrypt_ecc(public_key, hashfunc) ++ results.append((data, point)) ++ ++ # Check that all returned points have consistent marshaling ++ # (the data will be different due to random key generation, but point marshaling should be consistent) ++ point_sizes = [len(point) for _, point in results] ++ self.assertEqual(len(set(point_sizes)), 1, "All marshaled points should have the same size") ++ ++ # For P-521, the marshaled point should be 136 bytes (2+66+2+66) ++ expected_point_size = 2 + 66 + 2 + 66 ++ self.assertEqual( ++ point_sizes[0], expected_point_size, f"P-521 marshaled point should be {expected_point_size} bytes" ++ ) ++ ++ # All data results should be different (due to random ephemeral keys) ++ data_results = [data for data, _ in results] ++ self.assertEqual( ++ len(set(data_results)), ++ len(data_results), ++ "All data results should be different due to random ephemeral keys", ++ ) ++ ++ # All data results should have the same length (SHA256 digest size) ++ data_sizes = [len(data) for data, _ in results] ++ self.assertEqual(len(set(data_sizes)), 1, "All data results should have the same size") ++ self.assertEqual(data_sizes[0], hashfunc.digest_size, "Data size should match hash digest size") ++ ++ + if __name__ == "__main__": + unittest.main() +-- +2.52.0 + diff --git a/SOURCES/0021-tpm-fix-ECC-signature-parsing-to-support-variable-le.patch b/SOURCES/0021-tpm-fix-ECC-signature-parsing-to-support-variable-le.patch new file mode 100644 index 0000000..fbe8cbb --- /dev/null +++ b/SOURCES/0021-tpm-fix-ECC-signature-parsing-to-support-variable-le.patch @@ -0,0 +1,372 @@ +From ee4192df70384fa6b23f359a287e042103ba4ea9 Mon Sep 17 00:00:00 2001 +From: Sergio Correia +Date: Thu, 25 Sep 2025 14:37:10 +0100 +Subject: [PATCH 18/18] tpm: fix ECC signature parsing to support + variable-length coordinates + +The previous ECC signature validation implementation incorrectly assumed +fixed-length coordinate encoding, causing failures with mathematically +correct variable-length coordinates (especially P-521 curves where +coordinates are typically 65-66 bytes). + +This commit fixes the ecdsa_der_from_tpm() function to properly handle +TSS ESAPI signature format where the sig_size field contains only the +r component size, followed by the s component with its own size header. + +This enables proper ECC attestation for the supported NIST curves. + +Assisted-by: Claude 4 Sonnet +Signed-off-by: Sergio Correia +--- + keylime/tpm/tpm_main.py | 2 +- + keylime/tpm/tpm_util.py | 105 +++++++++++++++++++--- + test/test_tpm2_objects.py | 184 +++++++++++++++++++++++++++++++++++++- + 3 files changed, 277 insertions(+), 14 deletions(-) + +diff --git a/keylime/tpm/tpm_main.py b/keylime/tpm/tpm_main.py +index 6f2e89f..ecbacbe 100644 +--- a/keylime/tpm/tpm_main.py ++++ b/keylime/tpm/tpm_main.py +@@ -91,7 +91,7 @@ class Tpm: + if isinstance(iak_pub, EllipticCurvePublicKey): + if sig_alg in [tpm2_objects.TPM_ALG_ECDSA]: + try: +- der_sig = tpm_util.ecdsa_der_from_tpm(iak_sign) ++ der_sig = tpm_util.ecdsa_der_from_tpm(iak_sign, iak_pub) + tpm_util.verify(iak_pub, der_sig, digest, hashfunc) + logger.info("Agent %s AIK verified with IAK", uuid) + return True +diff --git a/keylime/tpm/tpm_util.py b/keylime/tpm/tpm_util.py +index fbbe557..f554f94 100644 +--- a/keylime/tpm/tpm_util.py ++++ b/keylime/tpm/tpm_util.py +@@ -59,6 +59,43 @@ logger = keylime_logging.init_logging("tpm_util") + + SupportedKeyTypes = Union[RSAPublicKey, EllipticCurvePublicKey] + ++# ECC signature parsing constants. ++# Raw signature sizes for different ECC curves (r||s concatenated format). ++# r and s are the two mathematical components of an ECDSA signature. ++ECC_SECP192R1_SIGNATURE_SIZE = 48 # 24 bytes each for r,s. ++ECC_SECP224R1_SIGNATURE_SIZE = 56 # 28 bytes each for r,s. ++ECC_SECP256R1_SIGNATURE_SIZE = 64 # 32 bytes each for r,s. ++ECC_SECP384R1_SIGNATURE_SIZE = 96 # 48 bytes each for r,s. ++ECC_SECP521R1_SIGNATURE_SIZE = 132 # 66 bytes each for r,s. ++ ++# TPM2B_ECDSA_SIGNATURE format constants. ++TPM2B_SIZE_FIELD_LENGTH = 2 # 2 bytes for size field. ++TPM2B_MIN_HEADER_SIZE = 4 # Minimum: 2 bytes r_size + 2 bytes s_size. ++ ++# DER encoding constants. ++DER_SEQUENCE_TAG = 0x30 ++ ++# Signature blob header offset (skip alg, hash_alg, sig_size headers). ++SIGNATURE_BLOB_HEADER_SIZE = 6 ++ ++# ECC curve to signature size mapping for raw r||s format. ++ECC_RAW_SIGNATURE_SIZES = { ++ "secp192r1": ECC_SECP192R1_SIGNATURE_SIZE, ++ "secp224r1": ECC_SECP224R1_SIGNATURE_SIZE, ++ "secp256r1": ECC_SECP256R1_SIGNATURE_SIZE, ++ "secp384r1": ECC_SECP384R1_SIGNATURE_SIZE, ++ "secp521r1": ECC_SECP521R1_SIGNATURE_SIZE, ++} ++ ++# ECC curve coordinate size ranges (min, max) for validation. ++ECC_COORDINATE_SIZE_RANGES = { ++ "secp192r1": (1, 24), # 192 bits = 24 bytes max ++ "secp224r1": (1, 28), # 224 bits = 28 bytes max ++ "secp256r1": (1, 32), # 256 bits = 32 bytes max ++ "secp384r1": (1, 48), # 384 bits = 48 bytes max ++ "secp521r1": (1, 66), # 521 bits = 66 bytes max (521 bits, not 512) ++} ++ + + def verify( + pubkey: SupportedKeyTypes, +@@ -106,17 +143,59 @@ def der_len(encoded_int_len: int) -> bytes: + return bytes((0x80 | len(bin_str),)) + bin_str + + +-def ecdsa_der_from_tpm(sigblob: bytes) -> bytes: +- _, _, sig_size_r = struct.unpack_from(">HHH", sigblob, 0) +- sig_r = sigblob[6 : 6 + sig_size_r] +- encoded_sig_r = der_int(sig_r) +- sigblob = sigblob[6 + sig_size_r :] +- sig_size_s = struct.unpack_from(">H", sigblob, 0)[0] +- sig_s = sigblob[2 : 2 + sig_size_s] +- encoded_sig_s = der_int(sig_s) +- total_size = len(encoded_sig_r) + len(encoded_sig_s) +- der_sig = bytes.fromhex(f"30{total_size:x}") + encoded_sig_r + encoded_sig_s +- return der_sig ++def ecdsa_der_from_tpm(sigblob: bytes, pubkey: EllipticCurvePublicKey) -> bytes: ++ """Convert ECC signature from TPM format to DER format for cryptographic verification. ++ ++ This function handles TSS ESAPI signature format where the signature header's ++ sig_size field contains the size of the r component, followed by the s component ++ with its own size header. ++ ++ Parameters ++ ---------- ++ sigblob: TPM signature blob containing signature headers and signature data ++ pubkey: ECC public key to determine expected signature size ++ ++ Returns ++ ------- ++ DER-encoded ECDSA signature suitable for cryptographic library verification ++ ++ Raises ++ ------ ++ ValueError: If signature format cannot be parsed or is invalid ++ """ ++ # Extract signature header information. ++ _sig_alg, _hash_alg, sig_size_r = struct.unpack_from(">HHH", sigblob, 0) ++ ++ # Extract the r component (size is in sig_size_r field). ++ sig_r = sigblob[SIGNATURE_BLOB_HEADER_SIZE : SIGNATURE_BLOB_HEADER_SIZE + sig_size_r] ++ ++ # The s component follows immediately after r, with its own size header. ++ s_offset = SIGNATURE_BLOB_HEADER_SIZE + sig_size_r ++ if s_offset + 2 <= len(sigblob): ++ sig_size_s = struct.unpack_from(">H", sigblob, s_offset)[0] ++ s_start = s_offset + 2 ++ if s_start + sig_size_s <= len(sigblob): ++ sig_s = sigblob[s_start : s_start + sig_size_s] ++ ++ # Validate coordinate sizes against curve requirements. ++ curve_name = pubkey.curve.name ++ coordinate_range = ECC_COORDINATE_SIZE_RANGES.get(curve_name) ++ if coordinate_range: ++ min_size, max_size = coordinate_range ++ if not min_size <= len(sig_r) <= max_size: ++ raise ValueError(f"Invalid r coordinate size {len(sig_r)} for curve {curve_name}") ++ if not min_size <= len(sig_s) <= max_size: ++ raise ValueError(f"Invalid s coordinate size {len(sig_s)} for curve {curve_name}") ++ ++ # Convert to DER format. ++ encoded_sig_r = der_int(sig_r) ++ encoded_sig_s = der_int(sig_s) ++ total_size = len(encoded_sig_r) + len(encoded_sig_s) ++ der_length = der_len(total_size) ++ der_sig = bytes([DER_SEQUENCE_TAG]) + der_length + encoded_sig_r + encoded_sig_s ++ return der_sig ++ ++ raise ValueError("Unable to parse ECC signature from TPM format") + + + def __get_pcrs_from_blob(pcrblob: bytes) -> Tuple[int, Dict[int, int], List[bytes]]: +@@ -238,7 +317,9 @@ def checkquote( + (sig_size,) = struct.unpack_from(">H", sigblob, 4) + (signature,) = struct.unpack_from(f"{sig_size}s", sigblob, 6) + elif sig_alg in [tpm2_objects.TPM_ALG_ECDSA]: +- signature = ecdsa_der_from_tpm(sigblob) ++ if not isinstance(pubkey, EllipticCurvePublicKey): ++ raise ValueError(f"ECDSA signature algorithm requires EllipticCurvePublicKey, got {type(pubkey)}") ++ signature = ecdsa_der_from_tpm(sigblob, pubkey) + else: + raise ValueError(f"Unsupported quote signature algorithm '{sig_alg:#x}'") + +diff --git a/test/test_tpm2_objects.py b/test/test_tpm2_objects.py +index 48d6a43..c0e4c0a 100644 +--- a/test/test_tpm2_objects.py ++++ b/test/test_tpm2_objects.py +@@ -16,7 +16,7 @@ from keylime.tpm.tpm2_objects import ( + pubkey_parms_from_tpm2b_public, + tpms_ecc_point_marshal, + ) +-from keylime.tpm.tpm_util import crypt_secret_encrypt_ecc ++from keylime.tpm.tpm_util import crypt_secret_encrypt_ecc, der_int, der_len, ecdsa_der_from_tpm + + + class TestTpm2Objects(unittest.TestCase): +@@ -543,5 +543,187 @@ class TestEccMarshaling(unittest.TestCase): + self.assertEqual(data_sizes[0], hashfunc.digest_size, "Data size should match hash digest size") + + ++class TestEccSignatureParsing(unittest.TestCase): ++ """Test ECC signature parsing improvements for variable-length coordinates""" ++ ++ def create_test_signature_blob(self, sig_r: bytes, sig_s: bytes) -> bytes: ++ """Create a test TPM signature blob with given r and s components""" ++ # TPM signature format: sig_alg(2) + hash_alg(2) + sig_size_r(2) + r_data + sig_size_s(2) + s_data ++ sig_alg = 0x0018 # TPM_ALG_ECDSA ++ hash_alg = 0x000B # TPM_ALG_SHA256 ++ ++ blob = struct.pack(">HHH", sig_alg, hash_alg, len(sig_r)) ++ blob += sig_r ++ blob += struct.pack(">H", len(sig_s)) ++ blob += sig_s ++ ++ return blob ++ ++ def test_p521_variable_length_coordinates(self): ++ """Test that P-521 signatures with variable-length coordinates are parsed correctly""" ++ # Generate a P-521 key for testing ++ private_key = ec.generate_private_key(ec.SECP521R1()) ++ public_key = private_key.public_key() ++ ++ # Test with 65-byte coordinates (leading zero stripped) ++ sig_r_65 = b"\x00" * 1 + b"\x01" * 64 # 65 bytes ++ sig_s_65 = b"\x00" * 1 + b"\x02" * 64 # 65 bytes ++ ++ blob_65 = self.create_test_signature_blob(sig_r_65, sig_s_65) ++ ++ # Should parse successfully ++ der_sig_65 = ecdsa_der_from_tpm(blob_65, public_key) ++ self.assertIsInstance(der_sig_65, bytes) ++ self.assertTrue(len(der_sig_65) > 0) ++ ++ # Test with 66-byte coordinates (full padding) ++ sig_r_66 = b"\x00" * 2 + b"\x01" * 64 # 66 bytes ++ sig_s_66 = b"\x00" * 2 + b"\x02" * 64 # 66 bytes ++ ++ blob_66 = self.create_test_signature_blob(sig_r_66, sig_s_66) ++ ++ # Should parse successfully ++ der_sig_66 = ecdsa_der_from_tpm(blob_66, public_key) ++ self.assertIsInstance(der_sig_66, bytes) ++ self.assertTrue(len(der_sig_66) > 0) ++ ++ def test_coordinate_size_validation(self): ++ """Test that coordinate size validation works for different curves""" ++ # Test P-256 with valid coordinates ++ p256_key = ec.generate_private_key(ec.SECP256R1()).public_key() ++ ++ # Valid P-256 coordinates (32 bytes each) ++ sig_r_32 = b"\x01" * 32 ++ sig_s_32 = b"\x02" * 32 ++ blob_p256_valid = self.create_test_signature_blob(sig_r_32, sig_s_32) ++ ++ # Should parse successfully ++ der_sig = ecdsa_der_from_tpm(blob_p256_valid, p256_key) ++ self.assertIsInstance(der_sig, bytes) ++ ++ # Test P-256 with invalid coordinates (too large) ++ sig_r_invalid = b"\x01" * 50 # Too large for P-256 ++ sig_s_invalid = b"\x02" * 50 # Too large for P-256 ++ blob_p256_invalid = self.create_test_signature_blob(sig_r_invalid, sig_s_invalid) ++ ++ # Should raise ValueError ++ with self.assertRaises(ValueError) as cm: ++ ecdsa_der_from_tpm(blob_p256_invalid, p256_key) ++ self.assertIn("Invalid r coordinate size", str(cm.exception)) ++ ++ def test_signature_parsing_edge_cases(self): ++ """Test edge cases in signature parsing""" ++ p256_key = ec.generate_private_key(ec.SECP256R1()).public_key() ++ ++ # Test with truncated blob (missing s component) ++ truncated_blob = struct.pack(">HHH", 0x0018, 0x000B, 32) + b"\x01" * 32 ++ # Missing s component ++ ++ with self.assertRaises(ValueError) as cm: ++ ecdsa_der_from_tpm(truncated_blob, p256_key) ++ self.assertIn("Unable to parse ECC signature", str(cm.exception)) ++ ++ # Test with blob too short for s size header ++ short_blob = struct.pack(">HHH", 0x0018, 0x000B, 32) + b"\x01" * 32 + b"\x00" # Only 1 byte for s size ++ ++ with self.assertRaises(ValueError) as cm: ++ ecdsa_der_from_tpm(short_blob, p256_key) ++ self.assertIn("Unable to parse ECC signature", str(cm.exception)) ++ ++ def test_der_encoding_correctness(self): ++ """Test that DER encoding produces correctly formatted output""" ++ p256_key = ec.generate_private_key(ec.SECP256R1()).public_key() ++ ++ # Create test coordinates ++ sig_r = b"\x01" * 32 ++ sig_s = b"\x02" * 32 ++ blob = self.create_test_signature_blob(sig_r, sig_s) ++ ++ der_sig = ecdsa_der_from_tpm(blob, p256_key) ++ ++ # DER signature should start with SEQUENCE tag (0x30) ++ self.assertEqual(der_sig[0], 0x30, "DER signature should start with SEQUENCE tag") ++ ++ # Should be parseable as DER format ++ # The structure should be: 0x30 + length + INTEGER(r) + INTEGER(s) ++ self.assertTrue(len(der_sig) >= 6, "DER signature should have minimum length") ++ ++ def test_multiple_curve_support(self): ++ """Test that signature parsing works for multiple curve types""" ++ test_cases = [ ++ (ec.SECP256R1(), 32), ++ (ec.SECP384R1(), 48), ++ (ec.SECP521R1(), 66), ++ ] ++ ++ for curve, coord_size in test_cases: ++ with self.subTest(curve=curve.name): ++ private_key = ec.generate_private_key(curve) ++ public_key = private_key.public_key() ++ ++ # Create test signature with appropriate coordinate size ++ sig_r = b"\x01" * coord_size ++ sig_s = b"\x02" * coord_size ++ blob = self.create_test_signature_blob(sig_r, sig_s) ++ ++ # Should parse successfully ++ der_sig = ecdsa_der_from_tpm(blob, public_key) ++ self.assertIsInstance(der_sig, bytes) ++ self.assertTrue(len(der_sig) > 0) ++ self.assertEqual(der_sig[0], 0x30) # DER SEQUENCE tag ++ ++ def test_der_int_encoding(self): ++ """Test DER integer encoding helper function""" ++ # Test positive number that doesn't need padding ++ test_bytes = b"\x7F" # 127, no padding needed ++ der_encoded = der_int(test_bytes) ++ expected = b"\x02\x01\x7F" # INTEGER tag + length + value ++ self.assertEqual(der_encoded, expected) ++ ++ # Test positive number that needs zero padding (high bit set) ++ test_bytes = b"\xFF" # 255, needs zero padding ++ der_encoded = der_int(test_bytes) ++ expected = b"\x02\x02\x00\xFF" # INTEGER tag + length + zero padding + value ++ self.assertEqual(der_encoded, expected) ++ ++ def test_der_len_encoding(self): ++ """Test DER length encoding helper function""" ++ # Test short form (< 128) ++ short_len = der_len(50) ++ self.assertEqual(short_len, b"\x32") # 50 in hex ++ ++ # Test long form (>= 128) ++ long_len = der_len(300) # 0x012C ++ expected = b"\x82\x01\x2C" # Long form: 0x80 | 2 bytes, then 0x012C ++ self.assertEqual(long_len, expected) ++ ++ def test_signature_format_validation_comprehensive(self): ++ """Comprehensive test of signature format validation""" ++ p521_key = ec.generate_private_key(ec.SECP521R1()).public_key() ++ ++ # Test minimum valid coordinate sizes for P-521 ++ valid_sizes = [65, 66] ++ for size in valid_sizes: ++ sig_r = b"\x01" * size ++ sig_s = b"\x02" * size ++ blob = self.create_test_signature_blob(sig_r, sig_s) ++ ++ # Should not raise exception ++ der_sig = ecdsa_der_from_tpm(blob, p521_key) ++ self.assertIsInstance(der_sig, bytes) ++ ++ # Test invalid coordinate sizes for P-521 (outside the 1-66 range) ++ invalid_sizes = [0, 67, 100] # 0 is too small, 67+ is too large ++ for size in invalid_sizes: ++ with self.subTest(size=size): ++ sig_r = b"\x01" * size if size > 0 else b"" ++ sig_s = b"\x02" * size if size > 0 else b"" ++ blob = self.create_test_signature_blob(sig_r, sig_s) ++ ++ with self.assertRaises(ValueError) as cm: ++ ecdsa_der_from_tpm(blob, p521_key) ++ self.assertIn("coordinate size", str(cm.exception)) ++ ++ + if __name__ == "__main__": + unittest.main() +-- +2.47.3 + diff --git a/SOURCES/0022-CVE-2026-1709.patch b/SOURCES/0022-CVE-2026-1709.patch new file mode 100644 index 0000000..b494256 --- /dev/null +++ b/SOURCES/0022-CVE-2026-1709.patch @@ -0,0 +1,20 @@ +diff --git a/keylime/web/base/server.py b/keylime/web/base/server.py +index 1d9a9c2..859b23a 100644 +--- a/keylime/web/base/server.py ++++ b/keylime/web/base/server.py +@@ -2,7 +2,6 @@ import asyncio + import multiprocessing + from abc import ABC, abstractmethod + from functools import wraps +-from ssl import CERT_OPTIONAL + from typing import TYPE_CHECKING, Any, Callable, Optional + + import tornado +@@ -252,7 +251,6 @@ class Server(ABC): + self._https_port = config.getint(component, "tls_port", fallback=0) + self._max_upload_size = config.getint(component, "max_upload_size", fallback=104857600) + self._ssl_ctx = web_util.init_mtls(component) +- self._ssl_ctx.verify_mode = CERT_OPTIONAL + + def _get(self, pattern: str, controller: type["Controller"], action: str, allow_insecure: bool = False) -> None: + """Creates a new route to handle incoming GET requests issued for paths which match the given diff --git a/SPECS/keylime.spec b/SPECS/keylime.spec index a31de45..2d89fd0 100644 --- a/SPECS/keylime.spec +++ b/SPECS/keylime.spec @@ -9,7 +9,7 @@ Name: keylime Version: 7.12.1 -Release: 11%{?dist} +Release: 15%{?dist} Summary: Open source TPM software for Bootstrapping and Maintaining Trust URL: https://github.com/keylime/keylime @@ -39,6 +39,30 @@ Patch: 0009-mb-support-vendor_db-as-logged-by-newer-shim-version.patch Patch: 0010-verifier-Gracefully-shutdown-on-signal.patch Patch: 0011-revocations-Try-to-send-notifications-on-shutdown.patch Patch: 0012-requests_client-close-the-session-at-the-end-of-the-.patch +Patch: 0013-fix-malformed-certs-workaround.patch + +# CVE-2025-13609 +# Backports from: +# - https://github.com/keylime/keylime/pull/1817/commits/1024e19d +# - https://github.com/keylime/keylime/pull/1825 +Patch: 0014-Add-shared-memory-infrastructure-for-multiprocess-co.patch +Patch: 0015-Fix-registrar-duplicate-UUID-vulnerability.patch + +# Backported from: +# - https://github.com/keylime/keylime/pull/1746 +# - https://github.com/keylime/keylime/pull/1803 +# - https://github.com/keylime/keylime/pull/1808 +# ECC attestation support. +Patch: 0016-algorithms-add-support-for-specific-ECC-curve-algori.patch +Patch: 0017-algorithms-add-support-for-specific-RSA-algorithms.patch +Patch: 0018-tpm_util-fix-quote-signature-extraction-for-ECDSA.patch +Patch: 0019-tpm-fix-ECC-P-521-coordinate-validation.patch +Patch: 0020-tpm-fix-ECC-P-521-credential-activation-with-consist.patch +Patch: 0021-tpm-fix-ECC-signature-parsing-to-support-variable-le.patch + +# CVE-2026-1709 +# Fix registrar authentication bypass +Patch: 0022-CVE-2026-1709.patch License: ASL 2.0 and MIT @@ -92,7 +116,8 @@ Requires: openssl %if 0%{?with_selinux} # This ensures that the *-selinux package and all it’s dependencies are not pulled # into containers and other systems that do not use SELinux -Recommends: (%{srcname}-selinux if selinux-policy-%{selinuxtype}) +Recommends: (%{srcname}-selinux = %{version}-%{release} if selinux-policy-%{selinuxtype}) + %endif %ifarch %efi @@ -418,8 +443,8 @@ fi %config(noreplace) %verify(not md5 size mode mtime) %attr(400,%{srcname},%{srcname}) %{_sysconfdir}/%{srcname}/logging.conf %attr(700,%{srcname},%{srcname}) %dir %{_rundir}/%{srcname} %attr(700,%{srcname},%{srcname}) %dir %{_sharedstatedir}/%{srcname} -%attr(500,%{srcname},%{srcname}) %dir %{_datadir}/%{srcname}/tpm_cert_store -%attr(400,%{srcname},%{srcname}) %{_datadir}/%{srcname}/tpm_cert_store/*.pem +%attr(755,root,root) %dir %{_datadir}/%{srcname}/tpm_cert_store +%attr(644,root,root) %{_datadir}/%{srcname}/tpm_cert_store/*.pem %attr(500,%{srcname},%{srcname}) %dir %{_sharedstatedir}/%{srcname}/tpm_cert_store %attr(400,%{srcname},%{srcname}) %{_sharedstatedir}/%{srcname}/tpm_cert_store/*.pem %{_tmpfilesdir}/%{srcname}.conf @@ -433,6 +458,26 @@ fi %license LICENSE %changelog +* Fri Feb 13 2026 Anderson Toshiyuki Sasaki - 7.12.1-15 +- Fix registrar authentication bypass (CVE-2026-1709) + Resolves: RHEL-145391 + +* Wed Feb 04 2026 Anderson Toshiyuki Sasaki - 7.12.1-14 +- Add support for TPM quotes using ECC keys + Resolves: RHEL-118150 + +* Tue Feb 03 2026 Sergio Correia - 7.12.1-13 +- Keylime: Registrar allows identity takeover via duplicate UUID registration + Resolves: RHEL-130761 + +* Mon Feb 02 2026 Sergio Correia - 7.12.1-12 +- Change ownership of /usr/share/keylime/tpm_cert_store to root + Resolves: RHEL-106024 + +* Mon Sep 15 2025 Anderson Toshiyuki Sasaki - 7.12.1-11.2 +- Properly fix the malformed certificate workaround + Resolves: RHEL-111244 + * Mon Aug 18 2025 Sergio Correia - 7.12.1-11 - Fix for revocation notifier not closing TLS session correctly Resolves: RHEL-109656