Merge "Adding tests to verify `EVP_PKEY_from_keystore2` API [Keystore2-engine]."
diff --git a/diced/open_dice/src/bcc.rs b/diced/open_dice/src/bcc.rs
index 1575113..f9c6a34 100644
--- a/diced/open_dice/src/bcc.rs
+++ b/diced/open_dice/src/bcc.rs
@@ -50,9 +50,12 @@
let mut buffer_size = 0;
// SAFETY: The function writes to the buffer, within the given bounds, and only reads the
// input values. It writes its result to buffer_size.
- check_result(unsafe {
- BccFormatConfigDescriptor(&values, buffer.len(), buffer.as_mut_ptr(), &mut buffer_size)
- })?;
+ check_result(
+ unsafe {
+ BccFormatConfigDescriptor(&values, buffer.len(), buffer.as_mut_ptr(), &mut buffer_size)
+ },
+ buffer_size,
+ )?;
Ok(buffer_size)
}
@@ -73,21 +76,24 @@
// to `next_bcc` and next CDI values within its bounds. It also reads
// `input_values` as a constant input and doesn't store any pointer.
// The first argument can be null and is not used in the current implementation.
- check_result(unsafe {
- BccMainFlow(
- ptr::null_mut(), // context
- current_cdi_attest.as_ptr(),
- current_cdi_seal.as_ptr(),
- current_bcc.as_ptr(),
- current_bcc.len(),
- input_values.as_ptr(),
- next_bcc.len(),
- next_bcc.as_mut_ptr(),
- &mut next_bcc_size,
- next_cdi_values.cdi_attest.as_mut_ptr(),
- next_cdi_values.cdi_seal.as_mut_ptr(),
- )
- })?;
+ check_result(
+ unsafe {
+ BccMainFlow(
+ ptr::null_mut(), // context
+ current_cdi_attest.as_ptr(),
+ current_cdi_seal.as_ptr(),
+ current_bcc.as_ptr(),
+ current_bcc.len(),
+ input_values.as_ptr(),
+ next_bcc.len(),
+ next_bcc.as_mut_ptr(),
+ &mut next_bcc_size,
+ next_cdi_values.cdi_attest.as_mut_ptr(),
+ next_cdi_values.cdi_seal.as_mut_ptr(),
+ )
+ },
+ next_bcc_size,
+ )?;
Ok(next_bcc_size)
}
@@ -106,17 +112,20 @@
// within its bounds,
// It also reads `input_values` as a constant input and doesn't store any pointer.
// The first argument can be null and is not used in the current implementation.
- check_result(unsafe {
- BccHandoverMainFlow(
- ptr::null_mut(), // context
- current_bcc_handover.as_ptr(),
- current_bcc_handover.len(),
- input_values.as_ptr(),
- next_bcc_handover.len(),
- next_bcc_handover.as_mut_ptr(),
- &mut next_bcc_handover_size,
- )
- })?;
+ check_result(
+ unsafe {
+ BccHandoverMainFlow(
+ ptr::null_mut(), // context
+ current_bcc_handover.as_ptr(),
+ current_bcc_handover.len(),
+ input_values.as_ptr(),
+ next_bcc_handover.len(),
+ next_bcc_handover.as_mut_ptr(),
+ &mut next_bcc_handover_size,
+ )
+ },
+ next_bcc_handover_size,
+ )?;
Ok(next_bcc_handover_size)
}
@@ -158,16 +167,19 @@
let mut bcc_size = 0;
// SAFETY: The `bcc_handover` is only read and never stored and the returned pointers should all
// point within the address range of the `bcc_handover` or be NULL.
- check_result(unsafe {
- BccHandoverParse(
- bcc_handover.as_ptr(),
- bcc_handover.len(),
- &mut cdi_attest,
- &mut cdi_seal,
- &mut bcc,
- &mut bcc_size,
- )
- })?;
+ check_result(
+ unsafe {
+ BccHandoverParse(
+ bcc_handover.as_ptr(),
+ bcc_handover.len(),
+ &mut cdi_attest,
+ &mut cdi_seal,
+ &mut bcc,
+ &mut bcc_size,
+ )
+ },
+ bcc_size,
+ )?;
let cdi_attest = sub_slice(bcc_handover, cdi_attest, CDI_SIZE)?;
let cdi_seal = sub_slice(bcc_handover, cdi_seal, CDI_SIZE)?;
let bcc = sub_slice(bcc_handover, bcc, bcc_size).ok();
diff --git a/diced/open_dice/src/dice.rs b/diced/open_dice/src/dice.rs
index 6e2df81..0704d21 100644
--- a/diced/open_dice/src/dice.rs
+++ b/diced/open_dice/src/dice.rs
@@ -219,13 +219,16 @@
let mut seed = PrivateKeySeed::default();
// SAFETY: The function writes to the buffer within the given bounds, and only reads the
// input values. The first argument context is not used in this function.
- check_result(unsafe {
- DiceDeriveCdiPrivateKeySeed(
- ptr::null_mut(), // context
- cdi_attest.as_ptr(),
- seed.as_mut_ptr(),
- )
- })?;
+ check_result(
+ unsafe {
+ DiceDeriveCdiPrivateKeySeed(
+ ptr::null_mut(), // context
+ cdi_attest.as_ptr(),
+ seed.as_mut_ptr(),
+ )
+ },
+ seed.0.len(),
+ )?;
Ok(seed)
}
@@ -234,14 +237,17 @@
let mut id = [0u8; ID_SIZE];
// SAFETY: The function writes to the buffer within the given bounds, and only reads the
// input values. The first argument context is not used in this function.
- check_result(unsafe {
- DiceDeriveCdiCertificateId(
- ptr::null_mut(), // context
- cdi_public_key.as_ptr(),
- cdi_public_key.len(),
- id.as_mut_ptr(),
- )
- })?;
+ check_result(
+ unsafe {
+ DiceDeriveCdiCertificateId(
+ ptr::null_mut(), // context
+ cdi_public_key.as_ptr(),
+ cdi_public_key.len(),
+ id.as_mut_ptr(),
+ )
+ },
+ id.len(),
+ )?;
Ok(id)
}
@@ -261,18 +267,21 @@
// SAFETY: The function only reads the current CDI values and inputs and writes
// to `next_cdi_certificate` and next CDI values within its bounds.
// The first argument can be null and is not used in the current implementation.
- check_result(unsafe {
- DiceMainFlow(
- ptr::null_mut(), // context
- current_cdi_attest.as_ptr(),
- current_cdi_seal.as_ptr(),
- input_values.as_ptr(),
- next_cdi_certificate.len(),
- next_cdi_certificate.as_mut_ptr(),
- &mut next_cdi_certificate_actual_size,
- next_cdi_values.cdi_attest.as_mut_ptr(),
- next_cdi_values.cdi_seal.as_mut_ptr(),
- )
- })?;
+ check_result(
+ unsafe {
+ DiceMainFlow(
+ ptr::null_mut(), // context
+ current_cdi_attest.as_ptr(),
+ current_cdi_seal.as_ptr(),
+ input_values.as_ptr(),
+ next_cdi_certificate.len(),
+ next_cdi_certificate.as_mut_ptr(),
+ &mut next_cdi_certificate_actual_size,
+ next_cdi_values.cdi_attest.as_mut_ptr(),
+ next_cdi_values.cdi_seal.as_mut_ptr(),
+ )
+ },
+ next_cdi_certificate_actual_size,
+ )?;
Ok(next_cdi_certificate_actual_size)
}
diff --git a/diced/open_dice/src/error.rs b/diced/open_dice/src/error.rs
index 4c67335..53ffd2d 100644
--- a/diced/open_dice/src/error.rs
+++ b/diced/open_dice/src/error.rs
@@ -26,7 +26,7 @@
/// Provided input was invalid.
InvalidInput,
/// Provided buffer was too small.
- BufferTooSmall,
+ BufferTooSmall(usize),
/// Platform error.
PlatformError,
}
@@ -39,7 +39,9 @@
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::InvalidInput => write!(f, "invalid input"),
- Self::BufferTooSmall => write!(f, "buffer too small"),
+ Self::BufferTooSmall(buffer_required_size) => {
+ write!(f, "buffer too small. Required {buffer_required_size} bytes.")
+ }
Self::PlatformError => write!(f, "platform error"),
}
}
@@ -49,11 +51,13 @@
pub type Result<T> = result::Result<T, DiceError>;
/// Checks the given `DiceResult`. Returns an error if it's not OK.
-pub fn check_result(result: DiceResult) -> Result<()> {
+pub(crate) fn check_result(result: DiceResult, buffer_required_size: usize) -> Result<()> {
match result {
DiceResult::kDiceResultOk => Ok(()),
DiceResult::kDiceResultInvalidInput => Err(DiceError::InvalidInput),
- DiceResult::kDiceResultBufferTooSmall => Err(DiceError::BufferTooSmall),
+ DiceResult::kDiceResultBufferTooSmall => {
+ Err(DiceError::BufferTooSmall(buffer_required_size))
+ }
DiceResult::kDiceResultPlatformError => Err(DiceError::PlatformError),
}
}
diff --git a/diced/open_dice/src/lib.rs b/diced/open_dice/src/lib.rs
index e7ec56b..4a85a1e 100644
--- a/diced/open_dice/src/lib.rs
+++ b/diced/open_dice/src/lib.rs
@@ -36,7 +36,7 @@
DiceArtifacts, DiceMode, Hash, Hidden, InlineConfig, InputValues, PrivateKey, PrivateKeySeed,
PublicKey, Signature, CDI_SIZE, HASH_SIZE, HIDDEN_SIZE, ID_SIZE, PRIVATE_KEY_SEED_SIZE,
};
-pub use error::{check_result, DiceError, Result};
+pub use error::{DiceError, Result};
pub use ops::{generate_certificate, hash, kdf, keypair_from_seed, sign, verify};
#[cfg(feature = "std")]
pub use retry::{
diff --git a/diced/open_dice/src/ops.rs b/diced/open_dice/src/ops.rs
index 8222b26..d978f86 100644
--- a/diced/open_dice/src/ops.rs
+++ b/diced/open_dice/src/ops.rs
@@ -31,14 +31,17 @@
let mut output: Hash = [0; HASH_SIZE];
// SAFETY: DiceHash takes a sized input buffer and writes to a constant-sized output buffer.
// The first argument context is not used in this function.
- check_result(unsafe {
- DiceHash(
- ptr::null_mut(), // context
- input.as_ptr(),
- input.len(),
- output.as_mut_ptr(),
- )
- })?;
+ check_result(
+ unsafe {
+ DiceHash(
+ ptr::null_mut(), // context
+ input.as_ptr(),
+ input.len(),
+ output.as_mut_ptr(),
+ )
+ },
+ output.len(),
+ )?;
Ok(output)
}
@@ -47,19 +50,22 @@
pub fn kdf(ikm: &[u8], salt: &[u8], info: &[u8], derived_key: &mut [u8]) -> Result<()> {
// SAFETY: The function writes to the `derived_key`, within the given bounds, and only reads the
// input values. The first argument context is not used in this function.
- check_result(unsafe {
- DiceKdf(
- ptr::null_mut(), // context
- derived_key.len(),
- ikm.as_ptr(),
- ikm.len(),
- salt.as_ptr(),
- salt.len(),
- info.as_ptr(),
- info.len(),
- derived_key.as_mut_ptr(),
- )
- })
+ check_result(
+ unsafe {
+ DiceKdf(
+ ptr::null_mut(), // context
+ derived_key.len(),
+ ikm.as_ptr(),
+ ikm.len(),
+ salt.as_ptr(),
+ salt.len(),
+ info.as_ptr(),
+ info.len(),
+ derived_key.as_mut_ptr(),
+ )
+ },
+ derived_key.len(),
+ )
}
/// Deterministically generates a public and private key pair from `seed`.
@@ -70,14 +76,17 @@
let mut private_key = PrivateKey::default();
// SAFETY: The function writes to the `public_key` and `private_key` within the given bounds,
// and only reads the `seed`. The first argument context is not used in this function.
- check_result(unsafe {
- DiceKeypairFromSeed(
- ptr::null_mut(), // context
- seed.as_ptr(),
- public_key.as_mut_ptr(),
- private_key.as_mut_ptr(),
- )
- })?;
+ check_result(
+ unsafe {
+ DiceKeypairFromSeed(
+ ptr::null_mut(), // context
+ seed.as_ptr(),
+ public_key.as_mut_ptr(),
+ private_key.as_mut_ptr(),
+ )
+ },
+ public_key.len(),
+ )?;
Ok((public_key, private_key))
}
@@ -86,15 +95,18 @@
let mut signature = [0u8; SIGNATURE_SIZE];
// SAFETY: The function writes to the `signature` within the given bounds, and only reads the
// message and the private key. The first argument context is not used in this function.
- check_result(unsafe {
- DiceSign(
- ptr::null_mut(), // context
- message.as_ptr(),
- message.len(),
- private_key.as_ptr(),
- signature.as_mut_ptr(),
- )
- })?;
+ check_result(
+ unsafe {
+ DiceSign(
+ ptr::null_mut(), // context
+ message.as_ptr(),
+ message.len(),
+ private_key.as_ptr(),
+ signature.as_mut_ptr(),
+ )
+ },
+ signature.len(),
+ )?;
Ok(signature)
}
@@ -102,15 +114,18 @@
pub fn verify(message: &[u8], signature: &Signature, public_key: &PublicKey) -> Result<()> {
// SAFETY: only reads the messages, signature and public key as constant values.
// The first argument context is not used in this function.
- check_result(unsafe {
- DiceVerify(
- ptr::null_mut(), // context
- message.as_ptr(),
- message.len(),
- signature.as_ptr(),
- public_key.as_ptr(),
- )
- })
+ check_result(
+ unsafe {
+ DiceVerify(
+ ptr::null_mut(), // context
+ message.as_ptr(),
+ message.len(),
+ signature.as_ptr(),
+ public_key.as_ptr(),
+ )
+ },
+ 0,
+ )
}
/// Generates an X.509 certificate from the given `subject_private_key_seed` and
@@ -127,16 +142,19 @@
let mut certificate_actual_size = 0;
// SAFETY: The function writes to the `certificate` within the given bounds, and only reads the
// input values and the key seeds. The first argument context is not used in this function.
- check_result(unsafe {
- DiceGenerateCertificate(
- ptr::null_mut(), // context
- subject_private_key_seed.as_ptr(),
- authority_private_key_seed.as_ptr(),
- input_values.as_ptr(),
- certificate.len(),
- certificate.as_mut_ptr(),
- &mut certificate_actual_size,
- )
- })?;
+ check_result(
+ unsafe {
+ DiceGenerateCertificate(
+ ptr::null_mut(), // context
+ subject_private_key_seed.as_ptr(),
+ authority_private_key_seed.as_ptr(),
+ input_values.as_ptr(),
+ certificate.len(),
+ certificate.as_mut_ptr(),
+ &mut certificate_actual_size,
+ )
+ },
+ certificate_actual_size,
+ )?;
Ok(certificate_actual_size)
}
diff --git a/diced/open_dice/src/retry.rs b/diced/open_dice/src/retry.rs
index 76a214c..3db4781 100644
--- a/diced/open_dice/src/retry.rs
+++ b/diced/open_dice/src/retry.rs
@@ -51,35 +51,21 @@
}
}
-/// Retries the given function with bigger output buffer size.
-fn retry_with_bigger_buffer<F>(mut f: F) -> Result<Vec<u8>>
+/// Retries the given function with bigger measured buffer size.
+fn retry_with_measured_buffer<F>(mut f: F) -> Result<Vec<u8>>
where
F: FnMut(&mut Vec<u8>) -> Result<usize>,
{
- const INITIAL_BUFFER_SIZE: usize = 256;
- const MAX_BUFFER_SIZE: usize = 64 * 1024 * 1024;
-
- let mut buffer = vec![0u8; INITIAL_BUFFER_SIZE];
- while buffer.len() <= MAX_BUFFER_SIZE {
- match f(&mut buffer) {
- Err(DiceError::BufferTooSmall) => {
- let new_size = buffer.len() * 2;
- buffer.resize(new_size, 0);
- }
- Err(e) => return Err(e),
- Ok(actual_size) => {
- if actual_size > buffer.len() {
- panic!(
- "actual_size larger than buffer size: open-dice function
- may have written past the end of the buffer."
- );
- }
- buffer.truncate(actual_size);
- return Ok(buffer);
- }
+ let mut buffer = Vec::new();
+ match f(&mut buffer) {
+ Err(DiceError::BufferTooSmall(actual_size)) => {
+ buffer.resize(actual_size, 0);
+ f(&mut buffer)?;
}
- }
- Err(DiceError::PlatformError)
+ Err(e) => return Err(e),
+ Ok(_) => {}
+ };
+ Ok(buffer)
}
/// Formats a configuration descriptor following the BCC's specification.
@@ -88,7 +74,7 @@
version: Option<u64>,
resettable: bool,
) -> Result<Vec<u8>> {
- retry_with_bigger_buffer(|buffer| {
+ retry_with_measured_buffer(|buffer| {
bcc_format_config_descriptor(name, version, resettable, buffer)
})
}
@@ -104,7 +90,7 @@
input_values: &InputValues,
) -> Result<OwnedDiceArtifacts> {
let mut next_cdi_values = CdiValues::default();
- let next_bcc = retry_with_bigger_buffer(|next_bcc| {
+ let next_bcc = retry_with_measured_buffer(|next_bcc| {
bcc_main_flow(
current_cdi_attest,
current_cdi_seal,
@@ -127,7 +113,7 @@
input_values: &InputValues,
) -> Result<(CdiValues, Vec<u8>)> {
let mut next_cdi_values = CdiValues::default();
- let next_cdi_certificate = retry_with_bigger_buffer(|next_cdi_certificate| {
+ let next_cdi_certificate = retry_with_measured_buffer(|next_cdi_certificate| {
dice_main_flow(
current_cdi_attest,
current_cdi_seal,
@@ -149,7 +135,7 @@
authority_private_key_seed: &[u8; PRIVATE_KEY_SEED_SIZE],
input_values: &InputValues,
) -> Result<Vec<u8>> {
- retry_with_bigger_buffer(|certificate| {
+ retry_with_measured_buffer(|certificate| {
generate_certificate(
subject_private_key_seed,
authority_private_key_seed,
diff --git a/identity/Android.bp b/identity/Android.bp
index f4fcc0a..da0df07 100644
--- a/identity/Android.bp
+++ b/identity/Android.bp
@@ -29,6 +29,7 @@
"identity_use_latest_hal_aidl_cpp_static",
"keymint_use_latest_hal_aidl_ndk_shared",
"keymint_use_latest_hal_aidl_cpp_static",
+ "android.hardware.identity-support-lib-deps",
],
srcs: [
@@ -43,27 +44,27 @@
],
init_rc: ["credstore.rc"],
shared_libs: [
- "android.hardware.identity-support-lib",
"android.hardware.keymaster@4.0",
"android.security.authorization-ndk",
"libbase",
"libbinder",
"libbinder_ndk",
- "libcredstore_aidl",
"libcrypto",
"libhidlbase",
+ "liblog",
+ "libutils",
+ "libutilscallstack",
+ ],
+ static_libs: [
+ "android.hardware.keymaster-V3-cpp",
+ "android.hardware.identity-support-lib",
+ "android.hardware.security.rkp-V3-cpp",
+ "android.security.rkp_aidl-cpp",
+ "libcppbor_external",
+ "libcredstore_aidl",
"libkeymaster4support",
"libkeystore-attestation-application-id",
"librkp_support",
- "libutils",
- "libutilscallstack",
- "libvintf",
- ],
- static_libs: [
- "android.hardware.security.rkp-V3-cpp",
- "android.hardware.keymaster-V3-cpp",
- "android.security.rkp_aidl-cpp",
- "libcppbor_external",
],
}
@@ -89,7 +90,7 @@
path: "binder",
}
-cc_library_shared {
+cc_library_static {
name: "libcredstore_aidl",
srcs: [
":credstore_aidl",
@@ -103,6 +104,8 @@
shared_libs: [
"libbinder",
"libutils",
+ ],
+ static_libs: [
"libkeymaster4support",
],
export_shared_lib_headers: [
diff --git a/identity/CredentialStore.cpp b/identity/CredentialStore.cpp
index cb2e8c7..57361c0 100644
--- a/identity/CredentialStore.cpp
+++ b/identity/CredentialStore.cpp
@@ -25,7 +25,6 @@
#include <binder/IPCThreadState.h>
#include <binder/IServiceManager.h>
#include <rkp/support/rkpd_client.h>
-#include <vintf/VintfObject.h>
#include "Credential.h"
#include "CredentialData.h"
diff --git a/keystore2/legacykeystore/lib.rs b/keystore2/legacykeystore/lib.rs
index 464f0a2..b826a65 100644
--- a/keystore2/legacykeystore/lib.rs
+++ b/keystore2/legacykeystore/lib.rs
@@ -29,9 +29,7 @@
legacy_blob::LegacyBlobLoader, maintenance::DeleteListener, maintenance::Domain,
utils::uid_to_android_user, utils::watchdog as wd,
};
-use rusqlite::{
- params, Connection, OptionalExtension, Transaction, TransactionBehavior, NO_PARAMS,
-};
+use rusqlite::{params, Connection, OptionalExtension, Transaction, TransactionBehavior};
use std::sync::Arc;
use std::{
collections::HashSet,
@@ -95,7 +93,7 @@
alias BLOB,
profile BLOB,
UNIQUE(owner, alias));",
- NO_PARAMS,
+ [],
)
.context("Failed to initialize \"profiles\" table.")?;
Ok(())
diff --git a/keystore2/src/authorization.rs b/keystore2/src/authorization.rs
index 1953920..4f2c7bd 100644
--- a/keystore2/src/authorization.rs
+++ b/keystore2/src/authorization.rs
@@ -19,7 +19,6 @@
use crate::error::anyhow_error_to_cstring;
use crate::globals::{ENFORCEMENTS, SUPER_KEY, DB, LEGACY_IMPORTER};
use crate::permission::KeystorePerm;
-use crate::super_key::UserState;
use crate::utils::{check_keystore_permission, watchdog as wd};
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
HardwareAuthToken::HardwareAuthToken,
@@ -158,31 +157,14 @@
let mut skm = SUPER_KEY.write().unwrap();
DB.with(|db| {
- skm.unlock_screen_lock_bound_key(
+ skm.unlock_user(
&mut db.borrow_mut(),
+ &LEGACY_IMPORTER,
user_id as u32,
&password,
)
})
- .context(ks_err!("unlock_screen_lock_bound_key failed"))?;
-
- // Unlock super key.
- if let UserState::Uninitialized = DB
- .with(|db| {
- skm.unlock_and_get_user_state(
- &mut db.borrow_mut(),
- &LEGACY_IMPORTER,
- user_id as u32,
- &password,
- )
- })
- .context(ks_err!("Unlock with password."))?
- {
- log::info!(
- "In on_lock_screen_event. Trying to unlock when LSKF is uninitialized."
- );
- }
-
+ .context(ks_err!("Unlock with password."))?;
Ok(())
}
(LockScreenEvent::UNLOCK, None) => {
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index d7c939c..d6bfaf2 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -82,7 +82,7 @@
types::FromSqlResult,
types::ToSqlOutput,
types::{FromSqlError, Value, ValueRef},
- Connection, OptionalExtension, ToSql, Transaction, TransactionBehavior, NO_PARAMS,
+ Connection, OptionalExtension, ToSql, Transaction, TransactionBehavior,
};
use std::{
@@ -905,21 +905,21 @@
alias BLOB,
state INTEGER,
km_uuid BLOB);",
- NO_PARAMS,
+ [],
)
.context("Failed to initialize \"keyentry\" table.")?;
tx.execute(
"CREATE INDEX IF NOT EXISTS persistent.keyentry_id_index
ON keyentry(id);",
- NO_PARAMS,
+ [],
)
.context("Failed to create index keyentry_id_index.")?;
tx.execute(
"CREATE INDEX IF NOT EXISTS persistent.keyentry_domain_namespace_index
ON keyentry(domain, namespace, alias);",
- NO_PARAMS,
+ [],
)
.context("Failed to create index keyentry_domain_namespace_index.")?;
@@ -929,14 +929,14 @@
subcomponent_type INTEGER,
keyentryid INTEGER,
blob BLOB);",
- NO_PARAMS,
+ [],
)
.context("Failed to initialize \"blobentry\" table.")?;
tx.execute(
"CREATE INDEX IF NOT EXISTS persistent.blobentry_keyentryid_index
ON blobentry(keyentryid);",
- NO_PARAMS,
+ [],
)
.context("Failed to create index blobentry_keyentryid_index.")?;
@@ -947,14 +947,14 @@
tag INTEGER,
data ANY,
UNIQUE (blobentryid, tag));",
- NO_PARAMS,
+ [],
)
.context("Failed to initialize \"blobmetadata\" table.")?;
tx.execute(
"CREATE INDEX IF NOT EXISTS persistent.blobmetadata_blobentryid_index
ON blobmetadata(blobentryid);",
- NO_PARAMS,
+ [],
)
.context("Failed to create index blobmetadata_blobentryid_index.")?;
@@ -964,14 +964,14 @@
tag INTEGER,
data ANY,
security_level INTEGER);",
- NO_PARAMS,
+ [],
)
.context("Failed to initialize \"keyparameter\" table.")?;
tx.execute(
"CREATE INDEX IF NOT EXISTS persistent.keyparameter_keyentryid_index
ON keyparameter(keyentryid);",
- NO_PARAMS,
+ [],
)
.context("Failed to create index keyparameter_keyentryid_index.")?;
@@ -981,14 +981,14 @@
tag INTEGER,
data ANY,
UNIQUE (keyentryid, tag));",
- NO_PARAMS,
+ [],
)
.context("Failed to initialize \"keymetadata\" table.")?;
tx.execute(
"CREATE INDEX IF NOT EXISTS persistent.keymetadata_keyentryid_index
ON keymetadata(keyentryid);",
- NO_PARAMS,
+ [],
)
.context("Failed to create index keymetadata_keyentryid_index.")?;
@@ -998,7 +998,7 @@
grantee INTEGER,
keyentryid INTEGER,
access_vector INTEGER);",
- NO_PARAMS,
+ [],
)
.context("Failed to initialize \"grant\" table.")?;
@@ -1611,7 +1611,7 @@
.context(ks_err!("Failed to insert blob."))?;
if let Some(blob_metadata) = blob_metadata {
let blob_id = tx
- .query_row("SELECT MAX(id) FROM persistent.blobentry;", NO_PARAMS, |row| {
+ .query_row("SELECT MAX(id) FROM persistent.blobentry;", [], |row| {
row.get(0)
})
.context(ks_err!("Failed to get new blob id."))?;
@@ -2859,7 +2859,6 @@
use android_hardware_security_secureclock::aidl::android::hardware::security::secureclock::{
Timestamp::Timestamp,
};
- use rusqlite::NO_PARAMS;
use rusqlite::TransactionBehavior;
use std::cell::RefCell;
use std::collections::BTreeMap;
@@ -2898,7 +2897,7 @@
#[test]
fn datetime() -> Result<()> {
let conn = Connection::open_in_memory()?;
- conn.execute("CREATE TABLE test (ts DATETIME);", NO_PARAMS)?;
+ conn.execute("CREATE TABLE test (ts DATETIME);", [])?;
let now = SystemTime::now();
let duration = Duration::from_secs(1000);
let then = now.checked_sub(duration).unwrap();
@@ -2908,7 +2907,7 @@
params![DateTime::try_from(now)?, DateTime::try_from(then)?, DateTime::try_from(soon)?],
)?;
let mut stmt = conn.prepare("SELECT ts FROM test ORDER BY ts ASC;")?;
- let mut rows = stmt.query(NO_PARAMS)?;
+ let mut rows = stmt.query([])?;
assert_eq!(DateTime::try_from(then)?, rows.next()?.unwrap().get(0)?);
assert_eq!(DateTime::try_from(now)?, rows.next()?.unwrap().get(0)?);
assert_eq!(DateTime::try_from(soon)?, rows.next()?.unwrap().get(0)?);
@@ -3259,15 +3258,9 @@
let mut stmt = db
.conn
.prepare("SELECT id, grantee, keyentryid, access_vector FROM persistent.grant;")?;
- let mut rows =
- stmt.query_map::<(i64, u32, i64, KeyPermSet), _, _>(NO_PARAMS, |row| {
- Ok((
- row.get(0)?,
- row.get(1)?,
- row.get(2)?,
- KeyPermSet::from(row.get::<_, i32>(3)?),
- ))
- })?;
+ let mut rows = stmt.query_map::<(i64, u32, i64, KeyPermSet), _, _>([], |row| {
+ Ok((row.get(0)?, row.get(1)?, row.get(2)?, KeyPermSet::from(row.get::<_, i32>(3)?)))
+ })?;
let r = rows.next().unwrap().unwrap();
assert_eq!(r, (next_random, GRANTEE_UID, 1, PVEC1));
@@ -3311,7 +3304,7 @@
ORDER BY subcomponent_type ASC;",
)?;
let mut rows = stmt
- .query_map::<((SubComponentType, i64, Vec<u8>), i64), _, _>(NO_PARAMS, |row| {
+ .query_map::<((SubComponentType, i64, Vec<u8>), i64), _, _>([], |row| {
Ok(((row.get(0)?, row.get(1)?, row.get(2)?), row.get(3)?))
})?;
let (r, id) = rows.next().unwrap().unwrap();
@@ -4412,7 +4405,7 @@
fn get_keyentry(db: &KeystoreDB) -> Result<Vec<KeyEntryRow>> {
db.conn
.prepare("SELECT * FROM persistent.keyentry;")?
- .query_map(NO_PARAMS, |row| {
+ .query_map([], |row| {
Ok(KeyEntryRow {
id: row.get(0)?,
key_type: row.get(1)?,
@@ -4660,7 +4653,7 @@
params
}
- fn make_test_key_entry(
+ pub fn make_test_key_entry(
db: &mut KeystoreDB,
domain: Domain,
namespace: i64,
@@ -4719,7 +4712,7 @@
}
}
- fn make_bootlevel_key_entry(
+ pub fn make_bootlevel_key_entry(
db: &mut KeystoreDB,
domain: Domain,
namespace: i64,
@@ -4784,7 +4777,7 @@
"SELECT id, key_type, domain, namespace, alias, state, km_uuid FROM persistent.keyentry;",
)?;
let rows = stmt.query_map::<(i64, KeyType, i32, i64, String, KeyLifeCycle, Uuid), _, _>(
- NO_PARAMS,
+ [],
|row| {
Ok((
row.get(0)?,
@@ -4813,7 +4806,7 @@
let mut stmt = db
.conn
.prepare("SELECT id, grantee, keyentryid, access_vector FROM persistent.grant;")?;
- let rows = stmt.query_map::<(i64, i64, i64, i64), _, _>(NO_PARAMS, |row| {
+ let rows = stmt.query_map::<(i64, i64, i64, i64), _, _>([], |row| {
Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?))
})?;
diff --git a/keystore2/src/database/versioning.rs b/keystore2/src/database/versioning.rs
index e3a95c8..2c816f4 100644
--- a/keystore2/src/database/versioning.rs
+++ b/keystore2/src/database/versioning.rs
@@ -13,21 +13,19 @@
// limitations under the License.
use anyhow::{anyhow, Context, Result};
-use rusqlite::{params, OptionalExtension, Transaction, NO_PARAMS};
+use rusqlite::{params, OptionalExtension, Transaction};
pub fn create_or_get_version(tx: &Transaction, current_version: u32) -> Result<u32> {
tx.execute(
"CREATE TABLE IF NOT EXISTS persistent.version (
id INTEGER PRIMARY KEY,
version INTEGER);",
- NO_PARAMS,
+ [],
)
.context("In create_or_get_version: Failed to create version table.")?;
let version = tx
- .query_row("SELECT version FROM persistent.version WHERE id = 0;", NO_PARAMS, |row| {
- row.get(0)
- })
+ .query_row("SELECT version FROM persistent.version WHERE id = 0;", [], |row| row.get(0))
.optional()
.context("In create_or_get_version: Failed to read version.")?;
@@ -44,7 +42,7 @@
.query_row(
"SELECT name FROM persistent.sqlite_master
WHERE type = 'table' AND name = 'keyentry';",
- NO_PARAMS,
+ [],
|_| Ok(()),
)
.optional()
@@ -94,12 +92,12 @@
#[cfg(test)]
mod test {
use super::*;
- use rusqlite::{Connection, TransactionBehavior, NO_PARAMS};
+ use rusqlite::{Connection, TransactionBehavior};
#[test]
fn upgrade_database_test() {
let mut conn = Connection::open_in_memory().unwrap();
- conn.execute("ATTACH DATABASE 'file::memory:' as persistent;", NO_PARAMS).unwrap();
+ conn.execute("ATTACH DATABASE 'file::memory:' as persistent;", []).unwrap();
let upgraders: Vec<_> = (0..30_u32)
.map(move |i| {
@@ -125,19 +123,19 @@
alias BLOB,
state INTEGER,
km_uuid BLOB);",
- NO_PARAMS,
+ [],
)
.unwrap();
}
for from in 1..29 {
for to in from..30 {
- conn.execute("DROP TABLE IF EXISTS persistent.version;", NO_PARAMS).unwrap();
- conn.execute("DROP TABLE IF EXISTS persistent.test;", NO_PARAMS).unwrap();
+ conn.execute("DROP TABLE IF EXISTS persistent.version;", []).unwrap();
+ conn.execute("DROP TABLE IF EXISTS persistent.test;", []).unwrap();
conn.execute(
"CREATE TABLE IF NOT EXISTS persistent.test (
id INTEGER PRIMARY KEY,
test_field INTEGER);",
- NO_PARAMS,
+ [],
)
.unwrap();
@@ -163,7 +161,7 @@
to - from,
conn.query_row(
"SELECT COUNT(test_field) FROM persistent.test;",
- NO_PARAMS,
+ [],
|row| row.get(0)
)
.unwrap()
@@ -188,7 +186,7 @@
#[test]
fn create_or_get_version_new_database() {
let mut conn = Connection::open_in_memory().unwrap();
- conn.execute("ATTACH DATABASE 'file::memory:' as persistent;", NO_PARAMS).unwrap();
+ conn.execute("ATTACH DATABASE 'file::memory:' as persistent;", []).unwrap();
{
let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate).unwrap();
let version = create_or_get_version(&tx, 3).unwrap();
@@ -202,7 +200,7 @@
conn.query_row(
"SELECT name FROM persistent.sqlite_master
WHERE type = 'table' AND name = 'version';",
- NO_PARAMS,
+ [],
|row| row.get(0),
)
);
@@ -210,18 +208,14 @@
// There is exactly one row in the version table.
assert_eq!(
Ok(1),
- conn.query_row("SELECT COUNT(id) from persistent.version;", NO_PARAMS, |row| row
- .get(0))
+ conn.query_row("SELECT COUNT(id) from persistent.version;", [], |row| row.get(0))
);
// The version must be set to 3
assert_eq!(
Ok(3),
- conn.query_row(
- "SELECT version from persistent.version WHERE id = 0;",
- NO_PARAMS,
- |row| row.get(0)
- )
+ conn.query_row("SELECT version from persistent.version WHERE id = 0;", [], |row| row
+ .get(0))
);
// Will subsequent calls to create_or_get_version still return the same version even
@@ -236,8 +230,7 @@
// There is still exactly one row in the version table.
assert_eq!(
Ok(1),
- conn.query_row("SELECT COUNT(id) from persistent.version;", NO_PARAMS, |row| row
- .get(0))
+ conn.query_row("SELECT COUNT(id) from persistent.version;", [], |row| row.get(0))
);
// Bump the version.
@@ -258,25 +251,21 @@
// There is still exactly one row in the version table.
assert_eq!(
Ok(1),
- conn.query_row("SELECT COUNT(id) from persistent.version;", NO_PARAMS, |row| row
- .get(0))
+ conn.query_row("SELECT COUNT(id) from persistent.version;", [], |row| row.get(0))
);
// The version must be set to 5
assert_eq!(
Ok(5),
- conn.query_row(
- "SELECT version from persistent.version WHERE id = 0;",
- NO_PARAMS,
- |row| row.get(0)
- )
+ conn.query_row("SELECT version from persistent.version WHERE id = 0;", [], |row| row
+ .get(0))
);
}
#[test]
fn create_or_get_version_legacy_database() {
let mut conn = Connection::open_in_memory().unwrap();
- conn.execute("ATTACH DATABASE 'file::memory:' as persistent;", NO_PARAMS).unwrap();
+ conn.execute("ATTACH DATABASE 'file::memory:' as persistent;", []).unwrap();
// A legacy (version 0) database is detected if the keyentry table exists but no
// version table.
conn.execute(
@@ -288,7 +277,7 @@
alias BLOB,
state INTEGER,
km_uuid BLOB);",
- NO_PARAMS,
+ [],
)
.unwrap();
@@ -306,7 +295,7 @@
conn.query_row(
"SELECT name FROM persistent.sqlite_master
WHERE type = 'table' AND name = 'version';",
- NO_PARAMS,
+ [],
|row| row.get(0),
)
);
@@ -314,18 +303,14 @@
// There is exactly one row in the version table.
assert_eq!(
Ok(1),
- conn.query_row("SELECT COUNT(id) from persistent.version;", NO_PARAMS, |row| row
- .get(0))
+ conn.query_row("SELECT COUNT(id) from persistent.version;", [], |row| row.get(0))
);
// The version must be set to 0
assert_eq!(
Ok(0),
- conn.query_row(
- "SELECT version from persistent.version WHERE id = 0;",
- NO_PARAMS,
- |row| row.get(0)
- )
+ conn.query_row("SELECT version from persistent.version WHERE id = 0;", [], |row| row
+ .get(0))
);
// Will subsequent calls to create_or_get_version still return the same version even
@@ -340,8 +325,7 @@
// There is still exactly one row in the version table.
assert_eq!(
Ok(1),
- conn.query_row("SELECT COUNT(id) from persistent.version;", NO_PARAMS, |row| row
- .get(0))
+ conn.query_row("SELECT COUNT(id) from persistent.version;", [], |row| row.get(0))
);
// Bump the version.
@@ -362,18 +346,14 @@
// There is still exactly one row in the version table.
assert_eq!(
Ok(1),
- conn.query_row("SELECT COUNT(id) from persistent.version;", NO_PARAMS, |row| row
- .get(0))
+ conn.query_row("SELECT COUNT(id) from persistent.version;", [], |row| row.get(0))
);
// The version must be set to 5
assert_eq!(
Ok(5),
- conn.query_row(
- "SELECT version from persistent.version WHERE id = 0;",
- NO_PARAMS,
- |row| row.get(0)
- )
+ conn.query_row("SELECT version from persistent.version WHERE id = 0;", [], |row| row
+ .get(0))
);
}
}
diff --git a/keystore2/src/fuzzers/aidl-fuzzers/authorization_service_fuzzer.rs b/keystore2/src/fuzzers/aidl-fuzzers/authorization_service_fuzzer.rs
index c1b2098..9f83b40 100644
--- a/keystore2/src/fuzzers/aidl-fuzzers/authorization_service_fuzzer.rs
+++ b/keystore2/src/fuzzers/aidl-fuzzers/authorization_service_fuzzer.rs
@@ -16,11 +16,10 @@
#![allow(missing_docs)]
#![no_main]
-#[macro_use]
-extern crate libfuzzer_sys;
use binder_random_parcel_rs::fuzz_service;
use keystore2::authorization::AuthorizationManager;
+use libfuzzer_sys::fuzz_target;
fuzz_target!(|data: &[u8]| {
let authorization_service = AuthorizationManager::new_native_binder().unwrap_or_else(|e| {
diff --git a/keystore2/src/key_parameter.rs b/keystore2/src/key_parameter.rs
index 5da95d9..02a1f16 100644
--- a/keystore2/src/key_parameter.rs
+++ b/keystore2/src/key_parameter.rs
@@ -1216,7 +1216,7 @@
use crate::key_parameter::*;
use anyhow::Result;
use rusqlite::types::ToSql;
- use rusqlite::{params, Connection, NO_PARAMS};
+ use rusqlite::{params, Connection};
/// Test initializing a KeyParameter (with key parameter value corresponding to an enum of i32)
/// from a database table row.
@@ -1423,7 +1423,7 @@
tag INTEGER,
data ANY,
security_level INTEGER);",
- NO_PARAMS,
+ [],
)
.context("Failed to initialize \"keyparameter\" table.")?;
Ok(db)
@@ -1459,7 +1459,7 @@
fn query_from_keyparameter(db: &Connection) -> Result<KeyParameter> {
let mut stmt =
db.prepare("SELECT tag, data, security_level FROM persistent.keyparameter")?;
- let mut rows = stmt.query(NO_PARAMS)?;
+ let mut rows = stmt.query([])?;
let row = rows.next()?.unwrap();
KeyParameter::new_from_sql(
Tag(row.get(0)?),
diff --git a/keystore2/src/legacy_importer.rs b/keystore2/src/legacy_importer.rs
index 9eb702d..325c213 100644
--- a/keystore2/src/legacy_importer.rs
+++ b/keystore2/src/legacy_importer.rs
@@ -102,6 +102,11 @@
}
}
+ #[cfg(test)]
+ pub fn set_empty(&mut self) {
+ self.state = AtomicU8::new(Self::STATE_EMPTY);
+ }
+
/// The legacy importer must be initialized deferred, because keystore starts very early.
/// At this time the data partition may not be mounted. So we cannot open database connections
/// until we get actual key load requests. This sets the function that the legacy loader
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 5efb798..73dc881 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -83,26 +83,24 @@
.context(ks_err!("unlock_screen_lock_bound_key failed"))?;
}
- match DB
- .with(|db| {
- skm.reset_or_init_user_and_get_user_state(
- &mut db.borrow_mut(),
- &LEGACY_IMPORTER,
- user_id as u32,
- password.as_ref(),
- )
- })
- .context(ks_err!())?
+ if let UserState::LskfLocked = DB
+ .with(|db| skm.get_user_state(&mut db.borrow_mut(), &LEGACY_IMPORTER, user_id as u32))
+ .context(ks_err!("Could not get user state while changing password!"))?
{
- UserState::LskfLocked => {
- // Error - password can not be changed when the device is locked
- Err(Error::Rc(ResponseCode::LOCKED)).context(ks_err!("Device is locked."))
- }
- _ => {
- // LskfLocked is the only error case for password change
- Ok(())
- }
+ // Error - password can not be changed when the device is locked
+ return Err(Error::Rc(ResponseCode::LOCKED)).context(ks_err!("Device is locked."));
}
+
+ DB.with(|db| match password {
+ Some(pass) => {
+ skm.init_user(&mut db.borrow_mut(), &LEGACY_IMPORTER, user_id as u32, &pass)
+ }
+ None => {
+ // User transitioned to swipe.
+ skm.reset_user(&mut db.borrow_mut(), &LEGACY_IMPORTER, user_id as u32)
+ }
+ })
+ .context(ks_err!("Failed to change user password!"))
}
fn add_or_remove_user(&self, user_id: i32) -> Result<()> {
@@ -111,11 +109,10 @@
check_keystore_permission(KeystorePerm::ChangeUser).context(ks_err!())?;
DB.with(|db| {
- SUPER_KEY.write().unwrap().reset_user(
+ SUPER_KEY.write().unwrap().remove_user(
&mut db.borrow_mut(),
&LEGACY_IMPORTER,
user_id as u32,
- false,
)
})
.context(ks_err!("Trying to delete keys from db."))?;
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index 77cead8..5d311f0 100644
--- a/keystore2/src/metrics_store.rs
+++ b/keystore2/src/metrics_store.rs
@@ -43,9 +43,8 @@
RkpError::RkpError as MetricsRkpError, RkpErrorStats::RkpErrorStats,
SecurityLevel::SecurityLevel as MetricsSecurityLevel, Storage::Storage as MetricsStorage,
};
-use anyhow::{Context, Result};
+use anyhow::{anyhow, Context, Result};
use lazy_static::lazy_static;
-use rustutils::system_properties::PropertyWatcherError;
use std::collections::HashMap;
use std::sync::Mutex;
@@ -93,12 +92,15 @@
// Process keystore crash stats.
if AtomID::CRASH_STATS == atom_id {
- return Ok(vec![KeystoreAtom {
- payload: KeystoreAtomPayload::CrashStats(CrashStats {
- count_of_crash_events: read_keystore_crash_count()?,
- }),
- ..Default::default()
- }]);
+ return match read_keystore_crash_count()? {
+ Some(count) => Ok(vec![KeystoreAtom {
+ payload: KeystoreAtomPayload::CrashStats(CrashStats {
+ count_of_crash_events: count,
+ }),
+ ..Default::default()
+ }]),
+ None => Err(anyhow!("Crash count property is not set")),
+ };
}
// It is safe to call unwrap here since the lock can not be poisoned based on its usage
@@ -564,27 +566,21 @@
/// If the property is absent, it sets the property with value 0. If the property is present, it
/// increments the value. This helps tracking keystore crashes internally.
pub fn update_keystore_crash_sysprop() {
- let crash_count = read_keystore_crash_count();
- let new_count = match crash_count {
- Ok(count) => count + 1,
+ let new_count = match read_keystore_crash_count() {
+ Ok(Some(count)) => count + 1,
+ // If the property is absent, then this is the first start up during the boot.
+ // Proceed to write the system property with value 0.
+ Ok(None) => 0,
Err(error) => {
- // If the property is absent, this is the first start up during the boot.
- // Proceed to write the system property with value 0. Otherwise, log and return.
- if !matches!(
- error.root_cause().downcast_ref::<PropertyWatcherError>(),
- Some(PropertyWatcherError::SystemPropertyAbsent)
- ) {
- log::warn!(
- concat!(
- "In update_keystore_crash_sysprop: ",
- "Failed to read the existing system property due to: {:?}.",
- "Therefore, keystore crashes will not be logged."
- ),
- error
- );
- return;
- }
- 0
+ log::warn!(
+ concat!(
+ "In update_keystore_crash_sysprop: ",
+ "Failed to read the existing system property due to: {:?}.",
+ "Therefore, keystore crashes will not be logged."
+ ),
+ error
+ );
+ return;
}
};
@@ -602,12 +598,12 @@
}
/// Read the system property: keystore.crash_count.
-pub fn read_keystore_crash_count() -> Result<i32> {
- rustutils::system_properties::read("keystore.crash_count")
- .context(ks_err!("Failed read property."))?
- .context(ks_err!("Property not set."))?
- .parse::<i32>()
- .map_err(std::convert::Into::into)
+pub fn read_keystore_crash_count() -> Result<Option<i32>> {
+ match rustutils::system_properties::read("keystore.crash_count") {
+ Ok(Some(count)) => count.parse::<i32>().map(Some).map_err(std::convert::Into::into),
+ Ok(None) => Ok(None),
+ Err(e) => Err(e).context(ks_err!("Failed to read crash count property.")),
+ }
}
/// Enum defining the bit position for each padding mode. Since padding mode can be repeatable, it
diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs
index cb9960f..2e8b60f 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -26,7 +26,6 @@
error::ResponseCode,
key_parameter::{KeyParameter, KeyParameterValue},
ks_err,
- legacy_blob::LegacyBlobLoader,
legacy_importer::LegacyImporter,
raw_device::KeyMintDevice,
utils::{watchdog as wd, AesGcm, AID_KEYSTORE},
@@ -400,51 +399,6 @@
self.data.user_keys.get(&user_id).and_then(|e| e.per_boot.as_ref().cloned())
}
- /// This function unlocks the super keys for a given user.
- /// This means the key is loaded from the database, decrypted and placed in the
- /// super key cache. If there is no such key a new key is created, encrypted with
- /// a key derived from the given password and stored in the database.
- pub fn unlock_user_key(
- &mut self,
- db: &mut KeystoreDB,
- user: UserId,
- pw: &Password,
- legacy_blob_loader: &LegacyBlobLoader,
- ) -> Result<()> {
- let (_, entry) = db
- .get_or_create_key_with(
- Domain::APP,
- user as u64 as i64,
- USER_SUPER_KEY.alias,
- crate::database::KEYSTORE_UUID,
- || {
- // For backward compatibility we need to check if there is a super key present.
- let super_key = legacy_blob_loader
- .load_super_key(user, pw)
- .context(ks_err!("Failed to load legacy key blob."))?;
- let super_key = match super_key {
- None => {
- // No legacy file was found. So we generate a new key.
- generate_aes256_key()
- .context(ks_err!("Failed to generate AES 256 key."))?
- }
- Some(key) => key,
- };
- // Regardless of whether we loaded an old AES128 key or generated a new AES256
- // key as the super key, we derive a AES256 key from the password and re-encrypt
- // the super key before we insert it in the database. The length of the key is
- // preserved by the encryption so we don't need any extra flags to inform us
- // which algorithm to use it with.
- Self::encrypt_with_password(&super_key, pw).context("In create_new_key.")
- },
- )
- .context(ks_err!("Failed to get key id."))?;
-
- self.populate_cache_from_super_key_blob(user, USER_SUPER_KEY.algorithm, entry, pw)
- .context(ks_err!())?;
- Ok(())
- }
-
/// Check if a given key is super-encrypted, from its metadata. If so, unwrap the key using
/// the relevant super key.
pub fn unwrap_key_if_required<'a>(
@@ -526,83 +480,6 @@
}
}
- /// Checks if user has already setup LSKF (i.e. a super key is persisted in the database or the
- /// legacy database). If not, return Uninitialized state.
- /// Otherwise, decrypt the super key from the password and return LskfUnlocked state.
- pub fn check_and_unlock_super_key(
- &mut self,
- db: &mut KeystoreDB,
- legacy_importer: &LegacyImporter,
- user_id: UserId,
- pw: &Password,
- ) -> Result<UserState> {
- let alias = &USER_SUPER_KEY;
- let result = legacy_importer
- .with_try_import_super_key(user_id, pw, || db.load_super_key(alias, user_id))
- .context(ks_err!("Failed to load super key"))?;
-
- match result {
- Some((_, entry)) => {
- let super_key = self
- .populate_cache_from_super_key_blob(user_id, alias.algorithm, entry, pw)
- .context(ks_err!())?;
- Ok(UserState::LskfUnlocked(super_key))
- }
- None => Ok(UserState::Uninitialized),
- }
- }
-
- /// Checks if user has already setup LSKF (i.e. a super key is persisted in the database or the
- /// legacy database). If so, return LskfLocked state.
- /// If the password is provided, generate a new super key, encrypt with the password,
- /// store in the database and populate the super key cache for the new user
- /// and return LskfUnlocked state.
- /// If the password is not provided, return Uninitialized state.
- pub fn check_and_initialize_super_key(
- &mut self,
- db: &mut KeystoreDB,
- legacy_importer: &LegacyImporter,
- user_id: UserId,
- pw: Option<&Password>,
- ) -> Result<UserState> {
- let super_key_exists_in_db = self
- .super_key_exists_in_db_for_user(db, legacy_importer, user_id)
- .context(ks_err!("Failed to check if super key exists."))?;
- if super_key_exists_in_db {
- Ok(UserState::LskfLocked)
- } else if let Some(pw) = pw {
- // Generate a new super key.
- let super_key =
- generate_aes256_key().context(ks_err!("Failed to generate AES 256 key."))?;
- // Derive an AES256 key from the password and re-encrypt the super key
- // before we insert it in the database.
- let (encrypted_super_key, blob_metadata) =
- Self::encrypt_with_password(&super_key, pw).context(ks_err!())?;
-
- let key_entry = db
- .store_super_key(
- user_id,
- &USER_SUPER_KEY,
- &encrypted_super_key,
- &blob_metadata,
- &KeyMetaData::new(),
- )
- .context(ks_err!("Failed to store super key."))?;
-
- let super_key = self
- .populate_cache_from_super_key_blob(
- user_id,
- USER_SUPER_KEY.algorithm,
- key_entry,
- pw,
- )
- .context(ks_err!())?;
- Ok(UserState::LskfUnlocked(super_key))
- } else {
- Ok(UserState::Uninitialized)
- }
- }
-
// Helper function to populate super key cache from the super key blob loaded from the database.
fn populate_cache_from_super_key_blob(
&mut self,
@@ -613,7 +490,8 @@
) -> Result<Arc<SuperKey>> {
let super_key = Self::extract_super_key_from_key_entry(algorithm, entry, pw, None)
.context(ks_err!("Failed to extract super key from key entry"))?;
- self.install_per_boot_key_for_user(user_id, super_key.clone())?;
+ self.install_per_boot_key_for_user(user_id, super_key.clone())
+ .context(ks_err!("Failed to install per boot key for user!"))?;
Ok(super_key)
}
@@ -683,33 +561,6 @@
Ok((encrypted_key, metadata))
}
- // Encrypt the given key blob with the user's super key, if the super key exists and the device
- // is unlocked. If the super key exists and the device is locked, or LSKF is not setup,
- // return error. Note that it is out of the scope of this function to check if super encryption
- // is required. Such check should be performed before calling this function.
- fn super_encrypt_on_key_init(
- &self,
- db: &mut KeystoreDB,
- legacy_importer: &LegacyImporter,
- user_id: UserId,
- key_blob: &[u8],
- ) -> Result<(Vec<u8>, BlobMetaData)> {
- match self
- .get_user_state(db, legacy_importer, user_id)
- .context(ks_err!("Failed to get user state."))?
- {
- UserState::LskfUnlocked(super_key) => {
- Self::encrypt_with_aes_super_key(key_blob, &super_key)
- .context(ks_err!("Failed to encrypt the key."))
- }
- UserState::LskfLocked => {
- Err(Error::Rc(ResponseCode::LOCKED)).context(ks_err!("Device is locked."))
- }
- UserState::Uninitialized => Err(Error::Rc(ResponseCode::UNINITIALIZED))
- .context(ks_err!("LSKF is not setup for the user.")),
- }
- }
-
// Helper function to encrypt a key with the given super key. Callers should select which super
// key to be used. This is called when a key is super encrypted at its creation as well as at
// its upgrade.
@@ -729,6 +580,50 @@
Ok((encrypted_key, metadata))
}
+ // Encrypts a given key_blob using a hybrid approach, which can either use the symmetric super
+ // key or the public super key depending on which is available.
+ //
+ // If the symmetric_key is available, the key_blob is encrypted using symmetric encryption with
+ // the provided symmetric super key. Otherwise, the function loads the public super key from
+ // the KeystoreDB and encrypts the key_blob using ECDH encryption and marks the keyblob to be
+ // re-encrypted with the symmetric super key on the first use.
+ //
+ // This hybrid scheme allows lock-screen-bound keys to be added when the screen is locked.
+ fn encrypt_with_hybrid_super_key(
+ key_blob: &[u8],
+ symmetric_key: Option<&SuperKey>,
+ public_key_type: &SuperKeyType,
+ db: &mut KeystoreDB,
+ user_id: UserId,
+ ) -> Result<(Vec<u8>, BlobMetaData)> {
+ if let Some(super_key) = symmetric_key {
+ Self::encrypt_with_aes_super_key(key_blob, super_key)
+ .context(ks_err!("Failed to encrypt with ScreenLockBound super key."))
+ } else {
+ // Symmetric key is not available, use public key encryption
+ let loaded = db
+ .load_super_key(public_key_type, user_id)
+ .context(ks_err!("load_super_key failed."))?;
+ let (key_id_guard, key_entry) =
+ loaded.ok_or_else(Error::sys).context(ks_err!("User ECDH super key missing."))?;
+ let public_key = key_entry
+ .metadata()
+ .sec1_public_key()
+ .ok_or_else(Error::sys)
+ .context(ks_err!("sec1_public_key missing."))?;
+ let mut metadata = BlobMetaData::new();
+ let (ephem_key, salt, iv, encrypted_key, aead_tag) =
+ ECDHPrivateKey::encrypt_message(public_key, key_blob)
+ .context(ks_err!("ECDHPrivateKey::encrypt_message failed."))?;
+ metadata.add(BlobMetaEntry::PublicKey(ephem_key));
+ metadata.add(BlobMetaEntry::Salt(salt));
+ metadata.add(BlobMetaEntry::Iv(iv));
+ metadata.add(BlobMetaEntry::AeadTag(aead_tag));
+ SuperKeyIdentifier::DatabaseId(key_id_guard.id()).add_to_metadata(&mut metadata);
+ Ok((encrypted_key, metadata))
+ }
+ }
+
/// Check if super encryption is required and if so, super-encrypt the key to be stored in
/// the database.
#[allow(clippy::too_many_arguments)]
@@ -744,40 +639,40 @@
) -> Result<(Vec<u8>, BlobMetaData)> {
match Enforcements::super_encryption_required(domain, key_parameters, flags) {
SuperEncryptionType::None => Ok((key_blob.to_vec(), BlobMetaData::new())),
- SuperEncryptionType::LskfBound => self
- .super_encrypt_on_key_init(db, legacy_importer, user_id, key_blob)
- .context(ks_err!("Failed to super encrypt with LskfBound key.")),
- SuperEncryptionType::ScreenLockBound => {
- let entry =
- self.data.user_keys.get(&user_id).and_then(|e| e.screen_lock_bound.as_ref());
- if let Some(super_key) = entry {
- Self::encrypt_with_aes_super_key(key_blob, super_key)
- .context(ks_err!("Failed to encrypt with ScreenLockBound key."))
- } else {
- // Symmetric key is not available, use public key encryption
- let loaded = db
- .load_super_key(&USER_SCREEN_LOCK_BOUND_P521_KEY, user_id)
- .context(ks_err!("load_super_key failed."))?;
- let (key_id_guard, key_entry) =
- loaded.ok_or_else(Error::sys).context(ks_err!("User ECDH key missing."))?;
- let public_key = key_entry
- .metadata()
- .sec1_public_key()
- .ok_or_else(Error::sys)
- .context(ks_err!("sec1_public_key missing."))?;
- let mut metadata = BlobMetaData::new();
- let (ephem_key, salt, iv, encrypted_key, aead_tag) =
- ECDHPrivateKey::encrypt_message(public_key, key_blob)
- .context(ks_err!("ECDHPrivateKey::encrypt_message failed."))?;
- metadata.add(BlobMetaEntry::PublicKey(ephem_key));
- metadata.add(BlobMetaEntry::Salt(salt));
- metadata.add(BlobMetaEntry::Iv(iv));
- metadata.add(BlobMetaEntry::AeadTag(aead_tag));
- SuperKeyIdentifier::DatabaseId(key_id_guard.id())
- .add_to_metadata(&mut metadata);
- Ok((encrypted_key, metadata))
+ SuperEncryptionType::LskfBound => {
+ // Encrypt the given key blob with the user's per-boot super key. If the per-boot
+ // super key is not unlocked or the LSKF is not setup, an error is returned.
+ match self
+ .get_user_state(db, legacy_importer, user_id)
+ .context(ks_err!("Failed to get user state."))?
+ {
+ UserState::LskfUnlocked(super_key) => {
+ Self::encrypt_with_aes_super_key(key_blob, &super_key)
+ .context(ks_err!("Failed to encrypt with LskfBound key."))
+ }
+ UserState::LskfLocked => {
+ Err(Error::Rc(ResponseCode::LOCKED)).context(ks_err!("Device is locked."))
+ }
+ UserState::Uninitialized => Err(Error::Rc(ResponseCode::UNINITIALIZED))
+ .context(ks_err!("LSKF is not setup for the user.")),
}
}
+ SuperEncryptionType::ScreenLockBound => {
+ let screen_lock_bound_symmetric_key = self
+ .data
+ .user_keys
+ .get(&user_id)
+ .and_then(|e| e.screen_lock_bound.as_ref())
+ .map(|arc| arc.as_ref());
+ Self::encrypt_with_hybrid_super_key(
+ key_blob,
+ screen_lock_bound_symmetric_key,
+ &USER_SCREEN_LOCK_BOUND_P521_KEY,
+ db,
+ user_id,
+ )
+ .context(ks_err!("Failed to encrypt with ScreenLockBound hybrid scheme."))
+ }
SuperEncryptionType::BootLevel(level) => {
let key_id = SuperKeyIdentifier::BootLevel(level);
let super_key = self
@@ -1085,93 +980,147 @@
}
}
- /// If the given user is unlocked:
- /// * and `password` is None, the user is reset, all authentication bound keys are deleted and
- /// `Ok(UserState::Uninitialized)` is returned.
- /// * and `password` is Some, `Ok(UserState::LskfUnlocked)` is returned.
- /// If the given user is locked:
- /// * and the user was initialized before, `Ok(UserState::Locked)` is returned.
- /// * and the user was not initialized before:
- /// * and `password` is None, `Ok(Uninitialized)` is returned.
- /// * and `password` is Some, super keys are generated and `Ok(UserState::LskfUnlocked)` is
- /// returned.
- pub fn reset_or_init_user_and_get_user_state(
+ /// Deletes all keys and super keys for the given user.
+ /// This is called when a user is deleted.
+ pub fn remove_user(
&mut self,
db: &mut KeystoreDB,
legacy_importer: &LegacyImporter,
user_id: UserId,
- password: Option<&Password>,
- ) -> Result<UserState> {
- match self.get_per_boot_key_by_user_id_internal(user_id) {
- Some(_) if password.is_none() => {
- // Transitioning to swiping, delete only the super key in database and cache,
- // and super-encrypted keys in database (and in KM).
- self.reset_user(db, legacy_importer, user_id, true)
- .context(ks_err!("Trying to delete keys from the db."))?;
- // Lskf is now removed in Keystore.
- Ok(UserState::Uninitialized)
- }
- Some(super_key) => {
- // Keystore won't be notified when changing to a new password when LSKF is
- // already setup. Therefore, ideally this path wouldn't be reached.
- Ok(UserState::LskfUnlocked(super_key))
- }
- None => {
- // Check if a super key exists in the database or legacy database.
- // If so, return LskfLocked state.
- // Otherwise, i) if the password is provided, initialize the super key and return
- // LskfUnlocked state ii) if password is not provided, return Uninitialized state.
- self.check_and_initialize_super_key(db, legacy_importer, user_id, password)
- }
- }
+ ) -> Result<()> {
+ // Mark keys created on behalf of the user as unreferenced.
+ legacy_importer
+ .bulk_delete_user(user_id, false)
+ .context(ks_err!("Trying to delete legacy keys."))?;
+ db.unbind_keys_for_user(user_id, false).context(ks_err!("Error in unbinding keys."))?;
+
+ // Delete super key in cache, if exists.
+ self.forget_all_keys_for_user(user_id);
+ Ok(())
}
- /// Unlocks the given user with the given password. If the key was already unlocked or unlocking
- /// was successful, `Ok(UserState::LskfUnlocked)` is returned.
- /// If the user was never initialized `Ok(UserState::Uninitialized)` is returned.
- pub fn unlock_and_get_user_state(
- &mut self,
- db: &mut KeystoreDB,
- legacy_importer: &LegacyImporter,
- user_id: UserId,
- password: &Password,
- ) -> Result<UserState> {
- match self.get_per_boot_key_by_user_id_internal(user_id) {
- Some(super_key) => {
- log::info!("Trying to unlock when already unlocked.");
- Ok(UserState::LskfUnlocked(super_key))
- }
- None => {
- // Check if a super key exists in the database or legacy database.
- // If not, return Uninitialized state.
- // Otherwise, try to unlock the super key and if successful,
- // return LskfUnlocked.
- self.check_and_unlock_super_key(db, legacy_importer, user_id, password)
- .context(ks_err!("Failed to unlock super key."))
- }
- }
- }
-
- /// Delete all the keys created on behalf of the user.
- /// If 'keep_non_super_encrypted_keys' is set to true, delete only the super key and super
- /// encrypted keys.
+ /// Deletes all authentication bound keys and super keys for the given user. The user must be
+ /// unlocked before this function is called. This function is used to transition a user to
+ /// swipe.
pub fn reset_user(
&mut self,
db: &mut KeystoreDB,
legacy_importer: &LegacyImporter,
user_id: UserId,
- keep_non_super_encrypted_keys: bool,
) -> Result<()> {
- // Mark keys created on behalf of the user as unreferenced.
- legacy_importer
- .bulk_delete_user(user_id, keep_non_super_encrypted_keys)
- .context(ks_err!("Trying to delete legacy keys."))?;
- db.unbind_keys_for_user(user_id, keep_non_super_encrypted_keys)
- .context(ks_err!("Error in unbinding keys."))?;
+ match self.get_user_state(db, legacy_importer, user_id)? {
+ UserState::Uninitialized => {
+ Err(Error::sys()).context(ks_err!("Tried to reset an uninitialized user!"))
+ }
+ UserState::LskfLocked => {
+ Err(Error::sys()).context(ks_err!("Tried to reset a locked user's password!"))
+ }
+ UserState::LskfUnlocked(_) => {
+ // Mark keys created on behalf of the user as unreferenced.
+ legacy_importer
+ .bulk_delete_user(user_id, true)
+ .context(ks_err!("Trying to delete legacy keys."))?;
+ db.unbind_keys_for_user(user_id, true)
+ .context(ks_err!("Error in unbinding keys."))?;
- // Delete super key in cache, if exists.
- self.forget_all_keys_for_user(user_id);
- Ok(())
+ // Delete super key in cache, if exists.
+ self.forget_all_keys_for_user(user_id);
+ Ok(())
+ }
+ }
+ }
+
+ /// If the user hasn't been initialized yet, then this function generates the user's super keys
+ /// and sets the user's state to LskfUnlocked. Otherwise this function returns an error.
+ pub fn init_user(
+ &mut self,
+ db: &mut KeystoreDB,
+ legacy_importer: &LegacyImporter,
+ user_id: UserId,
+ password: &Password,
+ ) -> Result<()> {
+ match self.get_user_state(db, legacy_importer, user_id)? {
+ UserState::LskfUnlocked(_) | UserState::LskfLocked => {
+ Err(Error::sys()).context(ks_err!("Tried to re-init an initialized user!"))
+ }
+ UserState::Uninitialized => {
+ // Generate a new super key.
+ let super_key =
+ generate_aes256_key().context(ks_err!("Failed to generate AES 256 key."))?;
+ // Derive an AES256 key from the password and re-encrypt the super key
+ // before we insert it in the database.
+ let (encrypted_super_key, blob_metadata) =
+ Self::encrypt_with_password(&super_key, password)
+ .context(ks_err!("Failed to encrypt super key with password!"))?;
+
+ let key_entry = db
+ .store_super_key(
+ user_id,
+ &USER_SUPER_KEY,
+ &encrypted_super_key,
+ &blob_metadata,
+ &KeyMetaData::new(),
+ )
+ .context(ks_err!("Failed to store super key."))?;
+
+ self.populate_cache_from_super_key_blob(
+ user_id,
+ USER_SUPER_KEY.algorithm,
+ key_entry,
+ password,
+ )
+ .context(ks_err!("Failed to initialize user!"))?;
+ Ok(())
+ }
+ }
+ }
+
+ /// Unlocks the given user with the given password.
+ ///
+ /// If the user is LskfLocked:
+ /// - Unlock the per_boot super key
+ /// - Unlock the screen_lock_bound super key
+ ///
+ /// If the user is LskfUnlocked:
+ /// - Unlock the screen_lock_bound super key only
+ ///
+ pub fn unlock_user(
+ &mut self,
+ db: &mut KeystoreDB,
+ legacy_importer: &LegacyImporter,
+ user_id: UserId,
+ password: &Password,
+ ) -> Result<()> {
+ match self.get_user_state(db, legacy_importer, user_id)? {
+ UserState::LskfUnlocked(_) => self.unlock_screen_lock_bound_key(db, user_id, password),
+ UserState::Uninitialized => {
+ Err(Error::sys()).context(ks_err!("Tried to unlock an uninitialized user!"))
+ }
+ UserState::LskfLocked => {
+ let alias = &USER_SUPER_KEY;
+ let result = legacy_importer
+ .with_try_import_super_key(user_id, password, || {
+ db.load_super_key(alias, user_id)
+ })
+ .context(ks_err!("Failed to load super key"))?;
+
+ match result {
+ Some((_, entry)) => {
+ self.populate_cache_from_super_key_blob(
+ user_id,
+ alias.algorithm,
+ entry,
+ password,
+ )
+ .context(ks_err!("Failed when unlocking user."))?;
+ self.unlock_screen_lock_bound_key(db, user_id, password)
+ }
+ None => {
+ Err(Error::sys()).context(ks_err!("Locked user does not have a super key!"))
+ }
+ }
+ }
+ }
}
}
@@ -1233,3 +1182,390 @@
}
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::database::tests::make_bootlevel_key_entry;
+ use crate::database::tests::make_test_key_entry;
+ use crate::database::tests::new_test_db;
+ use rand::prelude::*;
+ const USER_ID: u32 = 0;
+ const TEST_KEY_ALIAS: &str = "TEST_KEY";
+ const TEST_BOOT_KEY_ALIAS: &str = "TEST_BOOT_KEY";
+
+ pub fn generate_password_blob() -> Password<'static> {
+ let mut rng = rand::thread_rng();
+ let mut password = vec![0u8; 64];
+ rng.fill_bytes(&mut password);
+
+ let mut zvec = ZVec::new(64).expect("Failed to create ZVec");
+ zvec[..].copy_from_slice(&password[..]);
+
+ Password::Owned(zvec)
+ }
+
+ fn setup_test(pw: &Password) -> (Arc<RwLock<SuperKeyManager>>, KeystoreDB, LegacyImporter) {
+ let mut keystore_db = new_test_db().unwrap();
+ let mut legacy_importer = LegacyImporter::new(Arc::new(Default::default()));
+ legacy_importer.set_empty();
+ let skm: Arc<RwLock<SuperKeyManager>> = Default::default();
+ assert!(skm
+ .write()
+ .unwrap()
+ .init_user(&mut keystore_db, &legacy_importer, USER_ID, pw)
+ .is_ok());
+ (skm, keystore_db, legacy_importer)
+ }
+
+ fn assert_unlocked(
+ skm: &Arc<RwLock<SuperKeyManager>>,
+ keystore_db: &mut KeystoreDB,
+ legacy_importer: &LegacyImporter,
+ user_id: u32,
+ err_msg: &str,
+ ) {
+ let user_state =
+ skm.write().unwrap().get_user_state(keystore_db, legacy_importer, user_id).unwrap();
+ match user_state {
+ UserState::LskfUnlocked(_) => {}
+ _ => panic!("{}", err_msg),
+ }
+ }
+
+ fn assert_locked(
+ skm: &Arc<RwLock<SuperKeyManager>>,
+ keystore_db: &mut KeystoreDB,
+ legacy_importer: &LegacyImporter,
+ user_id: u32,
+ err_msg: &str,
+ ) {
+ let user_state =
+ skm.write().unwrap().get_user_state(keystore_db, legacy_importer, user_id).unwrap();
+ match user_state {
+ UserState::LskfLocked => {}
+ _ => panic!("{}", err_msg),
+ }
+ }
+
+ fn assert_uninitialized(
+ skm: &Arc<RwLock<SuperKeyManager>>,
+ keystore_db: &mut KeystoreDB,
+ legacy_importer: &LegacyImporter,
+ user_id: u32,
+ err_msg: &str,
+ ) {
+ let user_state =
+ skm.write().unwrap().get_user_state(keystore_db, legacy_importer, user_id).unwrap();
+ match user_state {
+ UserState::Uninitialized => {}
+ _ => panic!("{}", err_msg),
+ }
+ }
+
+ #[test]
+ fn test_init_user() {
+ let pw: Password = generate_password_blob();
+ let (skm, mut keystore_db, legacy_importer) = setup_test(&pw);
+ assert_unlocked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was not unlocked after initialization!",
+ );
+ }
+
+ #[test]
+ fn test_unlock_user() {
+ let pw: Password = generate_password_blob();
+ let (skm, mut keystore_db, legacy_importer) = setup_test(&pw);
+ assert_unlocked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was not unlocked after initialization!",
+ );
+
+ skm.write().unwrap().data.user_keys.clear();
+ assert_locked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "Clearing the cache did not lock the user!",
+ );
+
+ assert!(skm
+ .write()
+ .unwrap()
+ .unlock_user(&mut keystore_db, &legacy_importer, USER_ID, &pw)
+ .is_ok());
+ assert_unlocked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user did not unlock!",
+ );
+ }
+
+ #[test]
+ fn test_unlock_wrong_password() {
+ let pw: Password = generate_password_blob();
+ let wrong_pw: Password = generate_password_blob();
+ let (skm, mut keystore_db, legacy_importer) = setup_test(&pw);
+ assert_unlocked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was not unlocked after initialization!",
+ );
+
+ skm.write().unwrap().data.user_keys.clear();
+ assert_locked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "Clearing the cache did not lock the user!",
+ );
+
+ assert!(skm
+ .write()
+ .unwrap()
+ .unlock_user(&mut keystore_db, &legacy_importer, USER_ID, &wrong_pw)
+ .is_err());
+ assert_locked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was unlocked with an incorrect password!",
+ );
+ }
+
+ #[test]
+ fn test_unlock_user_idempotent() {
+ let pw: Password = generate_password_blob();
+ let (skm, mut keystore_db, legacy_importer) = setup_test(&pw);
+ assert_unlocked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was not unlocked after initialization!",
+ );
+
+ skm.write().unwrap().data.user_keys.clear();
+ assert_locked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "Clearing the cache did not lock the user!",
+ );
+
+ for _ in 0..5 {
+ assert!(skm
+ .write()
+ .unwrap()
+ .unlock_user(&mut keystore_db, &legacy_importer, USER_ID, &pw)
+ .is_ok());
+ assert_unlocked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user did not unlock!",
+ );
+ }
+ }
+
+ fn test_user_removal(locked: bool) {
+ let pw: Password = generate_password_blob();
+ let (skm, mut keystore_db, legacy_importer) = setup_test(&pw);
+ assert_unlocked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was not unlocked after initialization!",
+ );
+
+ assert!(make_test_key_entry(
+ &mut keystore_db,
+ Domain::APP,
+ USER_ID.into(),
+ TEST_KEY_ALIAS,
+ None
+ )
+ .is_ok());
+ assert!(make_bootlevel_key_entry(
+ &mut keystore_db,
+ Domain::APP,
+ USER_ID.into(),
+ TEST_BOOT_KEY_ALIAS,
+ false
+ )
+ .is_ok());
+
+ assert!(keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+ assert!(keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_BOOT_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+
+ if locked {
+ skm.write().unwrap().data.user_keys.clear();
+ assert_locked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "Clearing the cache did not lock the user!",
+ );
+ }
+
+ assert!(skm
+ .write()
+ .unwrap()
+ .remove_user(&mut keystore_db, &legacy_importer, USER_ID)
+ .is_ok());
+ assert_uninitialized(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was not removed!",
+ );
+
+ assert!(!skm
+ .write()
+ .unwrap()
+ .super_key_exists_in_db_for_user(&mut keystore_db, &legacy_importer, USER_ID)
+ .unwrap());
+
+ assert!(!keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+ assert!(!keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_BOOT_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+ }
+
+ fn test_user_reset(locked: bool) {
+ let pw: Password = generate_password_blob();
+ let (skm, mut keystore_db, legacy_importer) = setup_test(&pw);
+ assert_unlocked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was not unlocked after initialization!",
+ );
+
+ assert!(make_test_key_entry(
+ &mut keystore_db,
+ Domain::APP,
+ USER_ID.into(),
+ TEST_KEY_ALIAS,
+ None
+ )
+ .is_ok());
+ assert!(make_bootlevel_key_entry(
+ &mut keystore_db,
+ Domain::APP,
+ USER_ID.into(),
+ TEST_BOOT_KEY_ALIAS,
+ false
+ )
+ .is_ok());
+ assert!(keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+ assert!(keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_BOOT_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+
+ if locked {
+ skm.write().unwrap().data.user_keys.clear();
+ assert_locked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "Clearing the cache did not lock the user!",
+ );
+ assert!(skm
+ .write()
+ .unwrap()
+ .reset_user(&mut keystore_db, &legacy_importer, USER_ID)
+ .is_err());
+ assert_locked(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "User state should not have changed!",
+ );
+
+ // Keys should still exist.
+ assert!(keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+ assert!(keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_BOOT_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+ } else {
+ assert!(skm
+ .write()
+ .unwrap()
+ .reset_user(&mut keystore_db, &legacy_importer, USER_ID)
+ .is_ok());
+ assert_uninitialized(
+ &skm,
+ &mut keystore_db,
+ &legacy_importer,
+ USER_ID,
+ "The user was not reset!",
+ );
+ assert!(!skm
+ .write()
+ .unwrap()
+ .super_key_exists_in_db_for_user(&mut keystore_db, &legacy_importer, USER_ID)
+ .unwrap());
+
+ // Auth bound key should no longer exist.
+ assert!(!keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+ assert!(keystore_db
+ .key_exists(Domain::APP, USER_ID.into(), TEST_BOOT_KEY_ALIAS, KeyType::Client)
+ .unwrap());
+ }
+ }
+
+ #[test]
+ fn test_remove_unlocked_user() {
+ test_user_removal(false);
+ }
+
+ #[test]
+ fn test_remove_locked_user() {
+ test_user_removal(true);
+ }
+
+ #[test]
+ fn test_reset_unlocked_user() {
+ test_user_reset(false);
+ }
+
+ #[test]
+ fn test_reset_locked_user() {
+ test_user_reset(true);
+ }
+}
diff --git a/keystore2/test_utils/key_generations.rs b/keystore2/test_utils/key_generations.rs
index 3422ce8..ff40aa1 100644
--- a/keystore2/test_utils/key_generations.rs
+++ b/keystore2/test_utils/key_generations.rs
@@ -16,6 +16,10 @@
use anyhow::Result;
+use core::ops::Range;
+use std::collections::HashSet;
+use std::fmt::Write;
+
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
Algorithm::Algorithm, BlockMode::BlockMode, Digest::Digest, EcCurve::EcCurve,
ErrorCode::ErrorCode, HardwareAuthenticatorType::HardwareAuthenticatorType,
@@ -1085,3 +1089,23 @@
Err(e) => Err(e),
}
}
+
+/// Helper method to import AES keys `total_count` of times.
+pub fn import_aes_keys(
+ sec_level: &binder::Strong<dyn IKeystoreSecurityLevel>,
+ alias_prefix: String,
+ total_count: Range<i32>,
+) -> binder::Result<HashSet<String>> {
+ let mut imported_key_aliases = HashSet::new();
+
+ // Import Total number of keys with given alias prefix.
+ for count in total_count {
+ let mut alias = String::new();
+ write!(alias, "{}_{}", alias_prefix, count).unwrap();
+ imported_key_aliases.insert(alias.clone());
+
+ import_aes_key(sec_level, Domain::APP, -1, Some(alias))?;
+ }
+
+ Ok(imported_key_aliases)
+}
diff --git a/keystore2/tests/keystore2_client_list_entries_tests.rs b/keystore2/tests/keystore2_client_list_entries_tests.rs
index 3b656c3..809c01f 100644
--- a/keystore2/tests/keystore2_client_list_entries_tests.rs
+++ b/keystore2/tests/keystore2_client_list_entries_tests.rs
@@ -23,7 +23,7 @@
KeyPermission::KeyPermission, ResponseCode::ResponseCode,
};
-use crate::keystore2_client_test_utils::delete_app_key;
+use crate::keystore2_client_test_utils::{delete_all_entries, delete_app_key, verify_aliases};
use keystore2_test_utils::{get_keystore_service, key_generations, key_generations::Error, run_as};
/// Try to find a key with given key parameters using `listEntries` API.
@@ -251,3 +251,464 @@
})
};
}
+
+/// Import large number of Keystore entries with long aliases such that the
+/// aliases list would exceed the binder transaction size limit.
+/// Try to list aliases of all the entries in the keystore using `listEntriesBatched` API.
+#[test]
+fn keystore2_list_entries_batched_with_long_aliases_success() {
+ static CLIENT_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+
+ const USER_ID: u32 = 92;
+ const APPLICATION_ID: u32 = 10002;
+ static CLIENT_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
+ static CLIENT_GID: u32 = CLIENT_UID;
+
+ unsafe {
+ run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
+ let keystore2 = get_keystore_service();
+ let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
+
+ // Make sure there are no keystore entries exist before adding new entries.
+ delete_all_entries(&keystore2);
+
+ // Import 100 keys with aliases of length 6000.
+ let mut imported_key_aliases =
+ key_generations::import_aes_keys(&sec_level, "X".repeat(6000), 1..101).unwrap();
+ assert_eq!(
+ keystore2.getNumberOfEntries(Domain::APP, -1).unwrap(),
+ 100,
+ "Error while importing keys"
+ );
+
+ let mut start_past_alias = None;
+ let mut alias;
+ while !imported_key_aliases.is_empty() {
+ let key_descriptors =
+ keystore2.listEntriesBatched(Domain::APP, -1, start_past_alias).unwrap();
+
+ // Check retrieved key entries list is a subset of imported keys list.
+ assert!(key_descriptors
+ .iter()
+ .all(|key| imported_key_aliases.contains(key.alias.as_ref().unwrap())));
+
+ alias = key_descriptors.last().unwrap().alias.clone().unwrap();
+ start_past_alias = Some(alias.as_ref());
+ // Delete the listed key entries from imported keys list.
+ key_descriptors.into_iter().map(|key| key.alias.unwrap()).for_each(|alias| {
+ assert!(imported_key_aliases.remove(&alias));
+ });
+ }
+
+ assert!(imported_key_aliases.is_empty());
+ delete_all_entries(&keystore2);
+ assert_eq!(
+ keystore2.getNumberOfEntries(Domain::APP, -1).unwrap(),
+ 0,
+ "Error while doing cleanup"
+ );
+ })
+ };
+}
+
+/// Import keys from multiple processes with same user context and try to list the keystore entries
+/// using `listEntriesBatched` API.
+/// - Create two processes sharing user-id.
+/// - From process-1, import 3 keys and try to list the keys using `listEntriesBatched`
+/// without `startingPastAlias`, it should list all the 3 entries.
+/// - From process-2, import another 5 keys and try to list the keys using `listEntriesBatched`
+/// with the alias of the last key listed in process-1 as `startingPastAlias`. It should list
+/// all the entries whose alias is greater than the provided `startingPastAlias`.
+/// - From process-2 try to list all entries accessible to it by using `listEntriesBatched` with
+/// `startingPastAlias` as None. It should list all the keys imported in process-1 and process-2.
+#[test]
+fn keystore2_list_entries_batched_with_multi_procs_success() {
+ static CLIENT_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+
+ const USER_ID: u32 = 92;
+ const APPLICATION_ID: u32 = 10002;
+ static CLIENT_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
+ static CLIENT_GID: u32 = CLIENT_UID;
+ static ALIAS_PREFIX: &str = "key_test_batch_list";
+
+ unsafe {
+ run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
+ let keystore2 = get_keystore_service();
+ let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
+
+ // Make sure there are no keystore entries exist before adding new entries.
+ delete_all_entries(&keystore2);
+
+ // Import 3 keys with below aliases -
+ // [key_test_batch_list_1, key_test_batch_list_2, key_test_batch_list_3]
+ let imported_key_aliases =
+ key_generations::import_aes_keys(&sec_level, ALIAS_PREFIX.to_string(), 1..4)
+ .unwrap();
+ assert_eq!(
+ keystore2.getNumberOfEntries(Domain::APP, -1).unwrap(),
+ 3,
+ "Error while importing keys"
+ );
+
+ // List all entries in keystore for this user-id.
+ let key_descriptors = keystore2.listEntriesBatched(Domain::APP, -1, None).unwrap();
+ assert_eq!(key_descriptors.len(), 3);
+
+ // Makes sure all listed aliases are matching with imported keys aliases.
+ assert!(key_descriptors
+ .iter()
+ .all(|key| imported_key_aliases.contains(key.alias.as_ref().unwrap())));
+ })
+ };
+
+ unsafe {
+ run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
+ let keystore2 = get_keystore_service();
+ let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
+
+ // Import another 5 keys with below aliases -
+ // [ key_test_batch_list_4, key_test_batch_list_5, key_test_batch_list_6,
+ // key_test_batch_list_7, key_test_batch_list_8 ]
+ let mut imported_key_aliases =
+ key_generations::import_aes_keys(&sec_level, ALIAS_PREFIX.to_string(), 4..9)
+ .unwrap();
+
+ // Above context already 3 keys are imported, in this context 5 keys are imported,
+ // total 8 keystore entries are expected to be present in Keystore for this user-id.
+ assert_eq!(
+ keystore2.getNumberOfEntries(Domain::APP, -1).unwrap(),
+ 8,
+ "Error while importing keys"
+ );
+
+ // List keystore entries with `start_past_alias` as "key_test_batch_list_3".
+ // `listEntriesBatched` should list all the keystore entries with
+ // alias > "key_test_batch_list_3".
+ let key_descriptors = keystore2
+ .listEntriesBatched(Domain::APP, -1, Some("key_test_batch_list_3"))
+ .unwrap();
+ assert_eq!(key_descriptors.len(), 5);
+
+ // Make sure above listed aliases are matching with imported keys aliases.
+ assert!(key_descriptors
+ .iter()
+ .all(|key| imported_key_aliases.contains(key.alias.as_ref().unwrap())));
+
+ // List all keystore entries with `start_past_alias` as `None`.
+ // `listEntriesBatched` should list all the keystore entries.
+ let key_descriptors = keystore2.listEntriesBatched(Domain::APP, -1, None).unwrap();
+ assert_eq!(key_descriptors.len(), 8);
+
+ // Include previously imported keys aliases as well
+ imported_key_aliases.insert(ALIAS_PREFIX.to_owned() + "_1");
+ imported_key_aliases.insert(ALIAS_PREFIX.to_owned() + "_2");
+ imported_key_aliases.insert(ALIAS_PREFIX.to_owned() + "_3");
+
+ // Make sure all the above listed aliases are matching with imported keys aliases.
+ assert!(key_descriptors
+ .iter()
+ .all(|key| imported_key_aliases.contains(key.alias.as_ref().unwrap())));
+
+ delete_all_entries(&keystore2);
+ assert_eq!(
+ keystore2.getNumberOfEntries(Domain::APP, -1).unwrap(),
+ 0,
+ "Error while doing cleanup"
+ );
+ })
+ };
+}
+
+#[test]
+fn keystore2_list_entries_batched_with_empty_keystore_success() {
+ static CLIENT_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+
+ const USER_ID: u32 = 92;
+ const APPLICATION_ID: u32 = 10002;
+ static CLIENT_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
+ static CLIENT_GID: u32 = CLIENT_UID;
+
+ unsafe {
+ run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
+ let keystore2 = get_keystore_service();
+
+ // Make sure there are no keystore entries exist before adding new entries.
+ delete_all_entries(&keystore2);
+
+ // List all entries in keystore for this user-id, pass startingPastAlias = None
+ let key_descriptors = keystore2.listEntriesBatched(Domain::APP, -1, None).unwrap();
+ assert_eq!(key_descriptors.len(), 0);
+
+ // List all entries in keystore for this user-id, pass startingPastAlias = <random value>
+ let key_descriptors =
+ keystore2.listEntriesBatched(Domain::APP, -1, Some("startingPastAlias")).unwrap();
+ assert_eq!(key_descriptors.len(), 0);
+ })
+ };
+}
+
+/// Import a key with SELINUX as domain, list aliases using `listEntriesBatched`.
+/// Test should successfully list the imported key.
+#[test]
+fn keystore2_list_entries_batched_with_selinux_domain_success() {
+ let keystore2 = get_keystore_service();
+ let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
+
+ let alias = "test_selinux_key_list_alias_batched";
+ let _result = keystore2.deleteKey(&KeyDescriptor {
+ domain: Domain::SELINUX,
+ nspace: key_generations::SELINUX_SHELL_NAMESPACE,
+ alias: Some(alias.to_string()),
+ blob: None,
+ });
+
+ let initial_count = keystore2
+ .getNumberOfEntries(Domain::SELINUX, key_generations::SELINUX_SHELL_NAMESPACE)
+ .unwrap();
+
+ key_generations::import_aes_key(
+ &sec_level,
+ Domain::SELINUX,
+ key_generations::SELINUX_SHELL_NAMESPACE,
+ Some(alias.to_string()),
+ )
+ .unwrap();
+
+ assert_eq!(
+ keystore2
+ .getNumberOfEntries(Domain::SELINUX, key_generations::SELINUX_SHELL_NAMESPACE)
+ .unwrap(),
+ initial_count + 1,
+ "Error while getting number of keystore entries accessible."
+ );
+
+ let key_descriptors = keystore2
+ .listEntriesBatched(Domain::SELINUX, key_generations::SELINUX_SHELL_NAMESPACE, None)
+ .unwrap();
+ assert_eq!(key_descriptors.len(), (initial_count + 1) as usize);
+
+ let count =
+ key_descriptors.into_iter().map(|key| key.alias.unwrap()).filter(|a| a == alias).count();
+ assert_eq!(count, 1);
+
+ keystore2
+ .deleteKey(&KeyDescriptor {
+ domain: Domain::SELINUX,
+ nspace: key_generations::SELINUX_SHELL_NAMESPACE,
+ alias: Some(alias.to_string()),
+ blob: None,
+ })
+ .unwrap();
+}
+
+#[test]
+fn keystore2_list_entries_batched_validate_count_and_order_success() {
+ static CLIENT_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+
+ const USER_ID: u32 = 92;
+ const APPLICATION_ID: u32 = 10002;
+ static CLIENT_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
+ static CLIENT_GID: u32 = CLIENT_UID;
+ static ALIAS_PREFIX: &str = "key_test_batch_list";
+
+ unsafe {
+ run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
+ let keystore2 = get_keystore_service();
+ let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
+
+ // Make sure there are no keystore entries exist before adding new entries.
+ delete_all_entries(&keystore2);
+
+ // Import keys with below mentioned aliases -
+ // [
+ // key_test_batch_list_1,
+ // key_test_batch_list_2,
+ // key_test_batch_list_3,
+ // key_test_batch_list_4,
+ // key_test_batch_list_5,
+ // key_test_batch_list_10,
+ // key_test_batch_list_11,
+ // key_test_batch_list_12,
+ // key_test_batch_list_21,
+ // key_test_batch_list_22,
+ // ]
+ let _imported_key_aliases =
+ key_generations::import_aes_keys(&sec_level, ALIAS_PREFIX.to_string(), 1..6)
+ .unwrap();
+ assert_eq!(
+ keystore2.getNumberOfEntries(Domain::APP, -1).unwrap(),
+ 5,
+ "Error while importing keys"
+ );
+ let _imported_key_aliases =
+ key_generations::import_aes_keys(&sec_level, ALIAS_PREFIX.to_string(), 10..13)
+ .unwrap();
+ assert_eq!(
+ keystore2.getNumberOfEntries(Domain::APP, -1).unwrap(),
+ 8,
+ "Error while importing keys"
+ );
+ let _imported_key_aliases =
+ key_generations::import_aes_keys(&sec_level, ALIAS_PREFIX.to_string(), 21..23)
+ .unwrap();
+ assert_eq!(
+ keystore2.getNumberOfEntries(Domain::APP, -1).unwrap(),
+ 10,
+ "Error while importing keys"
+ );
+
+ // List the aliases using given `startingPastAlias` and verify the listed
+ // aliases with the expected list of aliases.
+ verify_aliases(&keystore2, Some(format!("{}{}", ALIAS_PREFIX, "_5").as_str()), vec![]);
+
+ verify_aliases(
+ &keystore2,
+ Some(format!("{}{}", ALIAS_PREFIX, "_4").as_str()),
+ vec![ALIAS_PREFIX.to_owned() + "_5"],
+ );
+
+ verify_aliases(
+ &keystore2,
+ Some(format!("{}{}", ALIAS_PREFIX, "_3").as_str()),
+ vec![ALIAS_PREFIX.to_owned() + "_4", ALIAS_PREFIX.to_owned() + "_5"],
+ );
+
+ verify_aliases(
+ &keystore2,
+ Some(format!("{}{}", ALIAS_PREFIX, "_2").as_str()),
+ vec![
+ ALIAS_PREFIX.to_owned() + "_21",
+ ALIAS_PREFIX.to_owned() + "_22",
+ ALIAS_PREFIX.to_owned() + "_3",
+ ALIAS_PREFIX.to_owned() + "_4",
+ ALIAS_PREFIX.to_owned() + "_5",
+ ],
+ );
+
+ verify_aliases(
+ &keystore2,
+ Some(format!("{}{}", ALIAS_PREFIX, "_1").as_str()),
+ vec![
+ ALIAS_PREFIX.to_owned() + "_10",
+ ALIAS_PREFIX.to_owned() + "_11",
+ ALIAS_PREFIX.to_owned() + "_12",
+ ALIAS_PREFIX.to_owned() + "_2",
+ ALIAS_PREFIX.to_owned() + "_21",
+ ALIAS_PREFIX.to_owned() + "_22",
+ ALIAS_PREFIX.to_owned() + "_3",
+ ALIAS_PREFIX.to_owned() + "_4",
+ ALIAS_PREFIX.to_owned() + "_5",
+ ],
+ );
+
+ verify_aliases(
+ &keystore2,
+ Some(ALIAS_PREFIX),
+ vec![
+ ALIAS_PREFIX.to_owned() + "_1",
+ ALIAS_PREFIX.to_owned() + "_10",
+ ALIAS_PREFIX.to_owned() + "_11",
+ ALIAS_PREFIX.to_owned() + "_12",
+ ALIAS_PREFIX.to_owned() + "_2",
+ ALIAS_PREFIX.to_owned() + "_21",
+ ALIAS_PREFIX.to_owned() + "_22",
+ ALIAS_PREFIX.to_owned() + "_3",
+ ALIAS_PREFIX.to_owned() + "_4",
+ ALIAS_PREFIX.to_owned() + "_5",
+ ],
+ );
+
+ verify_aliases(
+ &keystore2,
+ None,
+ vec![
+ ALIAS_PREFIX.to_owned() + "_1",
+ ALIAS_PREFIX.to_owned() + "_10",
+ ALIAS_PREFIX.to_owned() + "_11",
+ ALIAS_PREFIX.to_owned() + "_12",
+ ALIAS_PREFIX.to_owned() + "_2",
+ ALIAS_PREFIX.to_owned() + "_21",
+ ALIAS_PREFIX.to_owned() + "_22",
+ ALIAS_PREFIX.to_owned() + "_3",
+ ALIAS_PREFIX.to_owned() + "_4",
+ ALIAS_PREFIX.to_owned() + "_5",
+ ],
+ );
+ })
+ };
+}
+
+/// Try to list the key entries with domain SELINUX from user context where user doesn't possesses
+/// `GET_INFO` permission for specified namespace. Test should fail to list key entries with error
+/// response code `PERMISSION_DENIED`.
+#[test]
+fn keystore2_list_entries_batched_fails_perm_denied() {
+ let auid = 91 * AID_USER_OFFSET + 10001;
+ let agid = 91 * AID_USER_OFFSET + 10001;
+ static TARGET_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+
+ unsafe {
+ run_as::run_as(TARGET_CTX, Uid::from_raw(auid), Gid::from_raw(agid), move || {
+ let keystore2 = get_keystore_service();
+
+ let result = key_generations::map_ks_error(keystore2.listEntriesBatched(
+ Domain::SELINUX,
+ key_generations::SELINUX_SHELL_NAMESPACE,
+ None,
+ ));
+ assert!(result.is_err());
+ assert_eq!(Error::Rc(ResponseCode::PERMISSION_DENIED), result.unwrap_err());
+ })
+ };
+}
+
+/// Try to list key entries with domain BLOB. Test should fail with error response code
+/// `INVALID_ARGUMENT`.
+#[test]
+fn keystore2_list_entries_batched_fails_invalid_arg() {
+ let keystore2 = get_keystore_service();
+
+ let result = key_generations::map_ks_error(keystore2.listEntriesBatched(
+ Domain::BLOB,
+ key_generations::SELINUX_SHELL_NAMESPACE,
+ None,
+ ));
+ assert!(result.is_err());
+ assert_eq!(Error::Rc(ResponseCode::INVALID_ARGUMENT), result.unwrap_err());
+}
+
+/// Try to get the number of key entries with domain SELINUX from user context where user doesn't
+/// possesses `GET_INFO` permission for specified namespace. Test should fail to list key entries
+/// with error response code `PERMISSION_DENIED`.
+#[test]
+fn keystore2_get_number_of_entries_fails_perm_denied() {
+ let auid = 91 * AID_USER_OFFSET + 10001;
+ let agid = 91 * AID_USER_OFFSET + 10001;
+ static TARGET_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+
+ unsafe {
+ run_as::run_as(TARGET_CTX, Uid::from_raw(auid), Gid::from_raw(agid), move || {
+ let keystore2 = get_keystore_service();
+
+ let result = key_generations::map_ks_error(
+ keystore2
+ .getNumberOfEntries(Domain::SELINUX, key_generations::SELINUX_SHELL_NAMESPACE),
+ );
+ assert!(result.is_err());
+ assert_eq!(Error::Rc(ResponseCode::PERMISSION_DENIED), result.unwrap_err());
+ })
+ };
+}
+
+/// Try to get number of key entries with domain BLOB. Test should fail with error response code
+/// `INVALID_ARGUMENT`.
+#[test]
+fn keystore2_get_number_of_entries_fails_invalid_arg() {
+ let keystore2 = get_keystore_service();
+
+ let result = key_generations::map_ks_error(
+ keystore2.getNumberOfEntries(Domain::BLOB, key_generations::SELINUX_SHELL_NAMESPACE),
+ );
+ assert!(result.is_err());
+ assert_eq!(Error::Rc(ResponseCode::INVALID_ARGUMENT), result.unwrap_err());
+}
diff --git a/keystore2/tests/keystore2_client_test_utils.rs b/keystore2/tests/keystore2_client_test_utils.rs
index 58e6b7d..07c2183 100644
--- a/keystore2/tests/keystore2_client_test_utils.rs
+++ b/keystore2/tests/keystore2_client_test_utils.rs
@@ -376,6 +376,17 @@
})
}
+/// Deletes all entries from keystore.
+pub fn delete_all_entries(keystore2: &binder::Strong<dyn IKeystoreService>) {
+ while keystore2.getNumberOfEntries(Domain::APP, -1).unwrap() != 0 {
+ let key_descriptors = keystore2.listEntries(Domain::APP, -1).unwrap();
+ key_descriptors.into_iter().map(|key| key.alias.unwrap()).for_each(|alias| {
+ delete_app_key(keystore2, &alias).unwrap();
+ });
+ }
+ assert!(keystore2.getNumberOfEntries(Domain::APP, -1).unwrap() == 0);
+}
+
/// Encrypt the secure key with given transport key.
pub fn encrypt_secure_key(
sec_level: &binder::Strong<dyn IKeystoreSecurityLevel>,
@@ -417,3 +428,19 @@
Ok(encoded.to_vec())
}
+
+/// List aliases using given `startingPastAlias` and verify that the fetched list is matching with
+/// the expected list of aliases.
+pub fn verify_aliases(
+ keystore2: &binder::Strong<dyn IKeystoreService>,
+ starting_past_alias: Option<&str>,
+ expected_aliases: Vec<String>,
+) {
+ let key_descriptors =
+ keystore2.listEntriesBatched(Domain::APP, -1, starting_past_alias).unwrap();
+
+ assert_eq!(key_descriptors.len(), expected_aliases.len());
+ assert!(key_descriptors
+ .iter()
+ .all(|key| expected_aliases.contains(key.alias.as_ref().unwrap())));
+}
diff --git a/provisioner/rkp_factory_extraction_lib.cpp b/provisioner/rkp_factory_extraction_lib.cpp
index 8db62e6..ab7d17c 100644
--- a/provisioner/rkp_factory_extraction_lib.cpp
+++ b/provisioner/rkp_factory_extraction_lib.cpp
@@ -195,7 +195,11 @@
protectedData, *eekChain, eekId,
hwInfo.supportedEekCurve, irpc, challenge);
- std::cout << "Self test successful." << std::endl;
+ if (!result) {
+ std::cerr << "Self test failed for IRemotelyProvisionedComponent '" << componentName
+ << "'. Error message: '" << result.message() << "'." << std::endl;
+ exit(-1);
+ }
}
CborResult<Array> composeCertificateRequestV3(const std::vector<uint8_t>& csr) {
@@ -220,7 +224,7 @@
}
CborResult<cppbor::Array> getCsrV3(std::string_view componentName,
- IRemotelyProvisionedComponent* irpc) {
+ IRemotelyProvisionedComponent* irpc, bool selfTest) {
std::vector<uint8_t> csr;
std::vector<MacedPublicKey> emptyKeys;
const std::vector<uint8_t> challenge = generateChallenge();
@@ -232,32 +236,20 @@
exit(-1);
}
+ if (selfTest) {
+ auto result = verifyFactoryCsr(/*keysToSign=*/cppbor::Array(), csr, irpc, challenge);
+ if (!result) {
+ std::cerr << "Self test failed for IRemotelyProvisionedComponent '" << componentName
+ << "'. Error message: '" << result.message() << "'." << std::endl;
+ exit(-1);
+ }
+ }
+
return composeCertificateRequestV3(csr);
}
-void selfTestGetCsrV3(std::string_view componentName, IRemotelyProvisionedComponent* irpc) {
- std::vector<uint8_t> csr;
- std::vector<MacedPublicKey> emptyKeys;
- const std::vector<uint8_t> challenge = generateChallenge();
-
- auto status = irpc->generateCertificateRequestV2(emptyKeys, challenge, &csr);
- if (!status.isOk()) {
- std::cerr << "Bundle extraction failed for '" << componentName
- << "'. Error code: " << status.getServiceSpecificError() << "." << std::endl;
- exit(-1);
- }
-
- auto result = verifyFactoryCsr(/*keysToSign=*/cppbor::Array(), csr, irpc, challenge);
- if (!result) {
- std::cerr << "Self test failed for '" << componentName
- << "'. Error message: " << result.message() << "." << std::endl;
- exit(-1);
- }
-
- std::cout << "Self test successful." << std::endl;
-}
-
-CborResult<Array> getCsr(std::string_view componentName, IRemotelyProvisionedComponent* irpc) {
+CborResult<Array> getCsr(std::string_view componentName, IRemotelyProvisionedComponent* irpc,
+ bool selfTest) {
RpcHardwareInfo hwInfo;
auto status = irpc->getHardwareInfo(&hwInfo);
if (!status.isOk()) {
@@ -267,24 +259,11 @@
}
if (hwInfo.versionNumber < kVersionWithoutSuperencryption) {
+ if (selfTest) {
+ selfTestGetCsrV1(componentName, irpc);
+ }
return getCsrV1(componentName, irpc);
} else {
- return getCsrV3(componentName, irpc);
- }
-}
-
-void selfTestGetCsr(std::string_view componentName, IRemotelyProvisionedComponent* irpc) {
- RpcHardwareInfo hwInfo;
- auto status = irpc->getHardwareInfo(&hwInfo);
- if (!status.isOk()) {
- std::cerr << "Failed to get hardware info for '" << componentName
- << "'. Error code: " << status.getServiceSpecificError() << "." << std::endl;
- exit(-1);
- }
-
- if (hwInfo.versionNumber < kVersionWithoutSuperencryption) {
- selfTestGetCsrV1(componentName, irpc);
- } else {
- selfTestGetCsrV3(componentName, irpc);
+ return getCsrV3(componentName, irpc, selfTest);
}
}
diff --git a/provisioner/rkp_factory_extraction_lib.h b/provisioner/rkp_factory_extraction_lib.h
index a218338..ae8ea6b 100644
--- a/provisioner/rkp_factory_extraction_lib.h
+++ b/provisioner/rkp_factory_extraction_lib.h
@@ -46,7 +46,8 @@
// what went wrong.
CborResult<cppbor::Array>
getCsr(std::string_view componentName,
- aidl::android::hardware::security::keymint::IRemotelyProvisionedComponent* irpc);
+ aidl::android::hardware::security::keymint::IRemotelyProvisionedComponent* irpc,
+ bool selfTest);
// Generates a test certificate chain and validates it, exiting the process on error.
void selfTestGetCsr(
diff --git a/provisioner/rkp_factory_extraction_lib_test.cpp b/provisioner/rkp_factory_extraction_lib_test.cpp
index 72d7b71..3fe88da 100644
--- a/provisioner/rkp_factory_extraction_lib_test.cpp
+++ b/provisioner/rkp_factory_extraction_lib_test.cpp
@@ -180,7 +180,8 @@
SetArgPointee<6>(kFakeMac), //
Return(ByMove(ScopedAStatus::ok())))); //
- auto [csr, csrErrMsg] = getCsr("mock component name", mockRpc.get());
+ auto [csr, csrErrMsg] = getCsr("mock component name", mockRpc.get(),
+ /*selfTest=*/false);
ASSERT_THAT(csr, NotNull()) << csrErrMsg;
ASSERT_THAT(csr->asArray(), Pointee(Property(&Array::size, Eq(4))));
@@ -249,7 +250,8 @@
.WillOnce(DoAll(SaveArg<1>(&challenge), SetArgPointee<2>(kCsr),
Return(ByMove(ScopedAStatus::ok()))));
- auto [csr, csrErrMsg] = getCsr("mock component name", mockRpc.get());
+ auto [csr, csrErrMsg] = getCsr("mock component name", mockRpc.get(),
+ /*selfTest=*/false);
ASSERT_THAT(csr, NotNull()) << csrErrMsg;
ASSERT_THAT(csr, Pointee(Property(&Array::size, Eq(5))));
diff --git a/provisioner/rkp_factory_extraction_tool.cpp b/provisioner/rkp_factory_extraction_tool.cpp
index 2aeabe0..5ba777e 100644
--- a/provisioner/rkp_factory_extraction_tool.cpp
+++ b/provisioner/rkp_factory_extraction_tool.cpp
@@ -35,10 +35,10 @@
using namespace cppcose;
DEFINE_string(output_format, "build+csr", "How to format the output. Defaults to 'build+csr'.");
-DEFINE_bool(self_test, false,
- "If true, the tool does not output CSR data, but instead performs a self-test, "
- "validating a test payload for correctness. This may be used to verify a device on the "
- "factory line before attempting to upload the output to the device info service.");
+DEFINE_bool(self_test, true,
+ "If true, this tool performs a self-test, validating the payload for correctness. "
+ "This checks that the device on the factory line is producing valid output "
+ "before attempting to upload the output to the device info service.");
namespace {
@@ -81,17 +81,13 @@
exit(-1);
}
- if (FLAGS_self_test) {
- selfTestGetCsr(name, rkp_service.get());
- } else {
- auto [request, errMsg] = getCsr(name, rkp_service.get());
- if (!request) {
- std::cerr << "Unable to build CSR for '" << fullName << ": " << errMsg << std::endl;
- exit(-1);
- }
-
- writeOutput(std::string(name), *request);
+ auto [request, errMsg] = getCsr(name, rkp_service.get(), FLAGS_self_test);
+ if (!request) {
+ std::cerr << "Unable to build CSR for '" << fullName << ": " << errMsg << std::endl;
+ exit(-1);
}
+
+ writeOutput(std::string(name), *request);
}
} // namespace
diff --git a/provisioner/support/Android.bp b/provisioner/support/Android.bp
index 778b1e0..52b204f 100644
--- a/provisioner/support/Android.bp
+++ b/provisioner/support/Android.bp
@@ -31,7 +31,6 @@
"libbase",
"libbinder",
"libutils",
- "libvintf",
],
cflags: [
"-Wall",
diff --git a/provisioner/support/rkpd_client.cpp b/provisioner/support/rkpd_client.cpp
index 0643457..de1e3bb 100644
--- a/provisioner/support/rkpd_client.cpp
+++ b/provisioner/support/rkpd_client.cpp
@@ -26,7 +26,6 @@
#include <binder/IServiceManager.h>
#include <binder/Status.h>
#include <rkp/support/rkpd_client.h>
-#include <vintf/VintfObject.h>
namespace android::security::rkp::support {
namespace {
@@ -61,12 +60,10 @@
}
std::optional<String16> findRpcNameById(std::string_view targetRpcId) {
- auto deviceManifest = vintf::VintfObject::GetDeviceHalManifest();
- auto instances = deviceManifest->getAidlInstances("android.hardware.security.keymint",
- "IRemotelyProvisionedComponent");
- for (const std::string& instance : instances) {
- auto rpcName =
- IRemotelyProvisionedComponent::descriptor + String16("/") + String16(instance.c_str());
+ auto instances = android::defaultServiceManager()->getDeclaredInstances(
+ IRemotelyProvisionedComponent::descriptor);
+ for (const auto& instance : instances) {
+ auto rpcName = IRemotelyProvisionedComponent::descriptor + String16("/") + instance;
sp<IRemotelyProvisionedComponent> rpc =
android::waitForService<IRemotelyProvisionedComponent>(rpcName);
