Refactor encryption to use a read lock during operation

Author: jbaublitz

src/dbus/pool/pool_3_9/methods.rs

@@ -20,8 +20,8 @@ use crate::{
         },
     },
     engine::{
-        CreateAction, DeleteAction, Engine, InputEncryptionInfo, KeyDescription, Lockable,
-        PoolIdentifier, PoolUuid,
+        CreateAction, DeleteAction, EncryptedDevice, Engine, InputEncryptionInfo, KeyDescription,
+        Lockable, PoolIdentifier, PoolUuid,
     },
     stratis::StratisError,
 };
@@ -86,12 +86,27 @@ pub async fn encrypt_pool_method(
         .get_mut_pool(PoolIdentifier::Uuid(pool_uuid))
         .await
         .ok_or_else(|| StratisError::Msg(format!("No pool associated with uuid {pool_uuid}")));
+    let cloned_engine = Arc::clone(engine);
     match tokio::task::spawn_blocking(move || {
         let mut guard = guard_res?;
 
-        let (name, _, pool) = guard.as_mut_tuple();
-
-        handle_action!(pool.encrypt_pool(&name, pool_uuid, &iei))
+        handle_action!(guard
+            .start_encrypt_pool(pool_uuid, &iei)
+            .and_then(|action| match action {
+                CreateAction::Identity => Ok(CreateAction::Identity),
+                CreateAction::Created((sector_size, key_info)) => {
+                    let guard = guard.downgrade();
+                    guard
+                        .do_encrypt_pool(pool_uuid, sector_size, key_info)
+                        .map(|_| guard)
+                        .and_then(|guard| {
+                            let mut guard = block_on(cloned_engine.upgrade_pool(guard));
+                            let (name, _, _) = guard.as_mut_tuple();
+                            guard.finish_encrypt_pool(&name, pool_uuid)
+                        })
+                        .map(|_| CreateAction::Created(EncryptedDevice(pool_uuid)))
+                }
+            }))
     })
     .await
     {

src/engine/engine.rs

@@ -402,13 +402,23 @@ pub trait Pool: Debug + Send + Sync {
         limit: Option<Bytes>,
     ) -> StratisResult<PropChangeAction<Option<Sectors>>>;
 
-    /// Encrypted an unencrypted pool.
-    fn encrypt_pool(
+    /// Setup pool encryption operation and check for idempotence.
+    fn start_encrypt_pool(
         &mut self,
-        name: &Name,
         pool_uuid: PoolUuid,
         encryption_info: &InputEncryptionInfo,
-    ) -> StratisResult<CreateAction<EncryptedDevice>>;
+    ) -> StratisResult<CreateAction<(u32, (u32, SizedKeyMemory))>>;
+
+    /// Encrypt an unencrypted pool.
+    fn do_encrypt_pool(
+        &self,
+        pool_uuid: PoolUuid,
+        sector_size: u32,
+        key_info: (u32, SizedKeyMemory),
+    ) -> StratisResult<()>;
+
+    /// Update internal data structures with the result of the encryption operation.
+    fn finish_encrypt_pool(&mut self, name: &Name, pool_uuid: PoolUuid) -> StratisResult<()>;
 
     /// Start reencryption of an encrypted pool.
     ///

src/engine/mod.rs

@@ -20,12 +20,13 @@ pub use self::{
     },
     types::{
         ActionAvailability, BlockDevTier, ClevisInfo, CreateAction, DeleteAction, DevUuid, Diff,
-        EncryptionInfo, EngineAction, Features, FilesystemUuid, GrowAction, InputEncryptionInfo,
-        IntegritySpec, IntegrityTagSpec, KeyDescription, Lockable, LockedPoolInfo, LockedPoolsInfo,
-        MappingCreateAction, MappingDeleteAction, MaybeInconsistent, Name, OptionalTokenSlotInput,
-        PoolDiff, PoolEncryptionInfo, PoolIdentifier, PoolUuid, PropChangeAction, RenameAction,
-        ReportType, SetCreateAction, SetDeleteAction, SetUnlockAction, StartAction, StopAction,
-        StoppedPoolInfo, StoppedPoolsInfo, StratBlockDevDiff, StratFilesystemDiff, StratPoolDiff,
+        EncryptedDevice, EncryptionInfo, EngineAction, Features, FilesystemUuid, GrowAction,
+        InputEncryptionInfo, IntegritySpec, IntegrityTagSpec, KeyDescription, Lockable,
+        LockedPoolInfo, LockedPoolsInfo, MappingCreateAction, MappingDeleteAction,
+        MaybeInconsistent, Name, OptionalTokenSlotInput, PoolDiff, PoolEncryptionInfo,
+        PoolIdentifier, PoolUuid, PropChangeAction, RenameAction, ReportType, SetCreateAction,
+        SetDeleteAction, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo,
+        StoppedPoolsInfo, StratBlockDevDiff, StratFilesystemDiff, StratPoolDiff,
         StratSigblockVersion, StratisUuid, ThinPoolDiff, ToDisplay, TokenUnlockMethod,
         UdevEngineEvent, UnlockMethod, ValidatedIntegritySpec, DEFAULT_INTEGRITY_JOURNAL_SIZE,
         DEFAULT_INTEGRITY_TAG_SPEC,

src/engine/sim_engine/pool.rs

@@ -14,6 +14,7 @@ use itertools::Itertools;
 use serde_json::{Map, Value};
 
 use devicemapper::{Bytes, Sectors, IEC};
+use libcryptsetup_rs::SafeMemHandle;
 
 use crate::{
     engine::{
@@ -966,20 +967,30 @@ impl Pool for SimPool {
         }
     }
 
-    fn encrypt_pool(
+    fn start_encrypt_pool(
         &mut self,
-        _: &Name,
-        pool_uuid: PoolUuid,
+        _: PoolUuid,
         enc: &InputEncryptionInfo,
-    ) -> StratisResult<CreateAction<EncryptedDevice>> {
+    ) -> StratisResult<CreateAction<(u32, (u32, SizedKeyMemory))>> {
         if self.encryption_info.is_some() {
             Ok(CreateAction::Identity)
         } else {
             self.encryption_info = convert_encryption_info(Some(enc), None)?;
-            Ok(CreateAction::Created(EncryptedDevice(pool_uuid)))
+            Ok(CreateAction::Created((
+                0,
+                (0, SizedKeyMemory::new(SafeMemHandle::alloc(1)?, 0)),
+            )))
         }
     }
 
+    fn do_encrypt_pool(&self, _: PoolUuid, _: u32, _: (u32, SizedKeyMemory)) -> StratisResult<()> {
+        Ok(())
+    }
+
+    fn finish_encrypt_pool(&mut self, _: &Name, _: PoolUuid) -> StratisResult<()> {
+        Ok(())
+    }
+
     fn start_reencrypt_pool(&mut self) -> StratisResult<Vec<(u32, SizedKeyMemory, u32)>> {
         if self.encryption_info.is_none() {
             Err(StratisError::Msg(

src/engine/strat_engine/backstore/backstore/v2.rs

@@ -1291,17 +1291,17 @@ impl Backstore {
         self.data_tier.grow(dev)
     }
 
-    pub fn encrypt(
+    pub fn prepare_encrypt(
         &mut self,
         pool_uuid: PoolUuid,
         thinpool: &mut ThinPool<Self>,
         offset: Sectors,
         offset_direction: OffsetDirection,
         encryption_info: &InputEncryptionInfo,
-    ) -> StratisResult<()> {
+    ) -> StratisResult<(u32, (u32, SizedKeyMemory))> {
         let (dm_name, _) = format_backstore_ids(pool_uuid, CacheRole::Cache);
         let unencrypted_path = PathBuf::from(DEVICEMAPPER_PATH).join(dm_name.to_string());
-        let (mut device, sector_size, key_info) =
+        let (sector_size, key_info) =
             CryptHandle::setup_encrypt(pool_uuid, thinpool, &unencrypted_path, encryption_info)?;
 
         thinpool.suspend()?;
@@ -1314,14 +1314,23 @@ impl Backstore {
         self.shift_alloc_offset(offset, offset_direction);
         set_device_res?;
 
-        let handle = CryptHandle::do_encrypt(
-            &mut device,
-            &unencrypted_path,
-            pool_uuid,
-            sector_size,
-            key_info,
-        )?;
+        Ok((sector_size, key_info))
+    }
+
+    pub fn do_encrypt(
+        pool_uuid: PoolUuid,
+        sector_size: u32,
+        key_info: (u32, SizedKeyMemory),
+    ) -> StratisResult<()> {
+        let (dm_name, _) = format_backstore_ids(pool_uuid, CacheRole::Cache);
+        let unencrypted_path = PathBuf::from(DEVICEMAPPER_PATH).join(dm_name.to_string());
+        CryptHandle::do_encrypt(&unencrypted_path, pool_uuid, sector_size, key_info)
+    }
 
+    pub fn finish_encrypt(&mut self, pool_uuid: PoolUuid) -> StratisResult<()> {
+        let (dm_name, _) = format_backstore_ids(pool_uuid, CacheRole::Cache);
+        let unencrypted_path = PathBuf::from(DEVICEMAPPER_PATH).join(dm_name.to_string());
+        let handle = CryptHandle::finish_encrypt(&unencrypted_path, pool_uuid)?;
         self.cap_device.enc = Some(Either::Right(handle));
         Ok(())
     }

src/engine/strat_engine/crypt/handle/v2.rs

@@ -726,7 +726,7 @@ impl CryptHandle {
         thinpool: &ThinPool<v2::Backstore>,
         unencrypted_path: &Path,
         encryption_info: &InputEncryptionInfo,
-    ) -> StratisResult<(CryptDevice, u32, (u32, SizedKeyMemory))> {
+    ) -> StratisResult<(u32, (u32, SizedKeyMemory))> {
         let mut tmp_file = tempfile::NamedTempFile::new()?;
         tmp_file
             .as_file_mut()
@@ -804,7 +804,7 @@ impl CryptHandle {
             CryptActivate::SHARED,
         )?;
 
-        Ok((device, sector_size, (keyslot, key)))
+        Ok((sector_size, (keyslot, key)))
     }
 
     /// Perform the online encryption operation that was set up.
@@ -813,12 +813,12 @@ impl CryptHandle {
     /// Precondition: crypt device was already added as the backing device for the thin pool
     ///               failure to do so will result in corruption
     pub fn do_encrypt(
-        device: &mut CryptDevice,
         unencrypted_path: &Path,
         pool_uuid: PoolUuid,
         sector_size: u32,
         key_info: (u32, SizedKeyMemory),
-    ) -> StratisResult<Self> {
+    ) -> StratisResult<()> {
+        let mut device = acquire_crypt_device(unencrypted_path)?;
         let activation_name = &format_crypt_backstore_name(&pool_uuid).to_string();
         let (keyslot, key) = key_info;
         device.reencrypt_handle().reencrypt_init_by_passphrase(
@@ -850,7 +850,16 @@ impl CryptHandle {
         )?;
         info!("Starting encryption operation on pool with UUID {pool_uuid}; may take a while");
         device.reencrypt_handle().reencrypt2::<()>(None, None)?;
+        Ok(())
+    }
 
+    /// Generate the CryptHandle from the LUKS2 device path.
+    ///
+    /// Precondition: LUKS2 device path was fully encrypted successfully
+    pub fn finish_encrypt(
+        unencrypted_path: &Path,
+        pool_uuid: PoolUuid,
+    ) -> StratisResult<CryptHandle> {
         CryptHandle::setup(unencrypted_path, pool_uuid, TokenUnlockMethod::Any, None)
             .map(|h| h.expect("should have crypt device after online encrypt"))
     }

src/engine/strat_engine/pool/dispatch.rs

@@ -351,15 +351,33 @@ impl Pool for AnyPool {
         }
     }
 
-    fn encrypt_pool(
+    fn start_encrypt_pool(
         &mut self,
-        name: &Name,
         pool_uuid: PoolUuid,
         encryption_info: &InputEncryptionInfo,
-    ) -> StratisResult<CreateAction<EncryptedDevice>> {
+    ) -> StratisResult<CreateAction<(u32, (u32, SizedKeyMemory))>> {
+        match self {
+            AnyPool::V1(p) => p.start_encrypt_pool(pool_uuid, encryption_info),
+            AnyPool::V2(p) => p.start_encrypt_pool(pool_uuid, encryption_info),
+        }
+    }
+
+    fn do_encrypt_pool(
+        &self,
+        pool_uuid: PoolUuid,
+        sector_size: u32,
+        key_info: (u32, SizedKeyMemory),
+    ) -> StratisResult<()> {
+        match self {
+            AnyPool::V1(p) => p.do_encrypt_pool(pool_uuid, sector_size, key_info),
+            AnyPool::V2(p) => p.do_encrypt_pool(pool_uuid, sector_size, key_info),
+        }
+    }
+
+    fn finish_encrypt_pool(&mut self, name: &Name, pool_uuid: PoolUuid) -> StratisResult<()> {
         match self {
-            AnyPool::V1(p) => p.encrypt_pool(name, pool_uuid, encryption_info),
-            AnyPool::V2(p) => p.encrypt_pool(name, pool_uuid, encryption_info),
+            AnyPool::V1(p) => p.finish_encrypt_pool(name, pool_uuid),
+            AnyPool::V2(p) => p.finish_encrypt_pool(name, pool_uuid),
         }
     }
 

src/engine/strat_engine/pool/v1.rs

@@ -1340,12 +1340,26 @@ impl Pool for StratPool {
         }
     }
 
-    fn encrypt_pool(
+    #[pool_mutating_action("NoRequests")]
+    fn start_encrypt_pool(
         &mut self,
-        _: &Name,
         _: PoolUuid,
         _: &InputEncryptionInfo,
-    ) -> StratisResult<CreateAction<EncryptedDevice>> {
+    ) -> StratisResult<CreateAction<(u32, (u32, SizedKeyMemory))>> {
+        Err(StratisError::Msg(
+            "Encrypting an unencrypted device is only supported in V2 of the metadata".to_string(),
+        ))
+    }
+
+    #[pool_mutating_action("NoRequests")]
+    fn do_encrypt_pool(&self, _: PoolUuid, _: u32, _: (u32, SizedKeyMemory)) -> StratisResult<()> {
+        Err(StratisError::Msg(
+            "Encrypting an unencrypted device is only supported in V2 of the metadata".to_string(),
+        ))
+    }
+
+    #[pool_mutating_action("NoRequests")]
+    fn finish_encrypt_pool(&mut self, _: &Name, _: PoolUuid) -> StratisResult<()> {
         Err(StratisError::Msg(
             "Encrypting an unencrypted device is only supported in V2 of the metadata".to_string(),
         ))

src/engine/strat_engine/pool/v2.rs

@@ -1211,30 +1211,46 @@ impl Pool for StratPool {
     }
 
     #[pool_mutating_action("NoRequests")]
-    fn encrypt_pool(
+    fn start_encrypt_pool(
         &mut self,
-        name: &Name,
         pool_uuid: PoolUuid,
         encryption_info: &InputEncryptionInfo,
-    ) -> StratisResult<CreateAction<EncryptedDevice>> {
-        let offset = DEFAULT_CRYPT_DATA_OFFSET_V2;
-        let direction = OffsetDirection::Backwards;
+    ) -> StratisResult<CreateAction<(u32, (u32, SizedKeyMemory))>> {
         match self.backstore.encryption_info() {
             Some(_) => Ok(CreateAction::Identity),
             None => {
-                self.backstore.encrypt(
-                    pool_uuid,
-                    &mut self.thin_pool,
-                    offset,
-                    direction,
-                    encryption_info,
-                )?;
-                self.write_metadata(name)?;
-                Ok(CreateAction::Created(EncryptedDevice(pool_uuid)))
+                let offset = DEFAULT_CRYPT_DATA_OFFSET_V2;
+                let direction = OffsetDirection::Backwards;
+                self.backstore
+                    .prepare_encrypt(
+                        pool_uuid,
+                        &mut self.thin_pool,
+                        offset,
+                        direction,
+                        encryption_info,
+                    )
+                    .map(CreateAction::Created)
             }
         }
     }
 
+    #[pool_mutating_action("NoRequests")]
+    fn do_encrypt_pool(
+        &self,
+        pool_uuid: PoolUuid,
+        sector_size: u32,
+        key_info: (u32, SizedKeyMemory),
+    ) -> StratisResult<()> {
+        Backstore::do_encrypt(pool_uuid, sector_size, key_info)
+    }
+
+    #[pool_mutating_action("NoRequests")]
+    fn finish_encrypt_pool(&mut self, name: &Name, pool_uuid: PoolUuid) -> StratisResult<()> {
+        self.backstore.finish_encrypt(pool_uuid)?;
+        self.write_metadata(name)?;
+        Ok(())
+    }
+
     #[pool_mutating_action("NoRequests")]
     fn start_reencrypt_pool(&mut self) -> StratisResult<Vec<(u32, SizedKeyMemory, u32)>> {
         self.backstore.prepare_reencrypt()
@@ -2195,10 +2211,8 @@ mod tests {
             {
                 let mut handle =
                     test_async!(engine.get_mut_pool(PoolIdentifier::Uuid(pool_uuid))).unwrap();
-                let (name, _, pool) = handle.as_mut_tuple();
-                assert!(!pool.is_encrypted());
-                pool.encrypt_pool(
-                    &name,
+                assert!(!handle.is_encrypted());
+                let (sector_size, key_info) = handle.start_encrypt_pool(
                     pool_uuid,
                     &InputEncryptionInfo::new(
                         vec![(None, key_desc.to_owned())],
@@ -2216,8 +2230,17 @@ mod tests {
                     .unwrap()
                     .unwrap(),
                 )
+                .unwrap()
+                .changed()
                 .unwrap();
-                assert!(pool.is_encrypted());
+                let handle = handle.downgrade();
+                handle
+                    .do_encrypt_pool(pool_uuid, sector_size, key_info)
+                    .unwrap();
+                let mut handle = test_async!(engine.upgrade_pool(handle.into_dyn()));
+                let (name, _, _) = handle.as_mut_tuple();
+                handle.finish_encrypt_pool(&name, pool_uuid).unwrap();
+                assert!(handle.is_encrypted());
             }
 
             test_async!(engine.stop_pool(PoolIdentifier::Uuid(pool_uuid), true)).unwrap();