Re: [PATCH proxmox-backup 4/6] sync: wire up strict encryption mode

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On May 5, 2026 10:12 am, Shannon Sterz wrote:
> On Wed Apr 29, 2026 at 4:09 PM CEST, Fabian Grünbichler wrote:
>> enabling it requires configured encryption/decryption keys, disabling or
>> unsetting it does not.
>>
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>>  src/api2/config/sync.rs | 30 +++++++++++++++++++++++++++++-
>>  src/api2/pull.rs        | 13 +++++++++----
>>  src/api2/push.rs        | 10 ++++++++--
>>  src/server/sync.rs      |  2 +-
>>  4 files changed, 47 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
>> index eb82745d9..ac1e6350e 100644
>> --- a/src/api2/config/sync.rs
>> +++ b/src/api2/config/sync.rs
>> @@ -276,9 +276,16 @@ pub fn create_sync_job(
>>          .unwrap_or_else(|| Authid::root_auth_id());
>>
>>      if sync_direction == SyncDirection::Push {
>> +        if (config.strict_encryption_mode == Some(true)) != config.active_encryption_key.is_some() {
>> +            bail!("'strict-encryption-mode' requires encryption key");
>> +        }
> 
> is this also supposed to trigger if strict-encryption-mode is false or
> not specified, but an active encryption key is set? if i'm not
> misreading this, that's what would happen here.

right, this is wrong..

strict | key | result
---------------------
Y      | Y   | true  != true  => false => OK
N      | Y   | false != true  => true  => NOT OK
Y      | N   | true  != false => true  => OK (we want to warn)
N      | N   | false != false => false => OK

> 
> if no maybe:
> 
>     (data.strict_encryption_mode == Some(true)) && !data.active_encryption_key.is_some()
> 
> would be slightly more legible?

yes, or maybe

unwrap_or(false) && !..

since it's just a bool we don't have to worry about being smart here..

> 
>>          sync_user_can_access_optional_key(config.active_encryption_key.as_deref(), owner, true)?;
>>      } else {
>> -        for key in config.associated_key.as_deref().unwrap_or(&[]) {
>> +        let keys = config.associated_key.as_deref().unwrap_or(&[]);
>> +        if (config.strict_encryption_mode == Some(true)) == keys.is_empty() {
>> +            bail!("'strict-encryption-mode' requires encryption key(s)");
> 
> same here, is this supposed to fail when strict-mode is false and no
> keys are specified?

same as above :)

> 
>> +        }
>> +        for key in keys {
>>              sync_user_can_access_optional_key(Some(key), owner, false)?;
>>          }
>>      }
>> @@ -398,6 +405,8 @@ pub enum DeletableProperty {
>>      ActiveEncryptionKey,
>>      /// Delete associated key property,
>>      AssociatedKey,
>> +    /// Delete strict encryption_mode property,
>> +    StrictEncryptionMode,
>>  }
>>
>>  #[api(
>> @@ -538,6 +547,9 @@ pub fn update_sync_job(
>>                      // Previous active encryption key might be added as associated below.
>>                      data.associated_key = None;
>>                  }
>> +                DeletableProperty::StrictEncryptionMode => {
>> +                    data.strict_encryption_mode = None;
>> +                }
>>              }
>>          }
>>          keep_previous_key_as_associated(
>> @@ -654,6 +666,21 @@ pub fn update_sync_job(
>>          data.associated_key = Some(associated_key);
>>      }
>>
>> +    if let Some(strict_encryption_mode) = update.strict_encryption_mode {
>> +        data.strict_encryption_mode = Some(strict_encryption_mode);
>> +    }
>> +
>> +    if data.sync_direction == Some(SyncDirection::Push) {
>> +        if (data.strict_encryption_mode == Some(true)) != data.active_encryption_key.is_some() {
>> +            bail!("'strict-encryption-mode' requires encryption key");
> 
> see above
> 
>> +        }
>> +    } else {
>> +        let keys = data.associated_key.as_deref().unwrap_or(&[]);
>> +        if (data.strict_encryption_mode == Some(true)) == keys.is_empty() {
>> +            bail!("'strict-encryption-mode' requires encryption key(s)");
> 
> see above

I'll adapt (and maybe pull it into a helper) for v2, thanks for catching
it!