Merge changes from topic "encryptedstore_hctr2"
* changes:
dm_crypt: Extend unit tests to cover both ciphers
Change block cipher mode from XTS -> HCTR2
diff --git a/encryptedstore/src/main.rs b/encryptedstore/src/main.rs
index d7d2382..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};
@@ -45,7 +46,7 @@
.args(&[
arg!(--blkdevice <FILE> "the block device backing the encrypted storage")
.required(true),
- arg!(--key <KEY> "key (in hex) equivalent to 64 bytes)").required(true),
+ arg!(--key <KEY> "key (in hex) equivalent to 32 bytes)").required(true),
arg!(--mountpoint <MOUNTPOINT> "mount point for the storage").required(true),
])
.get_matches();
@@ -87,12 +88,11 @@
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() == 64, "We need 64 bytes' key for aes-xts cipher for block encryption");
// Create the dm-crypt spec
let target = dm::crypt::DmCryptTargetBuilder::default()
.data_device(data_device, dev_size)
- .cipher(CipherType::AES256XTS) // TODO(b/259253336) Move to HCTR2 based encryption.
+ .cipher(CipherType::AES256HCTR2)
.key(&key)
.build()
.context("Couldn't build the DMCrypt target")?;
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 9b715a5..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,10 +31,34 @@
// Documentation/admin-guide/device-mapper/dm-crypt.rst
/// Supported ciphers
+#[derive(Clone, Copy, Debug)]
pub enum CipherType {
- // TODO(b/253394457) Include ciphers with authenticated modes as well
+ // AES-256-HCTR2 takes a 32-byte key
+ AES256HCTR2,
+ // XTS requires key of twice the length of the underlying block cipher i.e., 64B for AES256
AES256XTS,
}
+impl CipherType {
+ fn get_kernel_crypto_name(&self) -> &str {
+ match *self {
+ // We use "plain64" as the IV/nonce generation algorithm -
+ // which basically is the sector number.
+ CipherType::AES256HCTR2 => "aes-hctr2-plain64",
+ 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]>);
@@ -59,7 +82,7 @@
impl<'a> Default for DmCryptTargetBuilder<'a> {
fn default() -> Self {
DmCryptTargetBuilder {
- cipher: CipherType::AES256XTS,
+ cipher: CipherType::AES256HCTR2,
key: None,
iv_offset: 0,
device_path: None,
@@ -112,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{...}
@@ -121,7 +149,7 @@
// <offset> [<#opt_params> <opt_params>]
let mut body = String::new();
use std::fmt::Write;
- write!(&mut body, "{} ", get_kernel_crypto_name(&self.cipher))?;
+ write!(&mut body, "{} ", self.cipher.get_kernel_crypto_name())?;
write!(&mut body, "{} ", key)?;
write!(&mut body, "{} ", self.iv_offset)?;
write!(&mut body, "{} ", device_path)?;
@@ -145,9 +173,3 @@
Ok(DmCryptTarget(buf.into_boxed_slice()))
}
}
-
-fn get_kernel_crypto_name(cipher: &CipherType) -> &str {
- match cipher {
- CipherType::AES256XTS => "aes-xts-plain64",
- }
-}
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());
}
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 17532d7..4f94bb4 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -85,7 +85,7 @@
const ENCRYPTEDSTORE_BACKING_DEVICE: &str = "/dev/block/by-name/encryptedstore";
const ENCRYPTEDSTORE_BIN: &str = "/system/bin/encryptedstore";
const ENCRYPTEDSTORE_KEY_IDENTIFIER: &str = "encryptedstore_key";
-const ENCRYPTEDSTORE_KEYSIZE: u32 = 64;
+const ENCRYPTEDSTORE_KEYSIZE: u32 = 32;
const ENCRYPTEDSTORE_MOUNTPOINT: &str = "/mnt/encryptedstore";
#[derive(thiserror::Error, Debug)]