Merge "Add AndroidTest.xml for keystore2_test to support coverage"
diff --git a/keystore/legacy_keymaster_device_wrapper.cpp b/keystore/legacy_keymaster_device_wrapper.cpp
index 86d286e..052f394 100644
--- a/keystore/legacy_keymaster_device_wrapper.cpp
+++ b/keystore/legacy_keymaster_device_wrapper.cpp
@@ -532,7 +532,7 @@
sp<IKeymasterDevice> makeSoftwareKeymasterDevice() {
keymaster2_device_t* dev = nullptr;
- dev = (new SoftKeymasterDevice)->keymaster2_device();
+ dev = (new SoftKeymasterDevice(keymaster::KmVersion::KEYMASTER_2))->keymaster2_device();
auto kmrc = ::keymaster::ConfigureDevice(dev);
if (kmrc != KM_ERROR_OK) {
diff --git a/keystore2/Android.bp b/keystore2/Android.bp
index de21251..4e819b8 100644
--- a/keystore2/Android.bp
+++ b/keystore2/Android.bp
@@ -45,7 +45,6 @@
"libandroid_logger",
"libanyhow",
"libbinder_rs",
- "libkeystore2_crypto_bindgen",
"libkeystore2_selinux",
"liblazy_static",
"liblibsqlite3_sys",
@@ -53,28 +52,6 @@
"librusqlite",
"libthiserror",
],
- shared_libs: ["libkeystore2_crypto"],
-}
-
-cc_library {
- name: "libkeystore2_crypto",
- srcs: [
- "src/crypto.cpp",
- "src/certificate_utils.cpp",
- ],
- export_include_dirs: ["include",],
- shared_libs: [
- "libcrypto",
- "liblog",
- ],
-}
-
-rust_bindgen {
- name: "libkeystore2_crypto_bindgen",
- wrapper_src: "src/crypto.hpp",
- crate_name: "keystore2_crypto_bindgen",
- source_stem: "bindings",
- host_supported: true,
}
rust_binary {
@@ -88,22 +65,3 @@
],
init_rc: ["keystore2.rc"],
}
-
-cc_test {
- cflags: [
- "-Wall",
- "-Werror",
- "-Wextra",
- ],
- srcs: [
- "src/tests/certificate_utils_test.cpp",
- "src/tests/gtest_main.cpp",
- ],
- static_libs: [
- "libkeystore2_crypto",
- ],
- shared_libs: [
- "libcrypto",
- ],
- name: "keystore2_crypto_test",
-}
diff --git a/keystore2/src/crypto/Android.bp b/keystore2/src/crypto/Android.bp
new file mode 100644
index 0000000..061cf9a
--- /dev/null
+++ b/keystore2/src/crypto/Android.bp
@@ -0,0 +1,84 @@
+// Copyright 2020, The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+rust_library {
+ name: "libkeystore2_crypto_rust",
+ crate_name: "keystore2_crypto",
+ srcs: ["lib.rs"],
+ rustlibs: [
+ "libkeystore2_crypto_bindgen",
+ ],
+ static_libs: ["libkeystore2_crypto"],
+ shared_libs: ["libcrypto"],
+}
+
+cc_library {
+ name: "libkeystore2_crypto",
+ srcs: [
+ "crypto.cpp",
+ "certificate_utils.cpp",
+ ],
+ export_include_dirs: ["include",],
+ shared_libs: [
+ "libcrypto",
+ "liblog",
+ ],
+}
+
+rust_bindgen {
+ name: "libkeystore2_crypto_bindgen",
+ wrapper_src: "crypto.hpp",
+ crate_name: "keystore2_crypto_bindgen",
+ source_stem: "bindings",
+ host_supported: true,
+}
+
+rust_test {
+ name: "keystore2_crypto_test_rust",
+ crate_name: "keystore2_crypto_test_rust",
+ srcs: ["lib.rs"],
+ test_suites: ["general-tests"],
+ auto_gen_config: true,
+ rustlibs: [
+ "libkeystore2_crypto_bindgen",
+ "libkeystore2_crypto_rust",
+ ],
+ static_libs: [
+ "libkeystore2_crypto",
+ ],
+ shared_libs: [
+ "libc++",
+ "libcrypto",
+ "liblog",
+ ],
+}
+
+cc_test {
+ cflags: [
+ "-Wall",
+ "-Werror",
+ "-Wextra",
+ ],
+ srcs: [
+ "tests/certificate_utils_test.cpp",
+ "tests/gtest_main.cpp",
+ ],
+ static_libs: [
+ "libkeystore2_crypto",
+ ],
+ shared_libs: [
+ "libcrypto",
+ ],
+ name: "keystore2_crypto_test",
+}
diff --git a/keystore2/src/certificate_utils.cpp b/keystore2/src/crypto/certificate_utils.cpp
similarity index 100%
rename from keystore2/src/certificate_utils.cpp
rename to keystore2/src/crypto/certificate_utils.cpp
diff --git a/keystore2/src/crypto.cpp b/keystore2/src/crypto/crypto.cpp
similarity index 100%
rename from keystore2/src/crypto.cpp
rename to keystore2/src/crypto/crypto.cpp
diff --git a/keystore2/src/crypto.hpp b/keystore2/src/crypto/crypto.hpp
similarity index 100%
rename from keystore2/src/crypto.hpp
rename to keystore2/src/crypto/crypto.hpp
diff --git a/keystore2/include/certificate_utils.h b/keystore2/src/crypto/include/certificate_utils.h
similarity index 100%
rename from keystore2/include/certificate_utils.h
rename to keystore2/src/crypto/include/certificate_utils.h
diff --git a/keystore2/src/crypto.rs b/keystore2/src/crypto/lib.rs
similarity index 95%
rename from keystore2/src/crypto.rs
rename to keystore2/src/crypto/lib.rs
index b25b648..6ec5edb 100644
--- a/keystore2/src/crypto.rs
+++ b/keystore2/src/crypto/lib.rs
@@ -12,6 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+// TODO: Once this is complete, remove this and document everything public.
+#![allow(missing_docs)]
+
#[cfg(test)]
mod tests {
diff --git a/keystore2/src/tests/certificate_utils_test.cpp b/keystore2/src/crypto/tests/certificate_utils_test.cpp
similarity index 100%
rename from keystore2/src/tests/certificate_utils_test.cpp
rename to keystore2/src/crypto/tests/certificate_utils_test.cpp
diff --git a/keystore2/src/tests/gtest_main.cpp b/keystore2/src/crypto/tests/gtest_main.cpp
similarity index 100%
rename from keystore2/src/tests/gtest_main.cpp
rename to keystore2/src/crypto/tests/gtest_main.cpp
diff --git a/keystore2/src/tests/test_keys.h b/keystore2/src/crypto/tests/test_keys.h
similarity index 100%
rename from keystore2/src/tests/test_keys.h
rename to keystore2/src/crypto/tests/test_keys.h
diff --git a/keystore2/src/database.rs b/keystore2/src/database.rs
index f612495..9d20c75 100644
--- a/keystore2/src/database.rs
+++ b/keystore2/src/database.rs
@@ -150,6 +150,7 @@
fn drop(&mut self) {
let mut locked_keys = KEY_ID_LOCK.locked_keys.lock().unwrap();
locked_keys.remove(&self.0);
+ drop(locked_keys);
KEY_ID_LOCK.cond_var.notify_all();
}
}
@@ -1077,7 +1078,7 @@
db.rebind_alias(&KEY_ID_LOCK.get(entries[1].id), "foo", Domain::APP, 42)?;
let entries = get_keyentry(&db)?;
assert_eq!(entries.len(), 2);
- assert_eq!(extractor(&entries[0]), (Some(Domain::APP), Some(42), None));
+ assert_eq!(extractor(&entries[0]), (None, None, None));
assert_eq!(extractor(&entries[1]), (Some(Domain::APP), Some(42), Some("foo")));
// Test that we must pass in a valid Domain.
@@ -1102,7 +1103,7 @@
// Test that we correctly abort the transaction in this case.
let entries = get_keyentry(&db)?;
assert_eq!(entries.len(), 2);
- assert_eq!(extractor(&entries[0]), (Some(Domain::APP), Some(42), None));
+ assert_eq!(extractor(&entries[0]), (None, None, None));
assert_eq!(extractor(&entries[1]), (Some(Domain::APP), Some(42), Some("foo")));
Ok(())
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index 63ebe62..49d72bb 100644
--- a/keystore2/src/error.rs
+++ b/keystore2/src/error.rs
@@ -99,34 +99,34 @@
}
/// This function should be used by Keystore service calls to translate error conditions
-/// into `android.system.keystore2.Result` which is imported here as `aidl::Result`
-/// and newtyped as AidlResult.
-/// All error conditions get logged by this function.
-/// All `Error::Rc(x)` variants get mapped onto `aidl::Result{x, 0}`.
-/// All `Error::Km(x)` variants get mapped onto
-/// `aidl::Result{aidl::ResponseCode::KeymintErrorCode, x}`.
-/// `selinux::Error::perm()` is mapped on `aidl::Result{aidl::ResponseCode::PERMISSION_DENIED, 0}`.
+/// into service specific exceptions.
///
-/// All non `Error` error conditions get mapped onto
-/// `aidl::Result{aidl::ResponseCode::SYSTEM_ERROR}`.
+/// All error conditions get logged by this function.
+///
+/// All `Error::Rc(x)` and `Error::Km(x)` variants get mapped onto a service specific error
+/// code of x. This is possible because KeyMint `ErrorCode` errors are always negative and
+/// `ResponseCode` codes are always positive.
+/// `selinux::Error::PermissionDenied` is mapped on `ResponseCode::PERMISSION_DENIED`.
+///
+/// All non `Error` error conditions and the Error::Binder variant get mapped onto
+/// ResponseCode::SYSTEM_ERROR`.
///
/// `handle_ok` will be called if `result` is `Ok(value)` where `value` will be passed
-/// as argument to `handle_ok`. `handle_ok` must generate an `AidlResult`, typically
-/// `AidlResult::ok()`, but other response codes may be used, e.g.,
-/// `aidl::ResponseCode::OpAuthNeeded` which does not required logging.
+/// as argument to `handle_ok`. `handle_ok` must generate a `BinderResult<T>`, but it
+/// typically returns Ok(value).
///
/// # Examples
///
/// ```
-/// fn loadKey() -> anyhow::Result<aidl::ResponseCode> {
+/// fn loadKey() -> anyhow::Result<Vec<u8>> {
/// if (good_but_auth_required) {
-/// Ok(aidl::ResponseCode::OpAuthRequired)
+/// Ok(vec!['k', 'e', 'y'])
/// } else {
-/// Err(anyhow!(Error::Rc(aidl::ResponseCode::KEY_NOT_FOUND)))
+/// Err(anyhow!(Error::Rc(ResponseCode::KEY_NOT_FOUND)))
/// }
/// }
///
-/// aidl_result_ = map_or_log_err(loadKey(), |r| { some_side_effect(); AidlResult::rc(r) });
+/// map_or_log_err(loadKey(), Ok)
/// ```
pub fn map_or_log_err<T, U, F>(result: anyhow::Result<U>, handle_ok: F) -> BinderResult<T>
where
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index dea0a93..ab00794 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-//! This crate implements Keystore 2.0.
+//! This crate implements the Keystore 2.0 service entry point.
use binder::Interface;
use keystore2::service::KeystoreService;
@@ -53,7 +53,10 @@
});
info!("Successfully registered Keystore 2.0 service.");
- info!("Joining threadpool now.");
+ info!("Starting thread pool now.");
+ binder::ProcessState::start_thread_pool();
+
+ info!("Joining thread pool now.");
binder::ProcessState::join_thread_pool();
}
diff --git a/keystore2/src/lib.rs b/keystore2/src/lib.rs
index 3e13c5f..067399e 100644
--- a/keystore2/src/lib.rs
+++ b/keystore2/src/lib.rs
@@ -14,7 +14,6 @@
//! This crate implements the Android Keystore 2.0 service.
-mod crypto;
pub mod database;
pub mod error;
pub mod globals;