Fix CVE-2021-3406

This ensures we verify the EK and AIK we get from the agent before trusting signatures by it. Advisory: GHSA-78f8-6c68-375m For details, see https://patrick.uiterwijk.org/blog/tpm2-attestation-keylime-vulnerability Signed-off-by: Patrick Uiterwijk <[email protected]> Signed-off-by: Michael Peters <[email protected]>
THS-on · Feb 24, 2021 · 8e86e3d · 8e86e3d
1 parent 6c9b0c5
commit 8e86e3d
Show file tree

Hide file tree

Showing 11 changed files with 215 additions and 155 deletions.
diff --git a/keylime/cloud_verifier_common.py b/keylime/cloud_verifier_common.py
@@ -199,7 +199,7 @@ def process_quote_response(agent, json_response):
         agent['nonce'],
         received_public_key,
         quote,
-        agent['registrar_keys']['aik'],
+        agent['registrar_keys']['aik_tpm'],
         agent['tpm_policy'],
         ima_measurement_list,
         agent['allowlist'],

diff --git a/keylime/db/registrar_db.py b/keylime/db/registrar_db.py
@@ -21,9 +21,9 @@ class RegistrarMain(Base):
     agent_id = Column(String(80),
                       primary_key=True)
     key = Column(String(45))
-    aik = Column(String(500))
-    ek = Column(String(500))
+    aik_tpm = Column(String(500))
     ekcert = Column(String(2048))
+    ek_tpm = Column(String(500))
     virtual = Column(Integer)
     active = Column(Integer)
     provider_keys = Column(JSONPickleType(pickler=json))

diff --git a/keylime/keylime_agent.py b/keylime/keylime_agent.py
@@ -495,7 +495,7 @@ def main():
     config.ch_dir(config.WORK_DIR, logger)
 
     # initialize tpm
-    (ek, ekcert, aik, ek_tpm, aik_name) = instance_tpm.tpm_init(self_activate=False, config_pw=config.get(
+    (ekcert, ek_tpm, aik_tpm) = instance_tpm.tpm_init(self_activate=False, config_pw=config.get(
         'cloud_agent', 'tpm_ownerpassword'))  # this tells initialize not to self activate the AIK
     virtual_agent = instance_tpm.is_vtpm()
     # try to get some TPM randomness into the system entropy pool
@@ -504,7 +504,7 @@ def main():
     if ekcert is None:
         if virtual_agent:
             ekcert = 'virtual'
-        elif tpm.is_emulator():
+        elif instance_tpm.is_emulator():
             ekcert = 'emulator'
 
     # now we need the UUID
@@ -515,7 +515,7 @@ def main():
     if agent_uuid == 'openstack':
         agent_uuid = openstack.get_openstack_uuid()
     elif agent_uuid == 'hash_ek':
-        agent_uuid = hashlib.sha256(ek).hexdigest()
+        agent_uuid = hashlib.sha256(ek_tpm).hexdigest()
     elif agent_uuid == 'generate' or agent_uuid is None:
         agent_uuid = str(uuid.uuid4())
     elif agent_uuid == 'dmidecode':
@@ -540,7 +540,7 @@ def main():
 
     # register it and get back a blob
     keyblob = registrar_client.doRegisterAgent(
-        registrar_ip, registrar_port, agent_uuid, ek, ekcert, aik, ek_tpm, aik_name)
+        registrar_ip, registrar_port, agent_uuid, ek_tpm, ekcert, aik_tpm)
 
     if keyblob is None:
         raise Exception("Registration failed")

diff --git a/keylime/migrations/versions/cc2630851a1f_receive_the_aik_tpm_from_the_agent.py b/keylime/migrations/versions/cc2630851a1f_receive_the_aik_tpm_from_the_agent.py
@@ -0,0 +1,52 @@
+"""receive the aik_tpm from the agent
+
+Revision ID: cc2630851a1f
+Revises: a7a64155ab3a
+Create Date: 2021-02-08 18:06:26.520283
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'cc2630851a1f'
+down_revision = 'a7a64155ab3a'
+branch_labels = None
+depends_on = None
+
+
+def upgrade(engine_name):
+    globals()["upgrade_%s" % engine_name]()
+
+
+def downgrade(engine_name):
+    globals()["downgrade_%s" % engine_name]()
+
+
+def upgrade_registrar():
+    with op.batch_alter_table('registrarmain') as batch_op:
+        batch_op.drop_column('aik')
+        batch_op.drop_column('ek')
+
+    with op.batch_alter_table('registrarmain') as batch_op:
+        batch_op.add_column(sa.Column('aik_tpm', sa.String(length=500), nullable=True))
+        batch_op.add_column(sa.Column('ek_tpm', sa.String(length=500), nullable=True))
+
+
+def downgrade_registrar():
+    with op.batch_alter_table('registrarmain') as batch_op:
+        batch_op.add_column(sa.Column('ek', sa.VARCHAR(length=500), nullable=True))
+        batch_op.add_column(sa.Column('aik', sa.VARCHAR(length=500), nullable=True))
+
+    with op.batch_alter_table('registrarmain') as batch_op:
+        batch_op.drop_column('ek_tpm')
+        batch_op.drop_column('aik_tpm')
+
+
+def upgrade_cloud_verifier():
+    pass
+
+
+def downgrade_cloud_verifier():
+    pass
diff --git a/keylime/registrar_client.py b/keylime/registrar_client.py
@@ -98,7 +98,7 @@ def getKeys(registrar_ip, registrar_port, agent_id):
                 "Error: unexpected http response body from Registrar Server: %s" % str(response.status_code))
             return None
 
-        if "aik" not in response_body["results"]:
+        if "aik_tpm" not in response_body["results"]:
             logger.critical(
                 "Error: did not receive AIK from Registrar Server: %s" % str(response.status_code))
             return None
@@ -118,14 +118,13 @@ def getKeys(registrar_ip, registrar_port, agent_id):
     return None
 
 
-def doRegisterAgent(registrar_ip, registrar_port, agent_id, pub_ek, ekcert, pub_aik, pub_ek_tpm=None, aik_name=None):
+def doRegisterAgent(registrar_ip, registrar_port, agent_id, ek_tpm, ekcert, aik_tpm):
     data = {
-        'ek': pub_ek,
         'ekcert': ekcert,
-        'aik': pub_aik,
-        'aik_name': aik_name,
-        'ek_tpm': pub_ek_tpm,
+        'aik_tpm': aik_tpm,
     }
+    if ekcert is None or ekcert == 'emulator':
+        data['ek_tpm'] = ek_tpm
     response = None
     try:
         client = RequestsClient(f'{registrar_ip}:{registrar_port}', tls_enabled)

diff --git a/keylime/registrar_common.py b/keylime/registrar_common.py
@@ -3,6 +3,7 @@
 Copyright 2017 Massachusetts Institute of Technology.
 '''
 
+import base64
 import threading
 import sys
 import signal
@@ -13,13 +14,16 @@
 from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.orm.exc import NoResultFound
 
+from cryptography.hazmat.backends import default_backend
+from cryptography.x509 import load_der_x509_certificate
 import simplejson as json
 
 from keylime.db.registrar_db import RegistrarMain
 from keylime.db.keylime_db import DBEngineManager, SessionManager
 from keylime import cloud_verifier_common
 from keylime import config
 from keylime import crypto
+from keylime.tpm import tpm2_objects
 from keylime import keylime_logging
 from keylime.tpm.tpm_main import tpm
 
@@ -85,8 +89,8 @@ def do_GET(self):
                 return
 
             response = {
-                'aik': agent.aik,
-                'ek': agent.ek,
+                'aik_tpm': agent.aik_tpm,
+                'ek_tpm': agent.ek_tpm,
                 'ekcert': agent.ekcert,
                 'regcount': agent.regcount,
             }
@@ -180,7 +184,6 @@ def do_POST(self):
         block sent in the body with 2 entries: ek and aik.
         """
         session = SessionManager().make_session(engine)
-        instance_tpm = tpm()
         rest_params = config.get_restful_params(self.path)
         if rest_params is None:
             config.echo_json_response(
@@ -213,14 +216,53 @@ def do_POST(self):
             post_body = self.rfile.read(content_length)
             json_body = json.loads(post_body)
 
-            ek = json_body['ek']
-            ek_tpm = json_body['ek_tpm']
             ekcert = json_body['ekcert']
-            aik = json_body['aik']
-            aik_name = json_body['aik_name']
+            aik_tpm = json_body['aik_tpm']
+
+            initialize_tpm = tpm()
+
+            if ekcert is None or ekcert == 'emulator':
+                logger.warning('Agent %s did not submit an ekcert' % agent_id)
+                ek_tpm = json_body['ek_tpm']
+            else:
+                if 'ek_tpm' in json_body:
+                    # This would mean the agent submitted both a non-None ekcert, *and*
+                    #  an ek_tpm... We can deal with it by just ignoring the ek_tpm they sent
+                    logger.warning('Overriding ek_tpm for agent %s from ekcert' % agent_id)
+                # If there's an EKCert, we just overwrite their ek_tpm
+                # Note, we don't validate the EKCert here, other than the implicit
+                #  "is it a valid x509 cert" check. So it's still untrusted.
+                # This will be validated by the tenant.
+                ek509 = load_der_x509_certificate(
+                    base64.b64decode(ekcert),
+                    backend=default_backend(),
+                )
+                ek_tpm = base64.b64encode(
+                    tpm2_objects.ek_low_tpm2b_public_from_pubkey(
+                        ek509.public_key(),
+                    )
+                )
+
+            aik_attrs = tpm2_objects.get_tpm2b_public_object_attributes(
+                base64.b64decode(aik_tpm),
+            )
+            if aik_attrs != tpm2_objects.AK_EXPECTED_ATTRS:
+                config.echo_json_response(
+                    self, 400, "Invalid AK attributes")
+                logger.warning(
+                    "Agent %s submitted AIK with invalid attributes! %s (provided) != %s (expected)",
+                    agent_id,
+                    tpm2_objects.object_attributes_description(aik_attrs),
+                    tpm2_objects.object_attributes_description(tpm2_objects.AK_EXPECTED_ATTRS),
+                )
+                return
 
             # try to encrypt the AIK
-            (blob, key) = instance_tpm.encryptAIK(agent_id, aik, ek, ek_tpm, aik_name)
+            (blob, key) = initialize_tpm.encryptAIK(
+                agent_id,
+                base64.b64decode(ek_tpm),
+                base64.b64decode(aik_tpm),
+            )
 
             # special behavior if we've registered this uuid before
             regcount = 1
@@ -237,7 +279,7 @@ def do_POST(self):
 
                 # keep track of how many ek-ekcerts have registered on this uuid
                 regcount = agent.regcount
-                if agent.ek != ek or agent.ekcert != ekcert:
+                if agent.ek_tpm != ek_tpm or agent.ekcert != ekcert:
                     logger.warning(
                         'WARNING: Overwriting previous registration for this UUID with new ek-ekcert pair!')
                     regcount += 1
@@ -255,8 +297,8 @@ def do_POST(self):
             # Add values to database
             d = {}
             d['agent_id'] = agent_id
-            d['ek'] = ek
-            d['aik'] = aik
+            d['ek_tpm'] = ek_tpm
+            d['aik_tpm'] = aik_tpm
             d['ekcert'] = ekcert
             d['virtual'] = int(ekcert == 'virtual')
             d['active'] = int(False)

diff --git a/keylime/tenant.py b/keylime/tenant.py
@@ -16,13 +16,15 @@
 import time
 import zipfile
 
+from cryptography.hazmat.primitives import serialization as crypto_serialization
 import simplejson as json
 
 from keylime.requests_client import RequestsClient
 from keylime.common import states
 from keylime import config
 from keylime import keylime_logging
 from keylime import registrar_client
+from keylime.tpm import tpm2_objects
 from keylime.tpm.tpm_main import tpm
 from keylime.tpm.tpm_abstract import TPM_Utilities
 from keylime import ima
@@ -414,13 +416,11 @@ def preloop(self):
             logger.debug(F"U: {base64.b64encode(self.U)}")
             logger.debug(F"Auth Tag: {self.auth_tag}")
 
-    def check_ek(self, ek, ekcert):
+    def check_ek(self, ekcert):
         """ Check the Entity Key
 
         Arguments:
-            ek {[type]} -- [description]
-            ekcert {[type]} -- [description]
-            tpm {[type]} -- [description]
+            ekcert {str} -- The endorsement key, either None, "emulator", or base64 encoded der cert
 
         Returns:
             [type] -- [description]
@@ -429,12 +429,12 @@ def check_ek(self, ek, ekcert):
             if config.STUB_TPM:
                 logger.debug("not checking ekcert due to STUB_TPM mode")
             elif ekcert == 'emulator' and config.DISABLE_EK_CERT_CHECK_EMULATOR:
-                logger.debug("not checking ekcert of TPM emulator")
+                logger.info("not checking ekcert of TPM emulator")
             elif ekcert is None:
                 logger.warning(
                     "No EK cert provided, require_ek_cert option in config set to True")
                 return False
-            elif not self.tpm_instance.verify_ek(base64.b64decode(ekcert), ek):
+            elif not self.tpm_instance.verify_ek(base64.b64decode(ekcert)):
                 logger.warning("Invalid EK certificate")
                 return False
 
@@ -461,7 +461,7 @@ def validate_tpm_quote(self, public_key, quote, hash_alg):
             logger.warning("AIK not found in registrar, quote not validated")
             return False
 
-        if not self.tpm_instance.check_quote(self.agent_uuid, self.nonce, public_key, quote, reg_keys['aik'], hash_alg=hash_alg):
+        if not self.tpm_instance.check_quote(self.agent_uuid, self.nonce, public_key, quote, reg_keys['aik_tpm'], hash_alg=hash_alg):
             if reg_keys['regcount'] > 1:
                 logger.error("WARNING: This UUID had more than one ek-ekcert registered to it!  This might indicate that your system is misconfigured or a malicious host is present.  Run 'regdelete' for this agent and restart")
                 sys.exit()
@@ -475,11 +475,11 @@ def validate_tpm_quote(self, public_key, quote, hash_alg):
                 "DANGER: EK cert checking is disabled and no additional checks on EKs have been specified with ek_check_script option. Keylime is not secure!!")
 
         # check EK cert and make sure it matches EK
-        if not self.check_ek(reg_keys['ek'], reg_keys['ekcert']):
+        if not self.check_ek(reg_keys['ekcert']):
             return False
         # if agent is virtual, check phyisical EK cert and make sure it matches phyiscal EK
         if 'provider_keys' in reg_keys:
-            if not self.check_ek(reg_keys['provider_keys']['ek'], reg_keys['provider_keys']['ekcert']):
+            if not self.check_ek(reg_keys['provider_keys']['ekcert']):
                 return False
 
         # check all EKs with optional script:
@@ -494,7 +494,13 @@ def validate_tpm_quote(self, public_key, quote, hash_alg):
         # now we need to exec the script with the ek and ek cert in vars
         env = os.environ.copy()
         env['AGENT_UUID'] = self.agent_uuid
-        env['EK'] = reg_keys['ek']
+        env['EK'] = tpm2_objects.pubkey_from_tpm2b_public(
+            reg_keys['ek_tpm'],
+            ).public_bytes(
+                crypto_serialization.Encoding.PEM,
+                crypto_serialization.PublicFormat.SubjectPublicKeyInfo,
+            )
+        env['EK_TPM'] = reg_keys['ek_tpm']
         if reg_keys['ekcert'] is not None:
             env['EK_CERT'] = reg_keys['ekcert']
         else:

diff --git a/keylime/tpm/tpm_abstract.py b/keylime/tpm/tpm_abstract.py
@@ -96,15 +96,15 @@ def flush_keys(self):
         pass
 
     @abstractmethod
-    def encryptAIK(self, uuid, pubaik, pubek, ek_tpm, aik_name):
+    def encryptAIK(self, uuid, ek_tpm: bytes, aik_tpm: bytes):
         pass
 
     @abstractmethod
     def activate_identity(self, keyblob):
         pass
 
     @abstractmethod
-    def verify_ek(self, ekcert, ekpem):
+    def verify_ek(self, ekcert):
         pass
 
     @abstractmethod
@@ -162,7 +162,7 @@ def create_quote(self, nonce, data=None, pcrmask=EMPTYMASK, hash_alg=None):
         pass
 
     @abstractmethod
-    def check_quote(self, agent_id, nonce, data, quote, aikFromRegistrar, tpm_policy={}, ima_measurement_list=None, allowlist={}, hash_alg=None, ima_keyring=None, mb_measurement_list=None, mb_intended_state={}):
+    def check_quote(self, agent_id, nonce, data, quote, aikTpmFromRegistrar, tpm_policy={}, ima_measurement_list=None, allowlist={}, hash_alg=None, ima_keyring=None, mb_measurement_list=None, mb_intended_state={}):
         pass
 
     def START_HASH(self, algorithm=None):