Merge "Log metrics related to Remote Key Provisioning (RKP)." into sc-dev
diff --git a/keystore2/Android.bp b/keystore2/Android.bp
index 388a1b3..0069f95 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -106,7 +106,7 @@
"libbinder_rs",
"libkeystore2",
"liblog_rust",
- "libvpnprofilestore-rust",
+ "liblegacykeystore-rust",
],
init_rc: ["keystore2.rc"],
diff --git a/keystore2/aidl/Android.bp b/keystore2/aidl/Android.bp
index 54353ab..4a7b7b4 100644
--- a/keystore2/aidl/Android.bp
+++ b/keystore2/aidl/Android.bp
@@ -151,8 +151,8 @@
}
aidl_interface {
- name: "android.security.vpnprofilestore",
- srcs: [ "android/security/vpnprofilestore/*.aidl" ],
+ name: "android.security.legacykeystore",
+ srcs: [ "android/security/legacykeystore/*.aidl" ],
unstable: true,
backend: {
java: {
@@ -162,6 +162,10 @@
rust: {
enabled: true,
},
+ ndk: {
+ enabled: true,
+ apps_enabled: false,
+ }
},
}
diff --git a/keystore2/aidl/android/security/legacykeystore/ILegacyKeystore.aidl b/keystore2/aidl/android/security/legacykeystore/ILegacyKeystore.aidl
new file mode 100644
index 0000000..e65efaa
--- /dev/null
+++ b/keystore2/aidl/android/security/legacykeystore/ILegacyKeystore.aidl
@@ -0,0 +1,98 @@
+/*
+ * Copyright (C) 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.
+ */
+
+package android.security.legacykeystore;
+
+/**
+ * Internal interface for accessing and storing legacy keystore blobs.
+ * Before Android S, Keystore offered a key-value store that was intended for storing
+ * data associated with certain types of keys. E.g., public certificates for asymmetric keys.
+ * This key value store no longer exists as part of the Keystore 2.0 protocol.
+ * However, there are some clients that used Keystore in an unintended way.
+ * This interface exists to give these clients a grace period to migrate their keys
+ * out of legacy keystore. In Android S, this legacy keystore may be used as keystore was
+ * used in earlier versions, and provides access to entries that were put into keystore
+ * before Android S.
+ *
+ * DEPRECATION NOTICE: In Android T, the `put` function is slated to be removed.
+ * This will allow clients to use the `get`, `list`, and `remove` API to migrate blobs out
+ * of legacy keystore.
+ * @hide
+ */
+interface ILegacyKeystore {
+
+ /**
+ * Special value indicating the callers uid.
+ */
+ const int UID_SELF = -1;
+
+ /**
+ * Service specific error code indicating that an unexpected system error occurred.
+ */
+ const int ERROR_SYSTEM_ERROR = 4;
+
+ /**
+ * Service specific error code indicating that the caller does not have the
+ * right to access the requested uid.
+ */
+ const int ERROR_PERMISSION_DENIED = 6;
+
+ /**
+ * Service specific error code indicating that the entry was not found.
+ */
+ const int ERROR_ENTRY_NOT_FOUND = 7;
+
+ /**
+ * Returns the blob stored under the given name.
+ *
+ * @param alias name of the blob entry.
+ * @param uid designates the legacy namespace. Specify UID_SELF for the caller's namespace.
+ * @return The unstructured blob that was passed as blob parameter into put()
+ */
+ byte[] get(in String alias, int uid);
+
+ /**
+ * Stores one entry as unstructured blob under the given alias.
+ * Overwrites existing entries with the same alias.
+ *
+ * @param alias name of the new entry.
+ * @param uid designates the legacy namespace. Specify UID_SELF for the caller's namespace.
+ * @param blob the payload of the new entry.
+ *
+ * IMPORTANT DEPRECATION NOTICE: This function is slated to be removed in Android T.
+ * Do not add new callers. The remaining functionality will remain for the purpose
+ * of migrating legacy configuration out.
+ */
+ void put(in String alias, int uid, in byte[] blob);
+
+ /**
+ * Deletes the entry under the given alias.
+ *
+ * @param alias name of the entry to be removed.
+ * @param uid designates the legacy namespace of the entry. Specify UID_SELF for the caller's
+ * namespace.
+ */
+ void remove(in String alias, int uid);
+
+ /**
+ * Returns a list of aliases of entries stored. The list is filtered by prefix.
+ * The resulting strings are the full aliases including the prefix.
+ *
+ * @param prefix used to filter results.
+ * @param uid legacy namespace to list. Specify UID_SELF for caller's namespace.
+ */
+ String[] list(in String prefix, int uid);
+}
\ No newline at end of file
diff --git a/keystore2/aidl/android/security/vpnprofilestore/IVpnProfileStore.aidl b/keystore2/aidl/android/security/vpnprofilestore/IVpnProfileStore.aidl
deleted file mode 100644
index 8375b7b..0000000
--- a/keystore2/aidl/android/security/vpnprofilestore/IVpnProfileStore.aidl
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * Copyright (C) 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.
- */
-
-package android.security.vpnprofilestore;
-
-/**
- * Internal interface for accessing and storing VPN profiles.
- * @hide
- */
-interface IVpnProfileStore {
- /**
- * Service specific error code indicating that the profile was not found.
- */
- const int ERROR_PROFILE_NOT_FOUND = 1;
-
- /**
- * Service specific error code indicating that an unexpected system error occurred.
- */
- const int ERROR_SYSTEM_ERROR = 2;
-
- /**
- * Returns the profile stored under the given alias.
- *
- * @param alias name of the profile.
- * @return The unstructured blob that was passed as profile parameter into put()
- */
- byte[] get(in String alias);
-
- /**
- * Stores one profile as unstructured blob under the given alias.
- */
- void put(in String alias, in byte[] profile);
-
- /**
- * Deletes the profile under the given alias.
- */
- void remove(in String alias);
-
- /**
- * Returns a list of aliases of profiles stored. The list is filtered by prefix.
- * The resulting strings are the full aliases including the prefix.
- */
- String[] list(in String prefix);
-}
\ No newline at end of file
diff --git a/keystore2/vpnprofilestore/Android.bp b/keystore2/legacykeystore/Android.bp
similarity index 85%
rename from keystore2/vpnprofilestore/Android.bp
rename to keystore2/legacykeystore/Android.bp
index 7ddf0d6..c2890cc 100644
--- a/keystore2/vpnprofilestore/Android.bp
+++ b/keystore2/legacykeystore/Android.bp
@@ -22,13 +22,13 @@
}
rust_library {
- name: "libvpnprofilestore-rust",
- crate_name: "vpnprofilestore",
+ name: "liblegacykeystore-rust",
+ crate_name: "legacykeystore",
srcs: [
"lib.rs",
],
rustlibs: [
- "android.security.vpnprofilestore-rust",
+ "android.security.legacykeystore-rust",
"libanyhow",
"libbinder_rs",
"libkeystore2",
@@ -39,13 +39,13 @@
}
rust_test {
- name: "vpnprofilestore_test",
- crate_name: "vpnprofilestore",
+ name: "legacykeystore_test",
+ crate_name: "legacykeystore",
srcs: ["lib.rs"],
test_suites: ["general-tests"],
auto_gen_config: true,
rustlibs: [
- "android.security.vpnprofilestore-rust",
+ "android.security.legacykeystore-rust",
"libanyhow",
"libbinder_rs",
"libkeystore2",
diff --git a/keystore2/vpnprofilestore/lib.rs b/keystore2/legacykeystore/lib.rs
similarity index 63%
rename from keystore2/vpnprofilestore/lib.rs
rename to keystore2/legacykeystore/lib.rs
index baa632f..8a4d7d8 100644
--- a/keystore2/vpnprofilestore/lib.rs
+++ b/keystore2/legacykeystore/lib.rs
@@ -12,13 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-//! Implements the android.security.vpnprofilestore interface.
+//! Implements the android.security.legacykeystore interface.
-use android_security_vpnprofilestore::aidl::android::security::vpnprofilestore::{
- IVpnProfileStore::BnVpnProfileStore, IVpnProfileStore::IVpnProfileStore,
- IVpnProfileStore::ERROR_PROFILE_NOT_FOUND, IVpnProfileStore::ERROR_SYSTEM_ERROR,
+use android_security_legacykeystore::aidl::android::security::legacykeystore::{
+ ILegacyKeystore::BnLegacyKeystore, ILegacyKeystore::ILegacyKeystore,
+ ILegacyKeystore::ERROR_ENTRY_NOT_FOUND, ILegacyKeystore::ERROR_PERMISSION_DENIED,
+ ILegacyKeystore::ERROR_SYSTEM_ERROR, ILegacyKeystore::UID_SELF,
};
-use android_security_vpnprofilestore::binder::{
+use android_security_legacykeystore::binder::{
BinderFeatures, ExceptionCode, Result as BinderResult, Status as BinderStatus, Strong,
ThreadState,
};
@@ -42,7 +43,7 @@
conn: Connection::open(db_file).context("Failed to initialize SQLite connection.")?,
};
- db.init_tables().context("Trying to initialize vpnstore db.")?;
+ db.init_tables().context("Trying to initialize legacy keystore db.")?;
Ok(db)
}
@@ -110,11 +111,11 @@
})
}
- fn put(&mut self, caller_uid: u32, alias: &str, profile: &[u8]) -> Result<()> {
+ fn put(&mut self, caller_uid: u32, alias: &str, entry: &[u8]) -> Result<()> {
self.with_transaction(TransactionBehavior::Immediate, |tx| {
tx.execute(
"INSERT OR REPLACE INTO profiles (owner, alias, profile) values (?, ?, ?)",
- params![caller_uid, alias, profile,],
+ params![caller_uid, alias, entry,],
)
.context("In put: Failed to insert or replace.")?;
Ok(())
@@ -129,7 +130,7 @@
|row| row.get(0),
)
.optional()
- .context("In get: failed loading profile.")
+ .context("In get: failed loading entry.")
})
}
@@ -145,11 +146,11 @@
}
}
-/// This is the main VpnProfileStore error type, it wraps binder exceptions and the
-/// VnpStore errors.
+/// This is the main LegacyKeystore error type, it wraps binder exceptions and the
+/// LegacyKeystore errors.
#[derive(Debug, thiserror::Error, PartialEq)]
pub enum Error {
- /// Wraps a VpnProfileStore error code.
+ /// Wraps a LegacyKeystore error code.
#[error("Error::Error({0:?})")]
Error(i32),
/// Wraps a Binder exception code other than a service specific exception.
@@ -163,16 +164,21 @@
Error::Error(ERROR_SYSTEM_ERROR)
}
- /// Short hand for `Error::Error(ERROR_PROFILE_NOT_FOUND)`
+ /// Short hand for `Error::Error(ERROR_ENTRY_NOT_FOUND)`
pub fn not_found() -> Self {
- Error::Error(ERROR_PROFILE_NOT_FOUND)
+ Error::Error(ERROR_ENTRY_NOT_FOUND)
+ }
+
+ /// Short hand for `Error::Error(ERROR_PERMISSION_DENIED)`
+ pub fn perm() -> Self {
+ Error::Error(ERROR_PERMISSION_DENIED)
}
}
-/// This function should be used by vpnprofilestore service calls to translate error conditions
+/// This function should be used by legacykeystore service calls to translate error conditions
/// into service specific exceptions.
///
-/// All error conditions get logged by this function, except for ERROR_PROFILE_NOT_FOUND error.
+/// All error conditions get logged by this function, except for ERROR_ENTRY_NOT_FOUND error.
///
/// `Error::Error(x)` variants get mapped onto a service specific error code of `x`.
///
@@ -189,8 +195,8 @@
|e| {
let root_cause = e.root_cause();
let (rc, log_error) = match root_cause.downcast_ref::<Error>() {
- // Make the profile not found errors silent.
- Some(Error::Error(ERROR_PROFILE_NOT_FOUND)) => (ERROR_PROFILE_NOT_FOUND, false),
+ // Make the entry not found errors silent.
+ Some(Error::Error(ERROR_ENTRY_NOT_FOUND)) => (ERROR_ENTRY_NOT_FOUND, false),
Some(Error::Error(e)) => (*e, true),
Some(Error::Binder(_, _)) | None => (ERROR_SYSTEM_ERROR, true),
};
@@ -203,8 +209,8 @@
)
}
-/// Implements IVpnProfileStore AIDL interface.
-pub struct VpnProfileStore {
+/// Implements ILegacyKeystore AIDL interface.
+pub struct LegacyKeystore {
db_path: PathBuf,
async_task: AsyncTask,
}
@@ -215,72 +221,93 @@
db_path: PathBuf,
}
-impl VpnProfileStore {
- /// Creates a new VpnProfileStore instance.
- pub fn new_native_binder(path: &Path) -> Strong<dyn IVpnProfileStore> {
+impl LegacyKeystore {
+ /// Note: The filename was chosen before the purpose of this module was extended.
+ /// It is kept for backward compatibility with early adopters.
+ const LEGACY_KEYSTORE_FILE_NAME: &'static str = "vpnprofilestore.sqlite";
+
+ /// Creates a new LegacyKeystore instance.
+ pub fn new_native_binder(path: &Path) -> Strong<dyn ILegacyKeystore> {
let mut db_path = path.to_path_buf();
- db_path.push("vpnprofilestore.sqlite");
+ db_path.push(Self::LEGACY_KEYSTORE_FILE_NAME);
let result = Self { db_path, async_task: Default::default() };
result.init_shelf(path);
- BnVpnProfileStore::new_binder(result, BinderFeatures::default())
+ BnLegacyKeystore::new_binder(result, BinderFeatures::default())
}
fn open_db(&self) -> Result<DB> {
DB::new(&self.db_path).context("In open_db: Failed to open db.")
}
- fn get(&self, alias: &str) -> Result<Vec<u8>> {
- let mut db = self.open_db().context("In get.")?;
+ fn get_effective_uid(uid: i32) -> Result<u32> {
+ const AID_SYSTEM: u32 = 1000;
+ const AID_WIFI: u32 = 1010;
let calling_uid = ThreadState::get_calling_uid();
+ let uid = uid as u32;
- if let Some(profile) =
- db.get(calling_uid, alias).context("In get: Trying to load profile from DB.")?
- {
- return Ok(profile);
+ if uid == UID_SELF as u32 || uid == calling_uid {
+ Ok(calling_uid)
+ } else if calling_uid == AID_SYSTEM && uid == AID_WIFI {
+ // The only exception for legacy reasons is allowing SYSTEM to access
+ // the WIFI namespace.
+ // IMPORTANT: If you attempt to add more exceptions, it means you are adding
+ // more callers to this deprecated feature. DON'T!
+ Ok(AID_WIFI)
+ } else {
+ Err(Error::perm()).with_context(|| {
+ format!("In get_effective_uid: caller: {}, requested uid: {}.", calling_uid, uid)
+ })
}
- if self.get_legacy(calling_uid, alias).context("In get: Trying to migrate legacy blob.")? {
+ }
+
+ fn get(&self, alias: &str, uid: i32) -> Result<Vec<u8>> {
+ let mut db = self.open_db().context("In get.")?;
+ let uid = Self::get_effective_uid(uid).context("In get.")?;
+
+ if let Some(entry) = db.get(uid, alias).context("In get: Trying to load entry from DB.")? {
+ return Ok(entry);
+ }
+ if self.get_legacy(uid, alias).context("In get: Trying to migrate legacy blob.")? {
// If we were able to migrate a legacy blob try again.
- if let Some(profile) =
- db.get(calling_uid, alias).context("In get: Trying to load profile from DB.")?
+ if let Some(entry) =
+ db.get(uid, alias).context("In get: Trying to load entry from DB.")?
{
- return Ok(profile);
+ return Ok(entry);
}
}
- Err(Error::not_found()).context("In get: No such profile.")
+ Err(Error::not_found()).context("In get: No such entry.")
}
- fn put(&self, alias: &str, profile: &[u8]) -> Result<()> {
- let calling_uid = ThreadState::get_calling_uid();
- // In order to make sure that we don't have stale legacy profiles, make sure they are
+ fn put(&self, alias: &str, uid: i32, entry: &[u8]) -> Result<()> {
+ let uid = Self::get_effective_uid(uid).context("In put.")?;
+ // In order to make sure that we don't have stale legacy entries, make sure they are
// migrated before replacing them.
- let _ = self.get_legacy(calling_uid, alias);
+ let _ = self.get_legacy(uid, alias);
let mut db = self.open_db().context("In put.")?;
- db.put(calling_uid, alias, profile).context("In put: Trying to insert profile into DB.")
+ db.put(uid, alias, entry).context("In put: Trying to insert entry into DB.")
}
- fn remove(&self, alias: &str) -> Result<()> {
- let calling_uid = ThreadState::get_calling_uid();
+ fn remove(&self, alias: &str, uid: i32) -> Result<()> {
+ let uid = Self::get_effective_uid(uid).context("In remove.")?;
let mut db = self.open_db().context("In remove.")?;
- // In order to make sure that we don't have stale legacy profiles, make sure they are
+ // In order to make sure that we don't have stale legacy entries, make sure they are
// migrated before removing them.
- let _ = self.get_legacy(calling_uid, alias);
- let removed = db
- .remove(calling_uid, alias)
- .context("In remove: Trying to remove profile from DB.")?;
+ let _ = self.get_legacy(uid, alias);
+ let removed =
+ db.remove(uid, alias).context("In remove: Trying to remove entry from DB.")?;
if removed {
Ok(())
} else {
- Err(Error::not_found()).context("In remove: No such profile.")
+ Err(Error::not_found()).context("In remove: No such entry.")
}
}
- fn list(&self, prefix: &str) -> Result<Vec<String>> {
+ fn list(&self, prefix: &str, uid: i32) -> Result<Vec<String>> {
let mut db = self.open_db().context("In list.")?;
- let calling_uid = ThreadState::get_calling_uid();
- let mut result = self.list_legacy(calling_uid).context("In list.")?;
- result
- .append(&mut db.list(calling_uid).context("In list: Trying to get list of profiles.")?);
+ let uid = Self::get_effective_uid(uid).context("In list.")?;
+ let mut result = self.list_legacy(uid).context("In list.")?;
+ result.append(&mut db.list(uid).context("In list: Trying to get list of entries.")?);
result = result.into_iter().filter(|s| s.starts_with(prefix)).collect();
result.sort_unstable();
result.dedup();
@@ -291,7 +318,7 @@
let mut db_path = path.to_path_buf();
self.async_task.queue_hi(move |shelf| {
let legacy_loader = LegacyBlobLoader::new(&db_path);
- db_path.push("vpnprofilestore.sqlite");
+ db_path.push(Self::LEGACY_KEYSTORE_FILE_NAME);
shelf.put(AsyncState { legacy_loader, db_path, recently_imported: Default::default() });
})
@@ -313,8 +340,8 @@
self.do_serialized(move |state| {
state
.legacy_loader
- .list_vpn_profiles(uid)
- .context("Trying to list legacy vnp profiles.")
+ .list_legacy_keystore_entries(uid)
+ .context("Trying to list legacy keystore entries.")
})
.context("In list_legacy.")
}
@@ -327,8 +354,8 @@
}
let mut db = DB::new(&state.db_path).context("In open_db: Failed to open db.")?;
let migrated =
- Self::migrate_one_legacy_profile(uid, &alias, &state.legacy_loader, &mut db)
- .context("Trying to migrate legacy vpn profile.")?;
+ Self::migrate_one_legacy_entry(uid, &alias, &state.legacy_loader, &mut db)
+ .context("Trying to migrate legacy keystore entries.")?;
if migrated {
state.recently_imported.insert((uid, alias));
}
@@ -337,21 +364,21 @@
.context("In get_legacy.")
}
- fn migrate_one_legacy_profile(
+ fn migrate_one_legacy_entry(
uid: u32,
alias: &str,
legacy_loader: &LegacyBlobLoader,
db: &mut DB,
) -> Result<bool> {
let blob = legacy_loader
- .read_vpn_profile(uid, alias)
- .context("In migrate_one_legacy_profile: Trying to read legacy vpn profile.")?;
- if let Some(profile) = blob {
- db.put(uid, alias, &profile)
- .context("In migrate_one_legacy_profile: Trying to insert profile into DB.")?;
+ .read_legacy_keystore_entry(uid, alias)
+ .context("In migrate_one_legacy_entry: Trying to read legacy keystore entry.")?;
+ if let Some(entry) = blob {
+ db.put(uid, alias, &entry)
+ .context("In migrate_one_legacy_entry: Trying to insert entry into DB.")?;
legacy_loader
- .remove_vpn_profile(uid, alias)
- .context("In migrate_one_legacy_profile: Trying to delete legacy profile.")?;
+ .remove_legacy_keystore_entry(uid, alias)
+ .context("In migrate_one_legacy_entry: Trying to delete legacy keystore entry.")?;
Ok(true)
} else {
Ok(false)
@@ -359,24 +386,24 @@
}
}
-impl binder::Interface for VpnProfileStore {}
+impl binder::Interface for LegacyKeystore {}
-impl IVpnProfileStore for VpnProfileStore {
- fn get(&self, alias: &str) -> BinderResult<Vec<u8>> {
- let _wp = wd::watch_millis("IVpnProfileStore::get", 500);
- map_or_log_err(self.get(alias), Ok)
+impl ILegacyKeystore for LegacyKeystore {
+ fn get(&self, alias: &str, uid: i32) -> BinderResult<Vec<u8>> {
+ let _wp = wd::watch_millis("ILegacyKeystore::get", 500);
+ map_or_log_err(self.get(alias, uid), Ok)
}
- fn put(&self, alias: &str, profile: &[u8]) -> BinderResult<()> {
- let _wp = wd::watch_millis("IVpnProfileStore::put", 500);
- map_or_log_err(self.put(alias, profile), Ok)
+ fn put(&self, alias: &str, uid: i32, entry: &[u8]) -> BinderResult<()> {
+ let _wp = wd::watch_millis("ILegacyKeystore::put", 500);
+ map_or_log_err(self.put(alias, uid, entry), Ok)
}
- fn remove(&self, alias: &str) -> BinderResult<()> {
- let _wp = wd::watch_millis("IVpnProfileStore::remove", 500);
- map_or_log_err(self.remove(alias), Ok)
+ fn remove(&self, alias: &str, uid: i32) -> BinderResult<()> {
+ let _wp = wd::watch_millis("ILegacyKeystore::remove", 500);
+ map_or_log_err(self.remove(alias, uid), Ok)
}
- fn list(&self, prefix: &str) -> BinderResult<Vec<String>> {
- let _wp = wd::watch_millis("IVpnProfileStore::list", 500);
- map_or_log_err(self.list(prefix), Ok)
+ fn list(&self, prefix: &str, uid: i32) -> BinderResult<Vec<String>> {
+ let _wp = wd::watch_millis("ILegacyKeystore::list", 500);
+ map_or_log_err(self.list(prefix, uid), Ok)
}
}
@@ -396,12 +423,12 @@
static TEST_BLOB4: &[u8] = &[3, 2, 3, 4, 5, 6, 7, 8, 9, 0];
#[test]
- fn test_profile_db() {
- let test_dir = TempDir::new("profiledb_test_").expect("Failed to create temp dir.");
- let mut db =
- DB::new(&test_dir.build().push("vpnprofile.sqlite")).expect("Failed to open database.");
+ fn test_entry_db() {
+ let test_dir = TempDir::new("entrydb_test_").expect("Failed to create temp dir.");
+ let mut db = DB::new(&test_dir.build().push(LegacyKeystore::LEGACY_KEYSTORE_FILE_NAME))
+ .expect("Failed to open database.");
- // Insert three profiles for owner 2.
+ // Insert three entries for owner 2.
db.put(2, "test1", TEST_BLOB1).expect("Failed to insert test1.");
db.put(2, "test2", TEST_BLOB2).expect("Failed to insert test2.");
db.put(2, "test3", TEST_BLOB3).expect("Failed to insert test3.");
@@ -409,76 +436,59 @@
// Check list returns all inserted aliases.
assert_eq!(
vec!["test1".to_string(), "test2".to_string(), "test3".to_string(),],
- db.list(2).expect("Failed to list profiles.")
+ db.list(2).expect("Failed to list entries.")
);
- // There should be no profiles for owner 1.
- assert_eq!(Vec::<String>::new(), db.list(1).expect("Failed to list profiles."));
+ // There should be no entries for owner 1.
+ assert_eq!(Vec::<String>::new(), db.list(1).expect("Failed to list entries."));
// Check the content of the three entries.
- assert_eq!(
- Some(TEST_BLOB1),
- db.get(2, "test1").expect("Failed to get profile.").as_deref()
- );
- assert_eq!(
- Some(TEST_BLOB2),
- db.get(2, "test2").expect("Failed to get profile.").as_deref()
- );
- assert_eq!(
- Some(TEST_BLOB3),
- db.get(2, "test3").expect("Failed to get profile.").as_deref()
- );
+ assert_eq!(Some(TEST_BLOB1), db.get(2, "test1").expect("Failed to get entry.").as_deref());
+ assert_eq!(Some(TEST_BLOB2), db.get(2, "test2").expect("Failed to get entry.").as_deref());
+ assert_eq!(Some(TEST_BLOB3), db.get(2, "test3").expect("Failed to get entry.").as_deref());
// Remove test2 and check and check that it is no longer retrievable.
- assert!(db.remove(2, "test2").expect("Failed to remove profile."));
- assert!(db.get(2, "test2").expect("Failed to get profile.").is_none());
+ assert!(db.remove(2, "test2").expect("Failed to remove entry."));
+ assert!(db.get(2, "test2").expect("Failed to get entry.").is_none());
// test2 should now no longer be in the list.
assert_eq!(
vec!["test1".to_string(), "test3".to_string(),],
- db.list(2).expect("Failed to list profiles.")
+ db.list(2).expect("Failed to list entries.")
);
// Put on existing alias replaces it.
// Verify test1 is TEST_BLOB1.
- assert_eq!(
- Some(TEST_BLOB1),
- db.get(2, "test1").expect("Failed to get profile.").as_deref()
- );
+ assert_eq!(Some(TEST_BLOB1), db.get(2, "test1").expect("Failed to get entry.").as_deref());
db.put(2, "test1", TEST_BLOB4).expect("Failed to replace test1.");
// Verify test1 is TEST_BLOB4.
- assert_eq!(
- Some(TEST_BLOB4),
- db.get(2, "test1").expect("Failed to get profile.").as_deref()
- );
+ assert_eq!(Some(TEST_BLOB4), db.get(2, "test1").expect("Failed to get entry.").as_deref());
}
#[test]
- fn concurrent_vpn_profile_test() -> Result<()> {
+ fn concurrent_legacy_keystore_entry_test() -> Result<()> {
let temp_dir = Arc::new(
- TempDir::new("concurrent_vpn_profile_test_").expect("Failed to create temp dir."),
+ TempDir::new("concurrent_legacy_keystore_entry_test_")
+ .expect("Failed to create temp dir."),
);
- let db_path = temp_dir.build().push("vpnprofile.sqlite").to_owned();
+ let db_path = temp_dir.build().push(LegacyKeystore::LEGACY_KEYSTORE_FILE_NAME).to_owned();
let test_begin = Instant::now();
let mut db = DB::new(&db_path).expect("Failed to open database.");
- const PROFILE_COUNT: u32 = 5000u32;
- const PROFILE_DB_COUNT: u32 = 5000u32;
+ const ENTRY_COUNT: u32 = 5000u32;
+ const ENTRY_DB_COUNT: u32 = 5000u32;
- let mode: String = db.conn.pragma_query_value(None, "journal_mode", |row| row.get(0))?;
- assert_eq!(mode, "wal");
-
- let mut actual_profile_count = PROFILE_COUNT;
- // First insert PROFILE_COUNT profiles.
- for count in 0..PROFILE_COUNT {
+ let mut actual_entry_count = ENTRY_COUNT;
+ // First insert ENTRY_COUNT entries.
+ for count in 0..ENTRY_COUNT {
if Instant::now().duration_since(test_begin) >= Duration::from_secs(15) {
- actual_profile_count = count;
+ actual_entry_count = count;
break;
}
let alias = format!("test_alias_{}", count);
- db.put(1, &alias, TEST_BLOB1).expect("Failed to add profile (1).");
+ db.put(1, &alias, TEST_BLOB1).expect("Failed to add entry (1).");
}
// Insert more keys from a different thread and into a different namespace.
@@ -486,16 +496,16 @@
let handle1 = thread::spawn(move || {
let mut db = DB::new(&db_path1).expect("Failed to open database.");
- for count in 0..actual_profile_count {
+ for count in 0..actual_entry_count {
if Instant::now().duration_since(test_begin) >= Duration::from_secs(40) {
return;
}
let alias = format!("test_alias_{}", count);
- db.put(2, &alias, TEST_BLOB2).expect("Failed to add profile (2).");
+ db.put(2, &alias, TEST_BLOB2).expect("Failed to add entry (2).");
}
// Then delete them again.
- for count in 0..actual_profile_count {
+ for count in 0..actual_entry_count {
if Instant::now().duration_since(test_begin) >= Duration::from_secs(40) {
return;
}
@@ -504,12 +514,12 @@
}
});
- // And start deleting the first set of profiles.
+ // And start deleting the first set of entries.
let db_path2 = db_path.clone();
let handle2 = thread::spawn(move || {
let mut db = DB::new(&db_path2).expect("Failed to open database.");
- for count in 0..actual_profile_count {
+ for count in 0..actual_entry_count {
if Instant::now().duration_since(test_begin) >= Duration::from_secs(40) {
return;
}
@@ -519,16 +529,16 @@
});
// While a lot of inserting and deleting is going on we have to open database connections
- // successfully and then insert and delete a specific profile.
+ // successfully and then insert and delete a specific entry.
let db_path3 = db_path.clone();
let handle3 = thread::spawn(move || {
- for _count in 0..PROFILE_DB_COUNT {
+ for _count in 0..ENTRY_DB_COUNT {
if Instant::now().duration_since(test_begin) >= Duration::from_secs(40) {
return;
}
let mut db = DB::new(&db_path3).expect("Failed to open database.");
- db.put(3, &TEST_ALIAS, TEST_BLOB3).expect("Failed to add profile (3).");
+ db.put(3, &TEST_ALIAS, TEST_BLOB3).expect("Failed to add entry (3).");
db.remove(3, &TEST_ALIAS).expect("Remove failed (3).");
}
@@ -537,14 +547,14 @@
// While thread 3 is inserting and deleting TEST_ALIAS, we try to get the alias.
// This may yield an entry or none, but it must not fail.
let handle4 = thread::spawn(move || {
- for _count in 0..PROFILE_DB_COUNT {
+ for _count in 0..ENTRY_DB_COUNT {
if Instant::now().duration_since(test_begin) >= Duration::from_secs(40) {
return;
}
let mut db = DB::new(&db_path).expect("Failed to open database.");
// This may return Some or None but it must not fail.
- db.get(3, &TEST_ALIAS).expect("Failed to get profile (4).");
+ db.get(3, &TEST_ALIAS).expect("Failed to get entry (4).");
}
});
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/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 84ed4e2..9535ecb 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -22,9 +22,9 @@
use keystore2::service::KeystoreService;
use keystore2::{apc::ApcManager, shared_secret_negotiation};
use keystore2::{authorization::AuthorizationManager, id_rotation::IdRotationState};
+use legacykeystore::LegacyKeystore;
use log::{error, info};
use std::{panic, path::Path, sync::mpsc::channel};
-use vpnprofilestore::VpnProfileStore;
static KS2_SERVICE_NAME: &str = "android.system.keystore2.IKeystoreService/default";
static APC_SERVICE_NAME: &str = "android.security.apc";
@@ -32,7 +32,7 @@
static METRICS_SERVICE_NAME: &str = "android.security.metrics";
static REMOTE_PROVISIONING_SERVICE_NAME: &str = "android.security.remoteprovisioning";
static USER_MANAGER_SERVICE_NAME: &str = "android.security.maintenance";
-static VPNPROFILESTORE_SERVICE_NAME: &str = "android.security.vpnprofilestore";
+static LEGACY_KEYSTORE_SERVICE_NAME: &str = "android.security.legacykeystore";
/// Keystore 2.0 takes one argument which is a path indicating its designated working directory.
fn main() {
@@ -128,14 +128,14 @@
});
}
- let vpnprofilestore = VpnProfileStore::new_native_binder(
+ let legacykeystore = LegacyKeystore::new_native_binder(
&keystore2::globals::DB_PATH.read().expect("Could not get DB_PATH."),
);
- binder::add_service(VPNPROFILESTORE_SERVICE_NAME, vpnprofilestore.as_binder()).unwrap_or_else(
+ binder::add_service(LEGACY_KEYSTORE_SERVICE_NAME, legacykeystore.as_binder()).unwrap_or_else(
|e| {
panic!(
"Failed to register service {} because of {:?}.",
- VPNPROFILESTORE_SERVICE_NAME, e
+ LEGACY_KEYSTORE_SERVICE_NAME, e
);
},
);
diff --git a/keystore2/src/km_compat/km_compat.cpp b/keystore2/src/km_compat/km_compat.cpp
index f6f8bfe..64849c1 100644
--- a/keystore2/src/km_compat/km_compat.cpp
+++ b/keystore2/src/km_compat/km_compat.cpp
@@ -304,33 +304,36 @@
static std::vector<KeyCharacteristics>
processLegacyCharacteristics(KeyMintSecurityLevel securityLevel,
const std::vector<KeyParameter>& genParams,
- const V4_0_KeyCharacteristics& legacyKc, bool hwEnforcedOnly = false) {
+ const V4_0_KeyCharacteristics& legacyKc, bool kmEnforcedOnly = false) {
- KeyCharacteristics hwEnforced{securityLevel,
- convertKeyParametersFromLegacy(legacyKc.hardwareEnforced)};
+ KeyCharacteristics kmEnforced{securityLevel, convertKeyParametersFromLegacy(
+ securityLevel == KeyMintSecurityLevel::SOFTWARE
+ ? legacyKc.softwareEnforced
+ : legacyKc.hardwareEnforced)};
- if (hwEnforcedOnly) {
- return {hwEnforced};
+ if (securityLevel == KeyMintSecurityLevel::SOFTWARE && legacyKc.hardwareEnforced.size() > 0) {
+ LOG(WARNING) << "Unexpected hardware enforced parameters.";
}
- KeyCharacteristics keystoreEnforced{KeyMintSecurityLevel::KEYSTORE,
- convertKeyParametersFromLegacy(legacyKc.softwareEnforced)};
+ if (kmEnforcedOnly) {
+ return {kmEnforced};
+ }
+
+ KeyCharacteristics keystoreEnforced{KeyMintSecurityLevel::KEYSTORE, {}};
+
+ if (securityLevel != KeyMintSecurityLevel::SOFTWARE) {
+ // Don't include these tags on software backends, else they'd end up duplicated
+ // across both the keystore-enforced and software keymaster-enforced tags.
+ keystoreEnforced.authorizations = convertKeyParametersFromLegacy(legacyKc.softwareEnforced);
+ }
// Add all parameters that we know can be enforced by keystore but not by the legacy backend.
auto unsupported_requested = extractNewAndKeystoreEnforceableParams(genParams);
- std::copy(unsupported_requested.begin(), unsupported_requested.end(),
- std::back_insert_iterator(keystoreEnforced.authorizations));
+ keystoreEnforced.authorizations.insert(keystoreEnforced.authorizations.end(),
+ std::begin(unsupported_requested),
+ std::end(unsupported_requested));
- if (securityLevel == KeyMintSecurityLevel::SOFTWARE) {
- // If the security level of the backend is `software` we expect the hardware enforced list
- // to be empty. Log a warning otherwise.
- if (legacyKc.hardwareEnforced.size() != 0) {
- LOG(WARNING) << "Unexpected hardware enforced parameters.";
- }
- return {keystoreEnforced};
- }
-
- return {hwEnforced, keystoreEnforced};
+ return {kmEnforced, keystoreEnforced};
}
static V4_0_KeyFormat convertKeyFormatToLegacy(const KeyFormat& kf) {
@@ -722,7 +725,7 @@
km_error = convert(errorCode);
*keyCharacteristics =
processLegacyCharacteristics(securityLevel_, {} /* getParams */,
- v40KeyCharacteristics, true /* hwEnforcedOnly */);
+ v40KeyCharacteristics, true /* kmEnforcedOnly */);
});
if (!ret.isOk()) {
diff --git a/keystore2/src/legacy_blob.rs b/keystore2/src/legacy_blob.rs
index 9eebb36..e0d2133 100644
--- a/keystore2/src/legacy_blob.rs
+++ b/keystore2/src/legacy_blob.rs
@@ -599,6 +599,15 @@
// * USRCERT was used for public certificates of USRPKEY entries. But KeyChain also
// used this for user installed certificates without private key material.
+ const KNOWN_KEYSTORE_PREFIXES: &'static [&'static str] =
+ &["USRPKEY_", "USRSKEY_", "USRCERT_", "CACERT_"];
+
+ fn is_keystore_alias(encoded_alias: &str) -> bool {
+ // We can check the encoded alias because the prefixes we are interested
+ // in are all in the printable range that don't get mangled.
+ Self::KNOWN_KEYSTORE_PREFIXES.iter().any(|prefix| encoded_alias.starts_with(prefix))
+ }
+
fn read_km_blob_file(&self, uid: u32, alias: &str) -> Result<Option<(Blob, String)>> {
let mut iter = ["USRPKEY", "USRSKEY"].iter();
@@ -630,28 +639,28 @@
Ok(Some(Self::new_from_stream(&mut file).context("In read_generic_blob.")?))
}
- /// Read a legacy vpn profile blob.
- pub fn read_vpn_profile(&self, uid: u32, alias: &str) -> Result<Option<Vec<u8>>> {
- let path = match self.make_vpn_profile_filename(uid, alias) {
+ /// Read a legacy keystore entry blob.
+ pub fn read_legacy_keystore_entry(&self, uid: u32, alias: &str) -> Result<Option<Vec<u8>>> {
+ let path = match self.make_legacy_keystore_entry_filename(uid, alias) {
Some(path) => path,
None => return Ok(None),
};
- let blob =
- Self::read_generic_blob(&path).context("In read_vpn_profile: Failed to read blob.")?;
+ let blob = Self::read_generic_blob(&path)
+ .context("In read_legacy_keystore_entry: Failed to read blob.")?;
Ok(blob.and_then(|blob| match blob.value {
BlobValue::Generic(blob) => Some(blob),
_ => {
- log::info!("Unexpected vpn profile blob type. Ignoring");
+ log::info!("Unexpected legacy keystore entry blob type. Ignoring");
None
}
}))
}
- /// Remove a vpn profile by the name alias with owner uid.
- pub fn remove_vpn_profile(&self, uid: u32, alias: &str) -> Result<()> {
- let path = match self.make_vpn_profile_filename(uid, alias) {
+ /// Remove a legacy keystore entry by the name alias with owner uid.
+ pub fn remove_legacy_keystore_entry(&self, uid: u32, alias: &str) -> Result<()> {
+ let path = match self.make_legacy_keystore_entry_filename(uid, alias) {
Some(path) => path,
None => return Ok(()),
};
@@ -659,25 +668,17 @@
if let Err(e) = Self::with_retry_interrupted(|| fs::remove_file(path.as_path())) {
match e.kind() {
ErrorKind::NotFound => return Ok(()),
- _ => return Err(e).context("In remove_vpn_profile."),
+ _ => return Err(e).context("In remove_legacy_keystore_entry."),
}
}
let user_id = uid_to_android_user(uid);
self.remove_user_dir_if_empty(user_id)
- .context("In remove_vpn_profile: Trying to remove empty user dir.")
+ .context("In remove_legacy_keystore_entry: Trying to remove empty user dir.")
}
- fn is_vpn_profile(encoded_alias: &str) -> bool {
- // We can check the encoded alias because the prefixes we are interested
- // in are all in the printable range that don't get mangled.
- encoded_alias.starts_with("VPN_")
- || encoded_alias.starts_with("PLATFORM_VPN_")
- || encoded_alias == "LOCKDOWN_VPN"
- }
-
- /// List all profiles belonging to the given uid.
- pub fn list_vpn_profiles(&self, uid: u32) -> Result<Vec<String>> {
+ /// List all entries belonging to the given uid.
+ pub fn list_legacy_keystore_entries(&self, uid: u32) -> Result<Vec<String>> {
let mut path = self.path.clone();
let user_id = uid_to_android_user(uid);
path.push(format!("user_{}", user_id));
@@ -688,7 +689,10 @@
ErrorKind::NotFound => return Ok(Default::default()),
_ => {
return Err(e).context(format!(
- "In list_vpn_profiles: Failed to open legacy blob database. {:?}",
+ concat!(
+ "In list_legacy_keystore_entries: ,",
+ "Failed to open legacy blob database: {:?}"
+ ),
path
))
}
@@ -696,14 +700,15 @@
};
let mut result: Vec<String> = Vec::new();
for entry in dir {
- let file_name =
- entry.context("In list_vpn_profiles: Trying to access dir entry")?.file_name();
+ let file_name = entry
+ .context("In list_legacy_keystore_entries: Trying to access dir entry")?
+ .file_name();
if let Some(f) = file_name.to_str() {
let encoded_alias = &f[uid_str.len() + 1..];
- if f.starts_with(&uid_str) && Self::is_vpn_profile(encoded_alias) {
+ if f.starts_with(&uid_str) && !Self::is_keystore_alias(encoded_alias) {
result.push(
Self::decode_alias(encoded_alias)
- .context("In list_vpn_profiles: Trying to decode alias.")?,
+ .context("In list_legacy_keystore_entries: Trying to decode alias.")?,
)
}
}
@@ -711,12 +716,15 @@
Ok(result)
}
- /// This function constructs the vpn_profile file name which has the form:
- /// user_<android user id>/<uid>_<alias>.
- fn make_vpn_profile_filename(&self, uid: u32, alias: &str) -> Option<PathBuf> {
- // legacy vpn entries must start with VPN_ or PLATFORM_VPN_ or are literally called
- // LOCKDOWN_VPN.
- if !Self::is_vpn_profile(alias) {
+ /// This function constructs the legacy blob file name which has the form:
+ /// user_<android user id>/<uid>_<alias>. Legacy blob file names must not use
+ /// known keystore prefixes.
+ fn make_legacy_keystore_entry_filename(&self, uid: u32, alias: &str) -> Option<PathBuf> {
+ // Legacy entries must not use known keystore prefixes.
+ if Self::is_keystore_alias(alias) {
+ log::warn!(
+ "Known keystore prefixes cannot be used with legacy keystore -> ignoring request."
+ );
return None;
}
@@ -1376,11 +1384,11 @@
}
#[test]
- fn list_vpn_profiles_on_non_existing_user() -> Result<()> {
- let temp_dir = TempDir::new("list_vpn_profiles_on_non_existing_user")?;
+ fn list_legacy_keystore_entries_on_non_existing_user() -> Result<()> {
+ let temp_dir = TempDir::new("list_legacy_keystore_entries_on_non_existing_user")?;
let legacy_blob_loader = LegacyBlobLoader::new(temp_dir.path());
- assert!(legacy_blob_loader.list_vpn_profiles(20)?.is_empty());
+ assert!(legacy_blob_loader.list_legacy_keystore_entries(20)?.is_empty());
Ok(())
}
diff --git a/keystore2/src/remote_provisioning.rs b/keystore2/src/remote_provisioning.rs
index aa0d218..6666f41 100644
--- a/keystore2/src/remote_provisioning.rs
+++ b/keystore2/src/remote_provisioning.rs
@@ -282,9 +282,17 @@
(mac.len() as u8),
];
cose_mac_0.append(&mut mac);
+ // If this is a test mode key, there is an extra 6 bytes added as an additional entry in
+ // the COSE_Key struct to denote that.
+ let test_mode_entry_shift = if test_mode { 0 } else { 6 };
+ let byte_dist_mac0_payload = 8;
+ let cose_key_size = 83 - test_mode_entry_shift;
for maced_public_key in keys_to_sign {
- if maced_public_key.macedKey.len() > 83 + 8 {
- cose_mac_0.extend_from_slice(&maced_public_key.macedKey[8..83 + 8]);
+ if maced_public_key.macedKey.len() > cose_key_size + byte_dist_mac0_payload {
+ cose_mac_0.extend_from_slice(
+ &maced_public_key.macedKey
+ [byte_dist_mac0_payload..cose_key_size + byte_dist_mac0_payload],
+ );
}
}
Ok(cose_mac_0)
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()),
)
diff --git a/ondevice-signing/VerityUtils.cpp b/ondevice-signing/VerityUtils.cpp
index cab92e2..3d5243a 100644
--- a/ondevice-signing/VerityUtils.cpp
+++ b/ondevice-signing/VerityUtils.cpp
@@ -227,13 +227,19 @@
while (!ec && it != end) {
if (it->is_regular_file()) {
- // Verify
+ // Verify the file is in fs-verity
auto result = isFileInVerity(it->path());
if (!result.ok()) {
return result.error();
}
digests[it->path()] = *result;
- } // TODO reject other types besides dirs?
+ } else if (it->is_directory()) {
+ // These are fine to ignore
+ } else if (it->is_symlink()) {
+ return Error() << "Rejecting artifacts, symlink at " << it->path();
+ } else {
+ return Error() << "Rejecting artifacts, unexpected file type for " << it->path();
+ }
++it;
}
if (ec) {
diff --git a/provisioner/Android.bp b/provisioner/Android.bp
index 12a21d1..86f0f12 100644
--- a/provisioner/Android.bp
+++ b/provisioner/Android.bp
@@ -43,15 +43,6 @@
},
}
-java_binary {
- name: "provisioner_cli",
- wrapper: "provisioner_cli",
- srcs: ["src/com/android/commands/provisioner/**/*.java"],
- static_libs: [
- "android.security.provisioner-java",
- ],
-}
-
cc_binary {
name: "rkp_factory_extraction_tool",
srcs: ["rkp_factory_extraction_tool.cpp"],
@@ -62,8 +53,9 @@
"libcppbor_external",
"libcppcose_rkp",
"libcrypto",
+ "libgflags",
"liblog",
+ "libkeymint_remote_prov_support",
"libvintf",
],
- //export_include_dirs: ["include"],
}
diff --git a/provisioner/provisioner_cli b/provisioner/provisioner_cli
deleted file mode 100755
index 7b53d6e..0000000
--- a/provisioner/provisioner_cli
+++ /dev/null
@@ -1,21 +0,0 @@
-#!/system/bin/sh
-#
-# Copyright (C) 2020 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.
-#
-# Script to start "provisioner_cli" on the device.
-#
-base=/system
-export CLASSPATH=$base/framework/provisioner_cli.jar
-exec app_process $base/bin com.android.commands.provisioner.Cli "$@"
diff --git a/provisioner/rkp_factory_extraction_tool.cpp b/provisioner/rkp_factory_extraction_tool.cpp
index d4842b1..bf6b9a6 100644
--- a/provisioner/rkp_factory_extraction_tool.cpp
+++ b/provisioner/rkp_factory_extraction_tool.cpp
@@ -20,8 +20,11 @@
#include <aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.h>
#include <android/binder_manager.h>
#include <cppbor.h>
+#include <gflags/gflags.h>
#include <keymaster/cppcose/cppcose.h>
#include <log/log.h>
+#include <remote_prov/remote_prov_utils.h>
+#include <sys/random.h>
#include <vintf/VintfObject.h>
using std::set;
@@ -32,6 +35,9 @@
using aidl::android::hardware::security::keymint::IRemotelyProvisionedComponent;
using aidl::android::hardware::security::keymint::MacedPublicKey;
using aidl::android::hardware::security::keymint::ProtectedData;
+using aidl::android::hardware::security::keymint::remote_prov::generateEekChain;
+using aidl::android::hardware::security::keymint::remote_prov::getProdEekChain;
+using aidl::android::hardware::security::keymint::remote_prov::jsonEncodeCsrWithBuild;
using android::vintf::HalManifest;
using android::vintf::VintfObject;
@@ -39,66 +45,42 @@
using namespace cppbor;
using namespace cppcose;
+DEFINE_bool(test_mode, false, "If enabled, a fake EEK key/cert are used.");
+
+DEFINE_string(output_format, "csr", "How to format the output. Defaults to 'csr'.");
+
namespace {
const string kPackage = "android.hardware.security.keymint";
const string kInterface = "IRemotelyProvisionedComponent";
const string kFormattedName = kPackage + "." + kInterface + "/";
-ErrMsgOr<vector<uint8_t>> generateEekChain(size_t length, const vector<uint8_t>& eekId) {
- auto eekChain = cppbor::Array();
+// Various supported --output_format values.
+constexpr std::string_view kBinaryCsrOutput = "csr"; // Just the raw csr as binary
+constexpr std::string_view kBuildPlusCsr = "build+csr"; // Text-encoded (JSON) build
+ // fingerprint plus CSR.
- vector<uint8_t> prevPrivKey;
- for (size_t i = 0; i < length - 1; ++i) {
- vector<uint8_t> pubKey(ED25519_PUBLIC_KEY_LEN);
- vector<uint8_t> privKey(ED25519_PRIVATE_KEY_LEN);
+constexpr size_t kChallengeSize = 16;
- ED25519_keypair(pubKey.data(), privKey.data());
+std::vector<uint8_t> generateChallenge() {
+ std::vector<uint8_t> challenge(kChallengeSize);
- // The first signing key is self-signed.
- if (prevPrivKey.empty()) prevPrivKey = privKey;
-
- auto coseSign1 = constructCoseSign1(prevPrivKey,
- cppbor::Map() /* payload CoseKey */
- .add(CoseKey::KEY_TYPE, OCTET_KEY_PAIR)
- .add(CoseKey::ALGORITHM, EDDSA)
- .add(CoseKey::CURVE, ED25519)
- .add(CoseKey::PUBKEY_X, pubKey)
- .canonicalize()
- .encode(),
- {} /* AAD */);
- if (!coseSign1) return coseSign1.moveMessage();
- eekChain.add(coseSign1.moveValue());
-
- prevPrivKey = privKey;
+ ssize_t bytesRemaining = static_cast<ssize_t>(challenge.size());
+ uint8_t* writePtr = challenge.data();
+ while (bytesRemaining > 0) {
+ int bytesRead = getrandom(writePtr, bytesRemaining, /*flags=*/0);
+ if (bytesRead < 0) {
+ LOG_FATAL_IF(errno != EINTR, "%d - %s", errno, strerror(errno));
+ }
+ bytesRemaining -= bytesRead;
+ writePtr += bytesRead;
}
- vector<uint8_t> pubKey(X25519_PUBLIC_VALUE_LEN);
- vector<uint8_t> privKey(X25519_PRIVATE_KEY_LEN);
- X25519_keypair(pubKey.data(), privKey.data());
-
- auto coseSign1 = constructCoseSign1(prevPrivKey,
- cppbor::Map() /* payload CoseKey */
- .add(CoseKey::KEY_TYPE, OCTET_KEY_PAIR)
- .add(CoseKey::KEY_ID, eekId)
- .add(CoseKey::ALGORITHM, ECDH_ES_HKDF_256)
- .add(CoseKey::CURVE, cppcose::X25519)
- .add(CoseKey::PUBKEY_X, pubKey)
- .canonicalize()
- .encode(),
- {} /* AAD */);
- if (!coseSign1) return coseSign1.moveMessage();
- eekChain.add(coseSign1.moveValue());
-
- return eekChain.encode();
+ return challenge;
}
-std::vector<uint8_t> getChallenge() {
- return std::vector<uint8_t>(0);
-}
-
-std::vector<uint8_t> composeCertificateRequest(ProtectedData&& protectedData,
- DeviceInfo&& deviceInfo) {
+Array composeCertificateRequest(ProtectedData&& protectedData, DeviceInfo&& deviceInfo,
+ const std::vector<uint8_t>& challenge) {
Array emptyMacedKeysToSign;
emptyMacedKeysToSign
.add(std::vector<uint8_t>(0)) // empty protected headers as bstr
@@ -107,10 +89,10 @@
.add(std::vector<uint8_t>(0)); // empty tag as bstr
Array certificateRequest;
certificateRequest.add(EncodedItem(std::move(deviceInfo.deviceInfo)))
- .add(getChallenge()) // fake challenge
+ .add(challenge)
.add(EncodedItem(std::move(protectedData.protectedData)))
.add(std::move(emptyMacedKeysToSign));
- return certificateRequest.encode();
+ return certificateRequest;
}
int32_t errorMsg(string name) {
@@ -118,9 +100,47 @@
return -1;
}
+std::vector<uint8_t> getEekChain() {
+ if (FLAGS_test_mode) {
+ const std::vector<uint8_t> kFakeEekId = {'f', 'a', 'k', 'e', 0};
+ auto eekOrErr = generateEekChain(3 /* chainlength */, kFakeEekId);
+ LOG_FATAL_IF(!eekOrErr, "Failed to generate test EEK somehow: %s",
+ eekOrErr.message().c_str());
+ auto [eek, ignored_pubkey, ignored_privkey] = eekOrErr.moveValue();
+ return eek;
+ }
+
+ return getProdEekChain();
+}
+
+void writeOutput(const Array& csr) {
+ if (FLAGS_output_format == kBinaryCsrOutput) {
+ auto bytes = csr.encode();
+ std::copy(bytes.begin(), bytes.end(), std::ostream_iterator<char>(std::cout));
+ } else if (FLAGS_output_format == kBuildPlusCsr) {
+ auto [json, error] = jsonEncodeCsrWithBuild(csr);
+ if (!error.empty()) {
+ std::cerr << "Error JSON encoding the output: " << error;
+ exit(1);
+ }
+ std::cout << json << std::endl;
+ } else {
+ std::cerr << "Unexpected output_format '" << FLAGS_output_format << "'" << std::endl;
+ std::cerr << "Valid formats:" << std::endl;
+ std::cerr << " " << kBinaryCsrOutput << std::endl;
+ std::cerr << " " << kBuildPlusCsr << std::endl;
+ exit(1);
+ }
+}
+
} // namespace
-int main() {
+int main(int argc, char** argv) {
+ gflags::ParseCommandLineFlags(&argc, &argv, /*remove_flags=*/true);
+
+ const std::vector<uint8_t> eek_chain = getEekChain();
+ const std::vector<uint8_t> challenge = generateChallenge();
+
std::shared_ptr<const HalManifest> manifest = VintfObject::GetDeviceHalManifest();
set<string> rkpNames = manifest->getAidlInstances(kPackage, kInterface);
for (auto name : rkpNames) {
@@ -136,31 +156,19 @@
std::vector<uint8_t> keysToSignMac;
std::vector<MacedPublicKey> emptyKeys;
- // Replace this eek chain generation with the actual production GEEK
- std::vector<uint8_t> eekId(10); // replace with real KID later (EEK fingerprint)
- auto eekOrErr = generateEekChain(3 /* chainlength */, eekId);
- if (!eekOrErr) {
- ALOGE("Failed to generate test EEK somehow: %s", eekOrErr.message().c_str());
- return errorMsg(name);
- }
-
- std::vector<uint8_t> eek = eekOrErr.moveValue();
DeviceInfo deviceInfo;
ProtectedData protectedData;
if (rkp_service) {
ALOGE("extracting bundle");
::ndk::ScopedAStatus status = rkp_service->generateCertificateRequest(
- true /* testMode */, emptyKeys, eek, getChallenge(), &deviceInfo, &protectedData,
+ FLAGS_test_mode, emptyKeys, eek_chain, challenge, &deviceInfo, &protectedData,
&keysToSignMac);
if (!status.isOk()) {
ALOGE("Bundle extraction failed. Error code: %d", status.getServiceSpecificError());
return errorMsg(name);
}
- std::cout << "\n";
- std::vector<uint8_t> certificateRequest =
- composeCertificateRequest(std::move(protectedData), std::move(deviceInfo));
- std::copy(certificateRequest.begin(), certificateRequest.end(),
- std::ostream_iterator<char>(std::cout));
+ writeOutput(composeCertificateRequest(std::move(protectedData), std::move(deviceInfo),
+ challenge));
}
}
}
diff --git a/provisioner/src/com/android/commands/provisioner/Cli.java b/provisioner/src/com/android/commands/provisioner/Cli.java
deleted file mode 100644
index 62afdac..0000000
--- a/provisioner/src/com/android/commands/provisioner/Cli.java
+++ /dev/null
@@ -1,141 +0,0 @@
-/*
- * Copyright 2020 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.
- */
-
-package com.android.commands.provisioner;
-
-import android.os.IBinder;
-import android.os.RemoteException;
-import android.os.ServiceManager;
-import android.security.provisioner.IProvisionerService;
-
-import com.android.internal.os.BaseCommand;
-
-import java.io.ByteArrayOutputStream;
-import java.io.InputStream;
-import java.io.IOException;
-import java.io.PrintStream;
-import java.lang.IllegalArgumentException;
-
-/**
- * Contains the implementation of the remote provisioning command-line interface.
- */
-public class Cli extends BaseCommand {
- /**
- * Creates an instance of the command-line interface and runs it. This is the entry point of
- * the tool.
- */
- public static void main(String[] args) {
- new Cli().run(args);
- }
-
- /**
- * Runs the command requested by the invoker. It parses the very first required argument, which
- * is the command, and calls the appropriate handler.
- */
- @Override
- public void onRun() throws Exception {
- String cmd = nextArgRequired();
- switch (cmd) {
- case "get-req":
- getRequest();
- break;
-
- case "help":
- onShowUsage(System.out);
- break;
-
- default:
- throw new IllegalArgumentException("unknown command: " + cmd);
- }
- }
-
- /**
- * Retrieves a 'certificate request' from the provisioning service. The COSE-encoded
- * 'certificate chain' describing the endpoint encryption key (EEK) to use for encryption is
- * read from the standard input. The retrieved request is written to the standard output.
- */
- private void getRequest() throws Exception {
- // Process options.
- boolean test = false;
- byte[] challenge = null;
- int count = 0;
- String arg;
- while ((arg = nextArg()) != null) {
- switch (arg) {
- case "--test":
- test = true;
- break;
-
- case "--challenge":
- // TODO: We may need a different encoding of the challenge.
- challenge = nextArgRequired().getBytes();
- break;
-
- case "--count":
- count = Integer.parseInt(nextArgRequired());
- if (count < 0) {
- throw new IllegalArgumentException(
- "--count must be followed by non-negative number");
- }
- break;
-
- default:
- throw new IllegalArgumentException("unknown argument: " + arg);
- }
- }
-
- // Send the request over to the provisioning service and write the result to stdout.
- byte[] res = getService().getCertificateRequest(test, count, readAll(System.in), challenge);
- if (res != null) {
- System.out.write(res);
- }
- }
-
- /**
- * Retrieves an implementation of the IProvisionerService interface. It allows the caller to
- * call into the service via binder.
- */
- private static IProvisionerService getService() throws RemoteException {
- IBinder binder = ServiceManager.getService("remote-provisioner");
- if (binder == null) {
- throw new RemoteException("Provisioning service is inaccessible");
- }
- return IProvisionerService.Stub.asInterface(binder);
- }
-
- /** Reads all data from the provided input stream and returns it as a byte array. */
- private static byte[] readAll(InputStream in) throws IOException {
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- byte[] buf = new byte[1024];
- int read;
- while ((read = in.read(buf)) != -1) {
- out.write(buf, 0, read);
- }
- return out.toByteArray();
- }
-
- /**
- * Writes the usage information to the given stream. This is displayed to users of the tool when
- * they ask for help or when they pass incorrect arguments to the tool.
- */
- @Override
- public void onShowUsage(PrintStream out) {
- out.println(
- "Usage: provisioner_cli <command> [options]\n" +
- "Commands: help\n" +
- " get-req [--count <n>] [--test] [--challenge <v>]");
- }
-}