commit 668d292fe74cbb2879f97ef666bbd4535c3156de
author Shaquille Johnson <ssjohnson@google.com> Tue Jul 02 18:03:47 2024 +0000
committer Shaquille Johnson <ssjohnson@google.com> Wed Jul 03 17:49:48 2024 +0000
tree b2c90dab2167cafadaaadf797c125a11868e07a3
parent cff76e7d5df2a89c255eb2097187ea4e0541c3a1

Log key params wo sensitive info

We log the key params while filtering out
information that is sensitive and should not
be included in the logs. This is currently
APPLICATION_ID and APPLICATION_DATA

Test: atest keystore2_test
Test: atest CtsKeystoreTestCases
Bug: 350986200
Change-Id: I3ff8da0c3728e120f3b339c47d3a9f7ff1cdc550

keystore2/src/security_level.rs

diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 4a8c418..1b66fa4 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -34,7 +34,8 @@
 use crate::utils::{
     check_device_attestation_permissions, check_key_permission,
     check_unique_id_attestation_permissions, is_device_id_attestation_tag,
-    key_characteristics_to_internal, uid_to_android_user, watchdog as wd, UNDEFINED_NOT_AFTER,
+    key_characteristics_to_internal, log_security_safe_params, uid_to_android_user, watchdog as wd,
+    UNDEFINED_NOT_AFTER,
 };
 use crate::{
     database::{
@@ -585,7 +586,11 @@
                         })
                     },
                 )
-                .context(ks_err!("Using user generated attestation key."))
+                .context(ks_err!(
+                    "While generating with a user-generated \
+                      attestation key, params: {:?}.",
+                    log_security_safe_params(&params)
+                ))
                 .map(|(result, _)| result),
             Some(AttestationKeyInfo::RkpdProvisioned { attestation_key, attestation_certs }) => {
                 self.upgrade_rkpd_keyblob_if_required_with(&attestation_key.keyBlob, &[], |blob| {
@@ -605,7 +610,12 @@
                         self.keymint.generateKey(&params, dynamic_attest_key.as_ref())
                     })
                 })
-                .context(ks_err!("While generating Key with remote provisioned attestation key."))
+                .context(ks_err!(
+                    "While generating Key {:?} with remote \
+                    provisioned attestation key and params: {:?}.",
+                    key.alias,
+                    log_security_safe_params(&params)
+                ))
                 .map(|(mut result, _)| {
                     result.certificateChain.push(attestation_certs);
                     result
@@ -621,7 +631,11 @@
                 );
                 self.keymint.generateKey(&params, None)
             })
-            .context(ks_err!("While generating Key without explicit attestation key.")),
+            .context(ks_err!(
+                "While generating without a provided \
+                 attestation key and params: {:?}.",
+                log_security_safe_params(&params)
+            )),
         }
         .context(ks_err!())?;
 
@@ -906,7 +920,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 196cac5..e78f7e4 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -591,6 +591,15 @@
     Ok((legacy_keys.len() + num_keys_in_db) as i32)
 }
 
+/// For params remove sensitive data before returning a string for logging
+pub fn log_security_safe_params(params: &[KmKeyParameter]) -> Vec<KmKeyParameter> {
+    params
+        .iter()
+        .filter(|kp| (kp.tag != Tag::APPLICATION_ID && kp.tag != Tag::APPLICATION_DATA))
+        .cloned()
+        .collect::<Vec<KmKeyParameter>>()
+}
+
 /// Trait implemented by objects that can be used to decrypt cipher text using AES-GCM.
 pub trait AesGcm {
     /// Deciphers `data` using the initialization vector `iv` and AEAD tag `tag`
@@ -716,4 +725,33 @@
         assert_eq!(aliases_from_key_descriptors(&result), vec!["key_d", "key_e", "key_f", "key_g"]);
         Ok(())
     }
+
+    #[test]
+    fn test_list_key_parameters_with_filter_on_security_sensitive_info() -> Result<()> {
+        let params = vec![
+            KmKeyParameter { tag: Tag::APPLICATION_ID, value: KeyParameterValue::Integer(0) },
+            KmKeyParameter { tag: Tag::APPLICATION_DATA, value: KeyParameterValue::Integer(0) },
+            KmKeyParameter {
+                tag: Tag::CERTIFICATE_NOT_AFTER,
+                value: KeyParameterValue::DateTime(UNDEFINED_NOT_AFTER),
+            },
+            KmKeyParameter {
+                tag: Tag::CERTIFICATE_NOT_BEFORE,
+                value: KeyParameterValue::DateTime(0),
+            },
+        ];
+        let wanted = vec![
+            KmKeyParameter {
+                tag: Tag::CERTIFICATE_NOT_AFTER,
+                value: KeyParameterValue::DateTime(UNDEFINED_NOT_AFTER),
+            },
+            KmKeyParameter {
+                tag: Tag::CERTIFICATE_NOT_BEFORE,
+                value: KeyParameterValue::DateTime(0),
+            },
+        ];
+
+        assert_eq!(log_security_safe_params(&params), wanted);
+        Ok(())
+    }
 }