Merge "Remove broken and unused support for expiring keys when off-body" into main

9 files changed, 30 insertions, 116 deletions

diff --git a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
index abea9582..50e98286 100644
--- a/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
+++ b/keystore2/aidl/android/security/maintenance/IKeystoreMaintenance.aidl
@@ -112,16 +112,6 @@ interface IKeystoreMaintenance {
      void earlyBootEnded();
 
     /**
-     * Informs Keystore 2.0 that the an off body event was detected.
-     *
-     * ## Error conditions:
-     * `ResponseCode::PERMISSION_DENIED` - if the caller does not have the `ReportOffBody`
-     *                                     permission.
-     * `ResponseCode::SYSTEM_ERROR` - if an unexpected error occurred.
-     */
-    void onDeviceOffBody();
-
-    /**
      * Migrate a key from one namespace to another. The caller must have use, grant, and delete
      * permissions on the source namespace and rebind permissions on the destination namespace.
      * The source may be specified by Domain::APP, Domain::SELINUX, or Domain::KEY_ID. The target
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index 2757313f..f343cb33 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -2864,26 +2864,11 @@ impl KeystoreDB {
     }
 
     /// Find the newest auth token matching the given predicate.
-    pub fn find_auth_token_entry<F>(&self, p: F) -> Option<(AuthTokenEntry, BootTime)>
+    pub fn find_auth_token_entry<F>(&self, p: F) -> Option<AuthTokenEntry>
     where
         F: Fn(&AuthTokenEntry) -> bool,
     {
-        self.perboot.find_auth_token_entry(p).map(|entry| (entry, self.get_last_off_body()))
-    }
-
-    /// Insert last_off_body into the metadata table at the initialization of auth token table
-    pub fn insert_last_off_body(&self, last_off_body: BootTime) {
-        self.perboot.set_last_off_body(last_off_body)
-    }
-
-    /// Update last_off_body when on_device_off_body is called
-    pub fn update_last_off_body(&self, last_off_body: BootTime) {
-        self.perboot.set_last_off_body(last_off_body)
-    }
-
-    /// Get last_off_body time when finding auth tokens
-    fn get_last_off_body(&self) -> BootTime {
-        self.perboot.get_last_off_body()
+        self.perboot.find_auth_token_entry(p)
     }
 
     /// Load descriptor of a key by key id
@@ -5051,23 +5036,6 @@ pub mod tests {
     }
 
     #[test]
-    fn test_last_off_body() -> Result<()> {
-        let mut db = new_test_db()?;
-        db.insert_last_off_body(BootTime::now());
-        let tx = db.conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
-        tx.commit()?;
-        let last_off_body_1 = db.get_last_off_body();
-        let one_second = Duration::from_secs(1);
-        thread::sleep(one_second);
-        db.update_last_off_body(BootTime::now());
-        let tx2 = db.conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
-        tx2.commit()?;
-        let last_off_body_2 = db.get_last_off_body();
-        assert!(last_off_body_1 < last_off_body_2);
-        Ok(())
-    }
-
-    #[test]
     fn test_unbind_keys_for_user() -> Result<()> {
         let mut db = new_test_db()?;
         db.unbind_keys_for_user(1, false)?;
@@ -5491,7 +5459,7 @@ pub mod tests {
         // All three entries are in the database
         assert_eq!(db.perboot.auth_tokens_len(), 3);
         // It selected the most recent timestamp
-        assert_eq!(db.find_auth_token_entry(|_| true).unwrap().0.auth_token.mac, b"mac2".to_vec());
+        assert_eq!(db.find_auth_token_entry(|_| true).unwrap().auth_token.mac, b"mac2".to_vec());
         Ok(())
     }
 
diff --git a/keystore2/src/database/perboot.rs b/keystore2/src/database/perboot.rs
index 1b7c80d6..4727015f 100644
--- a/keystore2/src/database/perboot.rs
+++ b/keystore2/src/database/perboot.rs
@@ -13,15 +13,14 @@
 // limitations under the License.
 
 //! This module implements a per-boot, shared, in-memory storage of auth tokens
-//! and last-time-on-body for the main Keystore 2.0 database module.
+//! for the main Keystore 2.0 database module.
 
-use super::{AuthTokenEntry, BootTime};
+use super::AuthTokenEntry;
 use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
     HardwareAuthToken::HardwareAuthToken, HardwareAuthenticatorType::HardwareAuthenticatorType,
 };
 use lazy_static::lazy_static;
 use std::collections::HashSet;
-use std::sync::atomic::{AtomicI64, Ordering};
 use std::sync::Arc;
 use std::sync::RwLock;
 
@@ -62,17 +61,13 @@ impl PartialEq<AuthTokenEntryWrap> for AuthTokenEntryWrap {
 
 impl Eq for AuthTokenEntryWrap {}
 
-/// Per-boot state structure. Currently only used to track auth tokens and
-/// last-off-body.
+/// Per-boot state structure. Currently only used to track auth tokens.
 #[derive(Default)]
 pub struct PerbootDB {
     // We can use a .unwrap() discipline on this lock, because only panicking
     // while holding a .write() lock will poison it. The only write usage is
     // an insert call which inserts a pre-constructed pair.
     auth_tokens: RwLock<HashSet<AuthTokenEntryWrap>>,
-    // Ordering::Relaxed is appropriate for accessing this atomic, since it
-    // does not currently need to be synchronized with anything else.
-    last_off_body: AtomicI64,
 }
 
 lazy_static! {
@@ -102,14 +97,6 @@ impl PerbootDB {
         matches.sort_by_key(|x| x.0.time_received);
         matches.last().map(|x| x.0.clone())
     }
-    /// Get the last time the device was off the user's body
-    pub fn get_last_off_body(&self) -> BootTime {
-        BootTime(self.last_off_body.load(Ordering::Relaxed))
-    }
-    /// Set the last time the device was off the user's body
-    pub fn set_last_off_body(&self, last_off_body: BootTime) {
-        self.last_off_body.store(last_off_body.0, Ordering::Relaxed)
-    }
     /// Return how many auth tokens are currently tracked.
     pub fn auth_tokens_len(&self) -> usize {
         self.auth_tokens.read().unwrap().len()
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs
index 55c9591f..95dd026d 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -476,7 +476,6 @@ impl Enforcements {
         let mut user_id: i32 = -1;
         let mut user_secure_ids = Vec::<i64>::new();
         let mut key_time_out: Option<i64> = None;
-        let mut allow_while_on_body = false;
         let mut unlocked_device_required = false;
         let mut key_usage_limited: Option<i64> = None;
         let mut confirmation_token_receiver: Option<Arc<Mutex<Option<Receiver<Vec<u8>>>>>> = None;
@@ -533,9 +532,6 @@ impl Enforcements {
                 KeyParameterValue::UnlockedDeviceRequired => {
                     unlocked_device_required = true;
                 }
-                KeyParameterValue::AllowWhileOnBody => {
-                    allow_while_on_body = true;
-                }
                 KeyParameterValue::UsageCountLimit(_) => {
                     // We don't examine the limit here because this is enforced on finish.
                     // Instead, we store the key_id so that finish can look up the key
@@ -607,13 +603,12 @@ impl Enforcements {
             let (hat, state) = if user_secure_ids.is_empty() {
                 (None, DeferredAuthState::NoAuthRequired)
             } else if let Some(key_time_out) = key_time_out {
-                let (hat, last_off_body) =
-                    Self::find_auth_token(|hat: &AuthTokenEntry| match user_auth_type {
-                        Some(auth_type) => hat.satisfies(&user_secure_ids, auth_type),
-                        None => false, // not reachable due to earlier check
-                    })
-                    .ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
-                    .context(ks_err!("No suitable auth token found."))?;
+                let hat = Self::find_auth_token(|hat: &AuthTokenEntry| match user_auth_type {
+                    Some(auth_type) => hat.satisfies(&user_secure_ids, auth_type),
+                    None => false, // not reachable due to earlier check
+                })
+                .ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
+                .context(ks_err!("No suitable auth token found."))?;
                 let now = BootTime::now();
                 let token_age = now
                     .checked_sub(&hat.time_received())
@@ -623,9 +618,7 @@ impl Enforcements {
                     Validity cannot be established."
                     ))?;
 
-                let on_body_extended = allow_while_on_body && last_off_body < hat.time_received();
-
-                if token_age.seconds() > key_time_out && !on_body_extended {
+                if token_age.seconds() > key_time_out {
                     return Err(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
                         .context(ks_err!("matching auth token is expired."));
                 }
@@ -660,8 +653,8 @@ impl Enforcements {
 
         let need_auth_token = timeout_bound || unlocked_device_required;
 
-        let hat_and_last_off_body = if need_auth_token {
-            let hat_and_last_off_body = Self::find_auth_token(|hat: &AuthTokenEntry| {
+        let hat = if need_auth_token {
+            let hat = Self::find_auth_token(|hat: &AuthTokenEntry| {
                 if let (Some(auth_type), true) = (user_auth_type, timeout_bound) {
                     hat.satisfies(&user_secure_ids, auth_type)
                 } else {
@@ -669,8 +662,7 @@ impl Enforcements {
                 }
             });
             Some(
-                hat_and_last_off_body
-                    .ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
+                hat.ok_or(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
                     .context(ks_err!("No suitable auth token found."))?,
             )
         } else {
@@ -678,8 +670,8 @@ impl Enforcements {
         };
 
         // Now check the validity of the auth token if the key is timeout bound.
-        let hat = match (hat_and_last_off_body, key_time_out) {
-            (Some((hat, last_off_body)), Some(key_time_out)) => {
+        let hat = match (hat, key_time_out) {
+            (Some(hat), Some(key_time_out)) => {
                 let now = BootTime::now();
                 let token_age = now
                     .checked_sub(&hat.time_received())
@@ -689,15 +681,13 @@ impl Enforcements {
                     Validity cannot be established."
                     ))?;
 
-                let on_body_extended = allow_while_on_body && last_off_body < hat.time_received();
-
-                if token_age.seconds() > key_time_out && !on_body_extended {
+                if token_age.seconds() > key_time_out {
                     return Err(Error::Km(Ec::KEY_USER_NOT_AUTHENTICATED))
                         .context(ks_err!("matching auth token is expired."));
                 }
                 Some(hat)
             }
-            (Some((hat, _)), None) => Some(hat),
+            (Some(hat), None) => Some(hat),
             // If timeout_bound is true, above code must have retrieved a HAT or returned with
             // KEY_USER_NOT_AUTHENTICATED. This arm should not be reachable.
             (None, Some(_)) => panic!("Logical error."),
@@ -728,7 +718,7 @@ impl Enforcements {
         })
     }
 
-    fn find_auth_token<F>(p: F) -> Option<(AuthTokenEntry, BootTime)>
+    fn find_auth_token<F>(p: F) -> Option<AuthTokenEntry>
     where
         F: Fn(&AuthTokenEntry) -> bool,
     {
@@ -848,7 +838,7 @@ impl Enforcements {
             (challenge == hat.challenge()) && hat.satisfies(&sids, auth_type)
         });
 
-        let auth_token = if let Some((auth_token_entry, _)) = result {
+        let auth_token = if let Some(auth_token_entry) = result {
             auth_token_entry.take_auth_token()
         } else {
             // Filter the matching auth tokens by age.
@@ -863,7 +853,7 @@ impl Enforcements {
                     token_valid && auth_token_entry.satisfies(&sids, auth_type)
                 });
 
-                if let Some((auth_token_entry, _)) = result {
+                if let Some(auth_token_entry) = result {
                     auth_token_entry.take_auth_token()
                 } else {
                     return Err(AuthzError::Rc(AuthzResponseCode::NO_AUTH_TOKEN_FOUND))
@@ -895,11 +885,7 @@ impl Enforcements {
         let result =
             Self::find_auth_token(|entry: &AuthTokenEntry| entry.satisfies(&sids, auth_type));
 
-        if let Some((auth_token_entry, _)) = result {
-            Some(auth_token_entry.time_received())
-        } else {
-            None
-        }
+        result.map(|auth_token_entry| auth_token_entry.time_received())
     }
 }
 
diff --git a/keystore2/src/globals.rs b/keystore2/src/globals.rs
index 8535df1b..7ac1038d 100644
--- a/keystore2/src/globals.rs
+++ b/keystore2/src/globals.rs
@@ -16,6 +16,7 @@
 //! database connections and connections to services that Keystore needs
 //! to talk to.
 
+use crate::async_task::AsyncTask;
 use crate::gc::Gc;
 use crate::km_compat::{BacklevelKeyMintWrapper, KeyMintV1};
 use crate::ks_err;
@@ -23,7 +24,6 @@ use crate::legacy_blob::LegacyBlobLoader;
 use crate::legacy_importer::LegacyImporter;
 use crate::super_key::SuperKeyManager;
 use crate::utils::watchdog as wd;
-use crate::{async_task::AsyncTask, database::BootTime};
 use crate::{
     database::KeystoreDB,
     database::Uuid,
@@ -68,7 +68,6 @@ pub fn create_thread_local_db() -> KeystoreDB {
 
     DB_INIT.call_once(|| {
         log::info!("Touching Keystore 2.0 database for this first time since boot.");
-        db.insert_last_off_body(BootTime::now());
         log::info!("Calling cleanup leftovers.");
         let n = db.cleanup_leftovers().expect("Failed to cleanup database on startup.");
         if n != 0 {
diff --git a/keystore2/src/key_parameter.rs b/keystore2/src/key_parameter.rs
index 02a1f16c..bd452073 100644
--- a/keystore2/src/key_parameter.rs
+++ b/keystore2/src/key_parameter.rs
@@ -912,7 +912,8 @@ pub enum KeyParameterValue {
     /// The time in seconds for which the key is authorized for use, after user authentication
     #[key_param(tag = AUTH_TIMEOUT, field = Integer)]
     AuthTimeout(i32),
-    /// The key may be used after authentication timeout if device is still on-body
+    /// The key's authentication timeout, if it has one, is automatically expired when the device is
+    /// removed from the user's body. No longer implemented; this tag is no longer enforced.
     #[key_param(tag = ALLOW_WHILE_ON_BODY, field = BoolValue)]
     AllowWhileOnBody,
     /// The key must be unusable except when the user has provided proof of physical presence
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 3e34cff3..8780e9e9 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -14,7 +14,7 @@
 
 //! This module implements IKeystoreMaintenance AIDL interface.
 
-use crate::database::{BootTime, KeyEntryLoadBits, KeyType};
+use crate::database::{KeyEntryLoadBits, KeyType};
 use crate::error::map_km_error;
 use crate::error::map_or_log_err;
 use crate::error::Error;
@@ -224,14 +224,6 @@ impl Maintenance {
         Maintenance::call_on_all_security_levels("earlyBootEnded", |dev| dev.earlyBootEnded())
     }
 
-    fn on_device_off_body() -> Result<()> {
-        // Security critical permission check. This statement must return on fail.
-        check_keystore_permission(KeystorePerm::ReportOffBody).context(ks_err!())?;
-
-        DB.with(|db| db.borrow_mut().update_last_off_body(BootTime::now()));
-        Ok(())
-    }
-
     fn migrate_key_namespace(source: &KeyDescriptor, destination: &KeyDescriptor) -> Result<()> {
         let calling_uid = ThreadState::get_calling_uid();
 
@@ -355,12 +347,6 @@ impl IKeystoreMaintenance for Maintenance {
         map_or_log_err(Self::early_boot_ended(), Ok)
     }
 
-    fn onDeviceOffBody(&self) -> BinderResult<()> {
-        log::info!("onDeviceOffBody()");
-        let _wp = wd::watch_millis("IKeystoreMaintenance::onDeviceOffBody", 500);
-        map_or_log_err(Self::on_device_off_body(), Ok)
-    }
-
     fn migrateKeyNamespace(
         &self,
         source: &KeyDescriptor,
diff --git a/keystore2/src/permission.rs b/keystore2/src/permission.rs
index bc73744f..982bc821 100644
--- a/keystore2/src/permission.rs
+++ b/keystore2/src/permission.rs
@@ -137,10 +137,7 @@ implement_class!(
         /// Checked when earlyBootEnded() is called.
         #[selinux(name = early_boot_ended)]
         EarlyBootEnded,
-        /// Checked when IKeystoreMaintenance::onDeviceOffBody is called.
-        #[selinux(name = report_off_body)]
-        ReportOffBody,
-        /// Checked when IkeystoreMetrics::pullMetrics is called.
+        /// Checked when IKeystoreMetrics::pullMetrics is called.
         #[selinux(name = pull_metrics)]
         PullMetrics,
         /// Checked when IKeystoreMaintenance::deleteAllKeys is called.
diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs
index 44ce9abf..11ab734f 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -1011,7 +1011,7 @@ impl SuperKeyManager {
             let mut errs = vec![];
             for sid in &biometric.sids {
                 let sid = *sid;
-                if let Some((auth_token_entry, _)) = db.find_auth_token_entry(|entry| {
+                if let Some(auth_token_entry) = db.find_auth_token_entry(|entry| {
                     entry.auth_token().userId == sid || entry.auth_token().authenticatorId == sid
                 }) {
                     let res: Result<(Arc<SuperKey>, Arc<SuperKey>)> = (|| {