commit a820ef5dc2c355d285099e1326d6d82e8ec1be24
author Shaquille Johnson <ssjohnson@google.com> Thu Jun 20 13:48:23 2024 +0000
committer Shaquille Johnson <ssjohnson@google.com> Mon Jun 24 12:06:27 2024 +0000
tree c352372c17094c3ac91bd5b05b6f22b2db3937ef
parent 76bf079cb18c9fa1e206534df32cbfa6ee421292

Enhance Security Logs for Clarity

Addressing numerous reports regarding
the output from the security_level file,
this update improves the logs and resolves
previous bugs.

Test: atest keystore2_test
Test: atest CtsKeystoreTestCases
Change-Id: I682c384383a39df4ee291c49500907e65d9e59aa

keystore2/src/legacy_importer.rs

diff --git a/keystore2/src/legacy_importer.rs b/keystore2/src/legacy_importer.rs
index f9d5d1b..045f848 100644
--- a/keystore2/src/legacy_importer.rs
+++ b/keystore2/src/legacy_importer.rs
@@ -928,6 +928,6 @@
         },
         |_| Ok(()),
     )
-    .context(ks_err!("Key blob upgrade failed, possibly invalid keyblob for uuid {uuid:?}"))?;
+    .context(ks_err!("getKeyCharacteristics failed: possibly invalid keyblob for uuid {uuid:?}"))?;
     Ok((key_characteristics_to_internal(characteristics), upgraded_blob))
 }

keystore2/src/security_level.rs

diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 951acb5..a53ccec 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -515,7 +515,7 @@
         flags: i32,
         _entropy: &[u8],
     ) -> Result<KeyMetadata> {
-        log::info!("security_level: generate_key(key={:?})", key);
+        log::info!("security_level: generate_key(key={:?})", key.alias);
         if key.domain != Domain::BLOB && key.alias.is_none() {
             return Err(error::Error::Km(ErrorCode::INVALID_ARGUMENT))
                 .context(ks_err!("Alias must be specified"));
@@ -586,9 +586,8 @@
                     },
                 )
                 .context(ks_err!(
-                    "While generating Key {:?} with remote \
-                    provisioned attestation key and params: {:?}.",
-                    key.alias,
+                    "While generating with a user-generated \
+                     attestation key, params: {:?}.",
                     log_security_safe_params(&params)
                 ))
                 .map(|(result, _)| result),
@@ -632,9 +631,8 @@
                 self.keymint.generateKey(&params, None)
             })
             .context(ks_err!(
-                "While generating Key {:?} with remote \
-                provisioned attestation key and params: {:?}.",
-                key.alias,
+                "While generating without a provided \
+                attestation key and params: {:?}.",
                 log_security_safe_params(&params)
             )),
         }
@@ -864,7 +862,6 @@
     where
         F: Fn(&[u8]) -> Result<T, Error>,
     {
-        log::info!("upgrade_keyblob_if_required_with(key_id={:?})", key_id_guard);
         let (v, upgraded_blob) = crate::utils::upgrade_keyblob_if_required_with(
             &*self.keymint,
             self.hw_info.versionNumber,
@@ -882,7 +879,7 @@
                 }
             },
         )
-        .context(ks_err!())?;
+        .context(ks_err!("upgrade_keyblob_if_required_with(key_id={:?})", key_id_guard))?;
 
         // If no upgrade was needed, use the opportunity to reencrypt the blob if required
         // and if the a key_id_guard is held. Note: key_id_guard can only be Some if no
@@ -905,10 +902,6 @@
     where
         F: Fn(&[u8]) -> Result<T, Error>,
     {
-        log::info!(
-            "upgrade_rkpd_keyblob_if_required_with(params={:?})",
-            log_security_safe_params(params)
-        );
         let rpc_name = get_remotely_provisioned_component_name(&self.security_level)
             .context(ks_err!("Trying to get IRPC name."))?;
         crate::utils::upgrade_keyblob_if_required_with(
@@ -926,7 +919,10 @@
                 }
             },
         )
-        .context(ks_err!())
+        .context(ks_err!(
+            "upgrade_rkpd_keyblob_if_required_with(params={:?})",
+            log_security_safe_params(params)
+        ))
     }
 
     fn convert_storage_key_to_ephemeral(

keystore2/src/utils.rs

diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index c80bfa5..190258e 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -551,8 +551,8 @@
     items_to_return
 }
 
-/// Log the key parameters, excluding sensitive ones such as
-/// APPLICATION_DATA and APPLICATION_ID
+/// List all key aliases for a given domain + namespace. whose alias is greater
+/// than start_past_alias (if provided).
 pub fn list_key_entries(
     db: &mut KeystoreDB,
     domain: Domain,