Merge "Adding an OWNERS file as requested in b/288143537" into main
diff --git a/OWNERS b/OWNERS
index 03e5769..6fdb550 100644
--- a/OWNERS
+++ b/OWNERS
@@ -1,5 +1,4 @@
alanstokes@google.com
-cbrubaker@google.com
drysdale@google.com
eranm@google.com
hasinitg@google.com
@@ -8,4 +7,5 @@
kroot@google.com
sethmo@google.com
swillden@google.com
+trong@google.com
zeuthen@google.com
diff --git a/diced/open_dice/Android.bp b/diced/open_dice/Android.bp
index 2505b42..c59419b 100644
--- a/diced/open_dice/Android.bp
+++ b/diced/open_dice/Android.bp
@@ -7,14 +7,6 @@
name: "libdiced_open_dice_defaults",
crate_name: "diced_open_dice",
srcs: ["src/lib.rs"],
- static_libs: [
- "libopen_dice_cbor",
- ],
- vendor_available: true,
- apex_available: [
- "//apex_available:platform",
- "com.android.virt",
- ],
}
rust_library_rlib {
@@ -37,6 +29,7 @@
rust_library {
name: "libdiced_open_dice",
defaults: ["libdiced_open_dice_defaults"],
+ vendor_available: true,
rustlibs: [
"libopen_dice_bcc_bindgen",
"libopen_dice_cbor_bindgen",
@@ -48,6 +41,9 @@
shared_libs: [
"libcrypto",
],
+ static_libs: [
+ "libopen_dice_cbor",
+ ],
whole_static_libs: [
"libopen_dice_bcc",
],
@@ -56,6 +52,10 @@
"//packages/modules/Virtualization:__subpackages__",
"//hardware/interfaces/security/dice/aidl:__subpackages__",
],
+ apex_available: [
+ "//apex_available:platform",
+ "com.android.virt",
+ ],
}
rust_defaults {
@@ -120,12 +120,10 @@
rust_defaults {
name: "libopen_dice_cbor_bindgen.rust_defaults",
- defaults: ["libopen_dice.rust_defaults"],
wrapper_src: "bindgen/dice.h",
crate_name: "open_dice_cbor_bindgen",
source_stem: "bindings",
bindgen_flags: [
- "--size_t-is-usize",
"--rustified-enum DiceConfigType",
"--rustified-enum DiceMode",
"--rustified-enum DiceResult",
@@ -157,7 +155,10 @@
rust_bindgen {
name: "libopen_dice_cbor_bindgen",
- defaults: ["libopen_dice_cbor_bindgen.rust_defaults"],
+ defaults: [
+ "libopen_dice.rust_defaults",
+ "libopen_dice_cbor_bindgen.rust_defaults",
+ ],
whole_static_libs: ["libopen_dice_cbor"],
}
@@ -172,13 +173,10 @@
rust_defaults {
name: "libopen_dice_bcc_bindgen.rust_defaults",
- defaults: ["libopen_dice.rust_defaults"],
wrapper_src: "bindgen/android/bcc.h",
crate_name: "open_dice_bcc_bindgen",
source_stem: "bindings",
bindgen_flags: [
- "--size_t-is-usize",
-
// By generating only essential functions, we can make bindings concise and
// optimize compilation time.
"--allowlist-function=BccFormatConfigDescriptor",
@@ -209,7 +207,10 @@
rust_bindgen {
name: "libopen_dice_bcc_bindgen",
- defaults: ["libopen_dice_bcc_bindgen.rust_defaults"],
+ defaults: [
+ "libopen_dice.rust_defaults",
+ "libopen_dice_bcc_bindgen.rust_defaults",
+ ],
rustlibs: [
"libopen_dice_cbor_bindgen",
],
diff --git a/diced/open_dice/src/bcc.rs b/diced/open_dice/src/bcc.rs
index f9c6a34..ca2136f 100644
--- a/diced/open_dice/src/bcc.rs
+++ b/diced/open_dice/src/bcc.rs
@@ -48,9 +48,9 @@
};
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(
+ // SAFETY: The function writes to the buffer, within the given bounds, and only reads the
+ // input values. It writes its result to buffer_size.
unsafe {
BccFormatConfigDescriptor(&values, buffer.len(), buffer.as_mut_ptr(), &mut buffer_size)
},
@@ -72,11 +72,11 @@
next_bcc: &mut [u8],
) -> Result<usize> {
let mut next_bcc_size = 0;
- // SAFETY: `BccMainFlow` only reads the current `bcc` and CDI values and writes
- // 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(
+ // SAFETY: `BccMainFlow` only reads the current `bcc` and CDI values and writes
+ // 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.
unsafe {
BccMainFlow(
ptr::null_mut(), // context
@@ -108,11 +108,11 @@
next_bcc_handover: &mut [u8],
) -> Result<usize> {
let mut next_bcc_handover_size = 0;
- // SAFETY - The function only reads `current_bcc_handover` and writes to `next_bcc_handover`
- // 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(
+ // SAFETY: The function only reads `current_bcc_handover` and writes to `next_bcc_handover`
+ // 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.
unsafe {
BccHandoverMainFlow(
ptr::null_mut(), // context
@@ -165,9 +165,9 @@
let mut cdi_seal: *const u8 = ptr::null();
let mut bcc: *const u8 = ptr::null();
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(
+ // 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.
unsafe {
BccHandoverParse(
bcc_handover.as_ptr(),
diff --git a/diced/open_dice/src/dice.rs b/diced/open_dice/src/dice.rs
index 0704d21..e42e373 100644
--- a/diced/open_dice/src/dice.rs
+++ b/diced/open_dice/src/dice.rs
@@ -217,9 +217,9 @@
/// Derives a CDI private key seed from a `cdi_attest` value.
pub fn derive_cdi_private_key_seed(cdi_attest: &Cdi) -> Result<PrivateKeySeed> {
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(
+ // 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.
unsafe {
DiceDeriveCdiPrivateKeySeed(
ptr::null_mut(), // context
@@ -235,9 +235,9 @@
/// Derives an ID from the given `cdi_public_key` value.
pub fn derive_cdi_certificate_id(cdi_public_key: &[u8]) -> Result<DiceId> {
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(
+ // 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.
unsafe {
DiceDeriveCdiCertificateId(
ptr::null_mut(), // context
@@ -264,10 +264,10 @@
next_cdi_values: &mut CdiValues,
) -> Result<usize> {
let mut next_cdi_certificate_actual_size = 0;
- // 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(
+ // 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.
unsafe {
DiceMainFlow(
ptr::null_mut(), // context
diff --git a/diced/open_dice/src/ops.rs b/diced/open_dice/src/ops.rs
index d978f86..6b9202a 100644
--- a/diced/open_dice/src/ops.rs
+++ b/diced/open_dice/src/ops.rs
@@ -29,9 +29,9 @@
/// Hashes the provided input using DICE's hash function `DiceHash`.
pub fn hash(input: &[u8]) -> Result<Hash> {
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(
+ // 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.
unsafe {
DiceHash(
ptr::null_mut(), // context
@@ -48,9 +48,9 @@
/// An implementation of HKDF-SHA512. Derives a key of `derived_key.len()` bytes from `ikm`, `salt`,
/// and `info`. The derived key is written to the `derived_key`.
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(
+ // 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.
unsafe {
DiceKdf(
ptr::null_mut(), // context
@@ -74,9 +74,10 @@
pub fn keypair_from_seed(seed: &[u8; PRIVATE_KEY_SEED_SIZE]) -> Result<(PublicKey, PrivateKey)> {
let mut public_key = [0u8; PUBLIC_KEY_SIZE];
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(
+ // 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.
unsafe {
DiceKeypairFromSeed(
ptr::null_mut(), // context
@@ -93,9 +94,9 @@
/// Signs the `message` with the give `private_key` using `DiceSign`.
pub fn sign(message: &[u8], private_key: &[u8; PRIVATE_KEY_SIZE]) -> Result<Signature> {
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(
+ // 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.
unsafe {
DiceSign(
ptr::null_mut(), // context
@@ -112,9 +113,9 @@
/// Verifies the `signature` of the `message` with the given `public_key` using `DiceVerify`.
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(
+ // SAFETY: only reads the messages, signature and public key as constant values.
+ // The first argument context is not used in this function.
unsafe {
DiceVerify(
ptr::null_mut(), // context
@@ -140,9 +141,10 @@
certificate: &mut [u8],
) -> Result<usize> {
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(
+ // 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.
unsafe {
DiceGenerateCertificate(
ptr::null_mut(), // context
diff --git a/fsverity/Android.bp b/fsverity/Android.bp
index ce3b499..93b4d93 100644
--- a/fsverity/Android.bp
+++ b/fsverity/Android.bp
@@ -44,6 +44,7 @@
name: "libfsverity_digests_proto_rust",
crate_name: "fsverity_digests_proto",
source_stem: "fsverity_digests_proto",
+ use_protobuf3: true,
protos: [
"fsverity_digests.proto",
],
diff --git a/fsverity_init/Android.bp b/fsverity_init/Android.bp
index 83c5945..07eaf6a 100644
--- a/fsverity_init/Android.bp
+++ b/fsverity_init/Android.bp
@@ -10,26 +10,10 @@
cc_binary {
name: "fsverity_init",
srcs: [
- "main.cpp",
+ "fsverity_init.cpp",
],
static_libs: [
"libc++fs",
- "libfsverity_init",
- "libmini_keyctl_static",
- ],
- shared_libs: [
- "libbase",
- "libkeyutils",
- "liblog",
- ],
- cflags: ["-Werror", "-Wall", "-Wextra"],
-}
-
-cc_library {
- name: "libfsverity_init",
- srcs: ["fsverity_init.cpp"],
- static_libs: [
- "libc++fs",
"libmini_keyctl_static",
],
shared_libs: [
@@ -38,6 +22,4 @@
"liblog",
],
cflags: ["-Werror", "-Wall", "-Wextra"],
- export_include_dirs: ["include"],
- recovery_available: true,
}
diff --git a/fsverity_init/fsverity_init.cpp b/fsverity_init/fsverity_init.cpp
index 61f84dd..797118d 100644
--- a/fsverity_init/fsverity_init.cpp
+++ b/fsverity_init/fsverity_init.cpp
@@ -14,6 +14,25 @@
* limitations under the License.
*/
+//
+// fsverity_init is a tool for loading X.509 certificates into the kernel keyring used by the
+// fsverity builtin signature verification kernel feature
+// (https://www.kernel.org/doc/html/latest/filesystems/fsverity.html#built-in-signature-verification).
+// Starting in Android 14, Android has actually stopped using this feature, as it was too inflexible
+// and caused problems. It has been replaced by userspace signature verification. Also, some uses
+// of fsverity in Android are now for integrity-only use cases.
+//
+// Regardless, there may exist fsverity files on-disk that were created by Android 13 or earlier.
+// These files still have builtin signatures. If the kernel is an older kernel that still has
+// CONFIG_FS_VERITY_BUILTIN_SIGNATURES enabled, these files cannot be opened unless the
+// corresponding key is in the ".fs-verity" keyring. Therefore, this tool still has to exist and be
+// used to load keys into the kernel, even though this has no security purpose anymore.
+//
+// This tool can be removed as soon as all supported kernels are guaranteed to have
+// CONFIG_FS_VERITY_BUILTIN_SIGNATURES disabled, or alternatively as soon as support for upgrades
+// from Android 13 or earlier is no longer required.
+//
+
#define LOG_TAG "fsverity_init"
#include <sys/types.h>
@@ -23,33 +42,10 @@
#include <android-base/file.h>
#include <android-base/logging.h>
-#include <android-base/properties.h>
#include <android-base/strings.h>
#include <log/log.h>
#include <mini_keyctl_utils.h>
-bool LoadKeyToKeyring(key_serial_t keyring_id, const char* desc, const char* data, size_t size) {
- key_serial_t key = add_key("asymmetric", desc, data, size, keyring_id);
- if (key < 0) {
- PLOG(ERROR) << "Failed to add key";
- return false;
- }
- return true;
-}
-
-bool LoadKeyFromStdin(key_serial_t keyring_id, const char* keyname) {
- std::string content;
- if (!android::base::ReadFdToString(STDIN_FILENO, &content)) {
- LOG(ERROR) << "Failed to read key from stdin";
- return false;
- }
- if (!LoadKeyToKeyring(keyring_id, keyname, content.c_str(), content.size())) {
- LOG(ERROR) << "Failed to load key from stdin";
- return false;
- }
- return true;
-}
-
void LoadKeyFromFile(key_serial_t keyring_id, const char* keyname, const std::string& path) {
LOG(INFO) << "LoadKeyFromFile path=" << path << " keyname=" << keyname;
std::string content;
@@ -57,8 +53,8 @@
LOG(ERROR) << "Failed to read key from " << path;
return;
}
- if (!LoadKeyToKeyring(keyring_id, keyname, content.c_str(), content.size())) {
- LOG(ERROR) << "Failed to load key from " << path;
+ if (add_key("asymmetric", keyname, content.c_str(), content.size(), keyring_id) < 0) {
+ PLOG(ERROR) << "Failed to add key from " << path;
}
}
@@ -81,3 +77,28 @@
LoadKeyFromDirectory(keyring_id, "fsv_system_", "/system/etc/security/fsverity");
LoadKeyFromDirectory(keyring_id, "fsv_product_", "/product/etc/security/fsverity");
}
+
+int main(int argc, const char** argv) {
+ if (argc < 2) {
+ LOG(ERROR) << "Not enough arguments";
+ return -1;
+ }
+
+ key_serial_t keyring_id = android::GetKeyringId(".fs-verity");
+ if (keyring_id < 0) {
+ // This is expected on newer kernels. See comment at the beginning of this file.
+ LOG(DEBUG) << "no initialization required";
+ return 0;
+ }
+
+ const std::string_view command = argv[1];
+
+ if (command == "--load-verified-keys") {
+ LoadKeyFromVerifiedPartitions(keyring_id);
+ } else {
+ LOG(ERROR) << "Unknown argument(s).";
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/fsverity_init/include/fsverity_init.h b/fsverity_init/include/fsverity_init.h
deleted file mode 100644
index c3bc93b..0000000
--- a/fsverity_init/include/fsverity_init.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * Copyright (C) 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.
- */
-
-#include <mini_keyctl_utils.h>
-
-bool LoadKeyFromStdin(key_serial_t keyring_id, const char* keyname);
-void LoadKeyFromFile(key_serial_t keyring_id, const char* keyname, const std::string& path);
-void LoadKeyFromVerifiedPartitions(key_serial_t keyring_id);
diff --git a/fsverity_init/main.cpp b/fsverity_init/main.cpp
deleted file mode 100644
index b502b91..0000000
--- a/fsverity_init/main.cpp
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Copyright (C) 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.
- */
-
-#include <string>
-
-#include <android-base/file.h>
-#include <android-base/logging.h>
-#include <android-base/properties.h>
-#include <fsverity_init.h>
-#include <log/log.h>
-#include <mini_keyctl_utils.h>
-
-int main(int argc, const char** argv) {
- if (argc < 2) {
- LOG(ERROR) << "Not enough arguments";
- return -1;
- }
-
- key_serial_t keyring_id = android::GetKeyringId(".fs-verity");
- if (keyring_id < 0) {
- LOG(ERROR) << "Failed to find .fs-verity keyring id";
- return -1;
- }
-
- const std::string_view command = argv[1];
-
- if (command == "--load-verified-keys") {
- LoadKeyFromVerifiedPartitions(keyring_id);
- } else if (command == "--load-extra-key") {
- if (argc != 3) {
- LOG(ERROR) << "--load-extra-key requires <key_name> argument.";
- return -1;
- }
- if (!LoadKeyFromStdin(keyring_id, argv[2])) {
- return -1;
- }
- } else if (command == "--lock") {
- if (!android::base::GetBoolProperty("ro.debuggable", false)) {
- if (keyctl_restrict_keyring(keyring_id, nullptr, nullptr) < 0) {
- PLOG(ERROR) << "Cannot restrict .fs-verity keyring";
- }
- }
- } else {
- LOG(ERROR) << "Unknown argument(s).";
- return -1;
- }
-
- return 0;
-}
diff --git a/identity/CredentialData.cpp b/identity/CredentialData.cpp
index 803e671..ecf2258 100644
--- a/identity/CredentialData.cpp
+++ b/identity/CredentialData.cpp
@@ -527,7 +527,6 @@
return nullptr;
}
- int n = 0;
for (AuthKeyData& data : authKeyDatas_) {
if (nowMilliSeconds > data.expirationDateMillisSinceEpoch) {
if (!allowUsingExpiredKeys) {
@@ -540,7 +539,6 @@
candidate = &data;
}
}
- n++;
}
if (candidate == nullptr) {
diff --git a/keystore/keystore_attestation_id.cpp b/keystore/keystore_attestation_id.cpp
index ccd3808..8eade97 100644
--- a/keystore/keystore_attestation_id.cpp
+++ b/keystore/keystore_attestation_id.cpp
@@ -74,8 +74,8 @@
}
KeyAttestationApplicationIdProvider::KeyAttestationApplicationIdProvider()
- : BpKeyAttestationApplicationIdProvider(
- android::defaultServiceManager()->getService(String16("sec_key_att_app_id_provider"))) {}
+ : BpKeyAttestationApplicationIdProvider(android::defaultServiceManager()->waitForService(
+ String16("sec_key_att_app_id_provider"))) {}
DECLARE_STACK_OF(ASN1_OCTET_STRING);
@@ -158,7 +158,7 @@
return BAD_VALUE;
}
- std::string pkg_name(String8(*pinfo.package_name()).string());
+ std::string pkg_name(String8(*pinfo.package_name()).c_str());
if (!ASN1_OCTET_STRING_set(attestation_package_info->package_name,
reinterpret_cast<const unsigned char*>(pkg_name.data()),
pkg_name.size())) {
@@ -209,7 +209,7 @@
ALOGE("Key attestation package info lacks package name");
return BAD_VALUE;
}
- std::string package_name(String8(*pinfo->package_name()).string());
+ std::string package_name(String8(*pinfo->package_name()).c_str());
std::unique_ptr<KM_ATTESTATION_PACKAGE_INFO> attestation_package_info;
auto rc = build_attestation_package_info(*pinfo, &attestation_package_info);
if (rc != NO_ERROR) {
@@ -283,7 +283,7 @@
// caller is unknown.
if (!status.isOk()) {
ALOGW("package manager request for key attestation ID failed with: %s %d",
- status.exceptionMessage().string(), status.exceptionCode());
+ status.exceptionMessage().c_str(), status.exceptionCode());
auto pinfo = std::make_optional<KeyAttestationPackageInfo>(
String16(kUnknownPackageName), 1 /* version code */,
std::make_shared<KeyAttestationPackageInfo::SignaturesVector>());
diff --git a/keystore2/aaid/Android.bp b/keystore2/aaid/Android.bp
index 3417960..d0a1e77 100644
--- a/keystore2/aaid/Android.bp
+++ b/keystore2/aaid/Android.bp
@@ -38,7 +38,6 @@
source_stem: "bindings",
bindgen_flags: [
- "--size_t-is-usize",
"--allowlist-function=aaid_keystore_attestation_id",
"--allowlist-var=KEY_ATTESTATION_APPLICATION_ID_MAX_SIZE",
],
diff --git a/keystore2/apc_compat/apc_compat.cpp b/keystore2/apc_compat/apc_compat.cpp
index 9f60db2..ffe7595 100644
--- a/keystore2/apc_compat/apc_compat.cpp
+++ b/keystore2/apc_compat/apc_compat.cpp
@@ -118,8 +118,7 @@
hidl_ui_options);
if (!rc.isOk()) {
LOG(ERROR) << "Communication error: promptUserConfirmation: " << rc.description();
- }
- if (rc == ResponseCode::OK) {
+ } else if (rc == ResponseCode::OK) {
callback_ = callback;
}
return responseCode2Compat(rc.withDefault(ResponseCode::SystemError));
diff --git a/keystore2/apc_compat/apc_compat.rs b/keystore2/apc_compat/apc_compat.rs
index 480f14d..e97ac59 100644
--- a/keystore2/apc_compat/apc_compat.rs
+++ b/keystore2/apc_compat/apc_compat.rs
@@ -53,7 +53,10 @@
/// ```
pub struct ApcHal(ApcCompatServiceHandle);
+// SAFETY: This is a wrapper around `ApcCompatSession`, which can be used from any thread.
unsafe impl Send for ApcHal {}
+// SAFETY: `ApcCompatSession` can be called simultaneously from different threads because AIDL and
+// HIDL are thread-safe.
unsafe impl Sync for ApcHal {}
impl Drop for ApcHal {
@@ -120,6 +123,7 @@
// `closeUserConfirmationService` when dropped.
let handle = unsafe { tryGetUserConfirmationService() };
match handle {
+ // SAFETY: This is just a constant.
h if h == unsafe { INVALID_SERVICE_HANDLE } => None,
h => Some(Self(h)),
}
diff --git a/keystore2/selinux/src/lib.rs b/keystore2/selinux/src/lib.rs
index e5c3091..32fdb59 100644
--- a/keystore2/selinux/src/lib.rs
+++ b/keystore2/selinux/src/lib.rs
@@ -20,6 +20,9 @@
//! * selabel_lookup for the keystore2_key backend.
//! And it provides an owning wrapper around context strings `Context`.
+// TODO(b/290018030): Remove this and add proper safety comments.
+#![allow(clippy::undocumented_unsafe_blocks)]
+
use anyhow::Context as AnyhowContext;
use anyhow::{anyhow, Result};
use lazy_static::lazy_static;
@@ -160,8 +163,9 @@
handle: *mut selinux::selabel_handle,
}
-// KeystoreKeyBackend is Sync because selabel_lookup is thread safe.
+// SAFETY: KeystoreKeyBackend is Sync because selabel_lookup is thread safe.
unsafe impl Sync for KeystoreKeyBackend {}
+// SAFETY: KeystoreKeyBackend is Send because selabel_lookup is thread safe.
unsafe impl Send for KeystoreKeyBackend {}
impl KeystoreKeyBackend {
diff --git a/keystore2/src/apc.rs b/keystore2/src/apc.rs
index 5d2083d..fbf9464 100644
--- a/keystore2/src/apc.rs
+++ b/keystore2/src/apc.rs
@@ -244,7 +244,7 @@
// If cancelled by the user or if aborted by the client.
(ResponseCode::CANCELLED, _, _) | (ResponseCode::ABORTED, true, _) => {
// Penalize.
- let mut rate_info = state.rate_limiting.entry(uid).or_default();
+ let rate_info = state.rate_limiting.entry(uid).or_default();
rate_info.counter += 1;
rate_info.timestamp = start;
}
diff --git a/keystore2/src/crypto/Android.bp b/keystore2/src/crypto/Android.bp
index 1ac6467..35fc5a9 100644
--- a/keystore2/src/crypto/Android.bp
+++ b/keystore2/src/crypto/Android.bp
@@ -69,7 +69,6 @@
vendor_available: true,
shared_libs: ["libcrypto"],
bindgen_flags: [
- "--size_t-is-usize",
"--allowlist-function", "hmacSha256",
"--allowlist-function", "randomBytes",
"--allowlist-function", "AES_gcm_encrypt",
diff --git a/keystore2/src/crypto/lib.rs b/keystore2/src/crypto/lib.rs
index 08b7589..8434651 100644
--- a/keystore2/src/crypto/lib.rs
+++ b/keystore2/src/crypto/lib.rs
@@ -49,8 +49,8 @@
/// Generate an AES256 key, essentially 32 random bytes from the underlying
/// boringssl library discretely stuffed into a ZVec.
pub fn generate_aes256_key() -> Result<ZVec, Error> {
- // Safety: key has the same length as the requested number of random bytes.
let mut key = ZVec::new(AES_256_KEY_LENGTH)?;
+ // Safety: key has the same length as the requested number of random bytes.
if unsafe { randomBytes(key.as_mut_ptr(), AES_256_KEY_LENGTH) } {
Ok(key)
} else {
@@ -65,8 +65,8 @@
/// Generate random data of the given size.
pub fn generate_random_data(size: usize) -> Result<Vec<u8>, Error> {
- // Safety: data has the same length as the requested number of random bytes.
let mut data = vec![0; size];
+ // Safety: data has the same length as the requested number of random bytes.
if unsafe { randomBytes(data.as_mut_ptr(), size) } {
Ok(data)
} else {
@@ -209,6 +209,8 @@
let pw = self.get_key();
let mut result = ZVec::new(key_length)?;
+ // Safety: We checked that the salt is exactly 16 bytes long. The other pointers are valid,
+ // and have matching lengths.
unsafe {
generateKeyFromPassword(
result.as_mut_ptr(),
@@ -324,10 +326,10 @@
/// Calls the boringssl ECDH_compute_key function.
pub fn ecdh_compute_key(pub_key: &EC_POINT, priv_key: &ECKey) -> Result<ZVec, Error> {
let mut buf = ZVec::new(EC_MAX_BYTES)?;
+ let result =
// Safety: Our ECDHComputeKey wrapper passes EC_MAX_BYES to ECDH_compute_key, which
// writes at most that many bytes to the output.
// The two keys are valid objects.
- let result =
unsafe { ECDHComputeKey(buf.as_mut_ptr() as *mut std::ffi::c_void, pub_key, priv_key.0) };
if result == -1 {
return Err(Error::ECDHComputeKeyFailed);
@@ -487,9 +489,11 @@
let input = vec![0; 16];
let mut out = vec![0; 16];
let mut out2 = vec![0; 16];
- let key = vec![0; 16];
- let iv = vec![0; 12];
+ let key = [0; 16];
+ let iv = [0; 12];
let mut tag = vec![0; 16];
+ // SAFETY: The various pointers are obtained from references so they are valid, and
+ // `AES_gcm_encrypt` and `AES_gcm_decrypt` don't do anything with them after they return.
unsafe {
let res = AES_gcm_encrypt(
input.as_ptr(),
@@ -519,10 +523,12 @@
#[test]
fn test_create_key_id() {
- let blob = vec![0; 16];
+ let blob = [0; 16];
let mut out: u64 = 0;
+ // SAFETY: The pointers are obtained from references so they are valid, the length matches
+ // the length of the array, and `CreateKeyId` doesn't access them after it returns.
unsafe {
- let res = CreateKeyId(blob.as_ptr(), 16, &mut out);
+ let res = CreateKeyId(blob.as_ptr(), blob.len(), &mut out);
assert!(res);
assert_ne!(out, 0);
}
@@ -531,10 +537,19 @@
#[test]
fn test_generate_key_from_password() {
let mut key = vec![0; 16];
- let pw = vec![0; 16];
- let salt = vec![0; 16];
+ let pw = [0; 16];
+ let salt = [0; 16];
+ // SAFETY: The pointers are obtained from references so they are valid, the salt is the
+ // expected length, the other lengths match the lengths of the arrays, and
+ // `generateKeyFromPassword` doesn't access them after it returns.
unsafe {
- generateKeyFromPassword(key.as_mut_ptr(), 16, pw.as_ptr(), 16, salt.as_ptr());
+ generateKeyFromPassword(
+ key.as_mut_ptr(),
+ key.len(),
+ pw.as_ptr(),
+ pw.len(),
+ salt.as_ptr(),
+ );
}
assert_ne!(key, vec![0; 16]);
}
diff --git a/keystore2/src/crypto/zvec.rs b/keystore2/src/crypto/zvec.rs
index 5a173c3..c917a89 100644
--- a/keystore2/src/crypto/zvec.rs
+++ b/keystore2/src/crypto/zvec.rs
@@ -45,6 +45,7 @@
let v: Vec<u8> = vec![0; size];
let b = v.into_boxed_slice();
if size > 0 {
+ // SAFETY: The address range is part of our address space.
unsafe { mlock(b.as_ptr() as *const std::ffi::c_void, b.len()) }?;
}
Ok(Self { elems: b, len: size })
@@ -71,11 +72,16 @@
impl Drop for ZVec {
fn drop(&mut self) {
for i in 0..self.elems.len() {
- unsafe { write_volatile(self.elems.as_mut_ptr().add(i), 0) };
+ // SAFETY: The pointer is valid and properly aligned because it came from a reference.
+ unsafe { write_volatile(&mut self.elems[i], 0) };
}
if !self.elems.is_empty() {
if let Err(e) =
- unsafe { munlock(self.elems.as_ptr() as *const std::ffi::c_void, self.elems.len()) }
+ // SAFETY: The address range is part of our address space, and was previously locked
+ // by `mlock` in `ZVec::new` or the `TryFrom<Vec<u8>>` implementation.
+ unsafe {
+ munlock(self.elems.as_ptr() as *const std::ffi::c_void, self.elems.len())
+ }
{
log::error!("In ZVec::drop: `munlock` failed: {:?}.", e);
}
@@ -130,6 +136,7 @@
v.resize(v.capacity(), 0);
let b = v.into_boxed_slice();
if !b.is_empty() {
+ // SAFETY: The address range is part of our address space.
unsafe { mlock(b.as_ptr() as *const std::ffi::c_void, b.len()) }?;
}
Ok(Self { elems: b, len })
diff --git a/keystore2/src/enforcements.rs b/keystore2/src/enforcements.rs
index 8d5e985..3bf582f 100644
--- a/keystore2/src/enforcements.rs
+++ b/keystore2/src/enforcements.rs
@@ -51,9 +51,9 @@
/// An outstanding per operation authorization request.
OpAuth,
/// An outstanding request for per operation authorization and secure timestamp.
- TimeStampedOpAuth(Receiver<Result<TimeStampToken, Error>>),
+ TimeStampedOpAuth(Mutex<Receiver<Result<TimeStampToken, Error>>>),
/// An outstanding request for a timestamp token.
- TimeStamp(Receiver<Result<TimeStampToken, Error>>),
+ TimeStamp(Mutex<Receiver<Result<TimeStampToken, Error>>>),
}
#[derive(Debug)]
@@ -64,8 +64,6 @@
hat: Mutex<Option<HardwareAuthToken>>,
}
-unsafe impl Sync for AuthRequest {}
-
impl AuthRequest {
fn op_auth() -> Arc<Self> {
Arc::new(Self { state: AuthRequestState::OpAuth, hat: Mutex::new(None) })
@@ -73,7 +71,7 @@
fn timestamped_op_auth(receiver: Receiver<Result<TimeStampToken, Error>>) -> Arc<Self> {
Arc::new(Self {
- state: AuthRequestState::TimeStampedOpAuth(receiver),
+ state: AuthRequestState::TimeStampedOpAuth(Mutex::new(receiver)),
hat: Mutex::new(None),
})
}
@@ -82,7 +80,10 @@
hat: HardwareAuthToken,
receiver: Receiver<Result<TimeStampToken, Error>>,
) -> Arc<Self> {
- Arc::new(Self { state: AuthRequestState::TimeStamp(receiver), hat: Mutex::new(Some(hat)) })
+ Arc::new(Self {
+ state: AuthRequestState::TimeStamp(Mutex::new(receiver)),
+ hat: Mutex::new(Some(hat)),
+ })
}
fn add_auth_token(&self, hat: HardwareAuthToken) {
@@ -100,7 +101,11 @@
let tst = match &self.state {
AuthRequestState::TimeStampedOpAuth(recv) | AuthRequestState::TimeStamp(recv) => {
- let result = recv.recv().context("In get_auth_tokens: Sender disconnected.")?;
+ let result = recv
+ .lock()
+ .unwrap()
+ .recv()
+ .context("In get_auth_tokens: Sender disconnected.")?;
Some(result.context(ks_err!(
"Worker responded with error \
from generating timestamp token.",
diff --git a/keystore2/src/entropy.rs b/keystore2/src/entropy.rs
index de38187..1dcdc86 100644
--- a/keystore2/src/entropy.rs
+++ b/keystore2/src/entropy.rs
@@ -29,7 +29,7 @@
/// Register the entropy feeder as an idle callback.
pub fn register_feeder() {
crate::globals::ASYNC_TASK.add_idle(|shelf| {
- let mut info = shelf.get_mut::<FeederInfo>();
+ let info = shelf.get_mut::<FeederInfo>();
let now = Instant::now();
let feed_needed = match info.last_feed {
None => true,
diff --git a/keystore2/src/fuzzers/Android.bp b/keystore2/src/fuzzers/Android.bp
index 9f3e104..0809dc8 100644
--- a/keystore2/src/fuzzers/Android.bp
+++ b/keystore2/src/fuzzers/Android.bp
@@ -20,7 +20,6 @@
name: "keystore2_unsafe_fuzzer",
srcs: ["keystore2_unsafe_fuzzer.rs"],
rustlibs: [
- "libbinder_rs",
"libkeystore2",
"libkeystore2_crypto_rust",
"libkeystore2_hal_names_rust",
diff --git a/keystore2/src/fuzzers/keystore2_unsafe_fuzzer.rs b/keystore2/src/fuzzers/keystore2_unsafe_fuzzer.rs
index b8259cf..8b8843d 100644
--- a/keystore2/src/fuzzers/keystore2_unsafe_fuzzer.rs
+++ b/keystore2/src/fuzzers/keystore2_unsafe_fuzzer.rs
@@ -16,7 +16,6 @@
#![no_main]
-use binder::get_declared_instances;
use keystore2::{legacy_blob::LegacyBlobLoader, utils::ui_opts_2_compat};
use keystore2_aaid::get_aaid;
use keystore2_apc_compat::ApcHal;
@@ -94,10 +93,6 @@
minor_version: usize,
hidl_interface_name: &'a str,
},
- GetAidlInstances {
- aidl_package: &'a str,
- aidl_interface_name: &'a str,
- },
GetAaid {
aaid_uid: u32,
},
@@ -189,12 +184,6 @@
} => {
get_hidl_instances(hidl_package, major_version, minor_version, hidl_interface_name);
}
- FuzzCommand::GetAidlInstances { aidl_package, aidl_interface_name } => {
- get_declared_instances(
- format!("{}.{}", aidl_package, aidl_interface_name).as_str(),
- )
- .unwrap();
- }
FuzzCommand::GetAaid { aaid_uid } => {
let _res = get_aaid(aaid_uid);
}
diff --git a/keystore2/src/id_rotation.rs b/keystore2/src/id_rotation.rs
index 460caa7..5904047 100644
--- a/keystore2/src/id_rotation.rs
+++ b/keystore2/src/id_rotation.rs
@@ -26,7 +26,7 @@
use std::fs;
use std::io::ErrorKind;
use std::path::{Path, PathBuf};
-use std::time::Duration;
+use std::time::{Duration, SystemTime};
const ID_ROTATION_PERIOD: Duration = Duration::from_secs(30 * 24 * 60 * 60); // Thirty days.
static TIMESTAMP_FILE_NAME: &str = "timestamp";
@@ -36,6 +36,8 @@
/// and passed down to the users of the feature which can then query the timestamp on demand.
#[derive(Debug, Clone)]
pub struct IdRotationState {
+ /// We consider the time of last factory reset to be the point in time when this timestamp file
+ /// is created.
timestamp_path: PathBuf,
}
@@ -47,25 +49,41 @@
Self { timestamp_path }
}
- /// Reads the metadata of or creates the timestamp file. It returns true if the timestamp
- /// file is younger than `ID_ROTATION_PERIOD`, i.e., 30 days.
- pub fn had_factory_reset_since_id_rotation(&self) -> Result<bool> {
+ /// Returns true iff a factory reset has occurred since the last ID rotation.
+ pub fn had_factory_reset_since_id_rotation(
+ &self,
+ creation_datetime: &SystemTime,
+ ) -> Result<bool> {
match fs::metadata(&self.timestamp_path) {
Ok(metadata) => {
- let duration_since_factory_reset = metadata
- .modified()
- .context("File creation time not supported.")?
- .elapsed()
- .context("Failed to compute time elapsed since factory reset.")?;
- Ok(duration_since_factory_reset < ID_ROTATION_PERIOD)
+ // For Tag::UNIQUE_ID, temporal counter value is defined as Tag::CREATION_DATETIME
+ // divided by 2592000000, dropping any remainder. Temporal counter value is
+ // effectively the index of the ID rotation period that we are currently in, with
+ // each ID rotation period being 30 days.
+ let temporal_counter_value = creation_datetime
+ .duration_since(SystemTime::UNIX_EPOCH)
+ .context(ks_err!("Failed to get epoch time"))?
+ .as_millis()
+ / ID_ROTATION_PERIOD.as_millis();
+
+ // Calculate the beginning of the current ID rotation period, which is also the
+ // last time ID was rotated.
+ let id_rotation_time: SystemTime = SystemTime::UNIX_EPOCH
+ .checked_add(ID_ROTATION_PERIOD * temporal_counter_value.try_into()?)
+ .context(ks_err!("Failed to get ID rotation time."))?;
+
+ let factory_reset_time =
+ metadata.modified().context(ks_err!("File creation time not supported."))?;
+
+ Ok(id_rotation_time <= factory_reset_time)
}
Err(e) => match e.kind() {
ErrorKind::NotFound => {
fs::File::create(&self.timestamp_path)
- .context("Failed to create timestamp file.")?;
+ .context(ks_err!("Failed to create timestamp file."))?;
Ok(true)
}
- _ => Err(e).context("Failed to open timestamp file."),
+ _ => Err(e).context(ks_err!("Failed to open timestamp file.")),
},
}
.context(ks_err!())
@@ -78,47 +96,75 @@
use keystore2_test_utils::TempDir;
use nix::sys::stat::utimes;
use nix::sys::time::{TimeVal, TimeValLike};
- use std::convert::TryInto;
- use std::time::UNIX_EPOCH;
+ use std::thread::sleep;
- #[test]
- fn test_had_factory_reset_since_id_rotation() -> Result<()> {
- let temp_dir = TempDir::new("test_had_factory_reset_since_id_rotation_")
- .expect("Failed to create temp dir.");
+ static TEMP_DIR_NAME: &str = "test_had_factory_reset_since_id_rotation_";
+
+ fn set_up() -> (TempDir, PathBuf, IdRotationState) {
+ let temp_dir = TempDir::new(TEMP_DIR_NAME).expect("Failed to create temp dir.");
+ let mut timestamp_file_path = temp_dir.path().to_owned();
+ timestamp_file_path.push(TIMESTAMP_FILE_NAME);
let id_rotation_state = IdRotationState::new(temp_dir.path());
- let mut temp_file_path = temp_dir.path().to_owned();
- temp_file_path.push(TIMESTAMP_FILE_NAME);
+ (temp_dir, timestamp_file_path, id_rotation_state)
+ }
+
+ #[test]
+ fn test_timestamp_creation() {
+ let (_temp_dir, timestamp_file_path, id_rotation_state) = set_up();
+ let creation_datetime = SystemTime::now();
// The timestamp file should not exist.
- assert!(!temp_file_path.exists());
+ assert!(!timestamp_file_path.exists());
- // This should return true.
- assert!(id_rotation_state.had_factory_reset_since_id_rotation()?);
+ // Trigger timestamp file creation one second later.
+ sleep(Duration::new(1, 0));
+ assert!(id_rotation_state.had_factory_reset_since_id_rotation(&creation_datetime).unwrap());
// Now the timestamp file should exist.
- assert!(temp_file_path.exists());
+ assert!(timestamp_file_path.exists());
- // We should still return true because the timestamp file is young.
- assert!(id_rotation_state.had_factory_reset_since_id_rotation()?);
+ let metadata = fs::metadata(×tamp_file_path).unwrap();
+ assert!(metadata.modified().unwrap() > creation_datetime);
+ }
- // Now let's age the timestamp file by backdating the modification time.
- let metadata = fs::metadata(&temp_file_path)?;
- let mtime = metadata.modified()?;
- let mtime = mtime.duration_since(UNIX_EPOCH)?;
- let mtime =
- mtime.checked_sub(ID_ROTATION_PERIOD).expect("Failed to subtract id rotation period");
- let mtime = TimeVal::seconds(mtime.as_secs().try_into().unwrap());
+ #[test]
+ fn test_existing_timestamp() {
+ let (_temp_dir, timestamp_file_path, id_rotation_state) = set_up();
- let atime = metadata.accessed()?;
- let atime = atime.duration_since(UNIX_EPOCH)?;
- let atime = TimeVal::seconds(atime.as_secs().try_into().unwrap());
+ // Let's start with at a known point in time, so that it's easier to control which ID
+ // rotation period we're in.
+ let mut creation_datetime = SystemTime::UNIX_EPOCH;
- utimes(&temp_file_path, &atime, &mtime)?;
+ // Create timestamp file and backdate it back to Unix epoch.
+ fs::File::create(×tamp_file_path).unwrap();
+ let mtime = TimeVal::seconds(0);
+ let atime = TimeVal::seconds(0);
+ utimes(×tamp_file_path, &atime, &mtime).unwrap();
- // Now that the file has aged we should see false.
- assert!(!id_rotation_state.had_factory_reset_since_id_rotation()?);
+ // Timestamp file was backdated to the very beginning of the current ID rotation period.
+ // So, this should return true.
+ assert!(id_rotation_state.had_factory_reset_since_id_rotation(&creation_datetime).unwrap());
- Ok(())
+ // Move time forward, but stay in the same ID rotation period.
+ creation_datetime += Duration::from_millis(1);
+
+ // We should still return true because we're in the same ID rotation period.
+ assert!(id_rotation_state.had_factory_reset_since_id_rotation(&creation_datetime).unwrap());
+
+ // Move time to the next ID rotation period.
+ creation_datetime += ID_ROTATION_PERIOD;
+
+ // Now we should see false.
+ assert!(!id_rotation_state
+ .had_factory_reset_since_id_rotation(&creation_datetime)
+ .unwrap());
+
+ // Move timestamp to the future. This shouldn't ever happen, but even in this edge case ID
+ // must be rotated.
+ let mtime = TimeVal::seconds((ID_ROTATION_PERIOD.as_secs() * 10).try_into().unwrap());
+ let atime = TimeVal::seconds((ID_ROTATION_PERIOD.as_secs() * 10).try_into().unwrap());
+ utimes(×tamp_file_path, &atime, &mtime).unwrap();
+ assert!(id_rotation_state.had_factory_reset_since_id_rotation(&creation_datetime).unwrap());
}
}
diff --git a/keystore2/src/keystore2_main.rs b/keystore2/src/keystore2_main.rs
index 31c1e29..059d59d 100644
--- a/keystore2/src/keystore2_main.rs
+++ b/keystore2/src/keystore2_main.rs
@@ -68,6 +68,8 @@
fn sqlite_log_handler(err: c_int, message: &str) {
log::error!("[SQLITE3] {}: {}", err, message);
}
+ // SAFETY: There are no other threads yet, `sqlite_log_handler` is threadsafe, and it doesn't
+ // invoke any SQLite calls.
unsafe { sqlite_trace::config_log(Some(sqlite_log_handler)) }
.expect("Error setting sqlite log callback.");
diff --git a/keystore2/src/km_compat/km_compat.cpp b/keystore2/src/km_compat/km_compat.cpp
index e27cd1c..6bfd47a 100644
--- a/keystore2/src/km_compat/km_compat.cpp
+++ b/keystore2/src/km_compat/km_compat.cpp
@@ -144,6 +144,11 @@
//
const uint8_t kKeyBlobMagic[7] = {'p', 'K', 'M', 'b', 'l', 'o', 'b'};
+// Per RFC 5280 4.1.2.5, an undefined expiration (not-after) field should be set
+// to 9999-12-31T23:59:59Z.
+//
+const uint64_t kUndefinedNotAfter = 253402300799000;
+
// Prefixes a keyblob returned by e.g. generateKey() with information on whether it
// originated from the real underlying KeyMaster HAL or from soft-KeyMint.
//
@@ -260,6 +265,16 @@
return result;
}
+std::vector<KMV1::KeyParameter>
+extractCombinedParams(const std::vector<KMV1::KeyCharacteristics>& characteristics) {
+ std::vector<KMV1::KeyParameter> result;
+ for (auto characteristic : characteristics) {
+ std::copy(characteristic.authorizations.begin(), characteristic.authorizations.end(),
+ std::back_inserter(result));
+ }
+ return result;
+}
+
ScopedAStatus convertErrorCode(KMV1::ErrorCode result) {
if (result == KMV1::ErrorCode::OK) {
return ScopedAStatus::ok();
@@ -587,6 +602,15 @@
LOG(ERROR) << __func__ << " transaction failed. " << result.description();
return convertErrorCode(KMV1::ErrorCode::UNKNOWN_ERROR);
}
+ if (errorCode == KMV1::ErrorCode::OK) {
+ auto params = extractCombinedParams(out_creationResult->keyCharacteristics);
+ auto cert = getCertificate(params, out_creationResult->keyBlob, true /* isWrappedKey */);
+ // importWrappedKey used to not generate a certificate. Ignore the error to preserve
+ // backwards compatibility with clients that can't successfully generate a certificate.
+ if (std::holds_alternative<std::vector<Certificate>>(cert)) {
+ out_creationResult->certificateChain = std::get<std::vector<Certificate>>(cert);
+ }
+ }
return convertErrorCode(errorCode);
}
@@ -1055,7 +1079,7 @@
static std::variant<keystore::X509_Ptr, KMV1::ErrorCode>
makeCert(::android::sp<Keymaster> mDevice, const std::vector<KeyParameter>& keyParams,
- const std::vector<uint8_t>& keyBlob) {
+ const std::vector<uint8_t>& keyBlob, bool isWrappedKey) {
// Start generating the certificate.
// Get public key for makeCert.
KMV1::ErrorCode errorCode;
@@ -1097,15 +1121,21 @@
serial = *blob;
}
+ // There is no way to specify CERTIFICATE_NOT_BEFORE and CERTIFICATE_NOT_AFTER for wrapped keys.
+ // So we provide default values.
int64_t activation;
- if (auto date = getParam(keyParams, KMV1::TAG_CERTIFICATE_NOT_BEFORE)) {
+ if (isWrappedKey) {
+ activation = 0;
+ } else if (auto date = getParam(keyParams, KMV1::TAG_CERTIFICATE_NOT_BEFORE)) {
activation = static_cast<int64_t>(*date);
} else {
return KMV1::ErrorCode::MISSING_NOT_BEFORE;
}
int64_t expiration;
- if (auto date = getParam(keyParams, KMV1::TAG_CERTIFICATE_NOT_AFTER)) {
+ if (isWrappedKey) {
+ expiration = kUndefinedNotAfter;
+ } else if (auto date = getParam(keyParams, KMV1::TAG_CERTIFICATE_NOT_AFTER)) {
expiration = static_cast<int64_t>(*date);
} else {
return KMV1::ErrorCode::MISSING_NOT_AFTER;
@@ -1235,7 +1265,7 @@
std::variant<std::vector<Certificate>, KMV1::ErrorCode>
KeyMintDevice::getCertificate(const std::vector<KeyParameter>& keyParams,
- const std::vector<uint8_t>& prefixedKeyBlob) {
+ const std::vector<uint8_t>& prefixedKeyBlob, bool isWrappedKey) {
const std::vector<uint8_t>& keyBlob = prefixedKeyBlobRemovePrefix(prefixedKeyBlob);
// There are no certificates for symmetric keys.
@@ -1278,7 +1308,7 @@
}
// makeCert
- auto certOrError = makeCert(mDevice, keyParams, keyBlob);
+ auto certOrError = makeCert(mDevice, keyParams, keyBlob, isWrappedKey);
if (std::holds_alternative<KMV1::ErrorCode>(certOrError)) {
return std::get<KMV1::ErrorCode>(certOrError);
}
diff --git a/keystore2/src/km_compat/km_compat.h b/keystore2/src/km_compat/km_compat.h
index 6654c4a..c4bcdaa 100644
--- a/keystore2/src/km_compat/km_compat.h
+++ b/keystore2/src/km_compat/km_compat.h
@@ -150,7 +150,8 @@
// These are public to allow testing code to use them directly.
// This class should not be used publicly anyway.
std::variant<std::vector<Certificate>, KMV1_ErrorCode>
- getCertificate(const std::vector<KeyParameter>& keyParams, const std::vector<uint8_t>& keyBlob);
+ getCertificate(const std::vector<KeyParameter>& keyParams, const std::vector<uint8_t>& keyBlob,
+ bool isWrappedKey = false);
void setNumFreeSlots(uint8_t numFreeSlots);
diff --git a/keystore2/src/km_compat/lib.rs b/keystore2/src/km_compat/lib.rs
index 2632ec4..e61a13a 100644
--- a/keystore2/src/km_compat/lib.rs
+++ b/keystore2/src/km_compat/lib.rs
@@ -21,6 +21,7 @@
#[allow(missing_docs)] // TODO remove this
pub fn add_keymint_device_service() -> i32 {
+ // SAFETY: This is always safe to call.
unsafe { addKeyMintDeviceService() }
}
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index 73dc881..8c9ed63 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -178,7 +178,7 @@
(SecurityLevel::TRUSTED_ENVIRONMENT, "TRUSTED_ENVIRONMENT"),
(SecurityLevel::STRONGBOX, "STRONGBOX"),
];
- sec_levels.iter().fold(Ok(()), move |result, (sec_level, sec_level_string)| {
+ sec_levels.iter().try_fold((), |_result, (sec_level, sec_level_string)| {
let curr_result = Maintenance::call_with_watchdog(*sec_level, name, &op);
match curr_result {
Ok(()) => log::info!(
@@ -193,7 +193,7 @@
e
),
}
- result.and(curr_result)
+ curr_result
})
}
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 5eed37c..db44d4b 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -405,8 +405,7 @@
) -> Result<Vec<KeyParameter>> {
let mut result = params.to_vec();
- // Unconditionally add the CREATION_DATETIME tag and prevent callers from
- // specifying it.
+ // Prevent callers from specifying the CREATION_DATETIME tag.
if params.iter().any(|kp| kp.tag == Tag::CREATION_DATETIME) {
return Err(Error::Rc(ResponseCode::INVALID_ARGUMENT)).context(ks_err!(
"KeystoreSecurityLevel::add_required_parameters: \
@@ -414,12 +413,16 @@
));
}
+ // Use this variable to refer to notion of "now". This eliminates discrepancies from
+ // quering the clock multiple times.
+ let creation_datetime = SystemTime::now();
+
// Add CREATION_DATETIME only if the backend version Keymint V1 (100) or newer.
if self.hw_info.versionNumber >= 100 {
result.push(KeyParameter {
tag: Tag::CREATION_DATETIME,
value: KeyParameterValue::DateTime(
- SystemTime::now()
+ creation_datetime
.duration_since(SystemTime::UNIX_EPOCH)
.context(ks_err!(
"KeystoreSecurityLevel::add_required_parameters: \
@@ -462,7 +465,7 @@
}
if self
.id_rotation_state
- .had_factory_reset_since_id_rotation()
+ .had_factory_reset_since_id_rotation(&creation_datetime)
.context(ks_err!("Call to had_factory_reset_since_id_rotation failed."))?
{
result.push(KeyParameter {
diff --git a/keystore2/src/super_key.rs b/keystore2/src/super_key.rs
index 2e8b60f..7fc3ed4 100644
--- a/keystore2/src/super_key.rs
+++ b/keystore2/src/super_key.rs
@@ -823,7 +823,7 @@
unlocking_sids: &[i64],
) {
log::info!("Locking screen bound for user {} sids {:?}", user_id, unlocking_sids);
- let mut entry = self.data.user_keys.entry(user_id).or_default();
+ let entry = self.data.user_keys.entry(user_id).or_default();
if !unlocking_sids.is_empty() {
if let (Some(aes), Some(ecdh)) = (
entry.screen_lock_bound.as_ref().cloned(),
@@ -899,7 +899,7 @@
db: &mut KeystoreDB,
user_id: UserId,
) -> Result<()> {
- let mut entry = self.data.user_keys.entry(user_id).or_default();
+ let entry = self.data.user_keys.entry(user_id).or_default();
if let Some(biometric) = entry.biometric_unlock.as_ref() {
let (key_id_guard, key_entry) = db
.load_key_entry(
diff --git a/keystore2/src/utils.rs b/keystore2/src/utils.rs
index acac7ee..584a51c 100644
--- a/keystore2/src/utils.rs
+++ b/keystore2/src/utils.rs
@@ -215,8 +215,8 @@
/// as an integer.
pub fn get_current_time_in_milliseconds() -> i64 {
let mut current_time = libc::timespec { tv_sec: 0, tv_nsec: 0 };
- // Following unsafe block includes one system call to get monotonic time.
- // Therefore, it is not considered harmful.
+ // SAFETY: The pointer is valid because it comes from a reference, and clock_gettime doesn't
+ // retain it beyond the call.
unsafe { libc::clock_gettime(libc::CLOCK_MONOTONIC_RAW, &mut current_time) };
current_time.tv_sec as i64 * 1000 + (current_time.tv_nsec as i64 / 1_000_000)
}
diff --git a/keystore2/test_utils/authorizations.rs b/keystore2/test_utils/authorizations.rs
index 4608bc5..514cbd3 100644
--- a/keystore2/test_utils/authorizations.rs
+++ b/keystore2/test_utils/authorizations.rs
@@ -161,6 +161,87 @@
.push(KeyParameter { tag: Tag::MIN_MAC_LENGTH, value: KeyParameterValue::Integer(l) });
self
}
+
+ /// Add Attestation-Device-Brand.
+ pub fn attestation_device_brand(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_BRAND,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
+
+ /// Add Attestation-Device-name.
+ pub fn attestation_device_name(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_DEVICE,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
+
+ /// Add Attestation-Device-Product-Name.
+ pub fn attestation_device_product_name(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_PRODUCT,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
+
+ /// Add Attestation-Device-Serial.
+ pub fn attestation_device_serial(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_SERIAL,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
+
+ /// Add Attestation-Device-IMEI.
+ pub fn attestation_device_imei(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_IMEI,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
+
+ /// Add Attestation-Device-IMEI.
+ pub fn attestation_device_second_imei(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_SECOND_IMEI,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
+
+ /// Add Attestation-Device-MEID.
+ pub fn attestation_device_meid(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_MEID,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
+
+ /// Add Attestation-Device-Manufacturer.
+ pub fn attestation_device_manufacturer(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_MANUFACTURER,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
+
+ /// Add Attestation-Device-Model.
+ pub fn attestation_device_model(mut self, b: Vec<u8>) -> Self {
+ self.0.push(KeyParameter {
+ tag: Tag::ATTESTATION_ID_MODEL,
+ value: KeyParameterValue::Blob(b),
+ });
+ self
+ }
}
impl Deref for AuthSetBuilder {
diff --git a/keystore2/test_utils/key_generations.rs b/keystore2/test_utils/key_generations.rs
index ff40aa1..0f9ecbe 100644
--- a/keystore2/test_utils/key_generations.rs
+++ b/keystore2/test_utils/key_generations.rs
@@ -46,6 +46,52 @@
/// Vold context
pub const TARGET_VOLD_CTX: &str = "u:r:vold:s0";
+/// Allowed tags in generated/imported key authorizations.
+/// See hardware/interfaces/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl for the
+/// list feature tags.
+/// Note: This list need to be updated whenever a new Tag is introduced and is expected to be added
+/// in key authorizations.
+pub const ALLOWED_TAGS_IN_KEY_AUTHS: &[Tag] = &[
+ Tag::ACTIVE_DATETIME,
+ Tag::ALGORITHM,
+ Tag::ALLOW_WHILE_ON_BODY,
+ Tag::AUTH_TIMEOUT,
+ Tag::BLOCK_MODE,
+ Tag::BOOTLOADER_ONLY,
+ Tag::BOOT_PATCHLEVEL,
+ Tag::CALLER_NONCE,
+ Tag::CREATION_DATETIME,
+ Tag::DIGEST,
+ Tag::EARLY_BOOT_ONLY,
+ Tag::EC_CURVE,
+ Tag::IDENTITY_CREDENTIAL_KEY,
+ Tag::INCLUDE_UNIQUE_ID,
+ Tag::KEY_SIZE,
+ Tag::MAX_BOOT_LEVEL,
+ Tag::MAX_USES_PER_BOOT,
+ Tag::MIN_MAC_LENGTH,
+ Tag::NO_AUTH_REQUIRED,
+ Tag::ORIGIN,
+ Tag::ORIGINATION_EXPIRE_DATETIME,
+ Tag::OS_PATCHLEVEL,
+ Tag::OS_VERSION,
+ Tag::PADDING,
+ Tag::PURPOSE,
+ Tag::ROLLBACK_RESISTANCE,
+ Tag::RSA_OAEP_MGF_DIGEST,
+ Tag::RSA_PUBLIC_EXPONENT,
+ Tag::STORAGE_KEY,
+ Tag::TRUSTED_CONFIRMATION_REQUIRED,
+ Tag::TRUSTED_USER_PRESENCE_REQUIRED,
+ Tag::UNLOCKED_DEVICE_REQUIRED,
+ Tag::USAGE_COUNT_LIMIT,
+ Tag::USAGE_EXPIRE_DATETIME,
+ Tag::USER_AUTH_TYPE,
+ Tag::USER_ID,
+ Tag::USER_SECURE_ID,
+ Tag::VENDOR_PATCHLEVEL,
+];
+
/// Key parameters to generate a key.
pub struct KeyParams {
/// Key Size.
@@ -306,6 +352,12 @@
/// Error code to indicate error while using keystore-engine API.
#[error("Failed to perform crypto op using keystore-engine APIs.")]
Keystore2EngineOpFailed,
+ /// Error code to indicate error in attestation-id validation.
+ #[error("Failed to validate attestation-id.")]
+ ValidateAttestIdFailed,
+ /// Error code to indicate error in getting value from attest record.
+ #[error("Failed to get value from attest record.")]
+ AttestRecordGetValueFailed,
}
/// Keystore2 error mapping.
@@ -331,6 +383,35 @@
})
}
+/// Verify that given key param is listed in given authorizations list.
+pub fn check_key_param(authorizations: &[Authorization], key_param: &KeyParameter) -> bool {
+ authorizations.iter().any(|auth| &auth.keyParameter == key_param)
+}
+
+fn check_key_authorizations(authorizations: &[Authorization], expected_params: &[KeyParameter]) {
+ // Make sure key authorizations contains only `ALLOWED_TAGS_IN_KEY_AUTHS`
+ authorizations.iter().all(|auth| {
+ assert!(
+ ALLOWED_TAGS_IN_KEY_AUTHS.contains(&auth.keyParameter.tag),
+ "key authorization is not allowed: {:#?}",
+ auth.keyParameter
+ );
+ true
+ });
+
+ //Check allowed-expected-key-parameters are present in given key authorizations list.
+ expected_params.iter().all(|key_param| {
+ if ALLOWED_TAGS_IN_KEY_AUTHS.contains(&key_param.tag) {
+ assert!(
+ check_key_param(authorizations, key_param),
+ "Key parameter not found: {:#?}",
+ key_param
+ );
+ }
+ true
+ });
+}
+
/// Generate EC Key using given security level and domain with below key parameters and
/// optionally allow the generated key to be attested with factory provisioned attest key using
/// given challenge and application id -
@@ -374,6 +455,7 @@
assert!(key_metadata.key.blob.is_some());
}
+ check_key_authorizations(&key_metadata.authorizations, &gen_params);
Ok(key_metadata)
}
Err(e) => Err(e),
@@ -416,6 +498,7 @@
} else {
assert!(key_metadata.key.blob.is_none());
}
+ check_key_authorizations(&key_metadata.authorizations, &gen_params);
Ok(key_metadata)
}
@@ -477,6 +560,19 @@
|| key_metadata.key.blob.is_none()
);
+ check_key_authorizations(&key_metadata.authorizations, &gen_params);
+ // If `RSA_OAEP_MGF_DIGEST` tag is not mentioned explicitly while generating/importing a key,
+ // then make sure `RSA_OAEP_MGF_DIGEST` tag with default value (SHA1) must not be included in
+ // key authorization list.
+ if key_params.mgf_digest.is_none() {
+ assert!(!check_key_param(
+ &key_metadata.authorizations,
+ &KeyParameter {
+ tag: Tag::RSA_OAEP_MGF_DIGEST,
+ value: KeyParameterValue::Digest(Digest::SHA1)
+ }
+ ));
+ }
Ok(key_metadata)
}
@@ -521,6 +617,7 @@
// Should not have an attestation record.
assert!(key_metadata.certificateChain.is_none());
+ check_key_authorizations(&key_metadata.authorizations, &gen_params);
Ok(key_metadata)
}
@@ -560,6 +657,7 @@
// Should not have an attestation record.
assert!(key_metadata.certificateChain.is_none());
+ check_key_authorizations(&key_metadata.authorizations, &gen_params);
Ok(key_metadata)
}
@@ -644,6 +742,7 @@
// Should have an attestation record.
assert!(attestation_key_metadata.certificateChain.is_some());
+ check_key_authorizations(&attestation_key_metadata.authorizations, &gen_params);
Ok(attestation_key_metadata)
}
@@ -678,20 +777,10 @@
// Shouldn't have an attestation record.
assert!(ec_key_metadata.certificateChain.is_none());
+ check_key_authorizations(&ec_key_metadata.authorizations, &ec_gen_params);
Ok(ec_key_metadata)
}
-/// Verify that given key param is listed in given authorizations list.
-pub fn check_key_param(authorizations: &[Authorization], key_param: KeyParameter) -> bool {
- for authrization in authorizations {
- if authrization.keyParameter == key_param {
- return true;
- }
- }
-
- false
-}
-
/// Imports above defined RSA key - `RSA_2048_KEY` and validates imported key parameters.
pub fn import_rsa_2048_key(
sec_level: &binder::Strong<dyn IKeystoreSecurityLevel>,
@@ -713,24 +802,27 @@
assert!(key_metadata.certificate.is_some());
assert!(key_metadata.certificateChain.is_none());
+ check_key_authorizations(&key_metadata.authorizations, &import_params);
+
+ // Check below auths explicitly, they might not be addd in import parameters.
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ALGORITHM, value: KeyParameterValue::Algorithm(Algorithm::RSA) }
+ &KeyParameter { tag: Tag::ALGORITHM, value: KeyParameterValue::Algorithm(Algorithm::RSA) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::KEY_SIZE, value: KeyParameterValue::Integer(2048) }
+ &KeyParameter { tag: Tag::KEY_SIZE, value: KeyParameterValue::Integer(2048) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::DIGEST, value: KeyParameterValue::Digest(Digest::SHA_2_256) }
+ &KeyParameter { tag: Tag::DIGEST, value: KeyParameterValue::Digest(Digest::SHA_2_256) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter {
+ &KeyParameter {
tag: Tag::RSA_PUBLIC_EXPONENT,
value: KeyParameterValue::LongInteger(65537)
}
@@ -738,7 +830,7 @@
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter {
+ &KeyParameter {
tag: Tag::PADDING,
value: KeyParameterValue::PaddingMode(PaddingMode::RSA_PSS)
}
@@ -746,7 +838,7 @@
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
+ &KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
));
Ok(key_metadata)
@@ -773,23 +865,26 @@
assert!(key_metadata.certificate.is_some());
assert!(key_metadata.certificateChain.is_none());
+ check_key_authorizations(&key_metadata.authorizations, &import_params);
+
+ // Check below auths explicitly, they might not be addd in import parameters.
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ALGORITHM, value: KeyParameterValue::Algorithm(Algorithm::EC) }
+ &KeyParameter { tag: Tag::ALGORITHM, value: KeyParameterValue::Algorithm(Algorithm::EC) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::EC_CURVE, value: KeyParameterValue::EcCurve(EcCurve::P_256) }
+ &KeyParameter { tag: Tag::EC_CURVE, value: KeyParameterValue::EcCurve(EcCurve::P_256) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::DIGEST, value: KeyParameterValue::Digest(Digest::SHA_2_256) }
+ &KeyParameter { tag: Tag::DIGEST, value: KeyParameterValue::Digest(Digest::SHA_2_256) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
+ &KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
));
Ok(key_metadata)
@@ -822,28 +917,31 @@
AES_KEY,
)?;
+ check_key_authorizations(&key_metadata.authorizations, &import_params);
+
+ // Check below auths explicitly, they might not be addd in import parameters.
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ALGORITHM, value: KeyParameterValue::Algorithm(Algorithm::AES) }
+ &KeyParameter { tag: Tag::ALGORITHM, value: KeyParameterValue::Algorithm(Algorithm::AES) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::KEY_SIZE, value: KeyParameterValue::Integer(128) }
+ &KeyParameter { tag: Tag::KEY_SIZE, value: KeyParameterValue::Integer(128) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter {
+ &KeyParameter {
tag: Tag::PADDING,
value: KeyParameterValue::PaddingMode(PaddingMode::PKCS7)
}
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::BLOCK_MODE, value: KeyParameterValue::BlockMode(BlockMode::ECB) }
+ &KeyParameter { tag: Tag::BLOCK_MODE, value: KeyParameterValue::BlockMode(BlockMode::ECB) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
+ &KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
));
Ok(key_metadata)
@@ -878,31 +976,34 @@
TRIPLE_DES_KEY,
)?;
+ check_key_authorizations(&key_metadata.authorizations, &import_params);
+
+ // Check below auths explicitly, they might not be addd in import parameters.
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter {
+ &KeyParameter {
tag: Tag::ALGORITHM,
value: KeyParameterValue::Algorithm(Algorithm::TRIPLE_DES)
}
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::KEY_SIZE, value: KeyParameterValue::Integer(168) }
+ &KeyParameter { tag: Tag::KEY_SIZE, value: KeyParameterValue::Integer(168) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter {
+ &KeyParameter {
tag: Tag::PADDING,
value: KeyParameterValue::PaddingMode(PaddingMode::PKCS7)
}
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::BLOCK_MODE, value: KeyParameterValue::BlockMode(BlockMode::ECB) }
+ &KeyParameter { tag: Tag::BLOCK_MODE, value: KeyParameterValue::BlockMode(BlockMode::ECB) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
+ &KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
));
Ok(key_metadata)
@@ -935,21 +1036,24 @@
HMAC_KEY,
)?;
+ check_key_authorizations(&key_metadata.authorizations, &import_params);
+
+ // Check below auths explicitly, they might not be addd in import parameters.
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ALGORITHM, value: KeyParameterValue::Algorithm(Algorithm::HMAC) }
+ &KeyParameter { tag: Tag::ALGORITHM, value: KeyParameterValue::Algorithm(Algorithm::HMAC) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::KEY_SIZE, value: KeyParameterValue::Integer(128) }
+ &KeyParameter { tag: Tag::KEY_SIZE, value: KeyParameterValue::Integer(128) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::DIGEST, value: KeyParameterValue::Digest(Digest::SHA_2_256) }
+ &KeyParameter { tag: Tag::DIGEST, value: KeyParameterValue::Digest(Digest::SHA_2_256) }
));
assert!(check_key_param(
&key_metadata.authorizations,
- KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
+ &KeyParameter { tag: Tag::ORIGIN, value: KeyParameterValue::Origin(KeyOrigin::IMPORTED) }
));
Ok(key_metadata)
@@ -1084,6 +1188,7 @@
assert!(key_metadata.key.blob.is_some());
}
+ check_key_authorizations(&key_metadata.authorizations, &gen_params);
Ok(key_metadata)
}
Err(e) => Err(e),
@@ -1109,3 +1214,77 @@
Ok(imported_key_aliases)
}
+
+/// Generate attested EC-P_256 key with device id attestation.
+pub fn generate_key_with_attest_id(
+ sec_level: &binder::Strong<dyn IKeystoreSecurityLevel>,
+ algorithm: Algorithm,
+ alias: Option<String>,
+ att_challenge: &[u8],
+ attest_key: &KeyDescriptor,
+ attest_id: Tag,
+ value: Vec<u8>,
+) -> binder::Result<KeyMetadata> {
+ assert!(algorithm == Algorithm::RSA || algorithm == Algorithm::EC);
+
+ let mut ec_gen_params;
+ if algorithm == Algorithm::EC {
+ ec_gen_params = AuthSetBuilder::new()
+ .no_auth_required()
+ .algorithm(Algorithm::EC)
+ .purpose(KeyPurpose::SIGN)
+ .purpose(KeyPurpose::VERIFY)
+ .digest(Digest::SHA_2_256)
+ .ec_curve(EcCurve::P_256)
+ .attestation_challenge(att_challenge.to_vec());
+ } else {
+ ec_gen_params = AuthSetBuilder::new()
+ .no_auth_required()
+ .algorithm(Algorithm::RSA)
+ .rsa_public_exponent(65537)
+ .key_size(2048)
+ .purpose(KeyPurpose::SIGN)
+ .purpose(KeyPurpose::VERIFY)
+ .digest(Digest::SHA_2_256)
+ .padding_mode(PaddingMode::RSA_PKCS1_1_5_SIGN)
+ .attestation_challenge(att_challenge.to_vec());
+ }
+
+ match attest_id {
+ Tag::ATTESTATION_ID_BRAND => {
+ ec_gen_params = ec_gen_params.attestation_device_brand(value);
+ }
+ Tag::ATTESTATION_ID_DEVICE => {
+ ec_gen_params = ec_gen_params.attestation_device_name(value);
+ }
+ Tag::ATTESTATION_ID_PRODUCT => {
+ ec_gen_params = ec_gen_params.attestation_device_product_name(value);
+ }
+ Tag::ATTESTATION_ID_SERIAL => {
+ ec_gen_params = ec_gen_params.attestation_device_serial(value);
+ }
+ Tag::ATTESTATION_ID_MANUFACTURER => {
+ ec_gen_params = ec_gen_params.attestation_device_manufacturer(value);
+ }
+ Tag::ATTESTATION_ID_MODEL => {
+ ec_gen_params = ec_gen_params.attestation_device_model(value);
+ }
+ Tag::ATTESTATION_ID_IMEI => {
+ ec_gen_params = ec_gen_params.attestation_device_imei(value);
+ }
+ Tag::ATTESTATION_ID_SECOND_IMEI => {
+ ec_gen_params = ec_gen_params.attestation_device_second_imei(value);
+ }
+ _ => {
+ panic!("Unknown attestation id");
+ }
+ }
+
+ sec_level.generateKey(
+ &KeyDescriptor { domain: Domain::APP, nspace: -1, alias, blob: None },
+ Some(attest_key),
+ &ec_gen_params,
+ 0,
+ b"entropy",
+ )
+}
diff --git a/keystore2/test_utils/run_as.rs b/keystore2/test_utils/run_as.rs
index 2485ab5..be643b6 100644
--- a/keystore2/test_utils/run_as.rs
+++ b/keystore2/test_utils/run_as.rs
@@ -255,7 +255,9 @@
let (response_reader, mut response_writer) =
pipe_channel().expect("Failed to create cmd pipe.");
- match fork() {
+ // SAFETY: Our caller guarantees that the process only has a single thread, so calling
+ // non-async-signal-safe functions in the child is in fact safe.
+ match unsafe { fork() } {
Ok(ForkResult::Parent { child, .. }) => {
drop(response_writer);
drop(cmd_reader);
@@ -314,7 +316,9 @@
selinux::Context::new(se_context).expect("Unable to construct selinux::Context.");
let (mut reader, mut writer) = pipe_channel::<R>().expect("Failed to create pipe.");
- match fork() {
+ // SAFETY: Our caller guarantees that the process only has a single thread, so calling
+ // non-async-signal-safe functions in the child is in fact safe.
+ match unsafe { fork() } {
Ok(ForkResult::Parent { child, .. }) => {
drop(writer);
let status = waitpid(child, None).expect("Failed while waiting for child.");
diff --git a/keystore2/tests/Android.bp b/keystore2/tests/Android.bp
index 0c8b0c8..32c39dc 100644
--- a/keystore2/tests/Android.bp
+++ b/keystore2/tests/Android.bp
@@ -58,6 +58,7 @@
"libkeymaster_messages",
"libcppbor_external",
"libkeystore-engine",
+ "libkeymint_support",
],
require_root: true,
}
diff --git a/keystore2/tests/ffi_test_utils.cpp b/keystore2/tests/ffi_test_utils.cpp
index 47dd7a4..7fbfb8b 100644
--- a/keystore2/tests/ffi_test_utils.cpp
+++ b/keystore2/tests/ffi_test_utils.cpp
@@ -18,6 +18,8 @@
#include <keymaster/km_openssl/openssl_err.h>
#include <keymaster/km_openssl/openssl_utils.h>
+#include <android-base/logging.h>
+
using aidl::android::hardware::security::keymint::ErrorCode;
#define TAG_SEQUENCE 0x30
@@ -503,3 +505,94 @@
#endif
return result;
}
+
+CxxResult getValueFromAttestRecord(rust::Vec<rust::u8> cert_buf, int32_t tag) {
+ CxxResult cxx_result{};
+ cxx_result.error = KM_ERROR_OK;
+
+ uint8_t* cert_data = cert_buf.data();
+ int cert_data_size = cert_buf.size();
+
+ std::vector<uint8_t> cert_bytes;
+ cert_bytes.insert(cert_bytes.end(), cert_data, (cert_data + cert_data_size));
+
+ aidl::android::hardware::security::keymint::X509_Ptr cert(
+ aidl::android::hardware::security::keymint::test::parse_cert_blob(cert_bytes));
+ if (!cert.get()) {
+ cxx_result.error = KM_ERROR_MEMORY_ALLOCATION_FAILED;
+ return cxx_result;
+ }
+
+ ASN1_OCTET_STRING* attest_rec =
+ aidl::android::hardware::security::keymint::test::get_attestation_record(cert.get());
+ if (!attest_rec) {
+ cxx_result.error = keymaster::TranslateLastOpenSslError();
+ return cxx_result;
+ }
+
+ aidl::android::hardware::security::keymint::AuthorizationSet att_sw_enforced;
+ aidl::android::hardware::security::keymint::AuthorizationSet att_hw_enforced;
+ uint32_t att_attestation_version;
+ uint32_t att_keymint_version;
+ aidl::android::hardware::security::keymint::SecurityLevel att_attestation_security_level;
+ aidl::android::hardware::security::keymint::SecurityLevel att_keymint_security_level;
+ std::vector<uint8_t> att_challenge;
+ std::vector<uint8_t> att_unique_id;
+ std::vector<uint8_t> att_app_id;
+
+ auto error = aidl::android::hardware::security::keymint::parse_attestation_record(
+ attest_rec->data, attest_rec->length, &att_attestation_version,
+ &att_attestation_security_level, &att_keymint_version, &att_keymint_security_level,
+ &att_challenge, &att_sw_enforced, &att_hw_enforced, &att_unique_id);
+ EXPECT_EQ(ErrorCode::OK, error);
+ if (error != ErrorCode::OK) {
+ cxx_result.error = static_cast<int32_t>(error);
+ return cxx_result;
+ }
+
+ aidl::android::hardware::security::keymint::Tag auth_tag =
+ static_cast<aidl::android::hardware::security::keymint::Tag>(tag);
+
+ if (auth_tag == aidl::android::hardware::security::keymint::Tag::ATTESTATION_APPLICATION_ID) {
+ int pos = att_sw_enforced.find(
+ aidl::android::hardware::security::keymint::Tag::ATTESTATION_APPLICATION_ID);
+ if (pos == -1) {
+ cxx_result.error = KM_ERROR_ATTESTATION_APPLICATION_ID_MISSING;
+ return cxx_result;
+ }
+ aidl::android::hardware::security::keymint::KeyParameter param = att_sw_enforced[pos];
+ std::vector<uint8_t> val =
+ param.value.get<aidl::android::hardware::security::keymint::KeyParameterValue::blob>();
+ std::move(val.begin(), val.end(), std::back_inserter(cxx_result.data));
+ return cxx_result;
+ }
+
+ if (auth_tag == aidl::android::hardware::security::keymint::Tag::ATTESTATION_CHALLENGE) {
+ if (att_challenge.size() == 0) {
+ cxx_result.error = KM_ERROR_ATTESTATION_CHALLENGE_MISSING;
+ return cxx_result;
+ }
+ std::move(att_challenge.begin(), att_challenge.end(), std::back_inserter(cxx_result.data));
+ return cxx_result;
+ }
+
+ if (auth_tag == aidl::android::hardware::security::keymint::Tag::UNIQUE_ID) {
+ if (att_unique_id.size() == 0) {
+ cxx_result.error = KM_ERROR_UNSUPPORTED_TAG;
+ return cxx_result;
+ }
+ std::move(att_unique_id.begin(), att_unique_id.end(), std::back_inserter(cxx_result.data));
+ return cxx_result;
+ }
+
+ int pos = att_hw_enforced.find(auth_tag);
+ if (pos == -1) {
+ cxx_result.error = KM_ERROR_UNSUPPORTED_TAG;
+ return cxx_result;
+ }
+ aidl::android::hardware::security::keymint::KeyParameter param = att_hw_enforced[pos];
+ std::vector<uint8_t> val =
+ param.value.get<aidl::android::hardware::security::keymint::KeyParameterValue::blob>();
+ std::move(val.begin(), val.end(), std::back_inserter(cxx_result.data));
+ return cxx_result;
+}
diff --git a/keystore2/tests/ffi_test_utils.hpp b/keystore2/tests/ffi_test_utils.hpp
index 32f29f4..3ed7edc 100644
--- a/keystore2/tests/ffi_test_utils.hpp
+++ b/keystore2/tests/ffi_test_utils.hpp
@@ -10,3 +10,4 @@
rust::Vec<rust::u8> tag);
CxxResult buildAsn1DerEncodedWrappedKeyDescription();
bool performCryptoOpUsingKeystoreEngine(int64_t grant_id);
+CxxResult getValueFromAttestRecord(rust::Vec<rust::u8> cert_buf, int32_t tag);
diff --git a/keystore2/tests/ffi_test_utils.rs b/keystore2/tests/ffi_test_utils.rs
index 689713a..c652174 100644
--- a/keystore2/tests/ffi_test_utils.rs
+++ b/keystore2/tests/ffi_test_utils.rs
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+use android_hardware_security_keymint::aidl::android::hardware::security::keymint::Tag::Tag;
use keystore2_test_utils::key_generations::Error;
#[cxx::bridge]
@@ -32,6 +33,7 @@
) -> CxxResult;
fn buildAsn1DerEncodedWrappedKeyDescription() -> CxxResult;
fn performCryptoOpUsingKeystoreEngine(grant_id: i64) -> bool;
+ fn getValueFromAttestRecord(cert_buf: Vec<u8>, tag: i32) -> CxxResult;
}
}
@@ -87,3 +89,11 @@
Err(Error::Keystore2EngineOpFailed)
}
+
+pub fn get_value_from_attest_record(cert_buf: &[u8], tag: Tag) -> Result<Vec<u8>, Error> {
+ let result = ffi::getValueFromAttestRecord(cert_buf.to_vec(), tag.0);
+ if result.error == 0 && !result.data.is_empty() {
+ return Ok(result.data);
+ }
+ Err(Error::AttestRecordGetValueFailed)
+}
diff --git a/keystore2/tests/keystore2_client_attest_key_tests.rs b/keystore2/tests/keystore2_client_attest_key_tests.rs
index 4febd9b..b8ad90d 100644
--- a/keystore2/tests/keystore2_client_attest_key_tests.rs
+++ b/keystore2/tests/keystore2_client_attest_key_tests.rs
@@ -17,21 +17,26 @@
use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
Algorithm::Algorithm, BlockMode::BlockMode, Digest::Digest, EcCurve::EcCurve,
ErrorCode::ErrorCode, KeyPurpose::KeyPurpose, PaddingMode::PaddingMode,
- SecurityLevel::SecurityLevel,
+ SecurityLevel::SecurityLevel, Tag::Tag,
};
use android_system_keystore2::aidl::android::system::keystore2::{
- Domain::Domain, KeyDescriptor::KeyDescriptor, ResponseCode::ResponseCode,
+ Domain::Domain, IKeystoreService::IKeystoreService, KeyDescriptor::KeyDescriptor,
+ ResponseCode::ResponseCode,
};
use keystore2_test_utils::{
authorizations, get_keystore_service, key_generations, key_generations::Error,
};
-use crate::ffi_test_utils::validate_certchain;
+use crate::ffi_test_utils::{get_value_from_attest_record, validate_certchain};
use crate::{
- keystore2_client_test_utils::app_attest_key_feature_exists,
- skip_test_if_no_app_attest_key_feature,
+ skip_test_if_no_app_attest_key_feature, skip_test_if_no_device_id_attestation_feature,
+};
+
+use crate::keystore2_client_test_utils::{
+ app_attest_key_feature_exists, device_id_attestation_feature_exists, get_attest_id_value,
+ is_second_imei_id_attestation_required,
};
/// Generate RSA and EC attestation keys and use them for signing RSA-signing keys.
@@ -480,3 +485,178 @@
// Should not have an attestation record.
assert!(aes_key_metadata.certificateChain.is_none());
}
+
+fn get_attestation_ids(keystore2: &binder::Strong<dyn IKeystoreService>) -> Vec<(Tag, Vec<u8>)> {
+ let attest_ids = vec![
+ (Tag::ATTESTATION_ID_BRAND, "ro.product.brand_for_attestation"),
+ (Tag::ATTESTATION_ID_DEVICE, "ro.product.device"),
+ (Tag::ATTESTATION_ID_PRODUCT, "ro.product.name_for_attestation"),
+ (Tag::ATTESTATION_ID_SERIAL, "ro.serialno"),
+ (Tag::ATTESTATION_ID_MANUFACTURER, "ro.product.manufacturer"),
+ (Tag::ATTESTATION_ID_MODEL, "ro.product.model_for_attestation"),
+ (Tag::ATTESTATION_ID_IMEI, ""), //Get this value from Telephony service.
+ (Tag::ATTESTATION_ID_SECOND_IMEI, ""), //Get this value from Telephony service.
+ ];
+
+ let mut attest_id_params: Vec<(Tag, Vec<u8>)> = vec![];
+ for (attest_id, prop_name) in attest_ids {
+ if attest_id == Tag::ATTESTATION_ID_SECOND_IMEI
+ && !is_second_imei_id_attestation_required(keystore2)
+ {
+ continue;
+ }
+
+ if let Some(value) = get_attest_id_value(attest_id, prop_name) {
+ if !value.is_empty() {
+ attest_id_params.push((attest_id, value));
+ }
+ }
+ }
+
+ attest_id_params
+}
+
+/// Generate an attested key with attestation of the device's identifiers. Test should succeed in
+/// generating a attested key with attestation of device identifiers. Test might fail on devices
+/// which don't support device id attestation with error response code `CANNOT_ATTEST_IDS or
+/// INVALID_TAG`
+fn generate_attested_key_with_device_attest_ids(algorithm: Algorithm) {
+ skip_test_if_no_device_id_attestation_feature!();
+ let keystore2 = get_keystore_service();
+ let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
+
+ let att_challenge: &[u8] = b"foo";
+
+ let attest_key_metadata =
+ key_generations::generate_attestation_key(&sec_level, algorithm, att_challenge).unwrap();
+
+ let attest_id_params = get_attestation_ids(&keystore2);
+
+ for (attest_id, value) in attest_id_params {
+ // Create RSA/EC key and use attestation key to sign it.
+ let key_alias = format!("ks_attested_test_key_{}", getuid());
+ let key_metadata =
+ key_generations::map_ks_error(key_generations::generate_key_with_attest_id(
+ &sec_level,
+ algorithm,
+ Some(key_alias),
+ att_challenge,
+ &attest_key_metadata.key,
+ attest_id,
+ value.clone(),
+ ))
+ .unwrap();
+
+ assert!(key_metadata.certificate.is_some());
+ assert!(key_metadata.certificateChain.is_none());
+
+ let mut cert_chain: Vec<u8> = Vec::new();
+ cert_chain.extend(key_metadata.certificate.as_ref().unwrap());
+ cert_chain.extend(attest_key_metadata.certificate.as_ref().unwrap());
+ cert_chain.extend(attest_key_metadata.certificateChain.as_ref().unwrap());
+
+ validate_certchain(&cert_chain).expect("Error while validating cert chain");
+ let attest_id_value =
+ get_value_from_attest_record(key_metadata.certificate.as_ref().unwrap(), attest_id)
+ .expect("Attest id verification failed.");
+ assert_eq!(attest_id_value, value);
+ }
+}
+
+#[test]
+fn keystore2_attest_ecdsa_attestation_id() {
+ generate_attested_key_with_device_attest_ids(Algorithm::EC);
+}
+
+#[test]
+fn keystore2_attest_rsa_attestation_id() {
+ generate_attested_key_with_device_attest_ids(Algorithm::RSA);
+}
+
+/// Try to generate an attested key with attestation of invalid device's identifiers. Test should
+/// fail with error response code `CANNOT_ATTEST_IDS`.
+#[test]
+fn keystore2_attest_key_fails_with_invalid_attestation_id() {
+ skip_test_if_no_device_id_attestation_feature!();
+
+ let keystore2 = get_keystore_service();
+ let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
+
+ let digest = Digest::SHA_2_256;
+ let att_challenge: &[u8] = b"foo";
+
+ // Create EC-Attestation key.
+ let attest_key_metadata = key_generations::generate_ec_attestation_key(
+ &sec_level,
+ att_challenge,
+ digest,
+ EcCurve::P_256,
+ )
+ .unwrap();
+
+ let attest_id_params = vec![
+ (Tag::ATTESTATION_ID_BRAND, b"invalid-brand".to_vec()),
+ (Tag::ATTESTATION_ID_DEVICE, b"invalid-device-name".to_vec()),
+ (Tag::ATTESTATION_ID_PRODUCT, b"invalid-product-name".to_vec()),
+ (Tag::ATTESTATION_ID_SERIAL, b"invalid-ro-serial".to_vec()),
+ (Tag::ATTESTATION_ID_MANUFACTURER, b"invalid-ro-product-manufacturer".to_vec()),
+ (Tag::ATTESTATION_ID_MODEL, b"invalid-ro-product-model".to_vec()),
+ (Tag::ATTESTATION_ID_IMEI, b"invalid-imei".to_vec()),
+ ];
+
+ for (attest_id, value) in attest_id_params {
+ // Create EC key and use attestation key to sign it.
+ let ec_key_alias = format!("ks_ec_attested_test_key_fail_{}{}", getuid(), digest.0);
+ let result = key_generations::map_ks_error(key_generations::generate_key_with_attest_id(
+ &sec_level,
+ Algorithm::EC,
+ Some(ec_key_alias),
+ att_challenge,
+ &attest_key_metadata.key,
+ attest_id,
+ value,
+ ));
+
+ assert!(result.is_err());
+ assert_eq!(result.unwrap_err(), Error::Km(ErrorCode::CANNOT_ATTEST_IDS));
+ }
+}
+
+/// If `DEVICE_ID_ATTESTATION_FEATURE` is not supported then test tries to generate an attested
+/// key with attestation of valid device's identifiers. Test should fail to generate key with
+/// error code `CANNOT_ATTEST_IDS`.
+#[test]
+fn keystore2_attest_key_without_attestation_id_support_fails_with_cannot_attest_id() {
+ if device_id_attestation_feature_exists() {
+ // Skip this test on device supporting `DEVICE_ID_ATTESTATION_FEATURE`.
+ return;
+ }
+
+ let keystore2 = get_keystore_service();
+ let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap();
+
+ let att_challenge: &[u8] = b"foo";
+ let attest_key_metadata =
+ key_generations::generate_attestation_key(&sec_level, Algorithm::RSA, att_challenge)
+ .unwrap();
+
+ let attest_id_params = get_attestation_ids(&keystore2);
+ for (attest_id, value) in attest_id_params {
+ // Create RSA/EC key and use attestation key to sign it.
+ let key_alias = format!("ks_attested_test_key_{}", getuid());
+ let result = key_generations::map_ks_error(key_generations::generate_key_with_attest_id(
+ &sec_level,
+ Algorithm::RSA,
+ Some(key_alias),
+ att_challenge,
+ &attest_key_metadata.key,
+ attest_id,
+ value.clone(),
+ ));
+ assert!(
+ result.is_err(),
+ "Expected to fail as FEATURE_DEVICE_ID_ATTESTATION is not supported."
+ );
+ assert_eq!(result.unwrap_err(), Error::Km(ErrorCode::CANNOT_ATTEST_IDS));
+ }
+}
diff --git a/keystore2/tests/keystore2_client_ec_key_tests.rs b/keystore2/tests/keystore2_client_ec_key_tests.rs
index c2034de..8267140 100644
--- a/keystore2/tests/keystore2_client_ec_key_tests.rs
+++ b/keystore2/tests/keystore2_client_ec_key_tests.rs
@@ -432,15 +432,18 @@
// Client#1: Generate a key and create an operation using generated key.
// Wait until the parent notifies to continue. Once the parent notifies, this operation
// is expected to be completed successfully.
- let mut child_handle = execute_op_run_as_child(
- TARGET_CTX,
- Domain::APP,
- -1,
- Some(alias.to_string()),
- Uid::from_raw(uid1),
- Gid::from_raw(gid1),
- ForcedOp(false),
- );
+ // SAFETY: The test is run in a separate process with no other threads.
+ let mut child_handle = unsafe {
+ execute_op_run_as_child(
+ TARGET_CTX,
+ Domain::APP,
+ -1,
+ Some(alias.to_string()),
+ Uid::from_raw(uid1),
+ Gid::from_raw(gid1),
+ ForcedOp(false),
+ )
+ };
// Wait until (client#1) child process notifies us to continue, so that there will be a key
// generated by client#1.
@@ -450,6 +453,7 @@
const APPLICATION_ID_2: u32 = 10602;
let uid2 = USER_ID * AID_USER_OFFSET + APPLICATION_ID_2;
let gid2 = USER_ID * AID_USER_OFFSET + APPLICATION_ID_2;
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(TARGET_CTX, Uid::from_raw(uid2), Gid::from_raw(gid2), move || {
let keystore2_inst = get_keystore_service();
diff --git a/keystore2/tests/keystore2_client_grant_key_tests.rs b/keystore2/tests/keystore2_client_grant_key_tests.rs
index bde872d..516869a 100644
--- a/keystore2/tests/keystore2_client_grant_key_tests.rs
+++ b/keystore2/tests/keystore2_client_grant_key_tests.rs
@@ -114,6 +114,7 @@
static GRANTEE_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
static GRANTEE_GID: u32 = GRANTEE_UID;
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(TARGET_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let empty_access_vector = KeyPermission::NONE.0;
@@ -132,6 +133,7 @@
// In grantee context try to load the key, it should fail to load the granted key as it is
// granted with empty access vector.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -169,6 +171,7 @@
static GRANTEE_GID: u32 = GRANTEE_UID;
// Generate a key and grant it to a user with GET_INFO|USE key permissions.
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(TARGET_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let access_vector = KeyPermission::GET_INFO.0 | KeyPermission::USE.0;
@@ -185,6 +188,7 @@
};
// In grantee context load the key and try to perform crypto operation.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -251,6 +255,7 @@
static ALIAS: &str = "ks_grant_key_delete_success";
// Generate a key and grant it to a user with DELETE permission.
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -270,6 +275,7 @@
};
// Grantee context, delete the key.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -290,6 +296,7 @@
};
// Verify whether key got deleted in grantor's context.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), move || {
let keystore2_inst = get_keystore_service();
@@ -325,6 +332,7 @@
static SEC_GRANTEE_GID: u32 = SEC_GRANTEE_UID;
// Generate a key and grant it to a user with GET_INFO permission.
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -345,6 +353,7 @@
};
// Grantee context, load the granted key and try to grant it to `SEC_GRANTEE_UID` grantee.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -375,6 +384,7 @@
};
// Make sure second grantee shouldn't have access to the above granted key.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -457,6 +467,7 @@
static GRANTEE_GID: u32 = GRANTEE_UID;
// Generate a key and grant it to a user with GET_INFO permission.
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -492,6 +503,7 @@
};
// Grantee context, try to load the ungranted key.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -527,6 +539,7 @@
static GRANTEE_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
static GRANTEE_GID: u32 = GRANTEE_UID;
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -576,6 +589,7 @@
// Make sure grant did not persist, try to access the earlier granted key in grantee context.
// Grantee context should fail to load the granted key as its associated key is deleted in
// grantor context.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -614,6 +628,7 @@
static GRANTEE_2_GID: u32 = GRANTEE_2_UID;
// Generate a key and grant it to multiple users with GET_INFO|USE permissions.
+ // SAFETY: The test is run in a separate process with no other threads.
let mut grant_keys = unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -636,6 +651,7 @@
&[(GRANTEE_1_UID, GRANTEE_1_GID), (GRANTEE_2_UID, GRANTEE_2_GID)]
{
let grant_key_nspace = grant_keys.remove(0);
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -678,6 +694,7 @@
static GRANTEE_2_GID: u32 = GRANTEE_2_UID;
// Generate a key and grant it to multiple users with GET_INFO permission.
+ // SAFETY: The test is run in a separate process with no other threads.
let mut grant_keys = unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -699,6 +716,7 @@
// Grantee #1 context
let grant_key1_nspace = grant_keys.remove(0);
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -733,6 +751,7 @@
// Grantee #2 context
let grant_key2_nspace = grant_keys.remove(0);
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
diff --git a/keystore2/tests/keystore2_client_keystore_engine_tests.rs b/keystore2/tests/keystore2_client_keystore_engine_tests.rs
index 1aed8e6..339eb60 100644
--- a/keystore2/tests/keystore2_client_keystore_engine_tests.rs
+++ b/keystore2/tests/keystore2_client_keystore_engine_tests.rs
@@ -167,6 +167,7 @@
static GRANTEE_GID: u32 = GRANTEE_UID;
// Generate a key and grant it to a user with GET_INFO|USE|DELETE key permissions.
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(TARGET_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -184,6 +185,7 @@
};
// In grantee context load the key and try to perform crypto operation.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -208,6 +210,7 @@
static GRANTEE_GID: u32 = GRANTEE_UID;
// Generate a key and grant it to a user with GET_INFO|USE|DELETE key permissions.
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(TARGET_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -225,6 +228,7 @@
};
// In grantee context load the key and try to perform crypto operation.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -250,6 +254,7 @@
// Generate a key and re-encode it's certificate as PEM and update it and
// grant it to a user with GET_INFO|USE|DELETE key permissions.
+ // SAFETY: The test is run in a separate process with no other threads.
let grant_key_nspace = unsafe {
run_as::run_as(TARGET_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -285,6 +290,7 @@
};
// In grantee context load the key and try to perform crypto operation.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
diff --git a/keystore2/tests/keystore2_client_list_entries_tests.rs b/keystore2/tests/keystore2_client_list_entries_tests.rs
index 809c01f..1c0c496 100644
--- a/keystore2/tests/keystore2_client_list_entries_tests.rs
+++ b/keystore2/tests/keystore2_client_list_entries_tests.rs
@@ -60,6 +60,7 @@
static GRANTEE_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
static GRANTEE_GID: u32 = GRANTEE_UID;
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -113,6 +114,7 @@
};
// In user context validate list of key entries associated with it.
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -161,6 +163,7 @@
let agid = 91 * AID_USER_OFFSET + 10001;
static TARGET_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(TARGET_CTX, Uid::from_raw(auid), Gid::from_raw(agid), move || {
let keystore2 = get_keystore_service();
@@ -198,6 +201,7 @@
static CLIENT_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
static CLIENT_GID: u32 = CLIENT_UID;
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
let keystore2 = get_keystore_service();
@@ -264,6 +268,7 @@
static CLIENT_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
static CLIENT_GID: u32 = CLIENT_UID;
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
let keystore2 = get_keystore_service();
@@ -331,6 +336,7 @@
static CLIENT_GID: u32 = CLIENT_UID;
static ALIAS_PREFIX: &str = "key_test_batch_list";
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
let keystore2 = get_keystore_service();
@@ -361,6 +367,7 @@
})
};
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
let keystore2 = get_keystore_service();
@@ -428,6 +435,7 @@
static CLIENT_UID: u32 = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
static CLIENT_GID: u32 = CLIENT_UID;
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
let keystore2 = get_keystore_service();
@@ -511,6 +519,7 @@
static CLIENT_GID: u32 = CLIENT_UID;
static ALIAS_PREFIX: &str = "key_test_batch_list";
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(CLIENT_CTX, Uid::from_raw(CLIENT_UID), Gid::from_raw(CLIENT_GID), || {
let keystore2 = get_keystore_service();
@@ -647,6 +656,7 @@
let agid = 91 * AID_USER_OFFSET + 10001;
static TARGET_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(TARGET_CTX, Uid::from_raw(auid), Gid::from_raw(agid), move || {
let keystore2 = get_keystore_service();
@@ -686,6 +696,7 @@
let agid = 91 * AID_USER_OFFSET + 10001;
static TARGET_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(TARGET_CTX, Uid::from_raw(auid), Gid::from_raw(agid), move || {
let keystore2 = get_keystore_service();
diff --git a/keystore2/tests/keystore2_client_operation_tests.rs b/keystore2/tests/keystore2_client_operation_tests.rs
index 19175dd..89b5a31 100644
--- a/keystore2/tests/keystore2_client_operation_tests.rs
+++ b/keystore2/tests/keystore2_client_operation_tests.rs
@@ -36,7 +36,11 @@
/// Create `max_ops` number child processes with the given context and perform an operation under each
/// child process.
-pub fn create_operations(
+///
+/// # Safety
+///
+/// Must be called from a process with no other threads.
+pub unsafe fn create_operations(
target_ctx: &'static str,
forced_op: ForcedOp,
max_ops: i32,
@@ -45,7 +49,8 @@
let base_gid = 99 * AID_USER_OFFSET + 10001;
let base_uid = 99 * AID_USER_OFFSET + 10001;
(0..max_ops)
- .map(|i| {
+ // SAFETY: The caller guarantees that there are no other threads.
+ .map(|i| unsafe {
execute_op_run_as_child(
target_ctx,
Domain::APP,
@@ -87,7 +92,8 @@
const MAX_OPS: i32 = 100;
static TARGET_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
- let mut child_handles = create_operations(TARGET_CTX, ForcedOp(false), MAX_OPS);
+ // SAFETY: The test is run in a separate process with no other threads.
+ let mut child_handles = unsafe { create_operations(TARGET_CTX, ForcedOp(false), MAX_OPS) };
// Wait until all child procs notifies us to continue,
// so that there are definitely enough operations outstanding to trigger a BACKEND_BUSY.
@@ -120,7 +126,8 @@
static TARGET_CTX: &str = "u:r:untrusted_app:s0:c91,c256,c10,c20";
// Create regular operations.
- let mut child_handles = create_operations(TARGET_CTX, ForcedOp(false), MAX_OPS);
+ // SAFETY: The test is run in a separate process with no other threads.
+ let mut child_handles = unsafe { create_operations(TARGET_CTX, ForcedOp(false), MAX_OPS) };
// Wait until all child procs notifies us to continue, so that there are enough
// operations outstanding to trigger a BACKEND_BUSY.
@@ -131,6 +138,7 @@
// Create a forced operation.
let auid = 99 * AID_USER_OFFSET + 10604;
let agid = 99 * AID_USER_OFFSET + 10604;
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
key_generations::TARGET_VOLD_CTX,
@@ -203,15 +211,18 @@
// Create initial forced operation in a child process
// and wait for the parent to notify to perform operation.
let alias = format!("ks_forced_op_key_{}", getuid());
- let mut first_op_handle = execute_op_run_as_child(
- key_generations::TARGET_SU_CTX,
- Domain::SELINUX,
- key_generations::SELINUX_SHELL_NAMESPACE,
- Some(alias),
- Uid::from_raw(auid),
- Gid::from_raw(agid),
- ForcedOp(true),
- );
+ // SAFETY: The test is run in a separate process with no other threads.
+ let mut first_op_handle = unsafe {
+ execute_op_run_as_child(
+ key_generations::TARGET_SU_CTX,
+ Domain::SELINUX,
+ key_generations::SELINUX_SHELL_NAMESPACE,
+ Some(alias),
+ Uid::from_raw(auid),
+ Gid::from_raw(agid),
+ ForcedOp(true),
+ )
+ };
// Wait until above child proc notifies us to continue, so that there is definitely a forced
// operation outstanding to perform a operation.
@@ -219,7 +230,8 @@
// Create MAX_OPS number of forced operations.
let mut child_handles =
- create_operations(key_generations::TARGET_SU_CTX, ForcedOp(true), MAX_OPS);
+ // SAFETY: The test is run in a separate process with no other threads.
+ unsafe { create_operations(key_generations::TARGET_SU_CTX, ForcedOp(true), MAX_OPS) };
// Wait until all child procs notifies us to continue, so that there are enough operations
// outstanding to trigger a BACKEND_BUSY.
@@ -282,15 +294,18 @@
// Create an operation in an untrusted_app context. Wait until the parent notifies to continue.
// Once the parent notifies, this operation is expected to be completed successfully.
let alias = format!("ks_reg_op_key_{}", getuid());
- let mut child_handle = execute_op_run_as_child(
- TARGET_CTX,
- Domain::APP,
- -1,
- Some(alias),
- Uid::from_raw(uid),
- Gid::from_raw(gid),
- ForcedOp(false),
- );
+ // SAFETY: The test is run in a separate process with no other threads.
+ let mut child_handle = unsafe {
+ execute_op_run_as_child(
+ TARGET_CTX,
+ Domain::APP,
+ -1,
+ Some(alias),
+ Uid::from_raw(uid),
+ Gid::from_raw(gid),
+ ForcedOp(false),
+ )
+ };
// Wait until child process notifies us to continue, so that an operation from child process is
// outstanding to complete the operation.
@@ -377,6 +392,7 @@
let gid = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
for context in TARGET_CTXS.iter() {
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(context, Uid::from_raw(uid), Gid::from_raw(gid), move || {
let alias = format!("ks_app_forced_op_test_key_{}", getuid());
@@ -406,6 +422,7 @@
let uid = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
let gid = USER_ID * AID_USER_OFFSET + APPLICATION_ID;
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(TARGET_CTX, Uid::from_raw(uid), Gid::from_raw(gid), move || {
let alias = format!("ks_vold_forced_op_key_{}", getuid());
diff --git a/keystore2/tests/keystore2_client_test_utils.rs b/keystore2/tests/keystore2_client_test_utils.rs
index 07c2183..f7e7985 100644
--- a/keystore2/tests/keystore2_client_test_utils.rs
+++ b/keystore2/tests/keystore2_client_test_utils.rs
@@ -15,6 +15,8 @@
use nix::unistd::{Gid, Uid};
use serde::{Deserialize, Serialize};
+use std::process::{Command, Output};
+
use openssl::encrypt::Encrypter;
use openssl::error::ErrorStack;
use openssl::hash::MessageDigest;
@@ -66,6 +68,7 @@
pub const PACKAGE_MANAGER_NATIVE_SERVICE: &str = "package_native";
pub const APP_ATTEST_KEY_FEATURE: &str = "android.hardware.keystore.app_attest_key";
+pub const DEVICE_ID_ATTESTATION_FEATURE: &str = "android.software.device_id_attestation";
/// Determines whether app_attest_key_feature is supported or not.
pub fn app_attest_key_feature_exists() -> bool {
@@ -75,6 +78,14 @@
pm.hasSystemFeature(APP_ATTEST_KEY_FEATURE, 0).expect("hasSystemFeature failed.")
}
+/// Determines whether device_id_attestation is supported or not.
+pub fn device_id_attestation_feature_exists() -> bool {
+ let pm = wait_for_interface::<dyn IPackageManagerNative>(PACKAGE_MANAGER_NATIVE_SERVICE)
+ .expect("Failed to get package manager native service.");
+
+ pm.hasSystemFeature(DEVICE_ID_ATTESTATION_FEATURE, 0).expect("hasSystemFeature failed.")
+}
+
#[macro_export]
macro_rules! skip_test_if_no_app_attest_key_feature {
() => {
@@ -84,6 +95,15 @@
};
}
+#[macro_export]
+macro_rules! skip_test_if_no_device_id_attestation_feature {
+ () => {
+ if !device_id_attestation_feature_exists() {
+ return;
+ }
+ };
+}
+
/// Indicate whether the default device is KeyMint (rather than Keymaster).
pub fn has_default_keymint() -> bool {
binder::is_declared("android.hardware.security.keymint.IKeyMintDevice/default")
@@ -246,7 +266,11 @@
}
/// Create new operation on child proc and perform simple operation after parent notification.
-pub fn execute_op_run_as_child(
+///
+/// # Safety
+///
+/// Must only be called from a single-threaded process.
+pub unsafe fn execute_op_run_as_child(
target_ctx: &'static str,
domain: Domain,
nspace: i64,
@@ -255,6 +279,7 @@
agid: Gid,
forced_op: ForcedOp,
) -> run_as::ChildHandle<TestOutcome, BarrierReached> {
+ // SAFETY: The caller guarantees that there are no other threads.
unsafe {
run_as::run_as_child(target_ctx, auid, agid, move |reader, writer| {
let result = key_generations::map_ks_error(create_signing_operation(
@@ -444,3 +469,84 @@
.iter()
.all(|key| expected_aliases.contains(key.alias.as_ref().unwrap())));
}
+
+// Get the value of the given system property, if the given system property doesn't exist
+// then returns an empty byte vector.
+pub fn get_system_prop(name: &str) -> Vec<u8> {
+ match rustutils::system_properties::read(name) {
+ Ok(Some(value)) => {
+ return value.as_bytes().to_vec();
+ }
+ _ => {
+ vec![]
+ }
+ }
+}
+
+/// Determines whether the SECOND-IMEI can be used as device attest-id.
+pub fn is_second_imei_id_attestation_required(
+ keystore2: &binder::Strong<dyn IKeystoreService>,
+) -> bool {
+ let api_level = std::str::from_utf8(&get_system_prop("ro.vendor.api_level"))
+ .unwrap()
+ .parse::<i32>()
+ .unwrap();
+ keystore2.getInterfaceVersion().unwrap() >= 3 && api_level > 33
+}
+
+/// Run a service command and collect the output.
+pub fn run_service_command(command: &[&str]) -> std::io::Result<Output> {
+ Command::new("cmd").args(command).output()
+}
+
+/// Get IMEI from telephony service.
+pub fn get_imei(slot: i32) -> Option<Vec<u8>> {
+ let mut cmd = vec!["phone", "get-imei"];
+ let slot_str = slot.to_string();
+ cmd.push(slot_str.as_str());
+ let output = run_service_command(&cmd).unwrap();
+ if output.status.success() {
+ let stdout = String::from_utf8(output.stdout).unwrap();
+ let mut split_out = stdout.split_whitespace();
+ let imei = split_out.next_back().unwrap();
+ if imei == "null" {
+ return None;
+ }
+ return Some(imei.as_bytes().to_vec());
+ }
+
+ None
+}
+
+/// Get value of the given attestation id.
+pub fn get_attest_id_value(attest_id: Tag, prop_name: &str) -> Option<Vec<u8>> {
+ match attest_id {
+ Tag::ATTESTATION_ID_IMEI => get_imei(0),
+ Tag::ATTESTATION_ID_SECOND_IMEI => get_imei(1),
+ Tag::ATTESTATION_ID_BRAND => {
+ let prop_val = get_system_prop(prop_name);
+ if prop_val.is_empty() {
+ Some(get_system_prop("ro.product.brand"))
+ } else {
+ Some(prop_val)
+ }
+ }
+ Tag::ATTESTATION_ID_PRODUCT => {
+ let prop_val = get_system_prop(prop_name);
+ if prop_val.is_empty() {
+ Some(get_system_prop("ro.product.name"))
+ } else {
+ Some(prop_val)
+ }
+ }
+ Tag::ATTESTATION_ID_MODEL => {
+ let prop_val = get_system_prop(prop_name);
+ if prop_val.is_empty() {
+ Some(get_system_prop("ro.product.model"))
+ } else {
+ Some(prop_val)
+ }
+ }
+ _ => Some(get_system_prop(prop_name)),
+ }
+}
diff --git a/keystore2/tests/keystore2_client_update_subcomponent_tests.rs b/keystore2/tests/keystore2_client_update_subcomponent_tests.rs
index 0be092f..d9576a8 100644
--- a/keystore2/tests/keystore2_client_update_subcomponent_tests.rs
+++ b/keystore2/tests/keystore2_client_update_subcomponent_tests.rs
@@ -167,6 +167,7 @@
static GRANTEE_2_GID: u32 = GRANTEE_2_UID;
// Generate a key and grant it to multiple users with different access permissions.
+ // SAFETY: The test is run in a separate process with no other threads.
let mut granted_keys = unsafe {
run_as::run_as(GRANTOR_SU_CTX, Uid::from_raw(0), Gid::from_raw(0), || {
let keystore2 = get_keystore_service();
@@ -205,6 +206,7 @@
// Grantee context, try to update the key public certs, permission denied error is expected.
let granted_key1_nspace = granted_keys.remove(0);
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
@@ -234,6 +236,7 @@
// Grantee context, update granted key public certs. Update should happen successfully.
let granted_key2_nspace = granted_keys.remove(0);
+ // SAFETY: The test is run in a separate process with no other threads.
unsafe {
run_as::run_as(
GRANTEE_CTX,
diff --git a/ondevice-signing/CertUtils.cpp b/ondevice-signing/CertUtils.cpp
index 8fe0816..bb2da5a 100644
--- a/ondevice-signing/CertUtils.cpp
+++ b/ondevice-signing/CertUtils.cpp
@@ -21,12 +21,10 @@
#include <openssl/bn.h>
#include <openssl/crypto.h>
-#include <openssl/pkcs7.h>
#include <openssl/rsa.h>
#include <openssl/x509.h>
#include <openssl/x509v3.h>
-#include <optional>
#include <vector>
#include "KeyConstants.h"
@@ -56,12 +54,6 @@
return cert;
}
-static X509V3_CTX makeContext(X509* issuer, X509* subject) {
- X509V3_CTX context = {};
- X509V3_set_ctx(&context, issuer, subject, nullptr, nullptr, 0);
- return context;
-}
-
static bool add_ext(X509V3_CTX* context, X509* cert, int nid, const char* value) {
bssl::UniquePtr<X509_EXTENSION> ex(X509V3_EXT_nconf_nid(nullptr, context, nid, value));
if (!ex) {
@@ -93,20 +85,6 @@
return rsaPubkey;
}
-static Result<bssl::UniquePtr<RSA>>
-getRsaFromRsaPublicKey(const std::vector<uint8_t>& rsaPublicKey) {
- auto derBytes = rsaPublicKey.data();
- bssl::UniquePtr<RSA> rsaKey(d2i_RSAPublicKey(nullptr, &derBytes, rsaPublicKey.size()));
- if (rsaKey.get() == nullptr) {
- return Error() << "Failed to parse RsaPublicKey";
- }
- if (derBytes != rsaPublicKey.data() + rsaPublicKey.size()) {
- return Error() << "Key has unexpected trailing data";
- }
-
- return rsaKey;
-}
-
static Result<bssl::UniquePtr<EVP_PKEY>> modulusToRsaPkey(const std::vector<uint8_t>& publicKey) {
// "publicKey" corresponds to the raw public key bytes - need to create
// a new RSA key with the correct exponent.
@@ -122,21 +100,6 @@
return public_key;
}
-static Result<bssl::UniquePtr<EVP_PKEY>>
-rsaPublicKeyToRsaPkey(const std::vector<uint8_t>& rsaPublicKey) {
- // rsaPublicKey contains both modulus and exponent, DER-encoded.
- auto rsaKey = getRsaFromRsaPublicKey(rsaPublicKey);
- if (!rsaKey.ok()) {
- return rsaKey.error();
- }
-
- bssl::UniquePtr<EVP_PKEY> public_key(EVP_PKEY_new());
- if (!EVP_PKEY_assign_RSA(public_key.get(), rsaKey->release())) {
- return Error() << "Failed to assign key";
- }
- return public_key;
-}
-
Result<void> verifySignature(const std::string& message, const std::string& signature,
const std::vector<uint8_t>& publicKey) {
auto rsaKey = getRsaFromModulus(publicKey);
@@ -156,34 +119,14 @@
return {};
}
-Result<void> verifyRsaPublicKeySignature(const std::string& message, const std::string& signature,
- const std::vector<uint8_t>& rsaPublicKey) {
- auto rsaKey = getRsaFromRsaPublicKey(rsaPublicKey);
- if (!rsaKey.ok()) {
- return rsaKey.error();
+Result<void> createSelfSignedCertificate(
+ const std::vector<uint8_t>& publicKey,
+ const std::function<Result<std::string>(const std::string&)>& signFunction,
+ const std::string& path) {
+ auto rsa_pkey = modulusToRsaPkey(publicKey);
+ if (!rsa_pkey.ok()) {
+ return rsa_pkey.error();
}
-
- uint8_t hashBuf[SHA256_DIGEST_LENGTH];
- SHA256(reinterpret_cast<const uint8_t*>(message.data()), message.size(), hashBuf);
-
- bool success = RSA_verify(NID_sha256, hashBuf, sizeof(hashBuf),
- reinterpret_cast<const uint8_t*>(signature.data()), signature.size(),
- rsaKey->get());
- if (!success) {
- return Error() << "Failed to verify signature";
- }
- return {};
-}
-
-static Result<void> createCertificate(
- const CertSubject& subject, EVP_PKEY* publicKey,
- const std::function<android::base::Result<std::string>(const std::string&)>& signFunction,
- const std::optional<std::string>& issuerCertPath, const std::string& path) {
-
- // If an issuer cert is specified, we are signing someone else's key.
- // Otherwise we are signing our key - a self-signed certificate.
- bool selfSigned = !issuerCertPath;
-
bssl::UniquePtr<X509> x509(X509_new());
if (!x509) {
return Error() << "Unable to allocate x509 container";
@@ -191,7 +134,7 @@
X509_set_version(x509.get(), 2);
X509_gmtime_adj(X509_get_notBefore(x509.get()), 0);
X509_gmtime_adj(X509_get_notAfter(x509.get()), kCertLifetimeSeconds);
- ASN1_INTEGER_set(X509_get_serialNumber(x509.get()), subject.serialNumber);
+ ASN1_INTEGER_set(X509_get_serialNumber(x509.get()), kRootSubject.serialNumber);
bssl::UniquePtr<X509_ALGOR> algor(X509_ALGOR_new());
if (!algor ||
@@ -201,7 +144,7 @@
return Error() << "Unable to set x509 signature algorithm";
}
- if (!X509_set_pubkey(x509.get(), publicKey)) {
+ if (!X509_set_pubkey(x509.get(), rsa_pkey.value().get())) {
return Error() << "Unable to set x509 public key";
}
@@ -211,44 +154,15 @@
}
addNameEntry(subjectName, "C", kIssuerCountry);
addNameEntry(subjectName, "O", kIssuerOrg);
- addNameEntry(subjectName, "CN", subject.commonName);
-
- if (selfSigned) {
- if (!X509_set_issuer_name(x509.get(), subjectName)) {
- return Error() << "Unable to set x509 issuer name";
- }
- } else {
- X509_NAME* issuerName = X509_get_issuer_name(x509.get());
- if (!issuerName) {
- return Error() << "Unable to get x509 issuer name";
- }
- addNameEntry(issuerName, "C", kIssuerCountry);
- addNameEntry(issuerName, "O", kIssuerOrg);
- addNameEntry(issuerName, "CN", kRootSubject.commonName);
+ addNameEntry(subjectName, "CN", kRootSubject.commonName);
+ if (!X509_set_issuer_name(x509.get(), subjectName)) {
+ return Error() << "Unable to set x509 issuer name";
}
- // Beware: context contains a pointer to issuerCert, so we need to keep it alive.
- bssl::UniquePtr<X509> issuerCert;
- X509V3_CTX context;
-
- if (selfSigned) {
- context = makeContext(x509.get(), x509.get());
- } else {
- auto certStatus = loadX509(*issuerCertPath);
- if (!certStatus.ok()) {
- return Error() << "Unable to load issuer cert: " << certStatus.error();
- }
- issuerCert = std::move(certStatus.value());
- context = makeContext(issuerCert.get(), x509.get());
- }
-
- // If it's a self-signed cert we use it for signing certs, otherwise only for signing data.
- const char* basicConstraints = selfSigned ? "CA:TRUE" : "CA:FALSE";
- const char* keyUsage =
- selfSigned ? "critical,keyCertSign,cRLSign,digitalSignature" : "critical,digitalSignature";
-
- add_ext(&context, x509.get(), NID_basic_constraints, basicConstraints);
- add_ext(&context, x509.get(), NID_key_usage, keyUsage);
+ X509V3_CTX context = {};
+ X509V3_set_ctx(&context, x509.get(), x509.get(), nullptr, nullptr, 0);
+ add_ext(&context, x509.get(), NID_basic_constraints, "CA:TRUE");
+ add_ext(&context, x509.get(), NID_key_usage, "critical,keyCertSign,cRLSign,digitalSignature");
add_ext(&context, x509.get(), NID_subject_key_identifier, "hash");
add_ext(&context, x509.get(), NID_authority_key_identifier, "keyid:always");
@@ -280,31 +194,7 @@
return {};
}
-Result<void> createSelfSignedCertificate(
- const std::vector<uint8_t>& publicKey,
- const std::function<Result<std::string>(const std::string&)>& signFunction,
- const std::string& path) {
- auto rsa_pkey = modulusToRsaPkey(publicKey);
- if (!rsa_pkey.ok()) {
- return rsa_pkey.error();
- }
-
- return createCertificate(kRootSubject, rsa_pkey.value().get(), signFunction, {}, path);
-}
-
-android::base::Result<void> createLeafCertificate(
- const CertSubject& subject, const std::vector<uint8_t>& rsaPublicKey,
- const std::function<android::base::Result<std::string>(const std::string&)>& signFunction,
- const std::string& issuerCertPath, const std::string& path) {
- auto rsa_pkey = rsaPublicKeyToRsaPkey(rsaPublicKey);
- if (!rsa_pkey.ok()) {
- return rsa_pkey.error();
- }
-
- return createCertificate(subject, rsa_pkey.value().get(), signFunction, issuerCertPath, path);
-}
-
-Result<std::vector<uint8_t>> extractPublicKey(EVP_PKEY* pkey) {
+static Result<std::vector<uint8_t>> extractPublicKey(EVP_PKEY* pkey) {
if (pkey == nullptr) {
return Error() << "Failed to extract public key from x509 cert";
}
@@ -325,14 +215,6 @@
return pubKey;
}
-Result<std::vector<uint8_t>>
-extractPublicKeyFromSubjectPublicKeyInfo(const std::vector<uint8_t>& keyData) {
- auto keyDataBytes = keyData.data();
- bssl::UniquePtr<EVP_PKEY> public_key(d2i_PUBKEY(nullptr, &keyDataBytes, keyData.size()));
-
- return extractPublicKey(public_key.get());
-}
-
Result<std::vector<uint8_t>> extractPublicKeyFromX509(const std::vector<uint8_t>& derCert) {
auto derCertBytes = derCert.data();
bssl::UniquePtr<X509> decoded_cert(d2i_X509(nullptr, &derCertBytes, derCert.size()));
@@ -351,121 +233,3 @@
}
return extractPublicKey(X509_get_pubkey(cert.value().get()));
}
-
-static Result<std::vector<uint8_t>> extractRsaPublicKey(EVP_PKEY* pkey) {
- RSA* rsa = EVP_PKEY_get0_RSA(pkey);
- if (rsa == nullptr) {
- return Error() << "The public key is not an RSA key";
- }
-
- uint8_t* out = nullptr;
- int size = i2d_RSAPublicKey(rsa, &out);
- if (size < 0 || !out) {
- return Error() << "Failed to convert to RSAPublicKey";
- }
-
- bssl::UniquePtr<uint8_t> buffer(out);
- std::vector<uint8_t> result(out, out + size);
- return result;
-}
-
-Result<CertInfo> verifyAndExtractCertInfoFromX509(const std::string& path,
- const std::vector<uint8_t>& publicKey) {
- auto public_key = modulusToRsaPkey(publicKey);
- if (!public_key.ok()) {
- return public_key.error();
- }
-
- auto cert = loadX509(path);
- if (!cert.ok()) {
- return cert.error();
- }
- X509* x509 = cert.value().get();
-
- // Make sure we signed it.
- if (X509_verify(x509, public_key.value().get()) != 1) {
- return Error() << "Failed to verify certificate.";
- }
-
- bssl::UniquePtr<EVP_PKEY> pkey(X509_get_pubkey(x509));
- auto subject_key = extractRsaPublicKey(pkey.get());
- if (!subject_key.ok()) {
- return subject_key.error();
- }
-
- // The pointers here are all owned by x509, and each function handles an
- // error return from the previous call correctly.
- X509_NAME* name = X509_get_subject_name(x509);
- int index = X509_NAME_get_index_by_NID(name, NID_commonName, -1);
- X509_NAME_ENTRY* entry = X509_NAME_get_entry(name, index);
- ASN1_STRING* asn1cn = X509_NAME_ENTRY_get_data(entry);
- unsigned char* utf8cn;
- int length = ASN1_STRING_to_UTF8(&utf8cn, asn1cn);
- if (length < 0) {
- return Error() << "Failed to read subject CN";
- }
-
- bssl::UniquePtr<unsigned char> utf8owner(utf8cn);
- std::string cn(reinterpret_cast<char*>(utf8cn), static_cast<size_t>(length));
-
- CertInfo cert_info{std::move(cn), std::move(subject_key.value())};
- return cert_info;
-}
-
-Result<std::vector<uint8_t>> createPkcs7(const std::vector<uint8_t>& signed_digest,
- const CertSubject& signer) {
- CBB out, outer_seq, wrapped_seq, seq, digest_algos_set, digest_algo, null;
- CBB content_info, issuer_and_serial, signer_infos, signer_info, sign_algo, signature;
- uint8_t *pkcs7_data, *name_der;
- size_t pkcs7_data_len, name_der_len;
- BIGNUM* serial = BN_new();
- int sig_nid = NID_rsaEncryption;
-
- X509_NAME* issuer_name = X509_NAME_new();
- if (!issuer_name) {
- return Error() << "Unable to create x509 subject name";
- }
- X509_NAME_add_entry_by_txt(issuer_name, "C", MBSTRING_ASC,
- reinterpret_cast<const unsigned char*>(kIssuerCountry), -1, -1, 0);
- X509_NAME_add_entry_by_txt(issuer_name, "O", MBSTRING_ASC,
- reinterpret_cast<const unsigned char*>(kIssuerOrg), -1, -1, 0);
- X509_NAME_add_entry_by_txt(issuer_name, "CN", MBSTRING_ASC,
- reinterpret_cast<const unsigned char*>(kRootSubject.commonName), -1,
- -1, 0);
-
- BN_set_word(serial, signer.serialNumber);
- name_der_len = i2d_X509_NAME(issuer_name, &name_der);
- CBB_init(&out, 1024);
-
- if (!CBB_add_asn1(&out, &outer_seq, CBS_ASN1_SEQUENCE) ||
- !OBJ_nid2cbb(&outer_seq, NID_pkcs7_signed) ||
- !CBB_add_asn1(&outer_seq, &wrapped_seq,
- CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0) ||
- // See https://tools.ietf.org/html/rfc2315#section-9.1
- !CBB_add_asn1(&wrapped_seq, &seq, CBS_ASN1_SEQUENCE) ||
- !CBB_add_asn1_uint64(&seq, 1 /* version */) ||
- !CBB_add_asn1(&seq, &digest_algos_set, CBS_ASN1_SET) ||
- !CBB_add_asn1(&digest_algos_set, &digest_algo, CBS_ASN1_SEQUENCE) ||
- !OBJ_nid2cbb(&digest_algo, NID_sha256) ||
- !CBB_add_asn1(&digest_algo, &null, CBS_ASN1_NULL) ||
- !CBB_add_asn1(&seq, &content_info, CBS_ASN1_SEQUENCE) ||
- !OBJ_nid2cbb(&content_info, NID_pkcs7_data) ||
- !CBB_add_asn1(&seq, &signer_infos, CBS_ASN1_SET) ||
- !CBB_add_asn1(&signer_infos, &signer_info, CBS_ASN1_SEQUENCE) ||
- !CBB_add_asn1_uint64(&signer_info, 1 /* version */) ||
- !CBB_add_asn1(&signer_info, &issuer_and_serial, CBS_ASN1_SEQUENCE) ||
- !CBB_add_bytes(&issuer_and_serial, name_der, name_der_len) ||
- !BN_marshal_asn1(&issuer_and_serial, serial) ||
- !CBB_add_asn1(&signer_info, &digest_algo, CBS_ASN1_SEQUENCE) ||
- !OBJ_nid2cbb(&digest_algo, NID_sha256) ||
- !CBB_add_asn1(&digest_algo, &null, CBS_ASN1_NULL) ||
- !CBB_add_asn1(&signer_info, &sign_algo, CBS_ASN1_SEQUENCE) ||
- !OBJ_nid2cbb(&sign_algo, sig_nid) || !CBB_add_asn1(&sign_algo, &null, CBS_ASN1_NULL) ||
- !CBB_add_asn1(&signer_info, &signature, CBS_ASN1_OCTETSTRING) ||
- !CBB_add_bytes(&signature, signed_digest.data(), signed_digest.size()) ||
- !CBB_finish(&out, &pkcs7_data, &pkcs7_data_len)) {
- return Error() << "Failed to create PKCS7 certificate.";
- }
-
- return std::vector<uint8_t>(&pkcs7_data[0], &pkcs7_data[pkcs7_data_len]);
-}
diff --git a/ondevice-signing/VerityUtils.cpp b/ondevice-signing/VerityUtils.cpp
index 0b631da..b9cb96c 100644
--- a/ondevice-signing/VerityUtils.cpp
+++ b/ondevice-signing/VerityUtils.cpp
@@ -24,7 +24,6 @@
#include <linux/fs.h>
#include <sys/stat.h>
#include <sys/types.h>
-#include <sys/wait.h>
#include "android-base/errors.h"
#include <android-base/file.h>
@@ -42,7 +41,6 @@
using android::base::Result;
using android::base::unique_fd;
-static const char* kFsVerityInitPath = "/system/bin/fsverity_init";
static const char* kFsVerityProcPath = "/proc/sys/fs/verity";
bool SupportsFsVerity() {
@@ -285,41 +283,3 @@
return {};
}
-
-Result<void> addCertToFsVerityKeyring(const std::string& path, const char* keyName) {
- const char* const argv[] = {kFsVerityInitPath, "--load-extra-key", keyName};
-
- int fd = open(path.c_str(), O_RDONLY | O_CLOEXEC);
- if (fd == -1) {
- return ErrnoError() << "Failed to open " << path;
- }
- pid_t pid = fork();
- if (pid == 0) {
- dup2(fd, STDIN_FILENO);
- close(fd);
- int argc = arraysize(argv);
- char* argv_child[argc + 1];
- memcpy(argv_child, argv, argc * sizeof(char*));
- argv_child[argc] = nullptr;
- execvp(argv_child[0], argv_child);
- PLOG(ERROR) << "exec in ForkExecvp";
- _exit(EXIT_FAILURE);
- } else {
- close(fd);
- }
- if (pid == -1) {
- return ErrnoError() << "Failed to fork.";
- }
- int status;
- if (waitpid(pid, &status, 0) == -1) {
- return ErrnoError() << "waitpid() failed.";
- }
- if (!WIFEXITED(status)) {
- return Error() << kFsVerityInitPath << ": abnormal process exit";
- }
- if (WEXITSTATUS(status) != 0) {
- return Error() << kFsVerityInitPath << " exited with " << WEXITSTATUS(status);
- }
-
- return {};
-}
diff --git a/ondevice-signing/include/CertUtils.h b/ondevice-signing/include/CertUtils.h
index fe703fa..9b9d2cc 100644
--- a/ondevice-signing/include/CertUtils.h
+++ b/ondevice-signing/include/CertUtils.h
@@ -34,39 +34,18 @@
unsigned serialNumber;
};
-// These are all the certificates we ever sign (the first one being our
-// self-signed cert). We shouldn't really re-use serial numbers for different
-// certificates for the same subject but we do; only one should be in use at a
-// time though.
+// This is our self-signed cert.
inline const CertSubject kRootSubject{"ODS", 1};
-inline const CertSubject kCompOsSubject{"CompOs", 2};
android::base::Result<void> createSelfSignedCertificate(
const std::vector<uint8_t>& publicKey,
const std::function<android::base::Result<std::string>(const std::string&)>& signFunction,
const std::string& path);
-android::base::Result<void> createLeafCertificate(
- const CertSubject& subject, const std::vector<uint8_t>& publicKey,
- const std::function<android::base::Result<std::string>(const std::string&)>& signFunction,
- const std::string& issuerCertPath, const std::string& outPath);
-
-android::base::Result<std::vector<uint8_t>> createPkcs7(const std::vector<uint8_t>& signedData,
- const CertSubject& signer);
-
android::base::Result<std::vector<uint8_t>>
extractPublicKeyFromX509(const std::vector<uint8_t>& x509);
-android::base::Result<std::vector<uint8_t>>
-extractPublicKeyFromSubjectPublicKeyInfo(const std::vector<uint8_t>& subjectKeyInfo);
android::base::Result<std::vector<uint8_t>> extractPublicKeyFromX509(const std::string& path);
-android::base::Result<CertInfo>
-verifyAndExtractCertInfoFromX509(const std::string& path, const std::vector<uint8_t>& publicKey);
-
android::base::Result<void> verifySignature(const std::string& message,
const std::string& signature,
const std::vector<uint8_t>& publicKey);
-
-android::base::Result<void> verifyRsaPublicKeySignature(const std::string& message,
- const std::string& signature,
- const std::vector<uint8_t>& rsaPublicKey);
diff --git a/ondevice-signing/include/VerityUtils.h b/ondevice-signing/include/VerityUtils.h
index 626bbdb..71f78cf 100644
--- a/ondevice-signing/include/VerityUtils.h
+++ b/ondevice-signing/include/VerityUtils.h
@@ -22,7 +22,6 @@
#include <string>
#include <vector>
-android::base::Result<void> addCertToFsVerityKeyring(const std::string& path, const char* keyName);
android::base::Result<std::vector<uint8_t>> createDigest(const std::string& path);
android::base::Result<std::string> enableFsVerity(int fd);
bool SupportsFsVerity();
@@ -34,7 +33,7 @@
android::base::Result<std::map<std::string, std::string>>
addFilesToVerityRecursive(const std::string& path);
-// Enable verity on the provided file, using the given PKCS7 signature.
+// Enable verity on the provided file.
android::base::Result<void> enableFsVerity(const std::string& path);
android::base::Result<void>
diff --git a/ondevice-signing/proto/Android.bp b/ondevice-signing/proto/Android.bp
index 356e661..6b1e35f 100644
--- a/ondevice-signing/proto/Android.bp
+++ b/ondevice-signing/proto/Android.bp
@@ -37,6 +37,7 @@
crate_name: "odsign_proto",
protos: ["odsign_info.proto"],
source_stem: "odsign_proto",
+ use_protobuf3: true,
host_supported: true,
apex_available: [
"com.android.compos",
diff --git a/prng_seeder/src/cutils_socket.rs b/prng_seeder/src/cutils_socket.rs
index ab2c869..b408be6 100644
--- a/prng_seeder/src/cutils_socket.rs
+++ b/prng_seeder/src/cutils_socket.rs
@@ -19,7 +19,11 @@
pub fn android_get_control_socket(name: &str) -> Result<UnixListener> {
let name = CString::new(name)?;
+ // SAFETY: name is a valid C string, and android_get_control_socket doesn't retain it after it
+ // returns.
let fd = unsafe { cutils_socket_bindgen::android_get_control_socket(name.as_ptr()) };
ensure!(fd >= 0, "android_get_control_socket failed");
+ // SAFETY: android_get_control_socket either returns a valid and open FD or -1, and we checked
+ // that it's not -1.
Ok(unsafe { UnixListener::from_raw_fd(fd) })
}
diff --git a/prng_seeder/src/drbg.rs b/prng_seeder/src/drbg.rs
index 89c5a88..808ea18 100644
--- a/prng_seeder/src/drbg.rs
+++ b/prng_seeder/src/drbg.rs
@@ -23,6 +23,9 @@
impl Drbg {
pub fn new(entropy: &Entropy) -> Result<Drbg> {
+ // SAFETY: entropy must be a valid pointer because it comes from a reference, and a null
+ // pointer is allowed for personalization. CTR_DRBG_new doesn't retain the entropy pointer
+ // for use after it returns.
let p = unsafe { bssl_sys::CTR_DRBG_new(entropy.as_ptr(), std::ptr::null(), 0) };
ensure!(!p.is_null(), "CTR_DRBG_new failed");
Ok(Drbg(p))
@@ -30,6 +33,9 @@
pub fn reseed(&mut self, entropy: &Entropy) -> Result<()> {
ensure!(
+ // SAFETY: We know that self.0 is valid because it was initialised from CTR_DRBG_new in
+ // Drbg::new above. The entropy pointer must be valid because it comes from a reference,
+ // and CTR_DRBG_reseed doesn't retain it after it returns.
unsafe { bssl_sys::CTR_DRBG_reseed(self.0, entropy.as_ptr(), std::ptr::null(), 0) }
== 1,
"CTR_DRBG_reseed failed"
@@ -39,6 +45,10 @@
pub fn generate(&mut self, buf: &mut [u8]) -> Result<()> {
ensure!(
+ // SAFETY: We know that self.0 is valid because it was initialised from CTR_DRBG_new in
+ // Drbg::new above. The out pointer and length must be valid and unaliased because they
+ // come from a mutable slice reference, and CTR_DRBG_generate doesn't retain them after
+ // it returns.
unsafe {
bssl_sys::CTR_DRBG_generate(
self.0,
@@ -56,10 +66,13 @@
impl Drop for Drbg {
fn drop(&mut self) {
+ // SAFETY: We know that self.0 is valid because it was initialised from CTR_DRBG_new in
+ // Drbg::new above, and this is the only place that frees it.
unsafe {
bssl_sys::CTR_DRBG_free(self.0);
}
}
}
+// SAFETY: CTR_DRBG functions can be called from any thread.
unsafe impl Send for Drbg {}
diff --git a/prng_seeder/src/main.rs b/prng_seeder/src/main.rs
index 924481a..f8b0c63 100644
--- a/prng_seeder/src/main.rs
+++ b/prng_seeder/src/main.rs
@@ -70,6 +70,7 @@
fn setup() -> Result<(ConditionerBuilder, UnixListener)> {
configure_logging()?;
let cli = Cli::try_parse()?;
+ // SAFETY: Nothing else sets the signal handler, so either it was set here or it is the default.
unsafe { signal::signal(signal::Signal::SIGPIPE, signal::SigHandler::SigIgn) }
.context("In setup, setting SIGPIPE to SIG_IGN")?;
diff --git a/provisioner/Android.bp b/provisioner/Android.bp
index 3b4c4fc..605abb4 100644
--- a/provisioner/Android.bp
+++ b/provisioner/Android.bp
@@ -29,18 +29,20 @@
"keymint_use_latest_hal_aidl_ndk_static",
],
shared_libs: [
- "libbinder",
"libbinder_ndk",
"libcrypto",
"liblog",
],
static_libs: [
+ "android.hardware.common-V2-ndk",
+ "android.hardware.drm-V1-ndk",
"android.hardware.security.rkp-V3-ndk",
"libbase",
"libcppbor_external",
"libcppcose_rkp",
"libjsoncpp",
"libkeymint_remote_prov_support",
+ "libmediadrmrkp",
],
}
@@ -77,4 +79,39 @@
"libgflags",
"librkp_factory_extraction",
],
+ dist: {
+ targets: [
+ "dist_files",
+ "rkp_factory_extraction_tool",
+ ],
+ dest: "rkp_factory_extraction_tool"
+ },
+ compile_multilib: "both",
+ multilib: {
+ lib64: {
+ suffix: "64",
+ },
+ },
+ target: {
+ android_arm: {
+ dist: {
+ dir: "rkp/arm",
+ },
+ },
+ android_arm64: {
+ dist: {
+ dir: "rkp/arm64",
+ },
+ },
+ android_x86: {
+ dist: {
+ dir: "rkp/x86",
+ },
+ },
+ android_x86_64: {
+ dist: {
+ dir: "rkp/x86_64",
+ },
+ },
+ },
}
diff --git a/provisioner/rkp_factory_extraction_tool.cpp b/provisioner/rkp_factory_extraction_tool.cpp
index 5ba777e..5765e05 100644
--- a/provisioner/rkp_factory_extraction_tool.cpp
+++ b/provisioner/rkp_factory_extraction_tool.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <aidl/android/hardware/drm/IDrmFactory.h>
#include <aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.h>
#include <android/binder_manager.h>
#include <cppbor.h>
@@ -26,8 +27,10 @@
#include <string>
#include <vector>
+#include "DrmRkpAdapter.h"
#include "rkp_factory_extraction_lib.h"
+using aidl::android::hardware::drm::IDrmFactory;
using aidl::android::hardware::security::keymint::IRemotelyProvisionedComponent;
using aidl::android::hardware::security::keymint::remote_prov::jsonEncodeCsrWithBuild;
@@ -47,6 +50,10 @@
constexpr std::string_view kBuildPlusCsr = "build+csr"; // Text-encoded (JSON) build
// fingerprint plus CSR.
+std::string getFullServiceName(const char* descriptor, const char* name) {
+ return std::string(descriptor) + "/" + name;
+}
+
void writeOutput(const std::string instance_name, const Array& csr) {
if (FLAGS_output_format == kBinaryCsrOutput) {
auto bytes = csr.encode();
@@ -67,12 +74,21 @@
}
}
+void getCsrForIRpc(const char* descriptor, const char* name, IRemotelyProvisionedComponent* irpc) {
+ auto [request, errMsg] = getCsr(name, irpc, FLAGS_self_test);
+ auto fullName = getFullServiceName(descriptor, name);
+ if (!request) {
+ std::cerr << "Unable to build CSR for '" << fullName << ": " << errMsg << std::endl;
+ exit(-1);
+ }
+
+ writeOutput(std::string(name), *request);
+}
+
// Callback for AServiceManager_forEachDeclaredInstance that writes out a CSR
// for every IRemotelyProvisionedComponent.
void getCsrForInstance(const char* name, void* /*context*/) {
- const std::vector<uint8_t> challenge = generateChallenge();
-
- auto fullName = std::string(IRemotelyProvisionedComponent::descriptor) + "/" + name;
+ auto fullName = getFullServiceName(IRemotelyProvisionedComponent::descriptor, name);
AIBinder* rkpAiBinder = AServiceManager_getService(fullName.c_str());
::ndk::SpAIBinder rkp_binder(rkpAiBinder);
auto rkp_service = IRemotelyProvisionedComponent::fromBinder(rkp_binder);
@@ -81,13 +97,7 @@
exit(-1);
}
- 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);
+ getCsrForIRpc(IRemotelyProvisionedComponent::descriptor, name, rkp_service.get());
}
} // namespace
@@ -98,5 +108,10 @@
AServiceManager_forEachDeclaredInstance(IRemotelyProvisionedComponent::descriptor,
/*context=*/nullptr, getCsrForInstance);
+ // Append drm csr's
+ for (auto const& e : android::mediadrm::getDrmRemotelyProvisionedComponents()) {
+ getCsrForIRpc(IDrmFactory::descriptor, e.first.c_str(), e.second.get());
+ }
+
return 0;
}