[automerger skipped] Validate artifacts before calling odrefresh --compile. am: 3469908edc -s ours
am skip reason: Merged-In I347794f456b9809f25489156a2370d1be93d40cd with SHA-1 7bf6e0a053 is already in history
Original change: https://googleplex-android-review.googlesource.com/c/platform/system/security/+/16511960
Change-Id: I4649f465652fe5603f50a9e091a7719383d72412
diff --git a/keystore-engine/keystore2_engine.cpp b/keystore-engine/keystore2_engine.cpp
index 69d2ca6..49c5e9a 100644
--- a/keystore-engine/keystore2_engine.cpp
+++ b/keystore-engine/keystore2_engine.cpp
@@ -23,11 +23,13 @@
#include <private/android_filesystem_config.h>
+#include <openssl/bio.h>
#include <openssl/bn.h>
#include <openssl/ec.h>
#include <openssl/ec_key.h>
#include <openssl/ecdsa.h>
#include <openssl/engine.h>
+#include <openssl/pem.h>
#include <openssl/rsa.h>
#include <openssl/x509.h>
@@ -327,6 +329,31 @@
return 1;
}
+bssl::UniquePtr<EVP_PKEY> extractPubKey(const std::vector<uint8_t>& cert_bytes) {
+ const uint8_t* p = cert_bytes.data();
+ bssl::UniquePtr<X509> decoded_cert(d2i_X509(nullptr, &p, cert_bytes.size()));
+ if (!decoded_cert) {
+ LOG(INFO) << AT << "Could not decode the cert, trying decoding as PEM";
+ bssl::UniquePtr<BIO> cert_bio(BIO_new_mem_buf(cert_bytes.data(), cert_bytes.size()));
+ if (!cert_bio) {
+ LOG(ERROR) << AT << "Failed to create BIO";
+ return {};
+ }
+ decoded_cert =
+ bssl::UniquePtr<X509>(PEM_read_bio_X509(cert_bio.get(), nullptr, nullptr, nullptr));
+ }
+ if (!decoded_cert) {
+ LOG(ERROR) << AT << "Could not decode the cert.";
+ return {};
+ }
+ bssl::UniquePtr<EVP_PKEY> pub_key(X509_get_pubkey(decoded_cert.get()));
+ if (!pub_key) {
+ LOG(ERROR) << AT << "Could not extract public key.";
+ return {};
+ }
+ return pub_key;
+}
+
} // namespace
/* EVP_PKEY_from_keystore returns an |EVP_PKEY| that contains either an RSA or
@@ -383,13 +410,7 @@
return nullptr;
}
- const uint8_t* p = response.metadata.certificate->data();
- bssl::UniquePtr<X509> x509(d2i_X509(nullptr, &p, response.metadata.certificate->size()));
- if (!x509) {
- LOG(ERROR) << AT << "Failed to parse x509 certificate.";
- return nullptr;
- }
- bssl::UniquePtr<EVP_PKEY> pkey(X509_get_pubkey(x509.get()));
+ auto pkey = extractPubKey(*response.metadata.certificate);
if (!pkey) {
LOG(ERROR) << AT << "Failed to extract public key.";
return nullptr;
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs
index 29a3f0b..b7b40f7 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -633,7 +633,7 @@
let hat_and_last_off_body = if need_auth_token {
let hat_and_last_off_body = Self::find_auth_token(|hat: &AuthTokenEntry| {
- if let (Some(auth_type), true) = (user_auth_type, has_sids) {
+ if let (Some(auth_type), true) = (user_auth_type, timeout_bound) {
hat.satisfies(&user_secure_ids, auth_type)
} else {
unlocked_device_required
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index 32067b9..a064f65 100644
--- a/keystore2/src/metrics_store.rs
+++ b/keystore2/src/metrics_store.rs
@@ -17,7 +17,7 @@
//! stores them in an in-memory store.
//! 2. Returns the collected metrics when requested by the statsd proxy.
-use crate::error::get_error_code;
+use crate::error::{get_error_code, Error};
use crate::globals::DB;
use crate::key_parameter::KeyParameterValue as KsKeyParamValue;
use crate::operation::Outcome;
@@ -44,6 +44,7 @@
RkpPoolStats::RkpPoolStats, SecurityLevel::SecurityLevel as MetricsSecurityLevel,
Storage::Storage as MetricsStorage,
};
+use android_system_keystore2::aidl::android::system::keystore2::ResponseCode::ResponseCode;
use anyhow::{Context, Result};
use keystore2_system_property::{write, PropertyWatcher, PropertyWatcherError};
use lazy_static::lazy_static;
@@ -560,10 +561,14 @@
fn pull_attestation_pool_stats() -> Result<Vec<KeystoreAtom>> {
let mut atoms = Vec::<KeystoreAtom>::new();
for sec_level in &[SecurityLevel::TRUSTED_ENVIRONMENT, SecurityLevel::STRONGBOX] {
+ // set the expired_by date to be three days from now
let expired_by = SystemTime::now()
+ .checked_add(Duration::from_secs(60 * 60 * 24 * 3))
+ .ok_or(Error::Rc(ResponseCode::SYSTEM_ERROR))
+ .context("In pull_attestation_pool_stats: Failed to compute expired by system time.")?
.duration_since(UNIX_EPOCH)
- .unwrap_or_else(|_| Duration::new(0, 0))
- .as_secs() as i64;
+ .context("In pull_attestation_pool_stats: Failed to compute expired by duration.")?
+ .as_millis() as i64;
let result = get_pool_status(expired_by, *sec_level);
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 5cb3afc..1b2e348 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -54,9 +54,11 @@
Domain::Domain, EphemeralStorageKeyResponse::EphemeralStorageKeyResponse,
IKeystoreOperation::IKeystoreOperation, IKeystoreSecurityLevel::BnKeystoreSecurityLevel,
IKeystoreSecurityLevel::IKeystoreSecurityLevel, KeyDescriptor::KeyDescriptor,
- KeyMetadata::KeyMetadata, KeyParameters::KeyParameters,
+ KeyMetadata::KeyMetadata, KeyParameters::KeyParameters, ResponseCode::ResponseCode,
};
use anyhow::{anyhow, Context, Result};
+use std::convert::TryInto;
+use std::time::SystemTime;
/// Implementation of the IKeystoreSecurityLevel Interface.
pub struct KeystoreSecurityLevel {
@@ -386,25 +388,50 @@
})
}
- fn add_certificate_parameters(
+ fn add_required_parameters(
&self,
uid: u32,
params: &[KeyParameter],
key: &KeyDescriptor,
) -> Result<Vec<KeyParameter>> {
let mut result = params.to_vec();
+
+ // Unconditionally add the CREATION_DATETIME tag and prevent callers from
+ // specifying it.
+ if params.iter().any(|kp| kp.tag == Tag::CREATION_DATETIME) {
+ return Err(Error::Rc(ResponseCode::INVALID_ARGUMENT)).context(
+ "In KeystoreSecurityLevel::add_required_parameters: \
+ Specifying Tag::CREATION_DATETIME is not allowed.",
+ );
+ }
+
+ result.push(KeyParameter {
+ tag: Tag::CREATION_DATETIME,
+ value: KeyParameterValue::DateTime(
+ SystemTime::now()
+ .duration_since(SystemTime::UNIX_EPOCH)
+ .context(
+ "In KeystoreSecurityLevel::add_required_parameters: \
+ Failed to get epoch time.",
+ )?
+ .as_millis()
+ .try_into()
+ .context(
+ "In KeystoreSecurityLevel::add_required_parameters: \
+ Failed to convert epoch time.",
+ )?,
+ ),
+ });
+
// If there is an attestation challenge we need to get an application id.
if params.iter().any(|kp| kp.tag == Tag::ATTESTATION_CHALLENGE) {
let aaid = {
let _wp = self.watch_millis(
- "In KeystoreSecurityLevel::add_certificate_parameters calling: get_aaid",
+ "In KeystoreSecurityLevel::add_required_parameters calling: get_aaid",
500,
);
keystore2_aaid::get_aaid(uid).map_err(|e| {
- anyhow!(format!(
- "In add_certificate_parameters: get_aaid returned status {}.",
- e
- ))
+ anyhow!(format!("In add_required_parameters: get_aaid returned status {}.", e))
})
}?;
@@ -416,13 +443,13 @@
if params.iter().any(|kp| kp.tag == Tag::INCLUDE_UNIQUE_ID) {
check_key_permission(KeyPerm::gen_unique_id(), key, &None).context(concat!(
- "In add_certificate_parameters: ",
+ "In add_required_parameters: ",
"Caller does not have the permission to generate a unique ID"
))?;
if self.id_rotation_state.had_factory_reset_since_id_rotation().context(
- "In add_certificate_parameters: Call to had_factory_reset_since_id_rotation failed."
+ "In add_required_parameters: Call to had_factory_reset_since_id_rotation failed.",
)? {
- result.push(KeyParameter{
+ result.push(KeyParameter {
tag: Tag::RESET_SINCE_ID_ROTATION,
value: KeyParameterValue::BoolValue(true),
})
@@ -433,7 +460,7 @@
// correct Android permission.
if params.iter().any(|kp| is_device_id_attestation_tag(kp.tag)) {
check_device_attestation_permissions().context(concat!(
- "In add_certificate_parameters: ",
+ "In add_required_parameters: ",
"Caller does not have the permission to attest device identifiers."
))?;
}
@@ -505,7 +532,7 @@
.context("In generate_key: Trying to get an attestation key")?,
};
let params = self
- .add_certificate_parameters(caller_uid, params, &key)
+ .add_required_parameters(caller_uid, params, &key)
.context("In generate_key: Trying to get aaid.")?;
let km_dev: Strong<dyn IKeyMintDevice> = self.keymint.get_interface()?;
@@ -606,7 +633,7 @@
check_key_permission(KeyPerm::rebind(), &key, &None).context("In import_key.")?;
let params = self
- .add_certificate_parameters(caller_uid, params, &key)
+ .add_required_parameters(caller_uid, params, &key)
.context("In import_key: Trying to get aaid.")?;
let format = params
diff --git a/ondevice-signing/VerityUtils.cpp b/ondevice-signing/VerityUtils.cpp
index 0bb3979..cd38308 100644
--- a/ondevice-signing/VerityUtils.cpp
+++ b/ondevice-signing/VerityUtils.cpp
@@ -172,32 +172,6 @@
return toHex(digest.value());
}
-Result<std::map<std::string, std::string>> addFilesToVerityRecursive(const std::string& path,
- const SigningKey& key) {
- std::map<std::string, std::string> digests;
- std::error_code ec;
-
- auto it = std::filesystem::recursive_directory_iterator(path, ec);
- auto end = std::filesystem::recursive_directory_iterator();
-
- while (!ec && it != end) {
- if (it->is_regular_file()) {
- LOG(INFO) << "Adding " << it->path() << " to fs-verity...";
- auto result = enableFsVerity(it->path(), key);
- if (!result.ok()) {
- return result.error();
- }
- digests[it->path()] = *result;
- }
- ++it;
- }
- if (ec) {
- return Error() << "Failed to iterate " << path << ": " << ec;
- }
-
- return digests;
-}
-
Result<std::string> isFileInVerity(const std::string& path) {
unsigned int flags;
@@ -223,6 +197,38 @@
return toHex({&d->digest[0], &d->digest[d->digest_size]});
}
+Result<std::map<std::string, std::string>> addFilesToVerityRecursive(const std::string& path,
+ const SigningKey& key) {
+ std::map<std::string, std::string> digests;
+ std::error_code ec;
+
+ auto it = std::filesystem::recursive_directory_iterator(path, ec);
+ auto end = std::filesystem::recursive_directory_iterator();
+
+ while (!ec && it != end) {
+ if (it->is_regular_file()) {
+ auto digest = isFileInVerity(it->path());
+ if (!digest.ok()) {
+ LOG(INFO) << "Adding " << it->path() << " to fs-verity...";
+ auto result = enableFsVerity(it->path(), key);
+ if (!result.ok()) {
+ return result.error();
+ }
+ digests[it->path()] = *result;
+ } else {
+ LOG(INFO) << it->path() << " was already in fs-verity.";
+ digests[it->path()] = *digest;
+ }
+ }
+ ++it;
+ }
+ if (ec) {
+ return Error() << "Failed to iterate " << path << ": " << ec;
+ }
+
+ return digests;
+}
+
Result<std::map<std::string, std::string>> verifyAllFilesInVerity(const std::string& path) {
std::map<std::string, std::string> digests;
std::error_code ec;
diff --git a/ondevice-signing/VerityUtils.h b/ondevice-signing/VerityUtils.h
index 84af319..a834e61 100644
--- a/ondevice-signing/VerityUtils.h
+++ b/ondevice-signing/VerityUtils.h
@@ -24,5 +24,8 @@
android::base::Result<std::vector<uint8_t>> createDigest(const std::string& path);
android::base::Result<std::map<std::string, std::string>>
verifyAllFilesInVerity(const std::string& path);
+
+// Note that this function will skip files that are already in fs-verity, and
+// for those files it will return the existing digest.
android::base::Result<std::map<std::string, std::string>>
addFilesToVerityRecursive(const std::string& path, const SigningKey& key);