Merge "Temporarily disable a bunch of new linter errors"
diff --git a/keystore2/Android.bp b/keystore2/Android.bp
index aff824b..af177be 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -86,6 +86,7 @@
rustlibs: [
"libandroid_logger",
"libkeystore2_test_utils",
+ "libnix",
],
}
diff --git a/keystore2/src/id_rotation.rs b/keystore2/src/id_rotation.rs
new file mode 100644
index 0000000..dbf0fc9
--- /dev/null
+++ b/keystore2/src/id_rotation.rs
@@ -0,0 +1,122 @@
+// Copyright 2021, The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! This module implements the unique id rotation privacy feature. Certain system components
+//! have the ability to include a per-app unique id into the key attestation. The key rotation
+//! feature assures that the unique id is rotated on factory reset at least once in a 30 day
+//! key rotation period.
+//!
+//! It is assumed that the timestamp file does not exist after a factory reset. So the creation
+//! time of the timestamp file provides a lower bound for the time since factory reset.
+
+use anyhow::{Context, Result};
+use std::fs;
+use std::io::ErrorKind;
+use std::path::{Path, PathBuf};
+use std::time::Duration;
+
+const ID_ROTATION_PERIOD: Duration = Duration::from_secs(30 * 24 * 60 * 60); // Thirty days.
+static TIMESTAMP_FILE_NAME: &str = &"timestamp";
+
+/// The IdRotationState stores the path to the timestamp file for deferred usage. The data
+/// partition is usually not available when Keystore 2.0 starts up. So this object is created
+/// and passed down to the users of the feature which can then query the timestamp on demand.
+#[derive(Debug, Clone)]
+pub struct IdRotationState {
+ timestamp_path: PathBuf,
+}
+
+impl IdRotationState {
+ /// Creates a new IdRotationState. It holds the path to the timestamp file for deferred usage.
+ pub fn new(keystore_db_path: &Path) -> Self {
+ let mut timestamp_path = keystore_db_path.to_owned();
+ timestamp_path.push(TIMESTAMP_FILE_NAME);
+ Self { timestamp_path }
+ }
+
+ /// Reads the metadata of or creates the timestamp file. It returns true if the timestamp
+ /// file is younger than `ID_ROTATION_PERIOD`, i.e., 30 days.
+ pub fn had_factory_reset_since_id_rotation(&self) -> Result<bool> {
+ match fs::metadata(&self.timestamp_path) {
+ Ok(metadata) => {
+ let duration_since_factory_reset = metadata
+ .modified()
+ .context("File creation time not supported.")?
+ .elapsed()
+ .context("Failed to compute time elapsed since factory reset.")?;
+ Ok(duration_since_factory_reset < ID_ROTATION_PERIOD)
+ }
+ Err(e) => match e.kind() {
+ ErrorKind::NotFound => {
+ fs::File::create(&self.timestamp_path)
+ .context("Failed to create timestamp file.")?;
+ Ok(true)
+ }
+ _ => Err(e).context("Failed to open timestamp file."),
+ },
+ }
+ .context("In had_factory_reset_since_id_rotation:")
+ }
+}
+
+#[cfg(test)]
+mod test {
+ use super::*;
+ use keystore2_test_utils::TempDir;
+ use nix::sys::stat::utimes;
+ use nix::sys::time::{TimeVal, TimeValLike};
+ use std::convert::TryInto;
+ use std::time::UNIX_EPOCH;
+
+ #[test]
+ fn test_had_factory_reset_since_id_rotation() -> Result<()> {
+ let temp_dir = TempDir::new("test_had_factory_reset_since_id_rotation_")
+ .expect("Failed to create temp dir.");
+ let id_rotation_state = IdRotationState::new(&temp_dir.path());
+
+ let mut temp_file_path = temp_dir.path().to_owned();
+ temp_file_path.push(TIMESTAMP_FILE_NAME);
+
+ // The timestamp file should not exist.
+ assert!(!temp_file_path.exists());
+
+ // This should return true.
+ assert!(id_rotation_state.had_factory_reset_since_id_rotation()?);
+
+ // Now the timestamp file should exist.
+ assert!(temp_file_path.exists());
+
+ // We should still return true because the timestamp file is young.
+ assert!(id_rotation_state.had_factory_reset_since_id_rotation()?);
+
+ // Now let's age the timestamp file by backdating the modification time.
+ let metadata = fs::metadata(&temp_file_path)?;
+ let mtime = metadata.modified()?;
+ let mtime = mtime.duration_since(UNIX_EPOCH)?;
+ let mtime =
+ mtime.checked_sub(ID_ROTATION_PERIOD).expect("Failed to subtract id rotation period");
+ let mtime = TimeVal::seconds(mtime.as_secs().try_into().unwrap());
+
+ let atime = metadata.accessed()?;
+ let atime = atime.duration_since(UNIX_EPOCH)?;
+ let atime = TimeVal::seconds(atime.as_secs().try_into().unwrap());
+
+ utimes(&temp_file_path, &atime, &mtime)?;
+
+ // Now that the file has aged we should see false.
+ assert!(!id_rotation_state.had_factory_reset_since_id_rotation()?);
+
+ Ok(())
+ }
+}
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index e745697..df7ba26 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -14,13 +14,13 @@
//! This crate implements the Keystore 2.0 service entry point.
-use keystore2::authorization::AuthorizationManager;
use keystore2::entropy;
use keystore2::globals::ENFORCEMENTS;
use keystore2::maintenance::Maintenance;
use keystore2::remote_provisioning::RemoteProvisioningService;
use keystore2::service::KeystoreService;
use keystore2::{apc::ApcManager, shared_secret_negotiation};
+use keystore2::{authorization::AuthorizationManager, id_rotation::IdRotationState};
use log::{error, info};
use std::{panic, path::Path, sync::mpsc::channel};
use vpnprofilestore::VpnProfileStore;
@@ -57,12 +57,14 @@
// startup as Keystore 1.0 did because Keystore 2.0 is intended to run much earlier than
// Keystore 1.0. Instead we set a global variable to the database path.
// For the ground truth check the service startup rule for init (typically in keystore2.rc).
- if let Some(dir) = args.next() {
+ let id_rotation_state = if let Some(dir) = args.next() {
+ let db_path = Path::new(&dir);
*keystore2::globals::DB_PATH.lock().expect("Could not lock DB_PATH.") =
- Path::new(&dir).to_path_buf();
+ db_path.to_path_buf();
+ IdRotationState::new(&db_path)
} else {
- panic!("Must specify a working directory.");
- }
+ panic!("Must specify a database directory.");
+ };
let (confirmation_token_sender, confirmation_token_receiver) = channel();
@@ -81,7 +83,7 @@
info!("Starting thread pool now.");
binder::ProcessState::start_thread_pool();
- let ks_service = KeystoreService::new_native_binder().unwrap_or_else(|e| {
+ let ks_service = KeystoreService::new_native_binder(id_rotation_state).unwrap_or_else(|e| {
panic!("Failed to create service {} because of {:?}.", KS2_SERVICE_NAME, e);
});
binder::add_service(KS2_SERVICE_NAME, ks_service.as_binder()).unwrap_or_else(|e| {
diff --git a/keystore2/src/lib.rs b/keystore2/src/lib.rs
index 62dc16a..154b5b3 100644
--- a/keystore2/src/lib.rs
+++ b/keystore2/src/lib.rs
@@ -24,6 +24,7 @@
pub mod entropy;
pub mod error;
pub mod globals;
+pub mod id_rotation;
/// Internal Representation of Key Parameter and convenience functions.
pub mod key_parameter;
pub mod legacy_blob;
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 50d697e..1cf770f 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -14,7 +14,7 @@
//! This crate implements the IKeystoreSecurityLevel interface.
-use crate::globals::get_keymint_device;
+use crate::{globals::get_keymint_device, id_rotation::IdRotationState};
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
Algorithm::Algorithm, AttestationKey::AttestationKey,
HardwareAuthenticatorType::HardwareAuthenticatorType, IKeyMintDevice::IKeyMintDevice,
@@ -67,6 +67,7 @@
km_uuid: Uuid,
operation_db: OperationDb,
rem_prov_state: RemProvState,
+ id_rotation_state: IdRotationState,
}
// Blob of 32 zeroes used as empty masking key.
@@ -83,6 +84,7 @@
/// we need it for checking keystore permissions.
pub fn new_native_binder(
security_level: SecurityLevel,
+ id_rotation_state: IdRotationState,
) -> Result<(Strong<dyn IKeystoreSecurityLevel>, Uuid)> {
let (dev, hw_info, km_uuid) = get_keymint_device(&security_level)
.context("In KeystoreSecurityLevel::new_native_binder.")?;
@@ -93,6 +95,7 @@
km_uuid,
operation_db: OperationDb::new(),
rem_prov_state: RemProvState::new(security_level, km_uuid),
+ id_rotation_state,
});
result.as_binder().set_requesting_sid(true);
Ok((result, km_uuid))
@@ -353,6 +356,7 @@
}
fn add_certificate_parameters(
+ &self,
uid: u32,
params: &[KeyParameter],
key: &KeyDescriptor,
@@ -374,6 +378,14 @@
"In add_certificate_parameters: ",
"Caller does not have the permission to generate a unique ID"
))?;
+ if self.id_rotation_state.had_factory_reset_since_id_rotation().context(
+ "In add_certificate_parameters: Call to had_factory_reset_since_id_rotation failed."
+ )? {
+ result.push(KeyParameter{
+ tag: Tag::RESET_SINCE_ID_ROTATION,
+ value: KeyParameterValue::BoolValue(true),
+ })
+ }
}
// If the caller requests any device identifier attestation tag, check that they hold the
@@ -451,7 +463,8 @@
})
.context("In generate_key: Trying to get an attestation key")?,
};
- let params = Self::add_certificate_parameters(caller_uid, params, &key)
+ let params = self
+ .add_certificate_parameters(caller_uid, params, &key)
.context("In generate_key: Trying to get aaid.")?;
let km_dev: Strong<dyn IKeyMintDevice> = self.keymint.get_interface()?;
@@ -524,7 +537,8 @@
// import_key requires the rebind permission.
check_key_permission(KeyPerm::rebind(), &key, &None).context("In import_key.")?;
- let params = Self::add_certificate_parameters(caller_uid, params, &key)
+ let params = self
+ .add_certificate_parameters(caller_uid, params, &key)
.context("In import_key: Trying to get aaid.")?;
let format = params
diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs
index 73bd526..1debe1b 100644
--- a/keystore2/src/service.rs
+++ b/keystore2/src/service.rs
@@ -17,7 +17,6 @@
use std::collections::HashMap;
-use crate::error::{self, map_or_log_err, ErrorCode};
use crate::permission::{KeyPerm, KeystorePerm};
use crate::security_level::KeystoreSecurityLevel;
use crate::utils::{
@@ -33,6 +32,10 @@
database::{KeyEntryLoadBits, KeyType, SubComponentType},
error::ResponseCode,
};
+use crate::{
+ error::{self, map_or_log_err, ErrorCode},
+ id_rotation::IdRotationState,
+};
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::SecurityLevel::SecurityLevel;
use android_system_keystore2::aidl::android::system::keystore2::{
Domain::Domain, IKeystoreSecurityLevel::IKeystoreSecurityLevel,
@@ -53,21 +56,26 @@
impl KeystoreService {
/// Create a new instance of the Keystore 2.0 service.
- pub fn new_native_binder() -> Result<Strong<dyn IKeystoreService>> {
+ pub fn new_native_binder(
+ id_rotation_state: IdRotationState,
+ ) -> Result<Strong<dyn IKeystoreService>> {
let mut result: Self = Default::default();
- let (dev, uuid) =
- KeystoreSecurityLevel::new_native_binder(SecurityLevel::TRUSTED_ENVIRONMENT)
- .context(concat!(
- "In KeystoreService::new_native_binder: ",
- "Trying to construct mandatory security level TEE."
- ))
- .map(|(dev, uuid)| (Asp::new(dev.as_binder()), uuid))?;
+ let (dev, uuid) = KeystoreSecurityLevel::new_native_binder(
+ SecurityLevel::TRUSTED_ENVIRONMENT,
+ id_rotation_state.clone(),
+ )
+ .context(concat!(
+ "In KeystoreService::new_native_binder: ",
+ "Trying to construct mandatory security level TEE."
+ ))
+ .map(|(dev, uuid)| (Asp::new(dev.as_binder()), uuid))?;
result.i_sec_level_by_uuid.insert(uuid, dev);
result.uuid_by_sec_level.insert(SecurityLevel::TRUSTED_ENVIRONMENT, uuid);
// Strongbox is optional, so we ignore errors and turn the result into an Option.
- if let Ok((dev, uuid)) = KeystoreSecurityLevel::new_native_binder(SecurityLevel::STRONGBOX)
- .map(|(dev, uuid)| (Asp::new(dev.as_binder()), uuid))
+ if let Ok((dev, uuid)) =
+ KeystoreSecurityLevel::new_native_binder(SecurityLevel::STRONGBOX, id_rotation_state)
+ .map(|(dev, uuid)| (Asp::new(dev.as_binder()), uuid))
{
result.i_sec_level_by_uuid.insert(uuid, dev);
result.uuid_by_sec_level.insert(SecurityLevel::STRONGBOX, uuid);
diff --git a/ondevice-signing/KeystoreKey.cpp b/ondevice-signing/KeystoreKey.cpp
index 840b683..9b5e505 100644
--- a/ondevice-signing/KeystoreKey.cpp
+++ b/ondevice-signing/KeystoreKey.cpp
@@ -239,5 +239,10 @@
}
Result<std::vector<uint8_t>> KeystoreKey::getPublicKey() const {
- return extractPublicKeyFromX509(mKeyMetadata.certificate.value());
+ auto cert = mKeyMetadata.certificate;
+ if (cert) {
+ return extractPublicKeyFromX509(cert.value());
+ } else {
+ return Error() << "Key did not have a certificate";
+ }
}