Log keystore2 crash events.
Ignore-AOSP-First: Merge conflict resolution of a CL in the topic.
Bug: 188590587
Test: statsd TestDrive script.
Merged-In: I0ad7c94187060dca28469ba262e1e4d93fe3df83
Change-Id: I0ad7c94187060dca28469ba262e1e4d93fe3df83
diff --git a/keystore2/aidl/android/security/metrics/AtomID.aidl b/keystore2/aidl/android/security/metrics/AtomID.aidl
index dc3768a..166e753 100644
--- a/keystore2/aidl/android/security/metrics/AtomID.aidl
+++ b/keystore2/aidl/android/security/metrics/AtomID.aidl
@@ -31,4 +31,5 @@
KEY_OPERATION_WITH_PURPOSE_AND_MODES_INFO = 10122,
KEY_OPERATION_WITH_GENERAL_INFO = 10123,
RKP_ERROR_STATS = 10124,
+ CRASH_STATS = 10125,
}
\ No newline at end of file
diff --git a/keystore2/aidl/android/security/metrics/CrashStats.aidl b/keystore2/aidl/android/security/metrics/CrashStats.aidl
new file mode 100644
index 0000000..8ca043b
--- /dev/null
+++ b/keystore2/aidl/android/security/metrics/CrashStats.aidl
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2021, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.security.metrics;
+
+/** @hide */
+@RustDerive(Clone=true, Eq=true, PartialEq=true, Ord=true, PartialOrd=true, Hash=true)
+parcelable CrashStats {
+ int count_of_crash_events;
+}
\ No newline at end of file
diff --git a/keystore2/aidl/android/security/metrics/KeystoreAtomPayload.aidl b/keystore2/aidl/android/security/metrics/KeystoreAtomPayload.aidl
index b8a3aba..a3e4dd6 100644
--- a/keystore2/aidl/android/security/metrics/KeystoreAtomPayload.aidl
+++ b/keystore2/aidl/android/security/metrics/KeystoreAtomPayload.aidl
@@ -25,6 +25,7 @@
import android.security.metrics.Keystore2AtomWithOverflow;
import android.security.metrics.RkpErrorStats;
import android.security.metrics.RkpPoolStats;
+import android.security.metrics.CrashStats;
/** @hide */
@RustDerive(Clone=true, Eq=true, PartialEq=true, Ord=true, PartialOrd=true, Hash=true)
@@ -38,4 +39,5 @@
KeyOperationWithPurposeAndModesInfo keyOperationWithPurposeAndModesInfo;
KeyOperationWithGeneralInfo keyOperationWithGeneralInfo;
RkpErrorStats rkpErrorStats;
+ CrashStats crashStats;
}
\ No newline at end of file
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 2c7d4a0..cf2ba04 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -18,6 +18,7 @@
use keystore2::globals::ENFORCEMENTS;
use keystore2::maintenance::Maintenance;
use keystore2::metrics::Metrics;
+use keystore2::metrics_store;
use keystore2::remote_provisioning::RemoteProvisioningService;
use keystore2::service::KeystoreService;
use keystore2::{apc::ApcManager, shared_secret_negotiation};
@@ -51,6 +52,9 @@
let mut args = std::env::args();
args.next().expect("That's odd. How is there not even a first argument?");
+ // Write/update keystore.crash_count system property.
+ metrics_store::update_keystore_crash_sysprop();
+
// Keystore 2.0 cannot change to the database directory (typically /data/misc/keystore) on
// startup as Keystore 1.0 did because Keystore 2.0 is intended to run much earlier than
// Keystore 1.0. Instead we set a global variable to the database path.
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index 32fe353..32067b9 100644
--- a/keystore2/src/metrics_store.rs
+++ b/keystore2/src/metrics_store.rs
@@ -29,7 +29,8 @@
SecurityLevel::SecurityLevel,
};
use android_security_metrics::aidl::android::security::metrics::{
- Algorithm::Algorithm as MetricsAlgorithm, AtomID::AtomID, EcCurve::EcCurve as MetricsEcCurve,
+ Algorithm::Algorithm as MetricsAlgorithm, AtomID::AtomID, CrashStats::CrashStats,
+ EcCurve::EcCurve as MetricsEcCurve,
HardwareAuthenticatorType::HardwareAuthenticatorType as MetricsHardwareAuthenticatorType,
KeyCreationWithAuthInfo::KeyCreationWithAuthInfo,
KeyCreationWithGeneralInfo::KeyCreationWithGeneralInfo,
@@ -43,12 +44,17 @@
RkpPoolStats::RkpPoolStats, SecurityLevel::SecurityLevel as MetricsSecurityLevel,
Storage::Storage as MetricsStorage,
};
-use anyhow::Result;
+use anyhow::{Context, Result};
+use keystore2_system_property::{write, PropertyWatcher, PropertyWatcherError};
use lazy_static::lazy_static;
use std::collections::HashMap;
use std::sync::Mutex;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
+// Note: Crash events are recorded at keystore restarts, based on the assumption that keystore only
+// gets restarted after a crash, during a boot cycle.
+const KEYSTORE_CRASH_COUNT_PROPERTY: &str = "keystore.crash_count";
+
lazy_static! {
/// Singleton for MetricsStore.
pub static ref METRICS_STORE: MetricsStore = Default::default();
@@ -92,6 +98,16 @@
return pull_attestation_pool_stats();
}
+ // 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()
+ }]);
+ }
+
// It is safe to call unwrap here since the lock can not be poisoned based on its usage
// in this module and the lock is not acquired in the same thread before.
let metrics_store_guard = self.metrics_store.lock().unwrap();
@@ -582,6 +598,55 @@
METRICS_STORE.insert_atom(AtomID::RKP_ERROR_STATS, rkp_error_stats);
}
+/// This function tries to read and update the system property: keystore.crash_count.
+/// 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,
+ 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
+ }
+ };
+
+ if let Err(e) = write(KEYSTORE_CRASH_COUNT_PROPERTY, &new_count.to_string()) {
+ log::error!(
+ concat!(
+ "In update_keystore_crash_sysprop:: ",
+ "Failed to write the system property due to error: {:?}"
+ ),
+ e
+ );
+ }
+}
+
+/// Read the system property: keystore.crash_count.
+pub fn read_keystore_crash_count() -> Result<i32> {
+ let mut prop_reader = PropertyWatcher::new("keystore.crash_count").context(concat!(
+ "In read_keystore_crash_count: Failed to create reader a PropertyWatcher."
+ ))?;
+ prop_reader
+ .read(|_n, v| v.parse::<i32>().map_err(std::convert::Into::into))
+ .context("In read_keystore_crash_count: Failed to read the existing system property.")
+}
+
/// Enum defining the bit position for each padding mode. Since padding mode can be repeatable, it
/// is represented using a bitmap.
#[allow(non_camel_case_types)]
diff --git a/keystore2/system_property/Android.bp b/keystore2/system_property/Android.bp
index 9e7b056..773804d 100644
--- a/keystore2/system_property/Android.bp
+++ b/keystore2/system_property/Android.bp
@@ -31,6 +31,7 @@
"--size_t-is-usize",
"--allowlist-function=__system_property_find",
"--allowlist-function=__system_property_read_callback",
+ "--allowlist-function=__system_property_set",
"--allowlist-function=__system_property_wait",
],
}
diff --git a/keystore2/system_property/lib.rs b/keystore2/system_property/lib.rs
index be13c88..b993c87 100644
--- a/keystore2/system_property/lib.rs
+++ b/keystore2/system_property/lib.rs
@@ -15,6 +15,7 @@
//! This crate provides the PropertyWatcher type, which watches for changes
//! in Android system properties.
+use anyhow::{anyhow, Context, Result as AnyhowResult};
use keystore2_system_property_bindgen::prop_info as PropInfo;
use std::os::raw::c_char;
use std::ptr::null;
@@ -48,6 +49,9 @@
/// read callback returned an error
#[error("Callback failed")]
CallbackError(#[from] anyhow::Error),
+ /// Failure in setting the system property
+ #[error("__system_property_set failed.")]
+ SetPropertyFailed,
}
/// Result type specific for this crate.
@@ -189,3 +193,25 @@
Ok(())
}
}
+
+/// Writes a system property.
+pub fn write(name: &str, value: &str) -> AnyhowResult<()> {
+ if
+ // Unsafe required for FFI call. Input and output are both const and valid strings.
+ unsafe {
+ // If successful, __system_property_set returns 0, otherwise, returns -1.
+ keystore2_system_property_bindgen::__system_property_set(
+ CString::new(name)
+ .context("In keystore2::system_property::write: Construction CString from name.")?
+ .as_ptr(),
+ CString::new(value)
+ .context("In keystore2::system_property::write: Constructing CString from value.")?
+ .as_ptr(),
+ )
+ } == 0
+ {
+ Ok(())
+ } else {
+ Err(anyhow!(PropertyWatcherError::SetPropertyFailed))
+ }
+}