Merge "rialto_test: Run non-protected tests sequentially" into main
diff --git a/android/fd_server/Android.bp b/android/fd_server/Android.bp
index b02c104..32a8fec 100644
--- a/android/fd_server/Android.bp
+++ b/android/fd_server/Android.bp
@@ -18,6 +18,7 @@
"liblog_rust",
"libnix",
"librpcbinder_rs",
+ "libsafe_ownedfd",
],
prefer_rlib: true,
apex_available: ["com.android.virt"],
@@ -39,6 +40,7 @@
"liblog_rust",
"libnix",
"librpcbinder_rs",
+ "libsafe_ownedfd",
],
prefer_rlib: true,
test_suites: ["general-tests"],
diff --git a/android/fd_server/src/aidl.rs b/android/fd_server/src/aidl.rs
index 5f91987..2f3697c 100644
--- a/android/fd_server/src/aidl.rs
+++ b/android/fd_server/src/aidl.rs
@@ -14,20 +14,21 @@
* limitations under the License.
*/
-use anyhow::Result;
+use anyhow::{Context, Result};
use log::error;
use nix::{
errno::Errno, fcntl::openat, fcntl::OFlag, sys::stat::fchmod, sys::stat::mkdirat,
sys::stat::mode_t, sys::stat::Mode, sys::statvfs::statvfs, sys::statvfs::Statvfs,
unistd::unlinkat, unistd::UnlinkatFlags,
};
+use safe_ownedfd::take_fd_ownership;
use std::cmp::min;
use std::collections::{btree_map, BTreeMap};
use std::convert::TryInto;
use std::fs::File;
use std::io;
use std::os::unix::fs::FileExt;
-use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd};
+use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, OwnedFd};
use std::path::{Component, Path, PathBuf, MAIN_SEPARATOR};
use std::sync::{Arc, RwLock};
@@ -38,7 +39,8 @@
get_fsverity_metadata_path, parse_fsverity_metadata, FSVerityMetadata,
};
use binder::{
- BinderFeatures, ExceptionCode, Interface, Result as BinderResult, Status, StatusCode, Strong,
+ BinderFeatures, ExceptionCode, Interface, IntoBinderResult, Result as BinderResult, Status,
+ StatusCode, Strong,
};
/// Bitflags of forbidden file mode, e.g. setuid, setgid and sticky bit.
@@ -299,9 +301,11 @@
mode,
)
.map_err(new_errno_error)?;
- // SAFETY: new_fd is just created and not an error.
- let new_file = unsafe { File::from_raw_fd(new_fd) };
- Ok((new_fd, FdConfig::ReadWrite(new_file)))
+ let new_fd = take_fd_ownership(new_fd)
+ .context("Failed to take ownership of fd for file")
+ .or_service_specific_exception(-1)?;
+ let new_file = File::from(new_fd);
+ Ok((new_file.as_raw_fd(), FdConfig::ReadWrite(new_file)))
}
_ => Err(new_errno_error(Errno::ENOTDIR)),
})
@@ -327,9 +331,10 @@
Mode::empty(),
)
.map_err(new_errno_error)?;
- // SAFETY: new_dir_fd is just created and not an error.
- let fd_owner = unsafe { OwnedFd::from_raw_fd(new_dir_fd) };
- Ok((new_dir_fd, FdConfig::OutputDir(fd_owner)))
+ let fd_owner = take_fd_ownership(new_dir_fd)
+ .context("Failed to take ownership of the fd for directory")
+ .or_service_specific_exception(-1)?;
+ Ok((fd_owner.as_raw_fd(), FdConfig::OutputDir(fd_owner)))
}
_ => Err(new_errno_error(Errno::ENOTDIR)),
})
@@ -408,9 +413,11 @@
fn open_readonly_at(dir_fd: BorrowedFd, path: &Path) -> nix::Result<File> {
let new_fd = openat(Some(dir_fd.as_raw_fd()), path, OFlag::O_RDONLY, Mode::empty())?;
- // SAFETY: new_fd is just created successfully and not owned.
- let new_file = unsafe { File::from_raw_fd(new_fd) };
- Ok(new_file)
+ let new_fd = take_fd_ownership(new_fd).map_err(|e| match e {
+ safe_ownedfd::Error::Errno(e) => e,
+ _ => Errno::UnknownErrno,
+ })?;
+ Ok(File::from(new_fd))
}
fn validate_and_cast_offset(offset: i64) -> Result<u64, Status> {
diff --git a/guest/authfs_service/Android.bp b/guest/authfs_service/Android.bp
index 2101a36..e508c17 100644
--- a/guest/authfs_service/Android.bp
+++ b/guest/authfs_service/Android.bp
@@ -18,6 +18,7 @@
"libnix",
"librpcbinder_rs",
"librustutils",
+ "libsafe_ownedfd",
"libshared_child",
],
prefer_rlib: true,
diff --git a/guest/authfs_service/src/main.rs b/guest/authfs_service/src/main.rs
index 97e684d..ff2f770 100644
--- a/guest/authfs_service/src/main.rs
+++ b/guest/authfs_service/src/main.rs
@@ -26,9 +26,10 @@
use log::*;
use rpcbinder::RpcServer;
use rustutils::sockets::android_get_control_socket;
+use safe_ownedfd::take_fd_ownership;
use std::ffi::OsString;
use std::fs::{create_dir, read_dir, remove_dir_all, remove_file};
-use std::os::unix::io::{FromRawFd, OwnedFd};
+use std::os::unix::io::OwnedFd;
use std::sync::atomic::{AtomicUsize, Ordering};
use authfs_aidl_interface::aidl::com::android::virt::fs::AuthFsConfig::AuthFsConfig;
@@ -109,22 +110,9 @@
}
/// Prepares a socket file descriptor for the authfs service.
-///
-/// # Safety requirement
-///
-/// The caller must ensure that this function is the only place that claims ownership
-/// of the file descriptor and it is called only once.
-unsafe fn prepare_authfs_service_socket() -> Result<OwnedFd> {
+fn prepare_authfs_service_socket() -> Result<OwnedFd> {
let raw_fd = android_get_control_socket(AUTHFS_SERVICE_SOCKET_NAME)?;
-
- // Creating OwnedFd for stdio FDs is not safe.
- if [libc::STDIN_FILENO, libc::STDOUT_FILENO, libc::STDERR_FILENO].contains(&raw_fd) {
- bail!("File descriptor {raw_fd} is standard I/O descriptor");
- }
- // SAFETY: Initializing OwnedFd for a RawFd created by the init.
- // We checked that the integer value corresponds to a valid FD and that the caller
- // ensures that this is the only place to claim its ownership.
- Ok(unsafe { OwnedFd::from_raw_fd(raw_fd) })
+ Ok(take_fd_ownership(raw_fd)?)
}
#[allow(clippy::eq_op)]
@@ -137,8 +125,7 @@
clean_up_working_directory()?;
- // SAFETY: This is the only place we take the ownership of the fd of the authfs service.
- let socket_fd = unsafe { prepare_authfs_service_socket()? };
+ let socket_fd = prepare_authfs_service_socket()?;
let service = AuthFsService::new_binder(debuggable).as_binder();
debug!("{} is starting as a rpc service.", AUTHFS_SERVICE_SOCKET_NAME);
let server = RpcServer::new_bound_socket(service, socket_fd)?;
diff --git a/guest/rialto/tests/test.rs b/guest/rialto/tests/test.rs
index 90721b9..582b69e 100644
--- a/guest/rialto/tests/test.rs
+++ b/guest/rialto/tests/test.rs
@@ -283,7 +283,9 @@
}
fn check_csr(csr: Vec<u8>) -> Result<()> {
- let _csr = rkp::Csr::from_cbor(&Session::default(), &csr[..]).context("Failed to parse CSR")?;
+ let mut session = Session::default();
+ session.set_allow_any_mode(true);
+ let _csr = rkp::Csr::from_cbor(&session, &csr[..]).context("Failed to parse CSR")?;
Ok(())
}
diff --git a/tests/ComposHostTestCases/java/android/compos/test/ComposTestCase.java b/tests/ComposHostTestCases/java/android/compos/test/ComposTestCase.java
index 7a35829..6e583c0 100644
--- a/tests/ComposHostTestCases/java/android/compos/test/ComposTestCase.java
+++ b/tests/ComposHostTestCases/java/android/compos/test/ComposTestCase.java
@@ -200,6 +200,7 @@
10000,
validator.getAbsolutePath(),
"dice-chain",
+ "--allow-any-mode",
bcc_file.getAbsolutePath());
assertWithMessage("hwtrust failed").about(command_results()).that(result).isSuccess();
}
diff --git a/tests/testapk/src/java/com/android/microdroid/test/HwTrustJni.java b/tests/testapk/src/java/com/android/microdroid/test/HwTrustJni.java
index 3b237aa..0cf0606 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/HwTrustJni.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/HwTrustJni.java
@@ -25,7 +25,8 @@
* Validates a DICE chain.
*
* @param diceChain The dice chain to validate.
+ * @param allowAnyMode Allow the chain's certificates to have any mode.
* @return true if the dice chain is valid, false otherwise.
*/
- public static native boolean validateDiceChain(byte[] diceChain);
+ public static native boolean validateDiceChain(byte[] diceChain, boolean allowAnyMode);
}
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index f21e18e..d38af45 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -1318,7 +1318,7 @@
grantPermission(VirtualMachine.USE_CUSTOM_VIRTUAL_MACHINE_PERMISSION);
VirtualMachineConfig config =
newVmConfigBuilderWithPayloadConfig("assets/vm_config.json")
- .setDebugLevel(DEBUG_LEVEL_FULL)
+ .setDebugLevel(DEBUG_LEVEL_NONE)
.build();
VirtualMachine vm = forceCreateNewVirtualMachine("bcc_vm_for_vsr", config);
TestResults testResults =
@@ -1331,7 +1331,11 @@
testResults.assertNoException();
byte[] bccBytes = testResults.mBcc;
assertThat(bccBytes).isNotNull();
- assertThat(HwTrustJni.validateDiceChain(bccBytes)).isTrue();
+
+ String buildType = SystemProperties.get("ro.build.type");
+ boolean nonUserBuild = !buildType.isEmpty() && buildType != "user";
+
+ assertThat(HwTrustJni.validateDiceChain(bccBytes, nonUserBuild)).isTrue();
}
@Test
diff --git a/tests/testapk/src/native/hwtrust_jni.rs b/tests/testapk/src/native/hwtrust_jni.rs
index 3b00364..058b1a6 100644
--- a/tests/testapk/src/native/hwtrust_jni.rs
+++ b/tests/testapk/src/native/hwtrust_jni.rs
@@ -29,6 +29,7 @@
env: JNIEnv,
_class: JClass,
dice_chain: JByteArray,
+ allow_any_mode: jboolean,
) -> jboolean {
android_logger::init_once(
android_logger::Config::default()
@@ -36,7 +37,7 @@
.with_max_level(log::LevelFilter::Debug),
);
debug!("Starting the DICE chain validation ...");
- match validate_dice_chain(env, dice_chain) {
+ match validate_dice_chain(env, dice_chain, allow_any_mode) {
Ok(_) => {
info!("DICE chain validated successfully");
true
@@ -49,9 +50,14 @@
.into()
}
-fn validate_dice_chain(env: JNIEnv, jdice_chain: JByteArray) -> Result<()> {
+fn validate_dice_chain(
+ env: JNIEnv,
+ jdice_chain: JByteArray,
+ allow_any_mode: jboolean,
+) -> Result<()> {
let dice_chain = env.convert_byte_array(jdice_chain)?;
- let session = Session::default();
+ let mut session = Session::default();
+ session.set_allow_any_mode(allow_any_mode == jboolean::from(true));
let _chain = dice::Chain::from_cbor(&session, &dice_chain)?;
Ok(())
}