Merge "Clear binder identity when executing callback"
diff --git a/TEST_MAPPING b/TEST_MAPPING
index 7be81a8..25908ad 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -17,6 +17,9 @@
},
{
"name": "compsvc_device_tests"
+ },
+ {
+ "name": "compos_key_tests"
}
],
"postsubmit": [
diff --git a/authfs/fd_server/src/aidl.rs b/authfs/fd_server/src/aidl.rs
index 125b991..f13f249 100644
--- a/authfs/fd_server/src/aidl.rs
+++ b/authfs/fd_server/src/aidl.rs
@@ -290,8 +290,10 @@
let new_fd = openat(
dir.as_raw_fd(),
basename,
- // TODO(205172873): handle the case when the file already exist, e.g. truncate
- // or fail, and possibly allow the client to specify. For now, always truncate.
+ // This function is supposed to be only called when FUSE/authfs thinks the file
+ // does not exist. However, if the file does exist from the view of fd_server
+ // (where the execution context is considered untrusted), we prefer to honor
+ // authfs and still allow the create to success. Therefore, always use O_TRUNC.
OFlag::O_CREAT | OFlag::O_RDWR | OFlag::O_TRUNC,
mode,
)
diff --git a/authfs/src/fsverity/verifier.rs b/authfs/src/fsverity/verifier.rs
index 61b8e13..aaf4bf7 100644
--- a/authfs/src/fsverity/verifier.rs
+++ b/authfs/src/fsverity/verifier.rs
@@ -112,8 +112,8 @@
}
pub struct VerifiedFileReader<F: ReadByChunk, M: ReadByChunk> {
+ pub file_size: u64,
chunked_file: F,
- file_size: u64,
merkle_tree: M,
root_hash: HashBuffer,
}
diff --git a/authfs/src/fusefs.rs b/authfs/src/fusefs.rs
index 84129b6..5bc25b0 100644
--- a/authfs/src/fusefs.rs
+++ b/authfs/src/fusefs.rs
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+mod file;
mod mount;
use anyhow::{anyhow, bail, Result};
@@ -37,12 +38,13 @@
use crate::common::{divide_roundup, ChunkedSizeIter, CHUNK_SIZE};
use crate::file::{
- validate_basename, Attr, EagerChunkReader, InMemoryDir, RandomWrite, ReadByChunk,
- RemoteDirEditor, RemoteFileEditor, RemoteFileReader,
+ validate_basename, Attr, InMemoryDir, RandomWrite, ReadByChunk, RemoteDirEditor,
+ RemoteFileEditor, RemoteFileReader,
};
use crate::fsstat::RemoteFsStatsReader;
-use crate::fsverity::{VerifiedFileEditor, VerifiedFileReader};
+use crate::fsverity::VerifiedFileEditor;
+pub use self::file::LazyVerifiedReadonlyFile;
pub use self::mount::mount_and_enter_message_loop;
use self::mount::MAX_WRITE_BYTES;
@@ -61,10 +63,7 @@
ReadonlyDirectory { dir: InMemoryDir },
/// A file type that is verified against fs-verity signature (thus read-only). The file is
/// served from a remote server.
- VerifiedReadonly {
- reader: VerifiedFileReader<RemoteFileReader, EagerChunkReader>,
- file_size: u64,
- },
+ VerifiedReadonly { reader: LazyVerifiedReadonlyFile },
/// A file type that is a read-only passthrough from a file on a remote server.
UnverifiedReadonly { reader: RemoteFileReader, file_size: u64 },
/// A file type that is initially empty, and the content is stored on a remote server. File
@@ -537,10 +536,12 @@
AuthFsEntry::ReadonlyDirectory { dir } => {
create_dir_stat(inode, dir.number_of_entries(), AccessMode::ReadOnly)
}
- AuthFsEntry::UnverifiedReadonly { file_size, .. }
- | AuthFsEntry::VerifiedReadonly { file_size, .. } => {
+ AuthFsEntry::UnverifiedReadonly { file_size, .. } => {
create_stat(inode, *file_size, AccessMode::ReadOnly)
}
+ AuthFsEntry::VerifiedReadonly { reader } => {
+ create_stat(inode, reader.file_size()?, AccessMode::ReadOnly)
+ }
AuthFsEntry::VerifiedNew { editor, attr, .. } => {
create_stat(inode, editor.size(), AccessMode::Variable(attr.mode()))
}
@@ -608,10 +609,12 @@
AuthFsEntry::ReadonlyDirectory { dir } => {
create_dir_stat(inode, dir.number_of_entries(), AccessMode::ReadOnly)
}
- AuthFsEntry::UnverifiedReadonly { file_size, .. }
- | AuthFsEntry::VerifiedReadonly { file_size, .. } => {
+ AuthFsEntry::UnverifiedReadonly { file_size, .. } => {
create_stat(inode, *file_size, AccessMode::ReadOnly)
}
+ AuthFsEntry::VerifiedReadonly { reader } => {
+ create_stat(inode, reader.file_size()?, AccessMode::ReadOnly)
+ }
AuthFsEntry::VerifiedNew { editor, attr, .. } => {
create_stat(inode, editor.size(), AccessMode::Variable(attr.mode()))
}
@@ -664,7 +667,6 @@
_flags: u32,
umask: u32,
) -> io::Result<(Entry, Option<Self::Handle>, FuseOpenOptions)> {
- // TODO(205172873): handle O_TRUNC and O_EXCL properly.
let new_inode = self.create_new_entry_with_ref_count(
parent,
name,
@@ -708,8 +710,8 @@
) -> io::Result<usize> {
self.handle_inode(&inode, |config| {
match config {
- AuthFsEntry::VerifiedReadonly { reader, file_size } => {
- read_chunks(w, reader, *file_size, offset, size)
+ AuthFsEntry::VerifiedReadonly { reader } => {
+ read_chunks(w, reader, reader.file_size()?, offset, size)
}
AuthFsEntry::UnverifiedReadonly { reader, file_size } => {
read_chunks(w, reader, *file_size, offset, size)
diff --git a/authfs/src/fusefs/file.rs b/authfs/src/fusefs/file.rs
new file mode 100644
index 0000000..8c02281
--- /dev/null
+++ b/authfs/src/fusefs/file.rs
@@ -0,0 +1,127 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+
+use log::error;
+use std::convert::TryInto;
+use std::io;
+use std::path::PathBuf;
+use std::sync::Mutex;
+
+use crate::file::{
+ ChunkBuffer, EagerChunkReader, ReadByChunk, RemoteFileReader, RemoteMerkleTreeReader,
+ VirtFdService,
+};
+use crate::fsverity::{merkle_tree_size, VerifiedFileReader};
+
+enum FileInfo {
+ ByPathUnderDirFd(i32, PathBuf),
+ ByFd(i32),
+}
+
+type Reader = VerifiedFileReader<RemoteFileReader, EagerChunkReader>;
+
+/// A lazily created read-only file that is verified against the given fs-verity digest.
+///
+/// The main purpose of this struct is to wrap and construct `VerifiedFileReader` lazily.
+pub struct LazyVerifiedReadonlyFile {
+ expected_digest: Vec<u8>,
+
+ service: VirtFdService,
+ file_info: FileInfo,
+
+ /// A lazily instantiated reader.
+ reader: Mutex<Option<Reader>>,
+}
+
+impl LazyVerifiedReadonlyFile {
+ /// Prepare the file by a remote path, related to a remote directory FD.
+ pub fn prepare_by_path(
+ service: VirtFdService,
+ remote_dir_fd: i32,
+ remote_path: PathBuf,
+ expected_digest: Vec<u8>,
+ ) -> Self {
+ LazyVerifiedReadonlyFile {
+ service,
+ file_info: FileInfo::ByPathUnderDirFd(remote_dir_fd, remote_path),
+ expected_digest,
+ reader: Mutex::new(None),
+ }
+ }
+
+ /// Prepare the file by a remote file FD.
+ pub fn prepare_by_fd(service: VirtFdService, remote_fd: i32, expected_digest: Vec<u8>) -> Self {
+ LazyVerifiedReadonlyFile {
+ service,
+ file_info: FileInfo::ByFd(remote_fd),
+ expected_digest,
+ reader: Mutex::new(None),
+ }
+ }
+
+ fn ensure_init_then<F, T>(&self, callback: F) -> io::Result<T>
+ where
+ F: FnOnce(&Reader) -> io::Result<T>,
+ {
+ let mut reader = self.reader.lock().unwrap();
+ if reader.is_none() {
+ let remote_file = match &self.file_info {
+ FileInfo::ByPathUnderDirFd(dir_fd, related_path) => {
+ RemoteFileReader::new_by_path(self.service.clone(), *dir_fd, related_path)?
+ }
+ FileInfo::ByFd(file_fd) => RemoteFileReader::new(self.service.clone(), *file_fd),
+ };
+ let remote_fd = remote_file.get_remote_fd();
+ let file_size = self
+ .service
+ .getFileSize(remote_fd)
+ .map_err(|e| {
+ error!("Failed to get file size of remote fd {}: {}", remote_fd, e);
+ io::Error::from_raw_os_error(libc::EIO)
+ })?
+ .try_into()
+ .map_err(|e| {
+ error!("Failed convert file size: {}", e);
+ io::Error::from_raw_os_error(libc::EIO)
+ })?;
+ let instance = VerifiedFileReader::new(
+ remote_file,
+ file_size,
+ &self.expected_digest,
+ EagerChunkReader::new(
+ RemoteMerkleTreeReader::new(self.service.clone(), remote_fd),
+ merkle_tree_size(file_size),
+ )?,
+ )
+ .map_err(|e| {
+ error!("Failed instantiate a verified file reader: {}", e);
+ io::Error::from_raw_os_error(libc::EIO)
+ })?;
+ *reader = Some(instance);
+ }
+ callback(reader.as_ref().unwrap())
+ }
+
+ pub fn file_size(&self) -> io::Result<u64> {
+ self.ensure_init_then(|reader| Ok(reader.file_size))
+ }
+}
+
+impl ReadByChunk for LazyVerifiedReadonlyFile {
+ fn read_chunk(&self, chunk_index: u64, buf: &mut ChunkBuffer) -> io::Result<usize> {
+ self.ensure_init_then(|reader| reader.read_chunk(chunk_index, buf))
+ }
+}
diff --git a/authfs/src/main.rs b/authfs/src/main.rs
index f664ca2..bdca5b4 100644
--- a/authfs/src/main.rs
+++ b/authfs/src/main.rs
@@ -47,14 +47,11 @@
mod fsverity;
mod fusefs;
-use file::{
- Attr, EagerChunkReader, InMemoryDir, RemoteDirEditor, RemoteFileEditor, RemoteFileReader,
- RemoteMerkleTreeReader,
-};
+use file::{Attr, InMemoryDir, RemoteDirEditor, RemoteFileEditor, RemoteFileReader};
use fsstat::RemoteFsStatsReader;
-use fsverity::{merkle_tree_size, VerifiedFileEditor, VerifiedFileReader};
+use fsverity::VerifiedFileEditor;
use fsverity_digests_proto::fsverity_digests::FSVerityDigests;
-use fusefs::{AuthFs, AuthFsEntry};
+use fusefs::{AuthFs, AuthFsEntry, LazyVerifiedReadonlyFile};
#[derive(StructOpt)]
struct Args {
@@ -186,19 +183,13 @@
service: file::VirtFdService,
remote_fd: i32,
expected_digest: &str,
- file_size: u64,
) -> Result<AuthFsEntry> {
Ok(AuthFsEntry::VerifiedReadonly {
- reader: VerifiedFileReader::new(
- RemoteFileReader::new(service.clone(), remote_fd),
- file_size,
- &from_hex_string(expected_digest)?,
- EagerChunkReader::new(
- RemoteMerkleTreeReader::new(service.clone(), remote_fd),
- merkle_tree_size(file_size),
- )?,
- )?,
- file_size,
+ reader: LazyVerifiedReadonlyFile::prepare_by_fd(
+ service,
+ remote_fd,
+ from_hex_string(expected_digest)?,
+ ),
})
}
@@ -239,12 +230,7 @@
for config in &args.remote_ro_file {
authfs.add_entry_at_root_dir(
remote_fd_to_path_buf(config.remote_fd),
- new_remote_verified_file_entry(
- service.clone(),
- config.remote_fd,
- &config.digest,
- service.getFileSize(config.remote_fd)?.try_into()?,
- )?,
+ new_remote_verified_file_entry(service.clone(), config.remote_fd, &config.digest)?,
)?;
}
@@ -294,25 +280,13 @@
let remote_path_str = path_str.strip_prefix(&config.prefix).ok_or_else(|| {
anyhow!("Expect path {} to match prefix {}", path_str, config.prefix)
})?;
- // TODO(205883847): Not all files will be used. Open the remote file lazily.
- let remote_file = RemoteFileReader::new_by_path(
- service.clone(),
- config.remote_dir_fd,
- Path::new(remote_path_str),
- )?;
- let remote_fd = remote_file.get_remote_fd();
- let file_size = service.getFileSize(remote_fd)?.try_into()?;
AuthFsEntry::VerifiedReadonly {
- reader: VerifiedFileReader::new(
- remote_file,
- file_size,
- &digest.digest,
- EagerChunkReader::new(
- RemoteMerkleTreeReader::new(service.clone(), remote_fd),
- merkle_tree_size(file_size),
- )?,
- )?,
- file_size,
+ reader: LazyVerifiedReadonlyFile::prepare_by_path(
+ service.clone(),
+ config.remote_dir_fd,
+ PathBuf::from(remote_path_str),
+ digest.digest.clone(),
+ ),
}
};
authfs.add_entry_at_ro_dir_by_path(dir_root_inode, Path::new(path_str), file_entry)?;
diff --git a/compos/apex/Android.bp b/compos/apex/Android.bp
index aec3c88..6550b4f 100644
--- a/compos/apex/Android.bp
+++ b/compos/apex/Android.bp
@@ -47,6 +47,7 @@
"composd_cmd",
// Used in VM
+ "compos_key_helper",
"compsvc",
],
diff --git a/compos/compos_key_helper/Android.bp b/compos/compos_key_helper/Android.bp
new file mode 100644
index 0000000..c53d88d
--- /dev/null
+++ b/compos/compos_key_helper/Android.bp
@@ -0,0 +1,47 @@
+package {
+ default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+cc_defaults {
+ name: "compos_key_defaults",
+ apex_available: ["com.android.compos"],
+
+ shared_libs: [
+ "libbase",
+ "libbinder_ndk",
+ "libcrypto",
+ ],
+}
+
+cc_library {
+ name: "libcompos_key",
+ defaults: ["compos_key_defaults"],
+ srcs: ["compos_key.cpp"],
+
+ shared_libs: [
+ "android.hardware.security.dice-V1-ndk",
+ "android.security.dice-ndk",
+ ],
+}
+
+cc_binary {
+ name: "compos_key_helper",
+ defaults: ["compos_key_defaults"],
+ srcs: ["compos_key_main.cpp"],
+
+ static_libs: ["libcompos_key"],
+ shared_libs: [
+ "android.security.dice-ndk",
+ ],
+}
+
+cc_test {
+ name: "compos_key_tests",
+ defaults: ["compos_key_defaults"],
+ test_suites: [
+ "general-tests",
+ ],
+
+ srcs: ["compos_key_test.cpp"],
+ static_libs: ["libcompos_key"],
+}
diff --git a/compos/compos_key_helper/compos_key.cpp b/compos/compos_key_helper/compos_key.cpp
new file mode 100644
index 0000000..84a736d
--- /dev/null
+++ b/compos/compos_key_helper/compos_key.cpp
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2022 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 "compos_key.h"
+
+#include <aidl/android/security/dice/IDiceNode.h>
+#include <android/binder_auto_utils.h>
+#include <android/binder_manager.h>
+#include <openssl/digest.h>
+#include <openssl/hkdf.h>
+#include <openssl/mem.h>
+#include <unistd.h>
+
+using aidl::android::hardware::security::dice::BccHandover;
+using aidl::android::hardware::security::dice::InputValues;
+using aidl::android::security::dice::IDiceNode;
+using android::base::ErrnoError;
+using android::base::Error;
+using android::base::Result;
+
+// Used to ensure the key we derive is distinct from any other.
+constexpr const char* kSigningKeyInfo = "CompOS signing key";
+
+Result<Ed25519KeyPair> deriveKeyFromSecret(const uint8_t* secret, size_t secret_size) {
+ // Ed25519 private keys are derived from a 32 byte seed:
+ // https://datatracker.ietf.org/doc/html/rfc8032#section-5.1.5
+ std::array<uint8_t, 32> seed;
+
+ // We derive the seed from the secret using HKDF - see
+ // https://datatracker.ietf.org/doc/html/rfc5869#section-2.
+ if (!HKDF(seed.data(), seed.size(), EVP_sha256(), secret, secret_size, /*salt=*/nullptr,
+ /*salt_len=*/0, reinterpret_cast<const uint8_t*>(kSigningKeyInfo),
+ strlen(kSigningKeyInfo))) {
+ return Error() << "HKDF failed";
+ }
+
+ Ed25519KeyPair result;
+ ED25519_keypair_from_seed(result.public_key.data(), result.private_key.data(), seed.data());
+ return result;
+}
+
+Result<Signature> sign(const PrivateKey& private_key, const uint8_t* data, size_t data_size) {
+ Signature result;
+ if (!ED25519_sign(result.data(), data, data_size, private_key.data())) {
+ return Error() << "Failed to sign";
+ }
+ return result;
+}
+
+bool verify(const PublicKey& public_key, const Signature& signature, const uint8_t* data,
+ size_t data_size) {
+ return ED25519_verify(data, data_size, signature.data(), public_key.data()) == 1;
+}
+
+Result<Ed25519KeyPair> deriveKeyFromDice() {
+ ndk::SpAIBinder binder{AServiceManager_getService("android.security.dice.IDiceNode")};
+ auto dice_node = IDiceNode::fromBinder(binder);
+ if (!dice_node) {
+ return Error() << "Unable to connect to IDiceNode";
+ }
+
+ const std::vector<InputValues> empty_input_values;
+ BccHandover bcc;
+ auto status = dice_node->derive(empty_input_values, &bcc);
+ if (!status.isOk()) {
+ return Error() << "Derive failed: " << status.getDescription();
+ }
+
+ // We use the sealing CDI because we want stability - the key needs to be the same
+ // for any instance of the "same" VM.
+ return deriveKeyFromSecret(bcc.cdiSeal.data(), bcc.cdiSeal.size());
+}
diff --git a/compos/compos_key_helper/compos_key.h b/compos/compos_key_helper/compos_key.h
new file mode 100644
index 0000000..520f846
--- /dev/null
+++ b/compos/compos_key_helper/compos_key.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2022 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.
+ */
+
+#pragma once
+
+#include <android-base/result.h>
+#include <openssl/curve25519.h>
+
+#include <array>
+
+using PrivateKey = std::array<uint8_t, ED25519_PRIVATE_KEY_LEN>;
+using PublicKey = std::array<uint8_t, ED25519_PUBLIC_KEY_LEN>;
+using Signature = std::array<uint8_t, ED25519_SIGNATURE_LEN>;
+
+struct Ed25519KeyPair {
+ PrivateKey private_key;
+ PublicKey public_key;
+};
+
+android::base::Result<Ed25519KeyPair> deriveKeyFromSecret(const uint8_t* secret,
+ size_t secret_size);
+
+android::base::Result<Ed25519KeyPair> deriveKeyFromDice();
+
+android::base::Result<Signature> sign(const PrivateKey& private_key, const uint8_t* data,
+ size_t data_size);
+
+bool verify(const PublicKey& public_key, const Signature& signature, const uint8_t* data,
+ size_t data_size);
diff --git a/compos/compos_key_helper/compos_key_main.cpp b/compos/compos_key_helper/compos_key_main.cpp
new file mode 100644
index 0000000..70f7539
--- /dev/null
+++ b/compos/compos_key_helper/compos_key_main.cpp
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2022 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 <android-base/file.h>
+#include <android-base/logging.h>
+#include <unistd.h>
+
+#include <iostream>
+#include <string_view>
+
+#include "compos_key.h"
+
+using android::base::ErrnoError;
+using android::base::WriteFully;
+using namespace std::literals;
+
+int main(int argc, char** argv) {
+ android::base::InitLogging(argv, android::base::LogdLogger(android::base::SYSTEM));
+
+ if (argc == 2) {
+ if (argv[1] == "public_key"sv) {
+ auto key_pair = deriveKeyFromDice();
+ if (!key_pair.ok()) {
+ LOG(ERROR) << key_pair.error();
+ return 1;
+ }
+ if (!WriteFully(STDOUT_FILENO, key_pair->public_key.data(),
+ key_pair->public_key.size())) {
+ PLOG(ERROR) << "Write failed";
+ return 1;
+ }
+ return 0;
+ }
+ }
+
+ std::cerr << "Usage:\n"
+ "compos_key_helper public_key Write current public key to stdout\n";
+ return 1;
+}
diff --git a/compos/compos_key_helper/compos_key_test.cpp b/compos/compos_key_helper/compos_key_test.cpp
new file mode 100644
index 0000000..e5c4768
--- /dev/null
+++ b/compos/compos_key_helper/compos_key_test.cpp
@@ -0,0 +1,90 @@
+/*
+ * Copyright 2022 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 "compos_key.h"
+
+#include <vector>
+
+#include "gtest/gtest.h"
+
+const std::vector<uint8_t> secret = {1, 2, 3};
+const std::vector<uint8_t> other_secret = {3, 2, 3};
+const std::vector<uint8_t> data = {42, 180, 65, 0};
+
+struct ComposKeyTest : public testing::Test {
+ Ed25519KeyPair key_pair;
+
+ void SetUp() override {
+ auto key_pair = deriveKeyFromSecret(secret.data(), secret.size());
+ ASSERT_TRUE(key_pair.ok()) << key_pair.error();
+ this->key_pair = *key_pair;
+ }
+};
+
+TEST_F(ComposKeyTest, SameSecretSameKey) {
+ auto other_key_pair = deriveKeyFromSecret(secret.data(), secret.size());
+ ASSERT_TRUE(other_key_pair.ok()) << other_key_pair.error();
+
+ ASSERT_EQ(key_pair.private_key, other_key_pair->private_key);
+ ASSERT_EQ(key_pair.public_key, other_key_pair->public_key);
+}
+
+TEST_F(ComposKeyTest, DifferentSecretDifferentKey) {
+ auto other_key_pair = deriveKeyFromSecret(other_secret.data(), other_secret.size());
+ ASSERT_TRUE(other_key_pair.ok()) << other_key_pair.error();
+
+ ASSERT_NE(key_pair.private_key, other_key_pair->private_key);
+ ASSERT_NE(key_pair.public_key, other_key_pair->public_key);
+}
+
+TEST_F(ComposKeyTest, CanVerifyValidSignature) {
+ auto signature = sign(key_pair.private_key, data.data(), data.size());
+ ASSERT_TRUE(signature.ok()) << signature.error();
+
+ bool verified = verify(key_pair.public_key, *signature, data.data(), data.size());
+ ASSERT_TRUE(verified);
+}
+
+TEST_F(ComposKeyTest, WrongSignatureDoesNotVerify) {
+ auto signature = sign(key_pair.private_key, data.data(), data.size());
+ ASSERT_TRUE(signature.ok()) << signature.error();
+
+ (*signature)[0] ^= 1;
+
+ bool verified = verify(key_pair.public_key, *signature, data.data(), data.size());
+ ASSERT_FALSE(verified);
+}
+
+TEST_F(ComposKeyTest, WrongDataDoesNotVerify) {
+ auto signature = sign(key_pair.private_key, data.data(), data.size());
+ ASSERT_TRUE(signature.ok()) << signature.error();
+
+ auto other_data = data;
+ other_data[0] ^= 1;
+
+ bool verified = verify(key_pair.public_key, *signature, other_data.data(), other_data.size());
+ ASSERT_FALSE(verified);
+}
+
+TEST_F(ComposKeyTest, WrongKeyDoesNotVerify) {
+ auto signature = sign(key_pair.private_key, data.data(), data.size());
+
+ auto other_key_pair = deriveKeyFromSecret(other_secret.data(), other_secret.size());
+ ASSERT_TRUE(other_key_pair.ok()) << other_key_pair.error();
+
+ bool verified = verify(other_key_pair->public_key, *signature, data.data(), data.size());
+ ASSERT_FALSE(verified);
+}
diff --git a/compos/compos_key_helper/tests/AndroidTest.xml b/compos/compos_key_helper/tests/AndroidTest.xml
new file mode 100644
index 0000000..3c1c657
--- /dev/null
+++ b/compos/compos_key_helper/tests/AndroidTest.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2022 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.
+-->
+<configuration description="Tests for compos_key">
+ <test class="com.android.tradefed.testtype.GTest" >
+ <option name="module-name" value="compos_key_tests" />
+ </test>
+</configuration>
diff --git a/compos/src/compilation.rs b/compos/src/compilation.rs
index 6049991..e14cd94 100644
--- a/compos/src/compilation.rs
+++ b/compos/src/compilation.rs
@@ -59,9 +59,10 @@
system_server_compiler_filter: &'a str,
) -> Result<Self> {
if compilation_mode != CompilationMode::NORMAL_COMPILE {
- let debuggable = is_property_set("ro.boot.microdroid.debuggable")
- || is_property_set("ro.boot.logd.enabled")
- || is_property_set("ro.boot.adb.enabled");
+ // Conservatively check debuggability.
+ let debuggable =
+ system_properties::read_bool("ro.boot.microdroid.app_debuggable", false)
+ .unwrap_or(false);
if !debuggable {
bail!("Requested compilation mode only available in debuggable VMs");
}
@@ -96,12 +97,6 @@
}
}
-// Return whether the named property is definitely enabled. Deliberately conservative; returns
-// false if the property does not exist or cannot be read or is malformed.
-fn is_property_set(name: &str) -> bool {
- system_properties::read_bool(name, false).unwrap_or(false)
-}
-
pub fn odrefresh<F>(
odrefresh_path: &Path,
context: OdrefreshContext,
diff --git a/microdroid/bootconfig.app_debuggable b/microdroid/bootconfig.app_debuggable
index 98d326a..0d85186 100644
--- a/microdroid/bootconfig.app_debuggable
+++ b/microdroid/bootconfig.app_debuggable
@@ -1,10 +1,15 @@
+# The app is debuggable.
+androidboot.microdroid.app_debuggable=1
+
# TODO(b/203369076) This should be 0 to disable adb rooting. For now, we can't do that because
# if this is set to 0, adbd enforces the host authentication but we don't put the adb
# public key (which represents the owner) in the VM yet.
androidboot.microdroid.debuggable=0
# Console output is not redirect to the host-side.
-kernel.console = null
+# TODO(b/219743539) This doesn't successfully disable the console
+kernel.printk.devkmsg=off
+kernel.console=null
# ADB is supported but rooting is prohibited.
androidboot.adb.enabled=1
diff --git a/microdroid/bootconfig.full_debuggable b/microdroid/bootconfig.full_debuggable
index fd8a83e..0bdd810 100644
--- a/microdroid/bootconfig.full_debuggable
+++ b/microdroid/bootconfig.full_debuggable
@@ -1,3 +1,6 @@
+# The app is debuggable as full_debuggable is a superser of app_debuggable.
+androidboot.microdroid.app_debuggable=1
+
# ro.debuggable is set.
androidboot.microdroid.debuggable=1
diff --git a/microdroid/bootconfig.normal b/microdroid/bootconfig.normal
index 9cfb55a..708d64b 100644
--- a/microdroid/bootconfig.normal
+++ b/microdroid/bootconfig.normal
@@ -1,8 +1,13 @@
+# The app is not debuggable.
+androidboot.microdroid.app_debuggable=0
+
# ro.debuggable is off
androidboot.microdroid.debuggable=0
# Console output is not redirect to the host-side.
-kernel.console = null
+# TODO(b/219743539) This doesn't successfully disable the console
+kernel.printk.devkmsg=off
+kernel.console=null
# ADB is not enabled.
androidboot.adb.enabled=0
diff --git a/microdroid/init.rc b/microdroid/init.rc
index ebe2464..5f0001f 100644
--- a/microdroid/init.rc
+++ b/microdroid/init.rc
@@ -196,6 +196,3 @@
"
chown logd logd /dev/event-log-tags
chmod 0644 /dev/event-log-tags
-
-on property:sys.boot_completed=1
- start logd-auditctl
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 005baf6..b644285 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -67,8 +67,7 @@
const APEX_CONFIG_DONE_PROP: &str = "apex_config.done";
const LOGD_ENABLED_PROP: &str = "ro.boot.logd.enabled";
-const ADBD_ENABLED_PROP: &str = "ro.boot.adb.enabled";
-const DEBUGGABLE_PROP: &str = "ro.boot.microdroid.debuggable";
+const APP_DEBUGGABLE_PROP: &str = "ro.boot.microdroid.app_debuggable";
#[derive(thiserror::Error, Debug)]
enum MicrodroidError {
@@ -121,7 +120,7 @@
}
fn try_main() -> Result<()> {
- kernlog::init()?;
+ let _ = kernlog::init();
info!("started.");
let service = get_vms_rpc_binder().context("cannot connect to VirtualMachineService")?;
@@ -145,15 +144,6 @@
}
}
-fn is_debuggable() -> Result<bool> {
- // Read all the properties so the behaviour is most similar between debug and non-debug boots.
- // Defensively default to debug enabled for unrecognised values.
- let adb = system_properties::read_bool(ADBD_ENABLED_PROP, true)?;
- let logd = system_properties::read_bool(LOGD_ENABLED_PROP, true)?;
- let debuggable = system_properties::read_bool(DEBUGGABLE_PROP, true)?;
- Ok(adb || logd || debuggable)
-}
-
fn dice_derivation(verified_data: MicrodroidData, payload_config_path: &str) -> Result<()> {
// Calculate compound digests of code and authorities
let mut code_hash_ctx = digest::Context::new(&digest::SHA512);
@@ -183,6 +173,9 @@
encode_header(3, config_path_bytes.len().try_into().unwrap(), &mut config_desc)?;
config_desc.extend_from_slice(config_path_bytes);
+ // Check app debuggability, conervatively assuming it is debuggable
+ let app_debuggable = system_properties::read_bool(APP_DEBUGGABLE_PROP, true)?;
+
// Send the details to diced
let diced =
wait_for_interface::<dyn IDiceMaintenance>("android.security.dice.IDiceMaintenance")
@@ -193,7 +186,7 @@
config: Config { desc: config_desc },
authorityHash: authority_hash,
authorityDescriptor: None,
- mode: if is_debuggable()? { Mode::DEBUG } else { Mode::NORMAL },
+ mode: if app_debuggable { Mode::DEBUG } else { Mode::NORMAL },
hidden: verified_data.salt.try_into().unwrap(),
}])
.context("IDiceMaintenance::demoteSelf failed")?;
diff --git a/tests/hostside/java/android/virt/test/MicrodroidTestCase.java b/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
index 25adc40..1e96246 100644
--- a/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
+++ b/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
@@ -18,12 +18,16 @@
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import com.android.tradefed.device.DeviceNotAvailableException;
+import com.android.tradefed.result.TestDescription;
+import com.android.tradefed.result.TestResult;
import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
+import com.android.tradefed.testtype.junit4.DeviceTestRunOptions;
import com.android.tradefed.util.CommandResult;
import com.android.tradefed.util.CommandStatus;
import com.android.tradefed.util.FileUtil;
@@ -35,6 +39,7 @@
import org.junit.runner.RunWith;
import java.io.File;
+import java.util.Map;
import java.util.Optional;
@RunWith(DeviceJUnit4ClassRunner.class)
@@ -73,6 +78,27 @@
}
@Test
+ public void testCreateVmRequiresPermission() throws Exception {
+ // Revoke the MANAGE_VIRTUAL_MACHINE permission for the test app
+ CommandRunner android = new CommandRunner(getDevice());
+ android.run("pm", "revoke", PACKAGE_NAME, "android.permission.MANAGE_VIRTUAL_MACHINE");
+
+ // Run MicrodroidTests#connectToVmService test, which should fail
+ final DeviceTestRunOptions options = new DeviceTestRunOptions(PACKAGE_NAME)
+ .setTestClassName(PACKAGE_NAME + ".MicrodroidTests")
+ .setTestMethodName("connectToVmService")
+ .setCheckResults(false);
+ assertFalse(runDeviceTests(options));
+
+ Map<TestDescription, TestResult> results = getLastDeviceRunResults().getTestResults();
+ assertThat(results.size(), is(1));
+ TestResult result = results.values().toArray(new TestResult[0])[0];
+ assertTrue("The test should fail with a permission error",
+ result.getStackTrace()
+ .contains("android.permission.MANAGE_VIRTUAL_MACHINE permission"));
+ }
+
+ @Test
public void testMicrodroidBoots() throws Exception {
final String configPath = "assets/vm_config.json"; // path inside the APK
final String cid =
@@ -119,6 +145,9 @@
assertThat(runOnMicrodroid("cat /proc/cpuinfo | grep processor | wc -l"),
is(Integer.toString(NUM_VCPUS)));
+ // Check that selinux is enabled
+ assertThat(runOnMicrodroid("getenforce"), is("Enforcing"));
+
// TODO(b/176805428): adb is broken for nested VM
if (!isCuttlefish()) {
// Check neverallow rules on microdroid
diff --git a/tests/vsock_test.cc b/tests/vsock_test.cc
index 9550651..c478f64 100644
--- a/tests/vsock_test.cc
+++ b/tests/vsock_test.cc
@@ -49,26 +49,14 @@
static constexpr const char kVmParams[] = "rdinit=/bin/init bin/vsock_client 2 45678 HelloWorld";
static constexpr const char kTestMessage[] = "HelloWorld";
-/** Returns true if the kernel supports protected VMs. */
-bool isProtectedVmSupported() {
- return android::sysprop::HypervisorProperties::hypervisor_protected_vm_supported().value_or(
- false);
-}
-
/** Returns true if the kernel supports unprotected VMs. */
bool isUnprotectedVmSupported() {
return android::sysprop::HypervisorProperties::hypervisor_vm_supported().value_or(false);
}
-void runTest(sp<IVirtualizationService> virtualization_service, bool protected_vm) {
- if (protected_vm) {
- if (!isProtectedVmSupported()) {
- GTEST_SKIP() << "Skipping as protected VMs are not supported on this device.";
- }
- } else {
- if (!isUnprotectedVmSupported()) {
- GTEST_SKIP() << "Skipping as unprotected VMs are not supported on this device.";
- }
+TEST_F(VirtualizationTest, TestVsock) {
+ if (!isUnprotectedVmSupported()) {
+ GTEST_SKIP() << "Skipping as unprotected VMs are not supported on this device.";
}
binder::Status status;
@@ -93,11 +81,11 @@
raw_config.kernel = ParcelFileDescriptor(unique_fd(open(kVmKernelPath, O_RDONLY | O_CLOEXEC)));
raw_config.initrd = ParcelFileDescriptor(unique_fd(open(kVmInitrdPath, O_RDONLY | O_CLOEXEC)));
raw_config.params = kVmParams;
- raw_config.protectedVm = protected_vm;
+ raw_config.protectedVm = false;
VirtualMachineConfig config(std::move(raw_config));
sp<IVirtualMachine> vm;
- status = virtualization_service->createVm(config, std::nullopt, std::nullopt, &vm);
+ status = mVirtualizationService->createVm(config, std::nullopt, std::nullopt, &vm);
ASSERT_TRUE(status.isOk()) << "Error creating VM: " << status;
int32_t cid;
@@ -124,12 +112,4 @@
ASSERT_EQ(msg, kTestMessage);
}
-TEST_F(VirtualizationTest, TestVsock) {
- runTest(mVirtualizationService, false);
-}
-
-TEST_F(VirtualizationTest, TestVsockProtected) {
- runTest(mVirtualizationService, true);
-}
-
} // namespace virt
diff --git a/virtualizationservice/src/main.rs b/virtualizationservice/src/main.rs
index 69ae076..43e46a3 100644
--- a/virtualizationservice/src/main.rs
+++ b/virtualizationservice/src/main.rs
@@ -40,7 +40,7 @@
fn main() {
android_logger::init_once(
- android_logger::Config::default().with_tag(LOG_TAG).with_min_level(Level::Trace),
+ android_logger::Config::default().with_tag(LOG_TAG).with_min_level(Level::Info),
);
clear_temporary_files().expect("Failed to delete old temporary files");