Merge changes I89c9c96d,I9c7b68d5,I6b3f2b0c

* changes:
  Fix test action that tampered file at wrong offset
  Verify the last incomplete chunk on resize
  Verify read_chunk for writable file
This commit is contained in:
Treehugger Robot 2022-03-07 20:03:32 +00:00 committed by Gerrit Code Review
commit 8c5e23c396
2 changed files with 116 additions and 13 deletions

View File

@ -52,6 +52,7 @@
//! Rollback attack is another possible attack, but can be addressed with a rollback counter when
//! possible.
use log::warn;
use std::io;
use std::sync::{Arc, RwLock};
@ -99,6 +100,44 @@ impl<F: ReadByChunk + RandomWrite> VerifiedFileEditor<F> {
merkle_tree.calculate_fsverity_digest().map_err(|e| io::Error::new(io::ErrorKind::Other, e))
}
fn read_backing_chunk_unverified(
&self,
chunk_index: u64,
buf: &mut ChunkBuffer,
) -> io::Result<usize> {
self.file.read_chunk(chunk_index, buf)
}
fn read_backing_chunk_verified(
&self,
chunk_index: u64,
buf: &mut ChunkBuffer,
merkle_tree_locked: &MerkleLeaves,
) -> io::Result<usize> {
let size = self.read_backing_chunk_unverified(chunk_index, buf)?;
// Ensure the returned buffer matches the known hash.
debug_assert_usize_is_u64();
if merkle_tree_locked.is_index_valid(chunk_index as usize) {
let hash = Sha256Hasher::new()?.update(buf)?.finalize()?;
if !merkle_tree_locked.is_consistent(chunk_index as usize, &hash) {
return Err(io::Error::new(io::ErrorKind::InvalidData, "Inconsistent hash"));
}
Ok(size)
} else {
if size != 0 {
// This is unexpected. For any reason that the file is changed and doesn't match
// the known state, ignore it at the moment. We can still generate correct
// fs-verity digest for an output file.
warn!(
"Ignoring the received {} bytes for index {} beyond the known file size",
size, chunk_index,
);
}
Ok(0)
}
}
fn new_hash_for_incomplete_write(
&self,
source: &[u8],
@ -114,7 +153,7 @@ impl<F: ReadByChunk + RandomWrite> VerifiedFileEditor<F> {
// If previous data exists, read back and verify against the known hash (since the
// storage / remote server is not trusted).
if merkle_tree.is_index_valid(output_chunk_index) {
self.read_chunk(output_chunk_index as u64, &mut orig_data)?;
self.read_backing_chunk_unverified(output_chunk_index as u64, &mut orig_data)?;
// Verify original content
let hash = Sha256Hasher::new()?.update(&orig_data)?.finalize()?;
@ -239,7 +278,7 @@ impl<F: ReadByChunk + RandomWrite> RandomWrite for VerifiedFileEditor<F> {
let chunk_index = size / CHUNK_SIZE;
if new_tail_size > 0 {
let mut buf: ChunkBuffer = [0; CHUNK_SIZE as usize];
let s = self.read_chunk(chunk_index, &mut buf)?;
let s = self.read_backing_chunk_verified(chunk_index, &mut buf, &merkle_tree)?;
debug_assert!(new_tail_size <= s);
let zeros = vec![0; CHUNK_SIZE as usize - new_tail_size];
@ -260,7 +299,8 @@ impl<F: ReadByChunk + RandomWrite> RandomWrite for VerifiedFileEditor<F> {
impl<F: ReadByChunk + RandomWrite> ReadByChunk for VerifiedFileEditor<F> {
fn read_chunk(&self, chunk_index: u64, buf: &mut ChunkBuffer) -> io::Result<usize> {
self.file.read_chunk(chunk_index, buf)
let merkle_tree = self.merkle_tree.read().unwrap();
self.read_backing_chunk_verified(chunk_index, buf, &merkle_tree)
}
}

View File

@ -265,8 +265,8 @@ public final class AuthFsHostTest extends VirtualizationTestCaseBase {
assertTrue(copyFileOnMicrodroid(srcPath, destPath));
// Action
// Tampering with the first 2 4K block of the backing file.
sAndroid.run("dd if=/dev/zero of=" + backendPath + " bs=1 count=8192");
// Tampering with the first 2 4K-blocks of the backing file.
zeroizeFileOnAndroid(backendPath, /* size */ 8192, /* offset */ 0);
// Verify
// Write to a block partially requires a read back to calculate the new hash. It should fail
@ -288,6 +288,52 @@ public final class AuthFsHostTest extends VirtualizationTestCaseBase {
destPath, /* offset */ 8192, /* number */ 1024, /* writeThrough */ false));
}
@Test
public void testReadFailedIfDetectsTampering() throws Exception {
// Setup
runFdServerOnAndroid("--open-rw 3:" + TEST_OUTPUT_DIR + "/out.file", "--rw-fds 3");
runAuthFsOnMicrodroid("--remote-new-rw-file 3 --cid " + VMADDR_CID_HOST);
String srcPath = "/system/bin/linker64";
String destPath = MOUNT_DIR + "/3";
String backendPath = TEST_OUTPUT_DIR + "/out.file";
assertTrue(copyFileOnMicrodroid(srcPath, destPath));
// Action
// Tampering with the first 4K-block of the backing file.
zeroizeFileOnAndroid(backendPath, /* size */ 4096, /* offset */ 0);
// Verify
// Force dropping the page cache, so that the next read can be validated.
runOnMicrodroid("echo 1 > /proc/sys/vm/drop_caches");
// A read will fail if the backing data has been tampered.
assertFalse(checkReadAtFileOffsetOnMicrodroid(
destPath, /* offset */ 0, /* number */ 4096));
assertTrue(checkReadAtFileOffsetOnMicrodroid(
destPath, /* offset */ 4096, /* number */ 4096));
}
@Test
public void testResizeFailedIfDetectsTampering() throws Exception {
// Setup
runFdServerOnAndroid("--open-rw 3:" + TEST_OUTPUT_DIR + "/out.file", "--rw-fds 3");
runAuthFsOnMicrodroid("--remote-new-rw-file 3 --cid " + VMADDR_CID_HOST);
String outputPath = MOUNT_DIR + "/3";
String backendPath = TEST_OUTPUT_DIR + "/out.file";
createFileWithOnesOnMicrodroid(outputPath, 8192);
// Action
// Tampering with the last 4K-block of the backing file.
zeroizeFileOnAndroid(backendPath, /* size */ 1, /* offset */ 4096);
// Verify
// A resize (to a non-multiple of 4K) will fail if the last backing chunk has been
// tampered. The original data is necessary (and has to be verified) to calculate the new
// hash with shorter data.
assertFalse(resizeFileOnMicrodroid(outputPath, 8000));
}
@Test
public void testFileResize() throws Exception {
// Setup
@ -304,14 +350,14 @@ public final class AuthFsHostTest extends VirtualizationTestCaseBase {
backendPath,
"684ad25fdc2bbb80cbc910dd1bde6d5499ccf860ca6ee44704b77ec445271353");
resizeFileOnMicrodroid(outputPath, 15000);
assertTrue(resizeFileOnMicrodroid(outputPath, 15000));
assertEquals(getFileSizeInBytesOnMicrodroid(outputPath), 15000);
expectBackingFileConsistency(
outputPath,
backendPath,
"567c89f62586e0d33369157afdfe99a2fa36cdffb01e91dcdc0b7355262d610d");
resizeFileOnMicrodroid(outputPath, 5000);
assertTrue(resizeFileOnMicrodroid(outputPath, 5000));
assertEquals(getFileSizeInBytesOnMicrodroid(outputPath), 5000);
expectBackingFileConsistency(
outputPath,
@ -340,7 +386,7 @@ public final class AuthFsHostTest extends VirtualizationTestCaseBase {
"684ad25fdc2bbb80cbc910dd1bde6d5499ccf860ca6ee44704b77ec445271353");
// Regular file operations work, e.g. resize.
resizeFileOnMicrodroid(authfsPath, 15000);
assertTrue(resizeFileOnMicrodroid(authfsPath, 15000));
assertEquals(getFileSizeInBytesOnMicrodroid(authfsPath), 15000);
expectBackingFileConsistency(
authfsPath,
@ -698,8 +744,9 @@ public final class AuthFsHostTest extends VirtualizationTestCaseBase {
assertEquals("Inconsistent mode for " + androidPath + " (android)", expected, actual);
}
private void resizeFileOnMicrodroid(String path, long size) {
runOnMicrodroid("truncate -c -s " + size + " " + path);
private boolean resizeFileOnMicrodroid(String path, long size) {
CommandResult result = runOnMicrodroidForResult("truncate -c -s " + size + " " + path);
return result.getStatus() == CommandStatus.SUCCESS;
}
private long getFileSizeInBytesOnMicrodroid(String path) {
@ -711,12 +758,22 @@ public final class AuthFsHostTest extends VirtualizationTestCaseBase {
"yes $'\\x01' | tr -d '\\n' | dd bs=1 count=" + numberOfOnes + " of=" + filePath);
}
private boolean writeZerosAtFileOffsetOnMicrodroid(
String filePath, long offset, long numberOfZeros, boolean writeThrough) {
String cmd = "dd if=/dev/zero of=" + filePath + " bs=1 count=" + numberOfZeros;
private boolean checkReadAtFileOffsetOnMicrodroid(String filePath, long offset, long size) {
String cmd = "dd if=" + filePath + " of=/dev/null bs=1 count=" + size;
if (offset > 0) {
cmd += " skip=" + offset;
}
CommandResult result = runOnMicrodroidForResult(cmd);
return result.getStatus() == CommandStatus.SUCCESS;
}
private boolean writeZerosAtFileOffsetOnMicrodroid(
String filePath, long offset, long numberOfZeros, boolean writeThrough) {
String cmd = "dd if=/dev/zero of=" + filePath + " bs=1 count=" + numberOfZeros
+ " conv=notrunc";
if (offset > 0) {
cmd += " seek=" + offset;
}
if (writeThrough) {
cmd += " direct";
}
@ -724,6 +781,12 @@ public final class AuthFsHostTest extends VirtualizationTestCaseBase {
return result.getStatus() == CommandStatus.SUCCESS;
}
private void zeroizeFileOnAndroid(String filePath, long size, long offset)
throws DeviceNotAvailableException {
sAndroid.run("dd if=/dev/zero of=" + filePath + " bs=1 count=" + size + " conv=notrunc"
+ " seek=" + offset);
}
private void runAuthFsOnMicrodroid(String flags) {
String cmd = AUTHFS_BIN + " " + MOUNT_DIR + " " + flags;