diff --git a/pegleg/engine/catalog/pki_utility.py b/pegleg/engine/catalog/pki_utility.py index 582209e7..0885252e 100644 --- a/pegleg/engine/catalog/pki_utility.py +++ b/pegleg/engine/catalog/pki_utility.py @@ -26,6 +26,7 @@ import pytz import yaml from pegleg.engine import exceptions +from pegleg.engine.util.catalog import decode_bytes from pegleg.engine.util.pegleg_managed_document import \ PeglegManagedSecretsDocument @@ -227,15 +228,14 @@ class PKIUtility(object): with tempfile.TemporaryDirectory() as tmp: for filename, data in files.items(): with open(os.path.join(tmp, filename), 'w') as f: - f.write(data) + f.write(decode_bytes(data)) # Ignore bandit false positive: # B603:subprocess_without_shell_equals_true # This method wraps cfssl calls originating from this module. result = subprocess.check_output( # nosec ['cfssl'] + command, cwd=tmp, stderr=subprocess.PIPE) - if not isinstance(result, str): - result = result.decode('utf-8') + result = decode_bytes(result) return json.loads(result) def _openssl(self, command, *, files=None): @@ -246,7 +246,7 @@ class PKIUtility(object): with tempfile.TemporaryDirectory() as tmp: for filename, data in files.items(): with open(os.path.join(tmp, filename), 'w') as f: - f.write(data) + f.write(decode_bytes(data)) # Ignore bandit false positive: # B603:subprocess_without_shell_equals_true diff --git a/pegleg/engine/site.py b/pegleg/engine/site.py index 4d94c772..ef7c9bc4 100644 --- a/pegleg/engine/site.py +++ b/pegleg/engine/site.py @@ -18,6 +18,7 @@ import os import click import git import yaml +from yaml.constructor import SafeConstructor from prettytable import PrettyTable @@ -99,6 +100,9 @@ def collect(site_name, save_location): def render(site_name, output_stream, validate): documents = [] + # Ignore YAML tags, only construct dicts + SafeConstructor.add_multi_constructor( + '', lambda loader, suffix, node: None) for filename in util.definition.site_files(site_name): with open(filename) as f: documents.extend(list(yaml.safe_load_all(f))) diff --git a/pegleg/engine/util/catalog.py b/pegleg/engine/util/catalog.py index 245d033e..505295f9 100644 --- a/pegleg/engine/util/catalog.py +++ b/pegleg/engine/util/catalog.py @@ -19,7 +19,23 @@ from pegleg.engine.util import definition LOG = logging.getLogger(__name__) -__all__ = ('iterate', ) +__all__ = ('iterate', 'decode_bytes') + + +def decode_bytes(obj): + """If the argument is bytes, decode it. + + :param Object obj: A string or byte object + :return: A string representation of obj + :rtype: str + + """ + if isinstance(obj, bytes): + return obj.decode('utf-8') + elif isinstance(obj, str): + return obj + else: + raise ValueError("ERROR: {} is not bytes or a string.".format(obj)) def iterate(kind, sitename=None, documents=None): diff --git a/pegleg/engine/util/files.py b/pegleg/engine/util/files.py index 573e2f0e..fdef14b9 100644 --- a/pegleg/engine/util/files.py +++ b/pegleg/engine/util/files.py @@ -18,6 +18,7 @@ import os import click import yaml +from yaml.constructor import SafeConstructor from pegleg import config from pegleg.engine import util @@ -227,6 +228,9 @@ def slurp(path): with open(path) as f: try: + # Ignore YAML tags, only construct dicts + SafeConstructor.add_multi_constructor( + '', lambda loader, suffix, node: None) return yaml.safe_load(f) except Exception as e: raise click.ClickException('Failed to parse %s:\n%s' % (path, e)) @@ -277,10 +281,14 @@ def read(path): document) with open(path) as stream: + # Ignore YAML tags, only construct dicts + SafeConstructor.add_multi_constructor( + '', lambda loader, suffix, node: None) try: return [ d for d in yaml.safe_load_all(stream) - if is_deckhand_document(d) or is_pegleg_managed_document(d) + if d and (is_deckhand_document(d) or + is_pegleg_managed_document(d)) ] except yaml.YAMLError as e: raise click.ClickException('Failed to parse %s:\n%s' % (path, e)) diff --git a/pegleg/engine/util/pegleg_secret_management.py b/pegleg/engine/util/pegleg_secret_management.py index f2e2671d..9feeef97 100644 --- a/pegleg/engine/util/pegleg_secret_management.py +++ b/pegleg/engine/util/pegleg_secret_management.py @@ -154,7 +154,7 @@ class PeglegSecretManagement(object): for doc in self.documents: # do not re-encrypt already encrypted data if doc.is_encrypted(): - doc_list.append(doc) + doc_list.append(doc.pegleg_document) continue # only encrypt if storagePolicy is set to encrypted. diff --git a/requirements.txt b/requirements.txt index 20cbd39f..f9a09151 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ gitpython click==6.7 jsonschema==2.6.0 -pyyaml==3.12 +pyyaml==5.1 cryptography==2.3.1 python-dateutil==2.7.3 diff --git a/tests/unit/engine/test_secrets.py b/tests/unit/engine/test_secrets.py index 520737c3..b2797ad8 100644 --- a/tests/unit/engine/test_secrets.py +++ b/tests/unit/engine/test_secrets.py @@ -18,13 +18,13 @@ from os import listdir import click import mock import pytest -import yaml import tempfile +import yaml from pegleg import config -from pegleg.engine import secrets -from pegleg.engine.catalog import pki_utility from pegleg.engine.catalog.pki_generator import PKIGenerator +from pegleg.engine.catalog import pki_utility +from pegleg.engine import secrets from pegleg.engine.util import encryption as crypt, catalog, git from pegleg.engine.util import files from pegleg.engine.util.pegleg_managed_document import \ @@ -120,7 +120,7 @@ data: {0}-password def test_pegleg_secret_management_constructor(): - test_data = yaml.load(TEST_DATA) + test_data = yaml.safe_load(TEST_DATA) doc = PeglegManagedSecretsDocument(test_data) assert doc.is_storage_policy_encrypted() assert not doc.is_encrypted() @@ -157,6 +157,18 @@ def test_pegleg_secret_management_constructor_with_invalid_arguments(): 'specified.' in str(err_info.value) +@mock.patch.dict(os.environ, { + ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', + ENV_SALT: 'MySecretSalt1234567890][' +}) +def test_pegleg_secret_management_double_encrypt(): + encrypted_doc = PeglegSecretManagement( + docs=[yaml.safe_load(TEST_DATA)]).get_encrypted_secrets()[0][0] + encrypted_doc_2 = PeglegSecretManagement( + docs=[encrypted_doc]).get_encrypted_secrets()[0][0] + assert encrypted_doc == encrypted_doc_2 + + @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', ENV_SALT: 'MySecretSalt1234567890][' @@ -236,7 +248,8 @@ def test_generate_pki_using_local_repo_path(create_tmp_deployment_files): repo_path = str(git.git_handler(TEST_PARAMS["repo_url"], ref=TEST_PARAMS["repo_rev"])) with mock.patch.dict(config.GLOBAL_CONTEXT, {"site_repo": repo_path}): - pki_generator = PKIGenerator(duration=365, sitename=TEST_PARAMS["site_name"]) + pki_generator = PKIGenerator(duration=365, + sitename=TEST_PARAMS["site_name"]) generated_files = pki_generator.generate() assert len(generated_files), 'No secrets were generated' @@ -258,7 +271,8 @@ def test_check_expiry(create_tmp_deployment_files): repo_path = str(git.git_handler(TEST_PARAMS["repo_url"], ref=TEST_PARAMS["repo_rev"])) with mock.patch.dict(config.GLOBAL_CONTEXT, {"site_repo": repo_path}): - pki_generator = PKIGenerator(duration=365, sitename=TEST_PARAMS["site_name"]) + pki_generator = PKIGenerator(duration=365, + sitename=TEST_PARAMS["site_name"]) generated_files = pki_generator.generate() pki_util = pki_utility.PKIUtility(duration=0) diff --git a/tests/unit/engine/util/test_files.py b/tests/unit/engine/util/test_files.py index 5f13dd4b..eeb49390 100644 --- a/tests/unit/engine/util/test_files.py +++ b/tests/unit/engine/util/test_files.py @@ -20,6 +20,7 @@ import yaml from pegleg import config from pegleg.engine.util import files from tests.unit.fixtures import create_tmp_deployment_files +from tests.unit.fixtures import temp_path class TestFileHelpers(object): @@ -65,3 +66,18 @@ def test_file_in_subdir(): assert not files.file_in_subdir("aaa/bbb/ccc.txt", "ccc") assert not files.file_in_subdir("aaa/bbb/ccc.txt", "bb") assert not files.file_in_subdir("aaa/bbb/../ccc.txt", "bbb") + + +def test_read(temp_path): + # This will throw an error if yaml attempts to read the tag. + with open(os.path.join(temp_path, "invalid.yaml"), "w") as invalid_yaml: + invalid_yaml.write("!!python/name:fake_class''\n") + files.read(os.path.join(temp_path, "invalid.yaml")) + + # Under PyYAML's default behavior, the tag !!python/name:builtins.int + # will be parsed into the method int. files.read should ignore this tag. + with open(os.path.join(temp_path, "valid.yaml"), "w") as valid_yaml: + valid_yaml.write("!!python/name:builtins.int ''\n") + read_files = files.read(os.path.join(temp_path, "valid.yaml")) + # Assert that the tag was not parsed into the method int + assert int not in read_files