commit e8b152a05b23ea97c2bdb7e4c142e957f6e77b37
author Shaquille Johnson <ssjohnson@google.com> Thu Feb 09 15:15:50 2023 +0000
committer Shaquille Johnson <ssjohnson@google.com> Thu Feb 09 15:19:26 2023 +0000
tree 3de2bf1e66c3ae1709a76dabe5b7e62680757da3
parent 9fe9eed821a69c05814567c7040ec0313890c309

Update ks_err on Keystore security code

Test: Run and tested using `atest keystore2_test` for Rust test and CTS test with `atest CtsKeystoreTestCases`
Change-Id: Id53c870260e9757227f6cb5d57787796f60fe9d5

keystore2/src/security_level.rs

diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index b928fb0..cc1f816 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -612,7 +612,7 @@
                         })
                     },
                 )
-                .context("While generating Key with remote provisioned attestation key.")
+                .context(ks_err!("While generating Key with remote provisioned attestation key."))
                 .map(|(mut result, _)| {
                     result.certificateChain.push(attestation_certs);
                     result
@@ -635,7 +635,7 @@
                         self.keymint.generateKey(&params, dynamic_attest_key.as_ref())
                     })
                 })
-                .context("While generating Key with remote provisioned attestation key.")
+                .context(ks_err!("While generating Key with remote provisioned attestation key."))
                 .map(|(mut result, _)| {
                     result.certificateChain.push(attestation_certs);
                     result
@@ -651,7 +651,7 @@
                 );
                 self.keymint.generateKey(&params, None)
             })
-            .context("While generating Key without explicit attestation key."),
+            .context(ks_err!("While generating Key without explicit attestation key.")),
         }
         .context(ks_err!())?;
 
@@ -684,7 +684,7 @@
         };
 
         // import_key requires the rebind permission.
-        check_key_permission(KeyPerm::Rebind, &key, &None).context("In import_key.")?;
+        check_key_permission(KeyPerm::Rebind, &key, &None).context(ks_err!("In import_key."))?;
 
         let params = self
             .add_required_parameters(caller_uid, params, &key)
@@ -694,7 +694,7 @@
             .iter()
             .find(|p| p.tag == Tag::ALGORITHM)
             .ok_or(error::Error::Km(ErrorCode::INVALID_ARGUMENT))
-            .context("No KeyParameter 'Algorithm'.")
+            .context(ks_err!("No KeyParameter 'Algorithm'."))
             .and_then(|p| match &p.value {
                 KeyParameterValue::Algorithm(Algorithm::AES)
                 | KeyParameterValue::Algorithm(Algorithm::HMAC)
@@ -780,7 +780,7 @@
                     )
                 })
             })
-            .context("Failed to load wrapping key.")?;
+            .context(ks_err!("Failed to load wrapping key."))?;
 
         let (wrapping_key_blob, wrapping_blob_metadata) =
             wrapping_key_entry.take_key_blob_info().ok_or_else(error::Error::sys).context(

keystore2/src/service.rs

diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs
index f43ba5c..1040228 100644
--- a/keystore2/src/service.rs
+++ b/keystore2/src/service.rs
@@ -205,17 +205,18 @@
             let mut db = db.borrow_mut();
             if let Some((key_id_guard, _key_entry)) = entry {
                 db.set_blob(&key_id_guard, SubComponentType::CERT, public_cert, None)
-                    .context("Failed to update cert subcomponent.")?;
+                    .context(ks_err!("Failed to update cert subcomponent."))?;
 
                 db.set_blob(&key_id_guard, SubComponentType::CERT_CHAIN, certificate_chain, None)
-                    .context("Failed to update cert chain subcomponent.")?;
+                    .context(ks_err!("Failed to update cert chain subcomponent."))?;
                 return Ok(());
             }
 
             // If we reach this point we have to check the special condition where a certificate
             // entry may be made.
             if !(public_cert.is_none() && certificate_chain.is_some()) {
-                return Err(Error::Rc(ResponseCode::KEY_NOT_FOUND)).context("No key to update.");
+                return Err(Error::Rc(ResponseCode::KEY_NOT_FOUND))
+                    .context(ks_err!("No key to update."));
             }
 
             // So we know that we have a certificate chain and no public cert.
@@ -230,13 +231,13 @@
                 (Domain::SELINUX, Some(_)) => key.clone(),
                 _ => {
                     return Err(Error::Rc(ResponseCode::INVALID_ARGUMENT))
-                        .context("Domain must be APP or SELINUX to insert a certificate.")
+                        .context(ks_err!("Domain must be APP or SELINUX to insert a certificate."))
                 }
             };
 
             // Security critical: This must return on failure. Do not remove the `?`;
             check_key_permission(KeyPerm::Rebind, &key, &None)
-                .context("Caller does not have permission to insert this certificate.")?;
+                .context(ks_err!("Caller does not have permission to insert this certificate."))?;
 
             db.store_new_certificate(
                 &key,
@@ -244,7 +245,7 @@
                 certificate_chain.unwrap(),
                 &KEYSTORE_UUID,
             )
-            .context("Failed to insert new certificate.")?;
+            .context(ks_err!("Failed to insert new certificate."))?;
             Ok(())
         })
         .context(ks_err!())
@@ -295,7 +296,8 @@
         DB.with(|db| {
             LEGACY_IMPORTER.with_try_import(key, caller_uid, super_key, || {
                 db.borrow_mut().unbind_key(key, KeyType::Client, caller_uid, |k, av| {
-                    check_key_permission(KeyPerm::Delete, k, &av).context("During delete_key.")
+                    check_key_permission(KeyPerm::Delete, k, &av)
+                        .context(ks_err!("During delete_key."))
                 })
             })
         })