Merge "keystore: Fix ID rotation window" into main
diff --git a/fsverity_init/Android.bp b/fsverity_init/Android.bp
deleted file mode 100644
index 07eaf6a..0000000
--- a/fsverity_init/Android.bp
+++ /dev/null
@@ -1,25 +0,0 @@
-package {
- // See: http://go/android-license-faq
- // A large-scale-change added 'default_applicable_licenses' to import
- // all of the 'license_kinds' from "system_security_license"
- // to get the below license kinds:
- // SPDX-license-identifier-Apache-2.0
- default_applicable_licenses: ["system_security_license"],
-}
-
-cc_binary {
- name: "fsverity_init",
- srcs: [
- "fsverity_init.cpp",
- ],
- static_libs: [
- "libc++fs",
- "libmini_keyctl_static",
- ],
- shared_libs: [
- "libbase",
- "libkeyutils",
- "liblog",
- ],
- cflags: ["-Werror", "-Wall", "-Wextra"],
-}
diff --git a/fsverity_init/OWNERS b/fsverity_init/OWNERS
deleted file mode 100644
index f9e7b25..0000000
--- a/fsverity_init/OWNERS
+++ /dev/null
@@ -1,5 +0,0 @@
-alanstokes@google.com
-ebiggers@google.com
-jeffv@google.com
-jiyong@google.com
-victorhsieh@google.com
diff --git a/fsverity_init/fsverity_init.cpp b/fsverity_init/fsverity_init.cpp
deleted file mode 100644
index 797118d..0000000
--- a/fsverity_init/fsverity_init.cpp
+++ /dev/null
@@ -1,104 +0,0 @@
-/*
- * Copyright (C) 2019 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.
- */
-
-//
-// 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>
-
-#include <filesystem>
-#include <string>
-
-#include <android-base/file.h>
-#include <android-base/logging.h>
-#include <android-base/strings.h>
-#include <log/log.h>
-#include <mini_keyctl_utils.h>
-
-void LoadKeyFromFile(key_serial_t keyring_id, const char* keyname, const std::string& path) {
- LOG(INFO) << "LoadKeyFromFile path=" << path << " keyname=" << keyname;
- std::string content;
- if (!android::base::ReadFileToString(path, &content)) {
- LOG(ERROR) << "Failed to read key from " << path;
- return;
- }
- if (add_key("asymmetric", keyname, content.c_str(), content.size(), keyring_id) < 0) {
- PLOG(ERROR) << "Failed to add key from " << path;
- }
-}
-
-void LoadKeyFromDirectory(key_serial_t keyring_id, const char* keyname_prefix, const char* dir) {
- if (!std::filesystem::exists(dir)) {
- return;
- }
- int counter = 0;
- for (const auto& entry : std::filesystem::directory_iterator(dir)) {
- if (!android::base::EndsWithIgnoreCase(entry.path().c_str(), ".der")) continue;
- std::string keyname = keyname_prefix + std::to_string(counter);
- counter++;
- LoadKeyFromFile(keyring_id, keyname.c_str(), entry.path());
- }
-}
-
-void LoadKeyFromVerifiedPartitions(key_serial_t keyring_id) {
- // NB: Directories need to be synced with FileIntegrityService.java in
- // frameworks/base.
- 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/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/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")?;