From 8cfa6d7b5511fa9d5f735071b29decab5428236c Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 24 Mar 2025 20:38:32 -0400 Subject: [PATCH 01/12] Update introspection data Signed-off-by: mulhern --- src/stratis_cli/_actions/_introspect.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/stratis_cli/_actions/_introspect.py b/src/stratis_cli/_actions/_introspect.py index 90c389d48..4088c2b39 100644 --- a/src/stratis_cli/_actions/_introspect.py +++ b/src/stratis_cli/_actions/_introspect.py @@ -185,12 +185,24 @@ + + + + + + + + + + + + @@ -229,6 +241,11 @@ + + + + + @@ -257,13 +274,12 @@ - - - + + From 7a6b5d3d55ec6720a439ffb96fe65f24a4aa7ba2 Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 2 Apr 2025 21:29:58 -0400 Subject: [PATCH 02/12] Set up clevis + keyring options for sharing Signed-off-by: mulhern --- src/stratis_cli/_parser/_pool.py | 31 ++---------------------------- src/stratis_cli/_parser/_shared.py | 24 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/stratis_cli/_parser/_pool.py b/src/stratis_cli/_parser/_pool.py index a3026a84f..485ef8afc 100644 --- a/src/stratis_cli/_parser/_pool.py +++ b/src/stratis_cli/_parser/_pool.py @@ -26,7 +26,6 @@ from .._actions import BindActions, PoolActions from .._alerts import PoolAlert from .._constants import ( - Clevis, EncryptionMethod, IntegrityOption, IntegrityTagSpec, @@ -36,6 +35,7 @@ from ._debug import POOL_DEBUG_SUBCMDS from ._encryption import BIND_SUBCMDS, ENCRYPTION_SUBCMDS, REBIND_SUBCMDS from ._shared import ( + CLEVIS_AND_KERNEL, KEYFILE_PATH_OR_STDIN, TRUST_URL_OR_THUMBPRINT, UUID_OR_NAME, @@ -114,34 +114,7 @@ def verify(self, namespace: Namespace, parser: ArgumentParser): "Encryption", { "description": "Arguments controlling creation with encryption", - "args": [ - ( - "--key-desc", - { - "help": ( - "Key description of key in kernel keyring to use " - "for encryption" - ), - }, - ), - ( - "--clevis", - { - "type": Clevis, - "help": "Specification for binding with Clevis.", - "choices": list(Clevis), - }, - ), - ( - "--tang-url", - { - "help": ( - "URL of Clevis tang server " - "(--clevis=[tang|nbde] must be set)" - ), - }, - ), - ], + "args": CLEVIS_AND_KERNEL, }, ), ( diff --git a/src/stratis_cli/_parser/_shared.py b/src/stratis_cli/_parser/_shared.py index 91fddd617..b91fe0d38 100644 --- a/src/stratis_cli/_parser/_shared.py +++ b/src/stratis_cli/_parser/_shared.py @@ -273,3 +273,27 @@ def verify(self, namespace, parser): else ClevisInfo(CLEVIS_PIN_TPM2, {}) ) ) + + +CLEVIS_AND_KERNEL = [ + ( + "--key-desc", + { + "help": ("Key description of key in kernel keyring"), + }, + ), + ( + "--clevis", + { + "type": Clevis, + "help": "Specification for binding with Clevis.", + "choices": list(Clevis), + }, + ), + ( + "--tang-url", + { + "help": "URL of Clevis tang server (--clevis=[tang|nbde] must be set)", + }, + ), +] From 12d54eca6c32d2c520629f5cb4672db656ef12ff Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 24 Mar 2025 21:05:36 -0400 Subject: [PATCH 03/12] Add commands for encrypt, re-encrypt, and unencrypt Signed-off-by: mulhern --- src/stratis_cli/_actions/__init__.py | 1 + src/stratis_cli/_actions/_crypt.py | 165 +++++++++++++++++ src/stratis_cli/_parser/_encryption.py | 82 ++++++++- tests/integration/pool/test_encryption.py | 212 ++++++++++++++++++++++ 4 files changed, 459 insertions(+), 1 deletion(-) create mode 100644 src/stratis_cli/_actions/_crypt.py diff --git a/src/stratis_cli/_actions/__init__.py b/src/stratis_cli/_actions/__init__.py index b95c02e1c..f16cf1953 100644 --- a/src/stratis_cli/_actions/__init__.py +++ b/src/stratis_cli/_actions/__init__.py @@ -22,6 +22,7 @@ MANAGER_0_INTERFACE, POOL_INTERFACE, ) +from ._crypt import CryptActions from ._debug import ( BlockdevDebugActions, FilesystemDebugActions, diff --git a/src/stratis_cli/_actions/_crypt.py b/src/stratis_cli/_actions/_crypt.py new file mode 100644 index 000000000..cb3f524fd --- /dev/null +++ b/src/stratis_cli/_actions/_crypt.py @@ -0,0 +1,165 @@ +# Copyright 2025 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Miscellaneous whole pool encryption actions. +""" + +# isort: STDLIB +import json +from argparse import Namespace + +from .._constants import PoolId +from .._errors import ( + StratisCliEngineError, + StratisCliIncoherenceError, + StratisCliNoChangeError, +) +from .._stratisd_constants import StratisdErrors +from ._connection import get_object +from ._constants import TOP_OBJECT + + +class CryptActions: + """ + Whole pool encryption actions. + """ + + @staticmethod + def encrypt(namespace: Namespace): + """ + Encrypt a previously unencrypted pool. + """ + + # pylint: disable=import-outside-toplevel + from ._data import MOPool, ObjectManager, Pool, pools + + pool_id = PoolId.from_parser_namespace(namespace) + assert pool_id is not None + + proxy = get_object(TOP_OBJECT) + + managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {}) + + (pool_object_path, mopool) = next( + pools(props=pool_id.managed_objects_key()) + .require_unique_match(True) + .search(managed_objects) + ) + + if bool(MOPool(mopool).Encrypted()): + raise StratisCliNoChangeError("encryption on", pool_id) + + (changed, return_code, message) = Pool.Methods.EncryptPool( + get_object(pool_object_path), + { + "key_descs": ( + [] + if namespace.key_desc is None + else [((False, 0), namespace.key_desc)] + ), + "clevis_infos": ( + [] + if namespace.clevis is None + else [ + ( + (False, 0), + namespace.clevis.pin, + json.dumps(namespace.clevis.config), + ) + ] + ), + }, + ) + + if return_code != StratisdErrors.OK: + raise StratisCliEngineError(return_code, message) + + if not changed: # pragma: no cover + raise StratisCliIncoherenceError( + f"Expected to change {pool_id} encryption status to " + "encrypted, but stratisd reports that it did not change " + "the encryption status" + ) + + @staticmethod + def unencrypt(namespace: Namespace): + """ + Unencrypt a previously encrypted pool. + """ + # pylint: disable=import-outside-toplevel + from ._data import MOPool, ObjectManager, Pool, pools + + pool_id = PoolId.from_parser_namespace(namespace) + assert pool_id is not None + + proxy = get_object(TOP_OBJECT) + + managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {}) + + (pool_object_path, mopool) = next( + pools(props=pool_id.managed_objects_key()) + .require_unique_match(True) + .search(managed_objects) + ) + + if not bool(MOPool(mopool).Encrypted()): + raise StratisCliNoChangeError("encryption off", pool_id) + + (changed, return_code, message) = Pool.Methods.DecryptPool( + get_object(pool_object_path), {} + ) + + if return_code != StratisdErrors.OK: # pragma: no cover + raise StratisCliEngineError(return_code, message) + + if not changed: # pragma: no cover + raise StratisCliIncoherenceError( + f"Expected to change {pool_id} encryption status to " + "unencrypted, but stratisd reports that it did not change " + "the pool's encryption status" + ) + + @staticmethod + def reencrypt(namespace: Namespace): + """ + Reencrypt an already encrypted pool with a new key. + """ + # pylint: disable=import-outside-toplevel + from ._data import ObjectManager, Pool, pools + + pool_id = PoolId.from_parser_namespace(namespace) + assert pool_id is not None + + proxy = get_object(TOP_OBJECT) + + managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {}) + + (pool_object_path, _) = next( + pools(props=pool_id.managed_objects_key()) + .require_unique_match(True) + .search(managed_objects) + ) + + (changed, return_code, message) = Pool.Methods.ReencryptPool( + get_object(pool_object_path), {} + ) + + if return_code != StratisdErrors.OK: + raise StratisCliEngineError(return_code, message) + + if not changed: # pragma: no cover + raise StratisCliIncoherenceError( + f"Expected to reencrypt {pool_id} with a new key but stratisd " + "reports that it did not perform the operation" + ) diff --git a/src/stratis_cli/_parser/_encryption.py b/src/stratis_cli/_parser/_encryption.py index a3a76d3b3..1ca065323 100644 --- a/src/stratis_cli/_parser/_encryption.py +++ b/src/stratis_cli/_parser/_encryption.py @@ -19,9 +19,10 @@ import copy from argparse import SUPPRESS, Namespace -from .._actions import BindActions, RebindActions +from .._actions import BindActions, CryptActions, RebindActions from .._constants import Clevis, EncryptionMethod from ._shared import ( + CLEVIS_AND_KERNEL, TRUST_URL_OR_THUMBPRINT, UUID_OR_NAME, ClevisEncryptionOptions, @@ -352,6 +353,85 @@ def __init__(self, namespace: Namespace): ] ENCRYPTION_SUBCMDS = [ + ( + "on", + { + "help": "Make encrypted a previously unencrypted pool", + "args": [ + ( + "--post-parser", + { + "action": RejectAction, + "default": ClevisEncryptionOptions, + "help": SUPPRESS, + "nargs": "?", + }, + ) + ], + "groups": [ + ( + "Pool Identifier", + { + "description": "Choose one option to specify the pool", + "mut_ex_args": [ + (True, UUID_OR_NAME), + ], + }, + ), + ( + "Encryption", + { + "description": "Arguments controlling encryption", + "args": CLEVIS_AND_KERNEL, + }, + ), + ( + "Tang Server Verification (only if --tang-url option is set)", + { + "description": "Choose one option", + "mut_ex_args": [(False, TRUST_URL_OR_THUMBPRINT)], + }, + ), + ], + "func": CryptActions.encrypt, + }, + ), + ( + "off", + { + "help": "Make unencrypted a previously encrypted pool", + "groups": [ + ( + "Pool Identifier", + { + "description": "Choose one option to specify the pool", + "mut_ex_args": [ + (True, UUID_OR_NAME), + ], + }, + ) + ], + "func": CryptActions.unencrypt, + }, + ), + ( + "reencrypt", + { + "help": "Reencrypt an encrypted pool with a new master key", + "groups": [ + ( + "Pool Identifier", + { + "description": "Choose one option to specify the pool", + "mut_ex_args": [ + (True, UUID_OR_NAME), + ], + }, + ) + ], + "func": CryptActions.reencrypt, + }, + ), ( "bind", { diff --git a/tests/integration/pool/test_encryption.py b/tests/integration/pool/test_encryption.py index 40c76c1fa..ea7af7e49 100644 --- a/tests/integration/pool/test_encryption.py +++ b/tests/integration/pool/test_encryption.py @@ -424,3 +424,215 @@ def test_unbind_with_unused_token_slot(self): "--token-slot=32", ] self.check_error(StratisCliNoChangeError, command_line, _ERROR) + + +class OffTestCase(SimTestCase): + """ + Test turning encryption off when pool is encrypted. + """ + + _MENU = ["--propagate", "pool", "encryption", "off"] + _POOLNAME = "poolname" + _KEY_DESC = "keydesc" + + def setUp(self): + super().setUp() + with RandomKeyTmpFile() as fname: + command_line = [ + "--propagate", + "key", + "set", + "--keyfile-path", + fname, + self._KEY_DESC, + ] + RUNNER(command_line) + + command_line = [ + "--propagate", + "pool", + "create", + "--key-desc", + self._KEY_DESC, + self._POOLNAME, + ] + _DEVICE_STRATEGY() + RUNNER(command_line) + + def test_decrypt_with_name(self): + """ + Decrypt an encrypted pool, specifying the pool by name. + """ + command_line = self._MENU + [ + f"--name={self._POOLNAME}", + ] + RUNNER(command_line) + + +class OffTestCase2(SimTestCase): + """ + Test turning encryption off when pool is not encrypted. + """ + + _MENU = ["--propagate", "pool", "encryption", "off"] + _POOLNAME = "poolname" + + def setUp(self): + super().setUp() + command_line = ["pool", "create", self._POOLNAME] + _DEVICE_STRATEGY() + RUNNER(command_line) + + def test_decrypt_with_name(self): + """ + Decrypting when unencrypted should return an error. + """ + command_line = self._MENU + [ + f"--name={self._POOLNAME}", + ] + self.check_error(StratisCliNoChangeError, command_line, _ERROR) + + +class ReencryptTestCase(SimTestCase): + """ + Test re-encrypting when pool is encrypted. + """ + + _MENU = ["--propagate", "pool", "encryption", "reencrypt"] + _POOLNAME = "poolname" + _KEY_DESC = "keydesc" + + def setUp(self): + super().setUp() + with RandomKeyTmpFile() as fname: + command_line = [ + "--propagate", + "key", + "set", + "--keyfile-path", + fname, + self._KEY_DESC, + ] + RUNNER(command_line) + + command_line = [ + "--propagate", + "pool", + "create", + "--key-desc", + self._KEY_DESC, + self._POOLNAME, + ] + _DEVICE_STRATEGY() + RUNNER(command_line) + + def test_reencrypt_with_name(self): + """ + Re-encrypt an encrypted pool, specifying the pool by name. + """ + command_line = self._MENU + [ + f"--name={self._POOLNAME}", + ] + RUNNER(command_line) + + +class ReencryptTestCase2(SimTestCase): + """ + Test reencryption when pool is not encrypted. + """ + + _MENU = ["--propagate", "pool", "encryption", "reencrypt"] + _POOLNAME = "poolname" + + def setUp(self): + super().setUp() + command_line = ["pool", "create", self._POOLNAME] + _DEVICE_STRATEGY() + RUNNER(command_line) + + def test_reencrypt_with_name(self): + """ + Reencrypting when unencrypted should return an error. + """ + command_line = self._MENU + [ + f"--name={self._POOLNAME}", + ] + self.check_error(StratisCliEngineError, command_line, _ERROR) + + +class EncryptTestCase(SimTestCase): + """ + Test encrypting when pool is already encrypted. + """ + + _MENU = ["--propagate", "pool", "encryption", "on"] + _POOLNAME = "poolname" + _KEY_DESC = "keydesc" + + def setUp(self): + super().setUp() + with RandomKeyTmpFile() as fname: + command_line = [ + "--propagate", + "key", + "set", + "--keyfile-path", + fname, + self._KEY_DESC, + ] + RUNNER(command_line) + + command_line = [ + "--propagate", + "pool", + "create", + "--key-desc", + self._KEY_DESC, + self._POOLNAME, + ] + _DEVICE_STRATEGY() + RUNNER(command_line) + + def test_encrypt_with_name(self): + """ + Encrypting when already encrypted should return an error. + """ + command_line = self._MENU + [ + f"--name={self._POOLNAME}", + "--clevis=tpm2", + ] + self.check_error(StratisCliNoChangeError, command_line, _ERROR) + + +class EncryptTestCase2(SimTestCase): + """ + Test encrypting when pool is not already encrypted. + """ + + _MENU = ["--propagate", "pool", "encryption", "on"] + _POOLNAME = "poolname" + _KEY_DESC = "keydesc" + + def setUp(self): + super().setUp() + command_line = [ + "--propagate", + "pool", + "create", + self._POOLNAME, + ] + _DEVICE_STRATEGY() + RUNNER(command_line) + + def test_encrypt_with_name(self): + """ + Encrypting when not already encrypted should succeed. + """ + command_line = self._MENU + [ + f"--name={self._POOLNAME}", + "--clevis=tpm2", + ] + TEST_RUNNER(command_line) + + def test_encryption_with_no_encryption_params(self): + """ + Encrypting without any encryption method fully specified should fail. + """ + command_line = self._MENU + [ + f"--name={self._POOLNAME}", + ] + self.check_error(StratisCliEngineError, command_line, _ERROR) From 8fdd64ff2585e2904cbc3aa42bac0146f8e39f1a Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 16 Apr 2025 09:36:02 -0400 Subject: [PATCH 04/12] Test parsing clevis options in encryption on Signed-off-by: mulhern --- tests/integration/test_parser.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_parser.py b/tests/integration/test_parser.py index 3a0125a0d..2953a64c3 100644 --- a/tests/integration/test_parser.py +++ b/tests/integration/test_parser.py @@ -241,14 +241,18 @@ def test_print_all_help(self): class TestClevisOptions(ParserTestCase): """ - Verify that invalid clevis encryption create options are detected. + Verify that invalid clevis encryption options are detected. """ def _do_clevis_test(self, clevis_args): """ Apply clevis args to create command_line and verify parser error. """ - self._do_test(["pool", "create", "pn", "/dev/n"] + clevis_args) + for subcommand in [ + ["pool", "create", "pn", "/dev/n"], + ["pool", "encryption", "on", "--name=pn"], + ]: + self._do_test(subcommand + clevis_args) def test_create_with_clevis_1(self): """ From 6d047a72fb8e8f59d56ad00ea4905011146c652e Mon Sep 17 00:00:00 2001 From: mulhern Date: Tue, 15 Jul 2025 16:18:57 -0400 Subject: [PATCH 05/12] Add an --in-place option Signed-off-by: mulhern --- src/stratis_cli/_actions/_crypt.py | 10 +++ src/stratis_cli/_errors.py | 16 +++++ src/stratis_cli/_parser/_encryption.py | 6 +- src/stratis_cli/_parser/_shared.py | 14 +++++ tests/integration/pool/test_encryption.py | 77 ++++++++++++++++++++--- 5 files changed, 115 insertions(+), 8 deletions(-) diff --git a/src/stratis_cli/_actions/_crypt.py b/src/stratis_cli/_actions/_crypt.py index cb3f524fd..e40c6f374 100644 --- a/src/stratis_cli/_actions/_crypt.py +++ b/src/stratis_cli/_actions/_crypt.py @@ -23,6 +23,7 @@ from .._errors import ( StratisCliEngineError, StratisCliIncoherenceError, + StratisCliInPlaceNotSpecified, StratisCliNoChangeError, ) from .._stratisd_constants import StratisdErrors @@ -41,6 +42,9 @@ def encrypt(namespace: Namespace): Encrypt a previously unencrypted pool. """ + if not namespace.in_place: + raise StratisCliInPlaceNotSpecified() + # pylint: disable=import-outside-toplevel from ._data import MOPool, ObjectManager, Pool, pools @@ -97,6 +101,9 @@ def unencrypt(namespace: Namespace): """ Unencrypt a previously encrypted pool. """ + if not namespace.in_place: + raise StratisCliInPlaceNotSpecified() + # pylint: disable=import-outside-toplevel from ._data import MOPool, ObjectManager, Pool, pools @@ -135,6 +142,9 @@ def reencrypt(namespace: Namespace): """ Reencrypt an already encrypted pool with a new key. """ + if not namespace.in_place: + raise StratisCliInPlaceNotSpecified() + # pylint: disable=import-outside-toplevel from ._data import ObjectManager, Pool, pools diff --git a/src/stratis_cli/_errors.py b/src/stratis_cli/_errors.py index b60731efb..1e6c7fe1a 100644 --- a/src/stratis_cli/_errors.py +++ b/src/stratis_cli/_errors.py @@ -454,3 +454,19 @@ def __init__(self, msg): def __str__(self): return self.msg + + +class StratisCliInPlaceNotSpecified(StratisCliUserError): + """ + Raised if the user requested in-place encryption but did not use the + --in-place option. + """ + + def __str__(self) -> str: + return ( + "Specify the --in-place option to demonstrate that you " + "understand the special nature of the procedure that you are " + "about to initiate. Please refer to the discussion of the " + '"--in-place" option in the man pages for further ' + "information." + ) diff --git a/src/stratis_cli/_parser/_encryption.py b/src/stratis_cli/_parser/_encryption.py index 1ca065323..8ad275540 100644 --- a/src/stratis_cli/_parser/_encryption.py +++ b/src/stratis_cli/_parser/_encryption.py @@ -23,6 +23,7 @@ from .._constants import Clevis, EncryptionMethod from ._shared import ( CLEVIS_AND_KERNEL, + IN_PLACE, TRUST_URL_OR_THUMBPRINT, UUID_OR_NAME, ClevisEncryptionOptions, @@ -367,7 +368,8 @@ def __init__(self, namespace: Namespace): "nargs": "?", }, ) - ], + ] + + IN_PLACE, "groups": [ ( "Pool Identifier", @@ -400,6 +402,7 @@ def __init__(self, namespace: Namespace): "off", { "help": "Make unencrypted a previously encrypted pool", + "args": IN_PLACE, "groups": [ ( "Pool Identifier", @@ -418,6 +421,7 @@ def __init__(self, namespace: Namespace): "reencrypt", { "help": "Reencrypt an encrypted pool with a new master key", + "args": IN_PLACE, "groups": [ ( "Pool Identifier", diff --git a/src/stratis_cli/_parser/_shared.py b/src/stratis_cli/_parser/_shared.py index b91fe0d38..2696e8fdd 100644 --- a/src/stratis_cli/_parser/_shared.py +++ b/src/stratis_cli/_parser/_shared.py @@ -297,3 +297,17 @@ def verify(self, namespace, parser): }, ), ] + +IN_PLACE = [ + ( + "--in-place", + { + "action": "store_true", + "help": ( + "Perform the operation in place; requires no additional " + "devices; see man page entry for this option for more " + "information" + ), + }, + ) +] diff --git a/tests/integration/pool/test_encryption.py b/tests/integration/pool/test_encryption.py index ea7af7e49..f646c5cce 100644 --- a/tests/integration/pool/test_encryption.py +++ b/tests/integration/pool/test_encryption.py @@ -17,7 +17,11 @@ # isort: LOCAL from stratis_cli import StratisCliErrorCodes -from stratis_cli._errors import StratisCliEngineError, StratisCliNoChangeError +from stratis_cli._errors import ( + StratisCliEngineError, + StratisCliInPlaceNotSpecified, + StratisCliNoChangeError, +) from .._keyutils import RandomKeyTmpFile from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list @@ -431,7 +435,7 @@ class OffTestCase(SimTestCase): Test turning encryption off when pool is encrypted. """ - _MENU = ["--propagate", "pool", "encryption", "off"] + _MENU = ["--propagate", "pool", "encryption", "off", "--in-place"] _POOLNAME = "poolname" _KEY_DESC = "keydesc" @@ -473,7 +477,7 @@ class OffTestCase2(SimTestCase): Test turning encryption off when pool is not encrypted. """ - _MENU = ["--propagate", "pool", "encryption", "off"] + _MENU = ["--propagate", "pool", "encryption", "off", "--in-place"] _POOLNAME = "poolname" def setUp(self): @@ -496,7 +500,7 @@ class ReencryptTestCase(SimTestCase): Test re-encrypting when pool is encrypted. """ - _MENU = ["--propagate", "pool", "encryption", "reencrypt"] + _MENU = ["--propagate", "pool", "encryption", "reencrypt", "--in-place"] _POOLNAME = "poolname" _KEY_DESC = "keydesc" @@ -538,7 +542,7 @@ class ReencryptTestCase2(SimTestCase): Test reencryption when pool is not encrypted. """ - _MENU = ["--propagate", "pool", "encryption", "reencrypt"] + _MENU = ["--propagate", "pool", "encryption", "reencrypt", "--in-place"] _POOLNAME = "poolname" def setUp(self): @@ -561,7 +565,7 @@ class EncryptTestCase(SimTestCase): Test encrypting when pool is already encrypted. """ - _MENU = ["--propagate", "pool", "encryption", "on"] + _MENU = ["--propagate", "pool", "encryption", "on", "--in-place"] _POOLNAME = "poolname" _KEY_DESC = "keydesc" @@ -604,7 +608,7 @@ class EncryptTestCase2(SimTestCase): Test encrypting when pool is not already encrypted. """ - _MENU = ["--propagate", "pool", "encryption", "on"] + _MENU = ["--propagate", "pool", "encryption", "on", "--in-place"] _POOLNAME = "poolname" _KEY_DESC = "keydesc" @@ -636,3 +640,62 @@ def test_encryption_with_no_encryption_params(self): f"--name={self._POOLNAME}", ] self.check_error(StratisCliEngineError, command_line, _ERROR) + + +class NoInPlaceTestCase(SimTestCase): + """ + Test encrypting when pool is not already encrypted. + """ + + _POOLNAME = "poolname" + _KEY_DESC = "keydesc" + + def setUp(self): + super().setUp() + command_line = [ + "--propagate", + "pool", + "create", + self._POOLNAME, + ] + _DEVICE_STRATEGY() + RUNNER(command_line) + + def test_on(self): + """ + In place must be specified for on. + """ + command_line = [ + "--propagate", + "pool", + "encryption", + "on", + f"--name={self._POOLNAME}", + "--clevis=tpm2", + ] + self.check_error(StratisCliInPlaceNotSpecified, command_line, _ERROR) + + def test_off(self): + """ + In place must be specified for off. + """ + command_line = [ + "--propagate", + "pool", + "encryption", + "off", + f"--name={self._POOLNAME}", + ] + self.check_error(StratisCliInPlaceNotSpecified, command_line, _ERROR) + + def test_reencrypt(self): + """ + In place must be specified for reencrypt. + """ + command_line = [ + "--propagate", + "pool", + "encryption", + "reencrypt", + f"--name={self._POOLNAME}", + ] + self.check_error(StratisCliInPlaceNotSpecified, command_line, _ERROR) From a590c2d3db13a034a565a04d3371e0a4e2d779aa Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 17 Jul 2025 16:17:33 -0400 Subject: [PATCH 06/12] Half D-Bus timeout We had it as high as two minutes to give a chance of returning on fairly long-running task, like creating encrypted pools, since each device had to be separately encrypted. We do not do that anymore, as the whole pool is encrypted these days, so the create method returns faster. We are about to introduce really long running commands, where 120 minutes will not be enough in almost all cases. So shortening the D-Bus timeout seems like a reasonable thing to do. 60 seconds is a plenty long time to wait for any error messsages that stratisd might return. Signed-off-by: mulhern --- src/stratis_cli/_actions/_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stratis_cli/_actions/_data.py b/src/stratis_cli/_actions/_data.py index 82b09b6ec..ee12be53e 100644 --- a/src/stratis_cli/_actions/_data.py +++ b/src/stratis_cli/_actions/_data.py @@ -44,7 +44,7 @@ "deferred until after the stratis_cli module has been fully loaded." ) -DBUS_TIMEOUT_SECONDS = 120 +DBUS_TIMEOUT_SECONDS = 60 # Specification for the lowest manager interface supported by the major # version of stratisd on which this version of the CLI depends. From 905361399948185e700a84436e07f78171922876 Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 17 Jul 2025 17:35:38 -0400 Subject: [PATCH 07/12] Add a decorator for our long running operations This way we can relatively cheaply avoid printing the timeout error message and return a zero exit code on the timeout. Signed-off-by: mulhern --- src/stratis_cli/_actions/__init__.py | 1 + src/stratis_cli/_actions/_crypt.py | 4 ++ src/stratis_cli/_actions/_utils.py | 63 +++++++++++++++++++++- src/stratis_cli/_error_reporting.py | 13 +---- tests/unit/test_running.py | 79 ++++++++++++++++++++++++++++ 5 files changed, 147 insertions(+), 13 deletions(-) create mode 100644 tests/unit/test_running.py diff --git a/src/stratis_cli/_actions/__init__.py b/src/stratis_cli/_actions/__init__.py index f16cf1953..45e079911 100644 --- a/src/stratis_cli/_actions/__init__.py +++ b/src/stratis_cli/_actions/__init__.py @@ -35,3 +35,4 @@ from ._stratis import StratisActions from ._stratisd_version import check_stratisd_version from ._top import TopActions +from ._utils import get_errors diff --git a/src/stratis_cli/_actions/_crypt.py b/src/stratis_cli/_actions/_crypt.py index e40c6f374..24a34770f 100644 --- a/src/stratis_cli/_actions/_crypt.py +++ b/src/stratis_cli/_actions/_crypt.py @@ -29,6 +29,7 @@ from .._stratisd_constants import StratisdErrors from ._connection import get_object from ._constants import TOP_OBJECT +from ._utils import long_running_operation class CryptActions: @@ -37,6 +38,7 @@ class CryptActions: """ @staticmethod + @long_running_operation(method_names=["EncryptPool"]) def encrypt(namespace: Namespace): """ Encrypt a previously unencrypted pool. @@ -97,6 +99,7 @@ def encrypt(namespace: Namespace): ) @staticmethod + @long_running_operation(method_names=["DecryptPool"]) def unencrypt(namespace: Namespace): """ Unencrypt a previously encrypted pool. @@ -138,6 +141,7 @@ def unencrypt(namespace: Namespace): ) @staticmethod + @long_running_operation(method_names=["ReencryptPool"]) def reencrypt(namespace: Namespace): """ Reencrypt an already encrypted pool with a new key. diff --git a/src/stratis_cli/_actions/_utils.py b/src/stratis_cli/_actions/_utils.py index ab21fa872..96ca1abe5 100644 --- a/src/stratis_cli/_actions/_utils.py +++ b/src/stratis_cli/_actions/_utils.py @@ -22,15 +22,21 @@ import os import sys import termios +from argparse import Namespace from enum import Enum -from typing import Any, Tuple +from functools import wraps +from typing import Any, Callable, Generator, Sequence, Tuple from uuid import UUID # isort: THIRDPARTY from dbus import Dictionary, Struct +from dbus.exceptions import DBusException from dbus.proxies import ProxyObject from justbytes import Range +# isort: FIRSTPARTY +from dbus_python_client_gen import DPClientInvocationError, DPClientMethodCallContext + from .._errors import ( StratisCliKeyfileNotFoundError, StratisCliPassphraseEmptyError, @@ -304,3 +310,58 @@ def free(self) -> Range | None: if self._used is None or self._total is None else self._total - self._used ) + + +def get_errors(exc: BaseException) -> Generator[BaseException, None, None]: + """ + Generates a sequence of exceptions starting with exc and following the chain + of causes. + """ + yield exc + while exc.__cause__ is not None: + yield exc.__cause__ + exc = exc.__cause__ + + +def long_running_operation( + *, method_names: Sequence[str] +) -> Callable[[Callable], Callable]: + """ + Mark a function as a long running operation and catch and ignore NoReply + D-Bus exception so long as the method raising the exception is one + of the methods specified in method_names. + """ + + def decorator(func: Callable[[Namespace], None]) -> Callable[[Namespace], None]: + """ + Decorator + """ + + @wraps(func) + def wrapper(namespace: Namespace): + """ + Wrapper + """ + try: + func(namespace) + except DPClientInvocationError as err: + if not any( + isinstance(e, DBusException) + and e.get_dbus_name() == "org.freedesktop.DBus.Error.NoReply" + for e in get_errors(err) + ): + raise err + + context = err.context + if ( + isinstance(context, DPClientMethodCallContext) + and context.method_name in method_names + ): + print("Operation initiated", file=sys.stderr) + + else: + raise err + + return wrapper + + return decorator diff --git a/src/stratis_cli/_error_reporting.py b/src/stratis_cli/_error_reporting.py index f0158f2a7..7a12941fe 100644 --- a/src/stratis_cli/_error_reporting.py +++ b/src/stratis_cli/_error_reporting.py @@ -17,7 +17,6 @@ # isort: STDLIB import os import sys -from collections.abc import Iterator from typing import List, Optional # isort: THIRDPARTY @@ -41,6 +40,7 @@ FILESYSTEM_INTERFACE, MANAGER_0_INTERFACE, POOL_INTERFACE, + get_errors, ) from ._errors import ( StratisCliActionError, @@ -87,17 +87,6 @@ def _interface_name_to_common_name(interface_name: str) -> str: raise StratisCliUnknownInterfaceError(interface_name) # pragma: no cover -def get_errors(exc: BaseException) -> Iterator[BaseException]: - """ - Generates a sequence of exceptions starting with exc and following the chain - of causes. - """ - yield exc - while exc.__cause__ is not None: - yield exc.__cause__ - exc = exc.__cause__ - - def _interpret_errors_0( error: dbus.exceptions.DBusException, ) -> Optional[str]: diff --git a/tests/unit/test_running.py b/tests/unit/test_running.py new file mode 100644 index 000000000..e9100be85 --- /dev/null +++ b/tests/unit/test_running.py @@ -0,0 +1,79 @@ +# Copyright 2025 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Test 'long_running_operation'. +""" + +# isort: STDLIB +import unittest + +# isort: THIRDPARTY +import dbus + +# isort: FIRSTPARTY +from dbus_python_client_gen import DPClientInvocationError, DPClientMethodCallContext + +# isort: LOCAL +from stratis_cli._actions._utils import long_running_operation + + +class LongRunningOperationTestCase(unittest.TestCase): + """ + Test long_running_operation error paths that don't show up in the sim + engine. + """ + + def test_catch_dbus_exception(self): + """ + Should succeed because it catches the distinguishing NoReply D-Bus + error from the identified method. + """ + + def raises_error(_): + raise DPClientInvocationError( + "fake", "intf", DPClientMethodCallContext("MethodName", []) + ) from dbus.exceptions.DBusException( + name="org.freedesktop.DBus.Error.NoReply" + ) + + self.assertIsNone( + long_running_operation(method_names=["MethodName"])(raises_error)(None) + ) + + def test_raise_dbus_exception_no_name_match(self): + """ + Should raise an exception because the method to be matched is not + MethodName. + """ + + def raises_error(_): + raise DPClientInvocationError( + "fake", "intf", DPClientMethodCallContext("MethodName", []) + ) from dbus.exceptions.DBusException( + name="org.freedesktop.DBus.Error.NoReply" + ) + + with self.assertRaises(DPClientInvocationError): + long_running_operation(method_names=["OtherMethodName"])(raises_error)(None) + + def test_no_dbus_exception(self): + """ + Should raise an exception that was previously raised. + """ + + def raises_error(_): + raise DPClientInvocationError("fake", "intf", None) + + with self.assertRaises(DPClientInvocationError): + long_running_operation(method_names=["OtherMethodName"])(raises_error)(None) From aa46061c314e167b8445fe341d43875bc7229ea2 Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 21 Jul 2025 17:49:20 -0400 Subject: [PATCH 08/12] Display last time re-encrypted in pool detail view Signed-off-by: mulhern --- src/stratis_cli/_actions/_list_pool.py | 13 +++++++++++++ tests/integration/pool/test_encryption.py | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/src/stratis_cli/_actions/_list_pool.py b/src/stratis_cli/_actions/_list_pool.py index 0ecec4ba5..e7a3f5f8c 100644 --- a/src/stratis_cli/_actions/_list_pool.py +++ b/src/stratis_cli/_actions/_list_pool.py @@ -28,6 +28,7 @@ from uuid import UUID # isort: THIRDPARTY +from dateutil import parser as date_parser from justbytes import Range # isort: FIRSTPARTY @@ -435,6 +436,18 @@ def _print_detail_view( elif encrypted is True: print("Encryption Enabled: Yes") + try: + reencrypted = get_property( + mopool.LastReencryptedTimestamp(), + lambda t: date_parser.isoparse(t) + .astimezone() + .strftime("%b %d %Y %H:%M"), + "Never", + ) + except DbusClientMissingPropertyError: + reencrypted = TABLE_UNKNOWN_STRING + print(f" Last Time Reencrypted: {reencrypted}") + if metadata_version is MetadataVersion.V1: # pragma: no cover try: key_description_str = _non_existent_or_inconsistent_to_str( diff --git a/tests/integration/pool/test_encryption.py b/tests/integration/pool/test_encryption.py index f646c5cce..6a93db20a 100644 --- a/tests/integration/pool/test_encryption.py +++ b/tests/integration/pool/test_encryption.py @@ -536,6 +536,10 @@ def test_reencrypt_with_name(self): ] RUNNER(command_line) + # Exercise detail view with last reencryption time set + command_line = ["--propagate", "pool", "list", f"--name={self._POOLNAME}"] + RUNNER(command_line) + class ReencryptTestCase2(SimTestCase): """ From d70d14d79037646d6140cda93730fe75a87e584f Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 14 Aug 2025 11:55:35 -0400 Subject: [PATCH 09/12] Add man page entries for the moved subcommands These are bind, unbind, and rebind. The new commands use the mandatory --name, --uuid option mechanism for specifying the pool to operate on, while the old commands just used name. Signed-off-by: mulhern --- docs/stratis.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/stratis.txt b/docs/stratis.txt index de09713e9..fc7228397 100644 --- a/docs/stratis.txt +++ b/docs/stratis.txt @@ -117,6 +117,25 @@ pool unbind <(clevis|keyring)> [--token-slot ]:: mechanism. MOVE NOTICE: The "unbind" subcommand can also be found under the "pool encryption" subcommand. The "pool unbind" subcommand that you are using now is deprecated and will be removed in stratis 3.10.0. +pool encryption bind <(nbde|tang)> <(--uuid |--name )> <(--thumbprint | --trust-url)> :: + Bind the devices in the specified pool to a supplementary encryption + mechanism that uses NBDE (Network-Bound Disc Encryption). *tang* is + an alias for *nbde*. +pool encryption bind tpm2 <(--uuid |--name )>:: + Bind the devices in the specified pool to a supplementary encryption + mechanism that uses TPM 2.0 (Trusted Platform Module). +pool encryption bind keyring <(--uuid |--name )> :: + Bind the devices in the specified pool to a supplementary encryption + mechanism using a key in the kernel keyring. +pool encryption rebind clevis <(--uuid |--name )> [--token-slot ]:: + Rebind the devices in the specified pool using the Clevis configuration + with which the devices in the pool were previously bound. +pool encryption rebind keyring <(--uuid |--name )> [--token-slot ]:: + Rebind the devices in the specified pool using the specified key + description. +pool encryption unbind <(clevis|keyring)> <(--uuid |--name )> [--token-slot ]:: + Unbind the devices in the specified pool from the specified encryption + mechanism. pool set-fs-limit :: Set the limit on the number of file systems allowed per-pool. This number may only be increased from its current value. From 3221491c7b5cef2d0587edf42118a30204f03dcf Mon Sep 17 00:00:00 2001 From: mulhern Date: Fri, 15 Aug 2025 10:06:14 -0400 Subject: [PATCH 10/12] man: Add entries for encryption on, off, and re-encrypt Signed-off-by: mulhern --- docs/stratis.txt | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/stratis.txt b/docs/stratis.txt index fc7228397..381102e5c 100644 --- a/docs/stratis.txt +++ b/docs/stratis.txt @@ -117,6 +117,15 @@ pool unbind <(clevis|keyring)> [--token-slot ]:: mechanism. MOVE NOTICE: The "unbind" subcommand can also be found under the "pool encryption" subcommand. The "pool unbind" subcommand that you are using now is deprecated and will be removed in stratis 3.10.0. +pool encryption on --in-place <(--uuid |--name )> [--key-desc ] [--clevis <(nbde|tang|tpm2)> [--tang-url ] [<(--thumbprint | --trust-url)>]:: + Turn encryption on for the specified pool. This operation takes time + proportional to the size of the pool. +pool encryption off --in-place <(--uuid |--name )>:: + Turn encryption off for the specified pool. This operation takes time + proportional to the size of the pool. +pool encryption reencrypt --in-place <(--uuid |--name )>:: + Reencrypt the pool with a new master key. This operation takes time + proportional to the size of the pool. pool encryption bind <(nbde|tang)> <(--uuid |--name )> <(--thumbprint | --trust-url)> :: Bind the devices in the specified pool to a supplementary encryption mechanism that uses NBDE (Network-Bound Disc Encryption). *tang* is @@ -262,6 +271,20 @@ OPTIONS --token-slot :: For V2 pools only. Use the token slot number to select among different bindings that use the same encryption method. +--in-place :: + This is a mandatory option that must be set when requesting a + long-running in-place encryption operation. These operations are a + kind that change the pool's data as well as its metadata. Hence these + operations take a time that is linear in the size of the pool. + Additionally, these operations are run in place, that is, the + pool's data blocks are directly modified while it is in use. While + the operation is taking place, automatic administrative actions, + for example, extending filesystems, can not be taken on the pool. + Furthermore, user-initiated actions, such as adding a new device to + a pool are also disabled. The pool administrator should therefore + ensure that no administrative operations will become urgently + necessary while the encryption operation is running. Consider + backing up your data before initiating this operation. SIZE SPECIFICATION FORMAT FOR INPUT From 7e12d43148d3a69e6ce0bd036e605b21cf118a32 Mon Sep 17 00:00:00 2001 From: mulhern Date: Fri, 15 Aug 2025 13:34:12 -0400 Subject: [PATCH 11/12] man: Add entry for the "Last Time Reencrypted" field Signed-off-by: mulhern --- docs/stratis.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/stratis.txt b/docs/stratis.txt index 381102e5c..920630ab2 100644 --- a/docs/stratis.txt +++ b/docs/stratis.txt @@ -380,6 +380,11 @@ Encryption Enabled: Clevis Configuration:: The Clevis configuration, if the pool is encrypted via Clevis. Only displayed if metadata version is 1 and encryption is enabled. +Encryption Enabled: Last Time Reencrypted:: + The last time the pool was re-encrypted. If the pool has never been + re-encrypted, the value is "Never". Only displayed if metadata + version is 2 and encryption is enabled. + Encryption Enabled: Free Token Slots Remaining:: The number of token slots remaining that can accommodate new bindings, followed by a list of binding descriptions, ordered by From 1c9b8736df2c8472c3fd05a3074105c07fe411bc Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 29 Jan 2026 10:08:10 -0500 Subject: [PATCH 12/12] Time out after ten seconds for long-running methods Signed-off-by: mulhern --- setup.cfg | 2 +- src/stratis_cli/_actions/_crypt.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/setup.cfg b/setup.cfg index 96f814e77..b3dcd19e8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -26,7 +26,7 @@ classifiers= python_requires = >=3.12 install_requires = dbus-client-gen>=0.4 - dbus-python-client-gen>=0.7 + dbus-python-client-gen>=0.8.4 justbytes>=0.14 packaging psutil diff --git a/src/stratis_cli/_actions/_crypt.py b/src/stratis_cli/_actions/_crypt.py index 24a34770f..59ca9a6ca 100644 --- a/src/stratis_cli/_actions/_crypt.py +++ b/src/stratis_cli/_actions/_crypt.py @@ -86,6 +86,7 @@ def encrypt(namespace: Namespace): ] ), }, + timeout=10, ) if return_code != StratisdErrors.OK: @@ -127,7 +128,9 @@ def unencrypt(namespace: Namespace): raise StratisCliNoChangeError("encryption off", pool_id) (changed, return_code, message) = Pool.Methods.DecryptPool( - get_object(pool_object_path), {} + get_object(pool_object_path), + {}, + timeout=10, ) if return_code != StratisdErrors.OK: # pragma: no cover @@ -166,7 +169,9 @@ def reencrypt(namespace: Namespace): ) (changed, return_code, message) = Pool.Methods.ReencryptPool( - get_object(pool_object_path), {} + get_object(pool_object_path), + {}, + timeout=10, ) if return_code != StratisdErrors.OK: