dm_crypt: Extend unit tests to cover both ciphers
The current tests (that maps crypt device & checks if data is accessible
or not) only covered for the default cipher. Extend the tests for both
the ciphers (and correct key size).
This patch also add logic to validate_key_size depending on chosen
cipher in dm_rust lib.
Test: atest libdm_rust.test
Bug: 259253336
Change-Id: Ie32c0882db364ac296f0ed55991ce079bee0fbb8
diff --git a/encryptedstore/src/main.rs b/encryptedstore/src/main.rs
index 5f27fd7..9c8311d 100644
--- a/encryptedstore/src/main.rs
+++ b/encryptedstore/src/main.rs
@@ -20,7 +20,8 @@
use anyhow::{ensure, Context, Result};
use clap::{arg, App};
-use dm::{crypt::CipherType, util};
+use dm::crypt::CipherType;
+use dm::util;
use log::info;
use std::ffi::CString;
use std::fs::{create_dir_all, OpenOptions};
@@ -87,7 +88,6 @@
fn enable_crypt(data_device: &Path, key: &str, name: &str) -> Result<PathBuf> {
let dev_size = util::blkgetsize64(data_device)?;
let key = hex::decode(key).context("Unable to decode hex key")?;
- ensure!(key.len() == 32, "We need 32 bytes' key for aes-hctr2 cipher for block encryption");
// Create the dm-crypt spec
let target = dm::crypt::DmCryptTargetBuilder::default()
diff --git a/libs/devicemapper/Android.bp b/libs/devicemapper/Android.bp
index 783fa79..9fa010c 100644
--- a/libs/devicemapper/Android.bp
+++ b/libs/devicemapper/Android.bp
@@ -13,6 +13,7 @@
"libbitflags",
"liblibc",
"libdata_model",
+ "libhex",
"libnix",
"libuuid",
],
@@ -33,6 +34,7 @@
defaults: ["libdm_rust.defaults"],
test_suites: ["general-tests"],
rustlibs: [
+ "librustutils",
"libscopeguard",
"libtempfile",
],
diff --git a/libs/devicemapper/src/crypt.rs b/libs/devicemapper/src/crypt.rs
index 75105ce..b2e677a 100644
--- a/libs/devicemapper/src/crypt.rs
+++ b/libs/devicemapper/src/crypt.rs
@@ -17,10 +17,9 @@
/// `crypt` module implements the "crypt" target in the device mapper framework. Specifically,
/// it provides `DmCryptTargetBuilder` struct which is used to construct a `DmCryptTarget` struct
/// which is then given to `DeviceMapper` to create a mapper device.
-use crate::util::*;
use crate::DmTargetSpec;
-use anyhow::{bail, Context, Result};
+use anyhow::{ensure, Context, Result};
use data_model::DataInit;
use std::io::Write;
use std::mem::size_of;
@@ -32,6 +31,7 @@
// Documentation/admin-guide/device-mapper/dm-crypt.rst
/// Supported ciphers
+#[derive(Clone, Copy, Debug)]
pub enum CipherType {
// AES-256-HCTR2 takes a 32-byte key
AES256HCTR2,
@@ -47,6 +47,17 @@
CipherType::AES256XTS => "aes-xts-plain64",
}
}
+
+ fn get_required_key_size(&self) -> usize {
+ match *self {
+ CipherType::AES256HCTR2 => 32,
+ CipherType::AES256XTS => 64,
+ }
+ }
+
+ fn validata_key_size(&self, key_size: usize) -> bool {
+ key_size == self.get_required_key_size()
+ }
}
pub struct DmCryptTarget(Box<[u8]>);
@@ -124,8 +135,13 @@
.to_str()
.context("data device path is not encoded in utf8")?;
- let key =
- if let Some(key) = self.key { hexstring_from(key) } else { bail!("key is not set") };
+ ensure!(self.key.is_some(), "key is not set");
+ // Unwrap is safe because we already made sure key.is_some()
+ ensure!(
+ self.cipher.validata_key_size(self.key.unwrap().len()),
+ format!("Invalid key size for cipher:{}", self.cipher.get_kernel_crypto_name())
+ );
+ let key = hex::encode(self.key.unwrap());
// Step2: serialize the information according to the spec, which is ...
// DmTargetSpec{...}
diff --git a/libs/devicemapper/src/lib.rs b/libs/devicemapper/src/lib.rs
index ebe71e4..9069eb2 100644
--- a/libs/devicemapper/src/lib.rs
+++ b/libs/devicemapper/src/lib.rs
@@ -234,12 +234,28 @@
#[cfg(test)]
mod tests {
use super::*;
- use crypt::DmCryptTargetBuilder;
+ use crypt::{CipherType, DmCryptTargetBuilder};
+ use rustutils::system_properties;
use std::fs::{read, File, OpenOptions};
use std::io::Write;
- const KEY: &[u8; 32] = b"thirtytwobyteslongreallylongword";
- const DIFFERENT_KEY: &[u8; 32] = b"drowgnolyllaergnolsetybowtytriht";
+ // Just a logical set of keys to make testing easy. This has no real meaning.
+ struct KeySet<'a> {
+ cipher: CipherType,
+ key: &'a [u8],
+ different_key: &'a [u8],
+ }
+
+ const KEY_SET_XTS: KeySet = KeySet {
+ cipher: CipherType::AES256XTS,
+ key: b"sixtyfourbyteslongsentencearerarebutletsgiveitatrycantbethathard",
+ different_key: b"drahtahtebtnacyrtatievigsteltuberareraecnetnesgnolsetybruofytxis",
+ };
+ const KEY_SET_HCTR2: KeySet = KeySet {
+ cipher: CipherType::AES256HCTR2,
+ key: b"thirtytwobyteslongreallylongword",
+ different_key: b"drowgnolyllaergnolsetybowtytriht",
+ };
// Create a file in given temp directory with given size
fn prepare_tmpfile(test_dir: &Path, filename: &str, sz: u64) -> PathBuf {
@@ -254,14 +270,54 @@
f.write_all(data).unwrap();
}
+ // TODO(b/250880499): delete_device() doesn't really delete it even without DM_DEFERRED_REMOVE.
+ // Hence, we have to create a new device with a different name for each test. Retrying
+ // the test on same machine without reboot will also fail.
fn delete_device(dm: &DeviceMapper, name: &str) -> Result<()> {
dm.delete_device_deferred(name)?;
wait_for_path_disappears(Path::new(MAPPER_DEV_ROOT).join(name))?;
Ok(())
}
+ // TODO(b/260692911): Find a better way to skip a test instead of silently passing it.
+ fn is_hctr2_supported() -> bool {
+ // hctr2 is NOT enabled in kernel 5.10 or lower. We run Microdroid tests on kernel versions
+ // 5.10 or above & therefore, we don't really care to skip test on other versions.
+ if let Some(version) = system_properties::read("ro.kernel.version")
+ .expect("Unable to read system property ro.kernel.version")
+ {
+ version != "5.10"
+ } else {
+ panic!("Could not read property: kernel.version!!");
+ }
+ }
+
#[test]
- fn mapping_again_keeps_data() {
+ fn mapping_again_keeps_data_xts() {
+ mapping_again_keeps_data(&KEY_SET_XTS, "name1");
+ }
+
+ #[test]
+ fn mapping_again_keeps_data_hctr2() {
+ if !is_hctr2_supported() {
+ return;
+ }
+ mapping_again_keeps_data(&KEY_SET_HCTR2, "name2");
+ }
+ #[test]
+ fn data_inaccessible_with_diff_key_xts() {
+ data_inaccessible_with_diff_key(&KEY_SET_XTS, "name3");
+ }
+
+ #[test]
+ fn data_inaccessible_with_diff_key_hctr2() {
+ if !is_hctr2_supported() {
+ return;
+ }
+ data_inaccessible_with_diff_key(&KEY_SET_HCTR2, "name4");
+ }
+
+ fn mapping_again_keeps_data(keyset: &KeySet, device: &str) {
// This test creates 2 different crypt devices using same key backed by same data_device
// -> Write data on dev1 -> Check the data is visible & same on dev2
let dm = DeviceMapper::new().unwrap();
@@ -278,28 +334,33 @@
/*writable*/ true,
)
.unwrap();
+ let device_diff = device.to_owned() + "_diff";
+
scopeguard::defer! {
loopdevice::detach(&data_device).unwrap();
- _ = delete_device(&dm, "crypt1");
- _ = delete_device(&dm, "crypt2");
+ _ = delete_device(&dm, device);
+ _ = delete_device(&dm, &device_diff);
}
- let target =
- DmCryptTargetBuilder::default().data_device(&data_device, sz).key(KEY).build().unwrap();
+ let target = DmCryptTargetBuilder::default()
+ .data_device(&data_device, sz)
+ .cipher(keyset.cipher)
+ .key(keyset.key)
+ .build()
+ .unwrap();
- let mut crypt_device = dm.create_crypt_device("crypt1", &target).unwrap();
+ let mut crypt_device = dm.create_crypt_device(device, &target).unwrap();
write_to_dev(&crypt_device, inputimg);
// Recreate another device using same target spec & check if the content is the same
- crypt_device = dm.create_crypt_device("crypt2", &target).unwrap();
+ crypt_device = dm.create_crypt_device(&device_diff, &target).unwrap();
let crypt = read(crypt_device).unwrap();
assert_eq!(inputimg.len(), crypt.len()); // fail early if the size doesn't match
assert_eq!(inputimg, crypt.as_slice());
}
- #[test]
- fn data_inaccessible_with_diff_key() {
+ fn data_inaccessible_with_diff_key(keyset: &KeySet, device: &str) {
// This test creates 2 different crypt devices using different keys backed
// by same data_device -> Write data on dev1 -> Check the data is visible but not the same on dev2
let dm = DeviceMapper::new().unwrap();
@@ -316,26 +377,32 @@
/*writable*/ true,
)
.unwrap();
+ let device_diff = device.to_owned() + "_diff";
scopeguard::defer! {
loopdevice::detach(&data_device).unwrap();
- _ = delete_device(&dm, "crypt3");
- _ = delete_device(&dm, "crypt4");
+ _ = delete_device(&dm, device);
+ _ = delete_device(&dm, &device_diff);
}
- let target =
- DmCryptTargetBuilder::default().data_device(&data_device, sz).key(KEY).build().unwrap();
+ let target = DmCryptTargetBuilder::default()
+ .data_device(&data_device, sz)
+ .cipher(keyset.cipher)
+ .key(keyset.key)
+ .build()
+ .unwrap();
let target2 = DmCryptTargetBuilder::default()
.data_device(&data_device, sz)
- .key(DIFFERENT_KEY)
+ .cipher(keyset.cipher)
+ .key(keyset.different_key)
.build()
.unwrap();
- let mut crypt_device = dm.create_crypt_device("crypt3", &target).unwrap();
+ let mut crypt_device = dm.create_crypt_device(device, &target).unwrap();
write_to_dev(&crypt_device, inputimg);
// Recreate the crypt device again diff key & check if the content is changed
- crypt_device = dm.create_crypt_device("crypt4", &target2).unwrap();
+ crypt_device = dm.create_crypt_device(&device_diff, &target2).unwrap();
let crypt = read(crypt_device).unwrap();
assert_ne!(inputimg, crypt.as_slice());
}