Keystore 2.0: Add error string to service specific exceptions.
This patch adds detailed error messages to the service specific
exceptions. The error messages are formated anyhow::Errors, the same
that already get logged to logcat. Returning them to the client, allows
them to be included in java stack traces which will lead to easier
diagnosis of bugreports.
Test: N/A
Bug: 197890905
Change-Id: Ie6178292650327a1382b04f478ed5fa9e5fd7feb
diff --git a/keystore2/legacykeystore/lib.rs b/keystore2/legacykeystore/lib.rs
index da60297..13a9143 100644
--- a/keystore2/legacykeystore/lib.rs
+++ b/keystore2/legacykeystore/lib.rs
@@ -25,8 +25,8 @@
};
use anyhow::{Context, Result};
use keystore2::{
- async_task::AsyncTask, legacy_blob::LegacyBlobLoader, maintenance::DeleteListener,
- maintenance::Domain, utils::watchdog as wd,
+ async_task::AsyncTask, error::anyhow_error_to_cstring, legacy_blob::LegacyBlobLoader,
+ maintenance::DeleteListener, maintenance::Domain, utils::watchdog as wd,
};
use rusqlite::{
params, Connection, OptionalExtension, Transaction, TransactionBehavior, NO_PARAMS,
@@ -226,7 +226,10 @@
if log_error {
log::error!("{:?}", e);
}
- Err(BinderStatus::new_service_specific_error(rc, None))
+ Err(BinderStatus::new_service_specific_error(
+ rc,
+ anyhow_error_to_cstring(&e).as_deref(),
+ ))
},
handle_ok,
)
diff --git a/keystore2/src/apc.rs b/keystore2/src/apc.rs
index 0096686..7d56dc9 100644
--- a/keystore2/src/apc.rs
+++ b/keystore2/src/apc.rs
@@ -21,6 +21,7 @@
sync::{mpsc::Sender, Arc, Mutex},
};
+use crate::error::anyhow_error_to_cstring;
use crate::utils::{compat_2_response_code, ui_opts_2_compat, watchdog as wd};
use android_security_apc::aidl::android::security::apc::{
IConfirmationCallback::IConfirmationCallback,
@@ -110,7 +111,10 @@
_ => ResponseCode::SYSTEM_ERROR.0,
},
};
- Err(BinderStatus::new_service_specific_error(rc, None))
+ Err(BinderStatus::new_service_specific_error(
+ rc,
+ anyhow_error_to_cstring(&e).as_deref(),
+ ))
},
handle_ok,
)
diff --git a/keystore2/src/authorization.rs b/keystore2/src/authorization.rs
index 04626bc..0c41507 100644
--- a/keystore2/src/authorization.rs
+++ b/keystore2/src/authorization.rs
@@ -15,6 +15,7 @@
//! This module implements IKeystoreAuthorization AIDL interface.
use crate::error::Error as KeystoreError;
+use crate::error::anyhow_error_to_cstring;
use crate::globals::{ENFORCEMENTS, SUPER_KEY, DB, LEGACY_MIGRATOR};
use crate::permission::KeystorePerm;
use crate::super_key::UserState;
@@ -88,7 +89,10 @@
// as well.
_ => ResponseCode::SYSTEM_ERROR.0,
};
- return Err(BinderStatus::new_service_specific_error(rc, None));
+ return Err(BinderStatus::new_service_specific_error(
+ rc,
+ anyhow_error_to_cstring(&e).as_deref(),
+ ));
}
let rc = match root_cause.downcast_ref::<Error>() {
Some(Error::Rc(rcode)) => rcode.0,
@@ -98,7 +102,10 @@
_ => ResponseCode::SYSTEM_ERROR.0,
},
};
- Err(BinderStatus::new_service_specific_error(rc, None))
+ Err(BinderStatus::new_service_specific_error(
+ rc,
+ anyhow_error_to_cstring(&e).as_deref(),
+ ))
},
handle_ok,
)
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index f969cb6..42dd3d2 100644
--- a/keystore2/src/error.rs
+++ b/keystore2/src/error.rs
@@ -37,6 +37,7 @@
};
use keystore2_selinux as selinux;
use std::cmp::PartialEq;
+use std::ffi::CString;
/// This is the main Keystore error type. It wraps the Keystore `ResponseCode` generated
/// from AIDL in the `Rc` variant and Keymint `ErrorCode` in the Km variant.
@@ -184,6 +185,20 @@
)
}
+/// This function turns an anyhow error into an optional CString.
+/// This is especially useful to add a message string to a service specific error.
+/// If the formatted string was not convertible because it contained a nul byte,
+/// None is returned and a warning is logged.
+pub fn anyhow_error_to_cstring(e: &anyhow::Error) -> Option<CString> {
+ match CString::new(format!("{:?}", e)) {
+ Ok(msg) => Some(msg),
+ Err(_) => {
+ log::warn!("Cannot convert error message to CStr. It contained a nul byte.");
+ None
+ }
+ }
+}
+
/// This function behaves similar to map_or_log_error, but it does not log the errors, instead
/// it calls map_err on the error before mapping it to a binder result allowing callers to
/// log or transform the error before mapping it.
@@ -200,7 +215,10 @@
|e| {
let e = map_err(e);
let rc = get_error_code(&e);
- Err(BinderStatus::new_service_specific_error(rc, None))
+ Err(BinderStatus::new_service_specific_error(
+ rc,
+ anyhow_error_to_cstring(&e).as_deref(),
+ ))
},
handle_ok,
)