Snap for 11540345 from f2843b6095 to 24Q3-release

Change-Id: I8fa3e76eeb1899f2550d544260f66886829525ff

keystore2/src/authorization.rs

@@ -128,7 +128,8 @@ impl AuthorizationManager {
 
     fn add_auth_token(&self, auth_token: &HardwareAuthToken) -> Result<()> {
         // Check keystore permission.
-        check_keystore_permission(KeystorePerm::AddAuth).context(ks_err!())?;
+        check_keystore_permission(KeystorePerm::AddAuth)
+            .context(ks_err!("caller missing AddAuth permissions"))?;
 
         log::info!(
             "add_auth_token(challenge={}, userId={}, authId={}, authType={:#x}, timestamp={}ms)",
@@ -149,7 +150,8 @@ impl AuthorizationManager {
             user_id,
             password.is_some(),
         );
-        check_keystore_permission(KeystorePerm::Unlock).context(ks_err!("Unlock."))?;
+        check_keystore_permission(KeystorePerm::Unlock)
+            .context(ks_err!("caller missing Unlock permissions"))?;
         ENFORCEMENTS.set_device_locked(user_id, false);
 
         let mut skm = SUPER_KEY.write().unwrap();
@@ -160,7 +162,7 @@ impl AuthorizationManager {
             .context(ks_err!("Unlock with password."))
         } else {
             DB.with(|db| skm.try_unlock_user_with_biometric(&mut db.borrow_mut(), user_id as u32))
-                .context(ks_err!("try_unlock_user_with_biometric failed"))
+                .context(ks_err!("try_unlock_user_with_biometric failed user_id={user_id}"))
         }
     }
 
@@ -179,7 +181,8 @@ impl AuthorizationManager {
         if !android_security_flags::fix_unlocked_device_required_keys_v2() {
             weak_unlock_enabled = false;
         }
-        check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
+        check_keystore_permission(KeystorePerm::Lock)
+            .context(ks_err!("caller missing Lock permission"))?;
         ENFORCEMENTS.set_device_locked(user_id, true);
         let mut skm = SUPER_KEY.write().unwrap();
         DB.with(|db| {
@@ -198,7 +201,8 @@ impl AuthorizationManager {
         if !android_security_flags::fix_unlocked_device_required_keys_v2() {
             return Ok(());
         }
-        check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
+        check_keystore_permission(KeystorePerm::Lock)
+            .context(ks_err!("caller missing Lock permission"))?;
         SUPER_KEY.write().unwrap().wipe_plaintext_unlocked_device_required_keys(user_id as u32);
         Ok(())
     }
@@ -208,7 +212,8 @@ impl AuthorizationManager {
         if !android_security_flags::fix_unlocked_device_required_keys_v2() {
             return Ok(());
         }
-        check_keystore_permission(KeystorePerm::Lock).context(ks_err!("Lock"))?;
+        check_keystore_permission(KeystorePerm::Lock)
+            .context(ks_err!("caller missing Lock permission"))?;
         SUPER_KEY.write().unwrap().wipe_all_unlocked_device_required_keys(user_id as u32);
         Ok(())
     }
@@ -221,7 +226,8 @@ impl AuthorizationManager {
     ) -> Result<AuthorizationTokens> {
         // Check permission. Function should return if this failed. Therefore having '?' at the end
         // is very important.
-        check_keystore_permission(KeystorePerm::GetAuthToken).context(ks_err!("GetAuthToken"))?;
+        check_keystore_permission(KeystorePerm::GetAuthToken)
+            .context(ks_err!("caller missing GetAuthToken permission"))?;
 
         // If the challenge is zero, return error
         if challenge == 0 {
@@ -240,7 +246,8 @@ impl AuthorizationManager {
         auth_types: &[HardwareAuthenticatorType],
     ) -> Result<i64> {
         // Check keystore permission.
-        check_keystore_permission(KeystorePerm::GetLastAuthTime).context(ks_err!())?;
+        check_keystore_permission(KeystorePerm::GetLastAuthTime)
+            .context(ks_err!("caller missing GetLastAuthTime permission"))?;
 
         let mut max_time: i64 = -1;
         for auth_type in auth_types.iter() {

keystore2/src/globals.rs

@@ -247,7 +247,11 @@ fn connect_keymint(
                     }
                     e => e,
                 })
-                .context(ks_err!("Trying to get Legacy wrapper."))?,
+                .context(ks_err!(
+                    "Trying to get Legacy wrapper. Attempt to get keystore \
+                    compat service for security level {:?}",
+                    *security_level
+                ))?,
             None,
         )
     };
@@ -394,7 +398,7 @@ fn connect_secureclock() -> Result<Strong<dyn ISecureClock>> {
                 }
                 e => e,
             })
-            .context(ks_err!("Trying to get Legacy wrapper."))
+            .context(ks_err!("Failed attempt to get legacy secure clock."))
     }?;
 
     Ok(secureclock)
@@ -437,5 +441,5 @@ pub fn get_remotely_provisioned_component_name(security_level: &SecurityLevel) -
         _ => None,
     }
     .ok_or(Error::Km(ErrorCode::HARDWARE_TYPE_UNAVAILABLE))
-    .context(ks_err!())
+    .context(ks_err!("Failed to get rpc for sec level {:?}", *security_level))
 }

keystore2/tests/AndroidTest.xml

@@ -14,6 +14,7 @@
      limitations under the License.
 -->
 <configuration description="Config to run keystore2_client_tests device tests.">
+    <option name="config-descriptor:metadata" key="parameter" value="not_multi_abi" />
 
     <target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer">
     </target_preparer>

keystore2/tests/keystore2_client_test_utils.rs

@@ -95,14 +95,11 @@ pub fn skip_device_id_attest_tests() -> bool {
     // only system update and not vendor update, newly added attestation properties
     // (ro.product.*_for_attestation) reading logic would not be available for such devices
     // hence skipping this test for such scenario.
-    let api_level = std::str::from_utf8(&get_system_prop("ro.board.first_api_level"))
-        .unwrap()
-        .parse::<i32>()
-        .unwrap();
-    // This file is only present on GSI builds.
-    let path_buf = PathBuf::from("/system/system_ext/etc/init/init.gsi.rc");
 
-    api_level < 34 && path_buf.as_path().is_file()
+    // This file is only present on GSI builds.
+    let gsi_marker = PathBuf::from("/system/system_ext/etc/init/init.gsi.rc");
+
+    get_vsr_api_level() < 34 && gsi_marker.as_path().is_file()
 }
 
 #[macro_export]
@@ -514,15 +511,38 @@ pub fn get_system_prop(name: &str) -> Vec<u8> {
     }
 }
 
+fn get_integer_system_prop(name: &str) -> Option<i32> {
+    let val = get_system_prop(name);
+    if val.is_empty() {
+        return None;
+    }
+    let val = std::str::from_utf8(&val).ok()?;
+    val.parse::<i32>().ok()
+}
+
+pub fn get_vsr_api_level() -> i32 {
+    if let Some(api_level) = get_integer_system_prop("ro.vendor.api_level") {
+        return api_level;
+    }
+
+    let vendor_api_level = get_integer_system_prop("ro.board.api_level")
+        .or_else(|| get_integer_system_prop("ro.board.first_api_level"));
+    let product_api_level = get_integer_system_prop("ro.product.first_api_level")
+        .or_else(|| get_integer_system_prop("ro.build.version.sdk"));
+
+    match (vendor_api_level, product_api_level) {
+        (Some(v), Some(p)) => std::cmp::min(v, p),
+        (Some(v), None) => v,
+        (None, Some(p)) => p,
+        _ => panic!("Could not determine VSR API level"),
+    }
+}
+
 /// Determines whether the SECOND-IMEI can be used as device attest-id.
 pub fn is_second_imei_id_attestation_required(
     keystore2: &binder::Strong<dyn IKeystoreService>,
 ) -> bool {
-    let api_level = std::str::from_utf8(&get_system_prop("ro.vendor.api_level"))
+    keystore2.getInterfaceVersion().unwrap() >= 3 && get_vsr_api_level() > 33
-        .unwrap()
-        .parse::<i32>()
-        .unwrap();
-    keystore2.getInterfaceVersion().unwrap() >= 3 && api_level > 33
 }
 
 /// Run a service command and collect the output.