Merge "Keystore 2.0 legacy Keystore: Cleanup when app/user removed." into sc-dev
diff --git a/keystore2/aidl/android/security/metrics/RkpPoolStats.aidl b/keystore2/aidl/android/security/metrics/RkpPoolStats.aidl
index b233842..016b6ff 100644
--- a/keystore2/aidl/android/security/metrics/RkpPoolStats.aidl
+++ b/keystore2/aidl/android/security/metrics/RkpPoolStats.aidl
@@ -16,14 +16,17 @@
package android.security.metrics;
-import android.security.metrics.PoolStatus;
+import android.security.metrics.SecurityLevel;
/**
- * Count of keys in the key pool related to Remote Key Provisioning (RKP).
+ * Count of keys in the attestation key pool related to Remote Key Provisioning (RKP).
* @hide
*/
@RustDerive(Clone=true, Eq=true, PartialEq=true, Ord=true, PartialOrd=true, Hash=true)
parcelable RkpPoolStats {
- PoolStatus pool_status;
- int count_of_keys;
+ SecurityLevel security_level;
+ int expiring;
+ int unassigned;
+ int attested;
+ int total;
}
\ No newline at end of file
diff --git a/keystore2/src/attestation_key_utils.rs b/keystore2/src/attestation_key_utils.rs
index 425eec6..ca00539 100644
--- a/keystore2/src/attestation_key_utils.rs
+++ b/keystore2/src/attestation_key_utils.rs
@@ -22,7 +22,7 @@
use crate::remote_provisioning::RemProvState;
use crate::utils::check_key_permission;
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
- AttestationKey::AttestationKey, Certificate::Certificate, KeyParameter::KeyParameter,
+ AttestationKey::AttestationKey, Certificate::Certificate, KeyParameter::KeyParameter, Tag::Tag,
};
use android_system_keystore2::aidl::android::system::keystore2::{
Domain::Domain, KeyDescriptor::KeyDescriptor,
@@ -47,8 +47,8 @@
}
/// This function loads and, optionally, assigns the caller's remote provisioned
-/// attestation key or, if `attest_key_descriptor` is given, it loads the user
-/// generated attestation key from the database.
+/// attestation key if a challenge is present. Alternatively, if `attest_key_descriptor` is given,
+/// it loads the user generated attestation key from the database.
pub fn get_attest_key_info(
key: &KeyDescriptor,
caller_uid: u32,
@@ -57,8 +57,9 @@
rem_prov_state: &RemProvState,
db: &mut KeystoreDB,
) -> Result<Option<AttestationKeyInfo>> {
+ let challenge_present = params.iter().any(|kp| kp.tag == Tag::ATTESTATION_CHALLENGE);
match attest_key_descriptor {
- None => rem_prov_state
+ None if challenge_present => rem_prov_state
.get_remotely_provisioned_attestation_key_and_certs(&key, caller_uid, params, db)
.context(concat!(
"In get_attest_key_and_cert_chain: ",
@@ -69,6 +70,7 @@
AttestationKeyInfo::RemoteProvisioned { attestation_key, attestation_certs }
})
}),
+ None => Ok(None),
Some(attest_key) => get_user_generated_attestation_key(&attest_key, caller_uid, db)
.context("In get_attest_key_and_cert_chain: Trying to load attest key")
.map(Some),
diff --git a/keystore2/src/crypto/crypto.cpp b/keystore2/src/crypto/crypto.cpp
index e4a1ac3..5d360a1 100644
--- a/keystore2/src/crypto/crypto.cpp
+++ b/keystore2/src/crypto/crypto.cpp
@@ -225,7 +225,7 @@
EC_KEY* ECKEYGenerateKey() {
EC_KEY* key = EC_KEY_new();
- EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
+ EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_secp521r1);
EC_KEY_set_group(key, group);
auto result = EC_KEY_generate_key(key);
if (result == 0) {
@@ -251,7 +251,7 @@
EC_KEY* ECKEYParsePrivateKey(const uint8_t* buf, size_t len) {
CBS cbs;
CBS_init(&cbs, buf, len);
- EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
+ EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_secp521r1);
auto result = EC_KEY_parse_private_key(&cbs, group);
EC_GROUP_free(group);
if (result != nullptr && CBS_len(&cbs) != 0) {
@@ -262,7 +262,7 @@
}
size_t ECPOINTPoint2Oct(const EC_POINT* point, uint8_t* buf, size_t len) {
- EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
+ EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_secp521r1);
point_conversion_form_t form = POINT_CONVERSION_UNCOMPRESSED;
auto result = EC_POINT_point2oct(group, point, form, buf, len, nullptr);
EC_GROUP_free(group);
@@ -270,7 +270,7 @@
}
EC_POINT* ECPOINTOct2Point(const uint8_t* buf, size_t len) {
- EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1);
+ EC_GROUP* group = EC_GROUP_new_by_curve_name(NID_secp521r1);
EC_POINT* point = EC_POINT_new(group);
auto result = EC_POINT_oct2point(group, point, buf, len, nullptr);
EC_GROUP_free(group);
diff --git a/keystore2/src/crypto/lib.rs b/keystore2/src/crypto/lib.rs
index db23d1f..5f8a2ef 100644
--- a/keystore2/src/crypto/lib.rs
+++ b/keystore2/src/crypto/lib.rs
@@ -346,7 +346,7 @@
/// Calls the boringssl EC_KEY_marshal_private_key function.
pub fn ec_key_marshal_private_key(key: &ECKey) -> Result<ZVec, Error> {
- let len = 39; // Empirically observed length of private key
+ let len = 73; // Empirically observed length of private key
let mut buf = ZVec::new(len)?;
// Safety: the key is valid.
// This will not write past the specified length of the buffer; if the
@@ -381,8 +381,8 @@
/// Calls the boringssl EC_POINT_point2oct.
pub fn ec_point_point_to_oct(point: &EC_POINT) -> Result<Vec<u8>, Error> {
- // We fix the length to 65 (1 + 2 * field_elem_size), as we get an error if it's too small.
- let len = 65;
+ // We fix the length to 133 (1 + 2 * field_elem_size), as we get an error if it's too small.
+ let len = 133;
let mut buf = vec![0; len];
// Safety: EC_POINT_point2oct writes at most len bytes. The point is valid.
let result = unsafe { ECPOINTPoint2Oct(point, buf.as_mut_ptr(), len) };
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index ca346e0..c788720 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -47,6 +47,7 @@
use crate::impl_metadata; // This is in db_utils.rs
use crate::key_parameter::{KeyParameter, Tag};
+use crate::metrics_store::log_rkp_error_stats;
use crate::permission::KeyPermSet;
use crate::utils::{get_current_time_in_milliseconds, watchdog as wd, AID_USER_OFFSET};
use crate::{
@@ -72,6 +73,7 @@
use android_security_metrics::aidl::android::security::metrics::{
StorageStats::StorageStats,
Storage::Storage as MetricsStorage,
+ RkpError::RkpError as MetricsRkpError,
};
use keystore2_crypto::ZVec;
@@ -1829,6 +1831,7 @@
)
.context("Failed to assign attestation key")?;
if result == 0 {
+ log_rkp_error_stats(MetricsRkpError::OUT_OF_KEYS);
return Err(KsError::Rc(ResponseCode::OUT_OF_KEYS)).context("Out of keys.");
} else if result > 1 {
return Err(KsError::sys())
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index e3de035..32fe353 100644
--- a/keystore2/src/metrics_store.rs
+++ b/keystore2/src/metrics_store.rs
@@ -21,6 +21,7 @@
use crate::globals::DB;
use crate::key_parameter::KeyParameterValue as KsKeyParamValue;
use crate::operation::Outcome;
+use crate::remote_provisioning::get_pool_status;
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
Algorithm::Algorithm, BlockMode::BlockMode, Digest::Digest, EcCurve::EcCurve,
HardwareAuthenticatorType::HardwareAuthenticatorType, KeyOrigin::KeyOrigin,
@@ -38,12 +39,15 @@
KeyOrigin::KeyOrigin as MetricsKeyOrigin, Keystore2AtomWithOverflow::Keystore2AtomWithOverflow,
KeystoreAtom::KeystoreAtom, KeystoreAtomPayload::KeystoreAtomPayload,
Outcome::Outcome as MetricsOutcome, Purpose::Purpose as MetricsPurpose,
- SecurityLevel::SecurityLevel as MetricsSecurityLevel, Storage::Storage as MetricsStorage,
+ RkpError::RkpError as MetricsRkpError, RkpErrorStats::RkpErrorStats,
+ RkpPoolStats::RkpPoolStats, SecurityLevel::SecurityLevel as MetricsSecurityLevel,
+ Storage::Storage as MetricsStorage,
};
use anyhow::Result;
use lazy_static::lazy_static;
use std::collections::HashMap;
use std::sync::Mutex;
+use std::time::{Duration, SystemTime, UNIX_EPOCH};
lazy_static! {
/// Singleton for MetricsStore.
@@ -83,7 +87,10 @@
return pull_storage_stats();
}
- // TODO (b/184301651): process and return RKP pool stats.
+ // Process and return RKP pool stats.
+ if AtomID::RKP_POOL_STATS == atom_id {
+ return pull_attestation_pool_stats();
+ }
// It is safe to call unwrap here since the lock can not be poisoned based on its usage
// in this module and the lock is not acquired in the same thread before.
@@ -534,6 +541,47 @@
Ok(atom_vec)
}
+fn pull_attestation_pool_stats() -> Result<Vec<KeystoreAtom>> {
+ let mut atoms = Vec::<KeystoreAtom>::new();
+ for sec_level in &[SecurityLevel::TRUSTED_ENVIRONMENT, SecurityLevel::STRONGBOX] {
+ let expired_by = SystemTime::now()
+ .duration_since(UNIX_EPOCH)
+ .unwrap_or_else(|_| Duration::new(0, 0))
+ .as_secs() as i64;
+
+ let result = get_pool_status(expired_by, *sec_level);
+
+ if let Ok(pool_status) = result {
+ let rkp_pool_stats = RkpPoolStats {
+ security_level: process_security_level(*sec_level),
+ expiring: pool_status.expiring,
+ unassigned: pool_status.unassigned,
+ attested: pool_status.attested,
+ total: pool_status.total,
+ };
+ atoms.push(KeystoreAtom {
+ payload: KeystoreAtomPayload::RkpPoolStats(rkp_pool_stats),
+ ..Default::default()
+ });
+ } else {
+ log::error!(
+ concat!(
+ "In pull_attestation_pool_stats: Failed to retrieve pool status",
+ " for security level: {:?}"
+ ),
+ sec_level
+ );
+ }
+ }
+ Ok(atoms)
+}
+
+/// Log error events related to Remote Key Provisioning (RKP).
+pub fn log_rkp_error_stats(rkp_error: MetricsRkpError) {
+ let rkp_error_stats = KeystoreAtomPayload::RkpErrorStats(RkpErrorStats { rkpError: rkp_error });
+ METRICS_STORE.insert_atom(AtomID::RKP_ERROR_STATS, rkp_error_stats);
+}
+
/// Enum defining the bit position for each padding mode. Since padding mode can be repeatable, it
/// is represented using a bitmap.
#[allow(non_camel_case_types)]
diff --git a/keystore2/src/remote_provisioning.rs b/keystore2/src/remote_provisioning.rs
index 40ffd0c..6666f41 100644
--- a/keystore2/src/remote_provisioning.rs
+++ b/keystore2/src/remote_provisioning.rs
@@ -234,26 +234,6 @@
Ok(BnRemoteProvisioning::new_binder(result, BinderFeatures::default()))
}
- /// Populates the AttestationPoolStatus parcelable with information about how many
- /// certs will be expiring by the date provided in `expired_by` along with how many
- /// keys have not yet been assigned.
- pub fn get_pool_status(
- &self,
- expired_by: i64,
- sec_level: SecurityLevel,
- ) -> Result<AttestationPoolStatus> {
- let (_, _, uuid) = get_keymint_device(&sec_level)?;
- DB.with::<_, Result<AttestationPoolStatus>>(|db| {
- let mut db = db.borrow_mut();
- // delete_expired_attestation_keys is always safe to call, and will remove anything
- // older than the date at the time of calling. No work should be done on the
- // attestation keys unless the pool status is checked first, so this call should be
- // enough to routinely clean out expired keys.
- db.delete_expired_attestation_keys()?;
- db.get_attestation_pool_status(expired_by, &uuid)
- })
- }
-
/// Generates a CBOR blob which will be assembled by the calling code into a larger
/// CBOR blob intended for delivery to a provisioning serever. This blob will contain
/// `num_csr` certificate signing requests for attestation keys generated in the TEE,
@@ -389,6 +369,22 @@
}
}
+/// Populates the AttestationPoolStatus parcelable with information about how many
+/// certs will be expiring by the date provided in `expired_by` along with how many
+/// keys have not yet been assigned.
+pub fn get_pool_status(expired_by: i64, sec_level: SecurityLevel) -> Result<AttestationPoolStatus> {
+ let (_, _, uuid) = get_keymint_device(&sec_level)?;
+ DB.with::<_, Result<AttestationPoolStatus>>(|db| {
+ let mut db = db.borrow_mut();
+ // delete_expired_attestation_keys is always safe to call, and will remove anything
+ // older than the date at the time of calling. No work should be done on the
+ // attestation keys unless the pool status is checked first, so this call should be
+ // enough to routinely clean out expired keys.
+ db.delete_expired_attestation_keys()?;
+ db.get_attestation_pool_status(expired_by, &uuid)
+ })
+}
+
impl binder::Interface for RemoteProvisioningService {}
// Implementation of IRemoteProvisioning. See AIDL spec at
@@ -400,7 +396,7 @@
sec_level: SecurityLevel,
) -> binder::public_api::Result<AttestationPoolStatus> {
let _wp = wd::watch_millis("IRemoteProvisioning::getPoolStatus", 500);
- map_or_log_err(self.get_pool_status(expired_by, sec_level), Ok)
+ map_or_log_err(get_pool_status(expired_by, sec_level), Ok)
}
fn generateCsr(
diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs
index 9fb267a..e02d9bc 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -68,8 +68,8 @@
pub enum SuperEncryptionAlgorithm {
/// Symmetric encryption with AES-256-GCM
Aes256Gcm,
- /// Public-key encryption with ECDH P-256
- EcdhP256,
+ /// Public-key encryption with ECDH P-521
+ EcdhP521,
}
/// A particular user may have several superencryption keys in the database, each for a
@@ -96,9 +96,9 @@
/// Key used for ScreenLockBound keys; the corresponding superencryption key is loaded in memory
/// each time the user enters their LSKF, and cleared from memory each time the device is locked.
/// Asymmetric, so keys can be encrypted when the device is locked.
-pub const USER_SCREEN_LOCK_BOUND_ECDH_KEY: SuperKeyType = SuperKeyType {
- alias: "USER_SCREEN_LOCK_BOUND_ECDH_KEY",
- algorithm: SuperEncryptionAlgorithm::EcdhP256,
+pub const USER_SCREEN_LOCK_BOUND_P521_KEY: SuperKeyType = SuperKeyType {
+ alias: "USER_SCREEN_LOCK_BOUND_P521_KEY",
+ algorithm: SuperEncryptionAlgorithm::EcdhP521,
};
/// Superencryption to apply to a new key.
@@ -468,7 +468,7 @@
tag.is_some(),
)),
},
- SuperEncryptionAlgorithm::EcdhP256 => {
+ SuperEncryptionAlgorithm::EcdhP521 => {
match (metadata.public_key(), metadata.salt(), metadata.iv(), metadata.aead_tag()) {
(Some(public_key), Some(salt), Some(iv), Some(aead_tag)) => {
ECDHPrivateKey::from_private_key(&key.key)
@@ -753,7 +753,7 @@
} else {
// Symmetric key is not available, use public key encryption
let loaded =
- db.load_super_key(&USER_SCREEN_LOCK_BOUND_ECDH_KEY, user_id).context(
+ db.load_super_key(&USER_SCREEN_LOCK_BOUND_P521_KEY, user_id).context(
"In handle_super_encryption_on_key_init: load_super_key failed.",
)?;
let (key_id_guard, key_entry) = loaded.ok_or_else(Error::sys).context(
@@ -836,7 +836,7 @@
.context("In get_or_create_super_key: Failed to generate AES 256 key.")?,
None,
),
- SuperEncryptionAlgorithm::EcdhP256 => {
+ SuperEncryptionAlgorithm::EcdhP521 => {
let key = ECDHPrivateKey::generate()
.context("In get_or_create_super_key: Failed to generate ECDH key")?;
(
@@ -903,7 +903,7 @@
Self::get_or_create_super_key(
db,
user_id,
- &USER_SCREEN_LOCK_BOUND_ECDH_KEY,
+ &USER_SCREEN_LOCK_BOUND_P521_KEY,
password,
Some(aes.clone()),
)