From 6c72cd912e7ff3ee5a12ebacb4cdbf034d461733 Mon Sep 17 00:00:00 2001 From: William Kimball <30981667+wwkimball@users.noreply.github.com> Date: Wed, 27 Oct 2021 10:47:44 -0500 Subject: [PATCH 1/4] Fix formatting loss during EYAML key rotation --- CHANGES | 5 +++ tests/test_commands_eyaml_rotate_keys.py | 55 +++++++++++++++++++++++- yamlpath/__init__.py | 2 +- yamlpath/commands/eyaml_rotate_keys.py | 5 ++- yamlpath/eyaml/eyamlprocessor.py | 37 ++++++++-------- yamlpath/processor.py | 12 +++--- 6 files changed, 87 insertions(+), 29 deletions(-) diff --git a/CHANGES b/CHANGES index 60be4e3..1e09b97 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +3.6.3 +Bug Fixes: +* The eyaml-rotate-keys command-line tool failed to preserve block-style EYAML + strings after key rotation. + 3.6.2: Bug Fixes: * The eyaml-rotate-keys command-line tool would generate a stack-dump when the diff --git a/tests/test_commands_eyaml_rotate_keys.py b/tests/test_commands_eyaml_rotate_keys.py index 7cd91c0..6419b78 100644 --- a/tests/test_commands_eyaml_rotate_keys.py +++ b/tests/test_commands_eyaml_rotate_keys.py @@ -59,7 +59,12 @@ def test_no_yaml_files(self, script_runner, old_eyaml_keys, new_eyaml_keys): assert "Not a file:" in result.stderr @requireseyaml - def test_good_multi_replacements(self, script_runner, tmp_path_factory, old_eyaml_keys, new_eyaml_keys): + def test_good_multi_replacements(self, script_runner, tmp_path_factory, old_eyaml_keys, new_eyaml_keys, quiet_logger): + from yamlpath.func import unwrap_node_coords + from yamlpath.common import Parsers + from yamlpath import Processor + from yamlpath.eyaml import EYAMLProcessor + simple_content = """--- encrypted_string: ENC[PKCS7,MIIBiQYJKoZIhvcNAQcDoIIBejCCAXYCAQAxggEhMIIBHQIBADAFMAACAQEwDQYJKoZIhvcNAQEBBQAEggEAHA4rPcTzvgzPLtnGz3yoyX/kVlQ5TnPXcScXK2bwjguGZLkuzv/JVPAsOm4t6GlnROpy4zb/lUMHRJDChJhPLrSj919B8//huoMgw0EU5XTcaN6jeDDjL+vhjswjvLFOux66UwvMo8sRci/e2tlFiam8VgxzV0hpF2qRrL/l84V04gL45kq4PCYDWrJNynOwYVbSIF+qc5HaF25H8kHq1lD3RB6Ob/J942Q7k5Qt7W9mNm9cKZmxwgtUgIZWXW6mcPJ2dXDB/RuPJJSrLsb1VU/DkhdgxaNzvLCA+MViyoFUkCfHFNZbaHKNkoYXBy7dLmoh/E5tKv99FeG/7CzL3DBMBgkqhkiG9w0BBwEwHQYJYIZIAWUDBAEqBBCVU5Mjt8+4dLkoqB9YArfkgCDkdIhXR9T1M4YYa1qTE6by61VPU3g1aMExRmo4tNZ8FQ==] encrypted_block: > @@ -114,7 +119,53 @@ def test_good_multi_replacements(self, script_runner, tmp_path_factory, old_eyam assert not simple_data == simple_content assert not anchored_data == anchored_content - # FIXME: Verify that block and string formatting is correct + # Verify that block and string formatting is correct + yaml = Parsers.get_yaml_editor() + + (yaml_rotated_data, doc_loaded) = Parsers.get_yaml_data( + yaml, quiet_logger, + anchored_data, literal=True) + if not doc_loaded: + # An error message has already been logged + assert False, "Rotated anchored data failed to load" + + source_processor = Processor(quiet_logger, yaml_rotated_data) + for node in source_processor.get_nodes('/block', mustexist=True): + assert not ' ' in unwrap_node_coords(node) + + + # Test that the pre- and post-rotated values are identical + (yaml_anchored_data, doc_loaded) = Parsers.get_yaml_data( + yaml, quiet_logger, + anchored_content, literal=True) + if not doc_loaded: + # An error message has already been logged + assert False, "Original anchored data failed to load" + + (yaml_rotated_data, doc_loaded) = Parsers.get_yaml_data( + yaml, quiet_logger, + anchored_data, literal=True) + if not doc_loaded: + # An error message has already been logged + assert False, "Rotated anchored data failed to load" + + source_processor = EYAMLProcessor( + quiet_logger, yaml_anchored_data, + privatekey=old_eyaml_keys[0], + publickey=old_eyaml_keys[1]) + for node in source_processor.get_eyaml_values( + '/block', True + ): + assert unwrap_node_coords(node) == 'This is a test value.' + + rotated_processor = EYAMLProcessor( + quiet_logger, yaml_rotated_data, + privatekey=new_eyaml_keys[0], + publickey=new_eyaml_keys[1]) + for node in rotated_processor.get_eyaml_values( + '/block', True + ): + assert unwrap_node_coords(node) == 'This is a test value.' def test_yaml_parsing_error(self, script_runner, imparsible_yaml_file, old_eyaml_keys, new_eyaml_keys): result = script_runner.run( diff --git a/yamlpath/__init__.py b/yamlpath/__init__.py index cab52f6..17f456c 100644 --- a/yamlpath/__init__.py +++ b/yamlpath/__init__.py @@ -1,6 +1,6 @@ """Core YAML Path classes.""" # Establish the version number common to all components -__version__ = "3.6.2" +__version__ = "3.6.3" from yamlpath.yamlpath import YAMLPath from yamlpath.processor import Processor diff --git a/yamlpath/commands/eyaml_rotate_keys.py b/yamlpath/commands/eyaml_rotate_keys.py index 14c8ad9..aac3b8d 100644 --- a/yamlpath/commands/eyaml_rotate_keys.py +++ b/yamlpath/commands/eyaml_rotate_keys.py @@ -17,6 +17,7 @@ from yamlpath import __version__ as YAMLPATH_VERSION from yamlpath.common import Anchors, Parsers from yamlpath.eyaml.exceptions import EYAMLCommandException +from yamlpath.eyaml.enums import EYAMLOutputFormats from yamlpath.eyaml import EYAMLProcessor from yamlpath.wrappers import ConsolePrinter @@ -163,9 +164,9 @@ def main(): # Prefer block (folded) values unless the original YAML value # was already a massivly long (string) line. - output = "block" + output = EYAMLOutputFormats.BLOCK if not isinstance(node, FoldedScalarString): - output = "string" + output = EYAMLOutputFormats.STRING # Re-encrypt the value with new EYAML keys processor.publickey = args.newpublickey diff --git a/yamlpath/eyaml/eyamlprocessor.py b/yamlpath/eyaml/eyamlprocessor.py index 2d7a43d..71508b0 100644 --- a/yamlpath/eyaml/eyamlprocessor.py +++ b/yamlpath/eyaml/eyamlprocessor.py @@ -196,20 +196,23 @@ def encrypt_eyaml( if not self._can_run_eyaml(): raise EYAMLCommandException( - "The eyaml binary is not executable at {}.".format(self.eyaml) + f"The eyaml binary is not executable at {self.eyaml}." ) - cmdstr: str = ("{} encrypt --quiet --stdin --output={}" - .format(self.eyaml, output)) + cmd: List[str] = [ + self.eyaml, + 'encrypt', + '--quiet', + '--stdin', + f"--output={output}" + ] if self.publickey: - cmdstr += " --pkcs7-public-key={}".format(self.publickey) + cmd.append(f"--pkcs7-public-key={self.publickey}") if self.privatekey: - cmdstr += " --pkcs7-private-key={}".format(self.privatekey) + cmd.append(f"--pkcs7-private-key={self.privatekey}") - cmd: List[str] = cmdstr.split() self.logger.debug( - "EYAMLPath::encrypt_eyaml: About to execute: {}" - .format(" ".join(cmd)) + f"EYAMLPath::encrypt_eyaml: About to execute: {' '.join(cmd)}" ) bval: bytes = value.encode("ascii") @@ -224,8 +227,8 @@ def encrypt_eyaml( ) except CalledProcessError as ex: raise EYAMLCommandException( - "The {} command cannot be run due to exit code: {}" - .format(self.eyaml, ex.returncode) + f"The {self.eyaml} command cannot be run due to exit code:" + " {ex.returncode}" ) from ex # While exceedingly rare and difficult to test for, it is possible @@ -234,16 +237,17 @@ def encrypt_eyaml( # that works multi-platform. So, ignore covering this case. if not retval: # pragma: no cover raise EYAMLCommandException( - ("The {} command was unable to encrypt your value. Please" - + " verify this process can run that command and read your" - + " EYAML keys.").format(self.eyaml) + "The {self.eyaml} command was unable to encrypt your value." + " Please verify this process can run that command and read" + " your EYAML keys." ) if output is EYAMLOutputFormats.BLOCK: - retval = re.sub(r" +", "", retval) + "\n" + retval = re.sub(r" +", "", retval) self.logger.debug( - "EYAMLPath::encrypt_eyaml: Encrypted result:\n{}".format(retval) + f"Encrypted result:\n{retval}", + prefix="EYAMLPath::encrypt_eyaml: " ) return retval @@ -269,8 +273,7 @@ def set_eyaml_value( - `YAMLPathException` when YAML Path is invalid """ self.logger.verbose( - "Encrypting value(s) for {}." - .format(yaml_path) + f"Encrypting value(s) for {yaml_path} using {output} format." ) encval: str = self.encrypt_eyaml(value, output) emit_format: YAMLValueFormats = YAMLValueFormats.FOLDED diff --git a/yamlpath/processor.py b/yamlpath/processor.py index 9478565..7c97027 100644 --- a/yamlpath/processor.py +++ b/yamlpath/processor.py @@ -163,8 +163,7 @@ def set_value( if mustexist: self.logger.debug( - "Processor::set_value: Seeking required node at {}." - .format(yaml_path) + f"Processor::set_value: Seeking required node at {yaml_path}." ) found_nodes: int = 0 for req_node in self._get_required_nodes(self.data, yaml_path): @@ -216,10 +215,9 @@ def _apply_change( YAMLValueFormats.DEFAULT) tag: str = kwargs.pop("tag", None) - self.logger.debug(( - "Attempting to change a node coordinate of type {} to value with" - " format <{}>:" - ).format(type(node_coord), value_format), + self.logger.debug( + "Attempting to change a node coordinate of type" + f" {type(node_coord)} to value with format <{value_format}>:", data={ "value": value, "node_coord": node_coord @@ -2349,7 +2347,7 @@ def recurse(data, parent, parentref, reference_node, replacement_node): change_node, value, value_format, tag=value_tag) self.logger.debug( - "Changing the following <{}> formatted node:".format(value_format), + f"Changing the following <{value_format}> formatted node:", prefix="Processor::_update_node: ", data={ "__FROM__": change_node, "___TO___": new_node }) From a1af759511a72d2b5e6a65efd1abd423e3f97949 Mon Sep 17 00:00:00 2001 From: William Kimball <30981667+wwkimball@users.noreply.github.com> Date: Wed, 27 Oct 2021 10:58:26 -0500 Subject: [PATCH 2/4] The eyaml command must be a str --- yamlpath/eyaml/eyamlprocessor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yamlpath/eyaml/eyamlprocessor.py b/yamlpath/eyaml/eyamlprocessor.py index 71508b0..eed6778 100644 --- a/yamlpath/eyaml/eyamlprocessor.py +++ b/yamlpath/eyaml/eyamlprocessor.py @@ -46,7 +46,7 @@ def __init__( Raises: N/A """ - self.eyaml: Optional[str] = kwargs.pop("binary", "eyaml") + self.eyaml: str = str(kwargs.pop("binary", "eyaml")) self.publickey: Optional[str] = kwargs.pop("publickey", None) self.privatekey: Optional[str] = kwargs.pop("privatekey", None) super().__init__(logger, data) From 745737e55c5ad37d02092bca07ab79816b5e2169 Mon Sep 17 00:00:00 2001 From: William Kimball <30981667+wwkimball@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:09:05 -0500 Subject: [PATCH 3/4] Add \n to EYAML blocks to avoid adding - to > --- yamlpath/eyaml/eyamlprocessor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yamlpath/eyaml/eyamlprocessor.py b/yamlpath/eyaml/eyamlprocessor.py index eed6778..37f41cd 100644 --- a/yamlpath/eyaml/eyamlprocessor.py +++ b/yamlpath/eyaml/eyamlprocessor.py @@ -243,7 +243,7 @@ def encrypt_eyaml( ) if output is EYAMLOutputFormats.BLOCK: - retval = re.sub(r" +", "", retval) + retval = re.sub(r" +", "", retval) + "\n" self.logger.debug( f"Encrypted result:\n{retval}", From 4c02d4ca2de8b20c5e3f88ae8642753602161b98 Mon Sep 17 00:00:00 2001 From: William Kimball <30981667+wwkimball@users.noreply.github.com> Date: Wed, 27 Oct 2021 11:52:54 -0500 Subject: [PATCH 4/4] Some C0209 --- yamlpath/eyaml/eyamlprocessor.py | 38 +++++++++++++++++--------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/yamlpath/eyaml/eyamlprocessor.py b/yamlpath/eyaml/eyamlprocessor.py index 37f41cd..d71040d 100644 --- a/yamlpath/eyaml/eyamlprocessor.py +++ b/yamlpath/eyaml/eyamlprocessor.py @@ -131,18 +131,22 @@ def decrypt_eyaml(self, value: str) -> str: if not self._can_run_eyaml(): raise EYAMLCommandException("No accessible eyaml command.") - cmdstr: str = "{} decrypt --quiet --stdin".format(self.eyaml) + cmd: List[str] = [ + self.eyaml, + 'decrypt', + '--quiet', + '--stdin' + ] if self.publickey: - cmdstr += " --pkcs7-public-key={}".format(self.publickey) + cmd.append(f"--pkcs7-public-key={self.publickey}") if self.privatekey: - cmdstr += " --pkcs7-private-key={}".format(self.privatekey) + cmd.append(f"--pkcs7-private-key={self.privatekey}") - cmd: List[str] = cmdstr.split() cleanval: str = str(value).replace("\n", "").replace(" ", "").rstrip() bval: bytes = cleanval.encode("ascii") self.logger.debug( - "EYAMLPath::decrypt_eyaml: About to execute {} against:\n{}" - .format(cmdstr, cleanval) + f"About to execute {' '.join(cmd)} against:\n{cleanval}", + prefix="EYAMLPath::decrypt_eyaml: " ) try: @@ -157,19 +161,19 @@ def decrypt_eyaml(self, value: str) -> str: ).stdout.decode('ascii').rstrip() except CalledProcessError as ex: raise EYAMLCommandException( - "The {} command cannot be run due to exit code: {}" - .format(self.eyaml, ex.returncode) + f"The {self.eyaml} command cannot be run due to exit code:" + f" {ex.returncode}" ) from ex # Check for bad decryptions self.logger.debug( - "EYAMLPath::decrypt_eyaml: Decrypted result: {}".format(retval) + f"EYAMLPath::decrypt_eyaml: Decrypted result: {retval}" ) if not retval or retval == cleanval: raise EYAMLCommandException( "Unable to decrypt value! Please verify you are using the" - + " correct old EYAML keys and the value is not corrupt: {}" - .format(cleanval) + " correct old EYAML keys and the value is not corrupt:" + " {cleanval}" ) return retval @@ -196,7 +200,7 @@ def encrypt_eyaml( if not self._can_run_eyaml(): raise EYAMLCommandException( - f"The eyaml binary is not executable at {self.eyaml}." + f"The eyaml binary is not executable at: {self.eyaml}" ) cmd: List[str] = [ @@ -222,13 +226,13 @@ def encrypt_eyaml( retval: str = ( run(cmd, stdout=PIPE, input=bval, check=True, shell=False) .stdout - .decode('ascii') + .decode("ascii") .rstrip() ) except CalledProcessError as ex: raise EYAMLCommandException( f"The {self.eyaml} command cannot be run due to exit code:" - " {ex.returncode}" + f" {ex.returncode}" ) from ex # While exceedingly rare and difficult to test for, it is possible @@ -237,7 +241,7 @@ def encrypt_eyaml( # that works multi-platform. So, ignore covering this case. if not retval: # pragma: no cover raise EYAMLCommandException( - "The {self.eyaml} command was unable to encrypt your value." + f"The {self.eyaml} command was unable to encrypt your value." " Please verify this process can run that command and read" " your EYAML keys." ) @@ -309,9 +313,7 @@ def get_eyaml_values( Raises: - `YAMLPathException` when YAML Path is invalid """ - self.logger.verbose( - "Decrypting value(s) at {}.".format(yaml_path) - ) + self.logger.verbose(f"Decrypting value(s) at {yaml_path}.") for node in self.get_nodes(yaml_path, mustexist=mustexist, default_value=default_value): plain_text: str = self.decrypt_eyaml(node.node)