From fce12add18869b8482cf9c0ec9c513079aa90d37 Mon Sep 17 00:00:00 2001 From: Alexander Hughes Date: Tue, 12 Mar 2019 13:21:31 -0500 Subject: [PATCH] Set salt when generating genesis bundle This patch: 1. Sets the salt in config when running genesis bundle 2. Updates the genesis bundle CLI method 3. Adds exception types for credentials 4. Updates unit tests to be compliant with new exceptions Change-Id: I8869f897e2c25b98c30eaa6be52356aae4ac63b6 --- doc/source/images/architecture-pegleg.png | Bin 37604 -> 37604 bytes pegleg/cli.py | 29 ++++++------------ pegleg/config.py | 26 +++++++++++++--- pegleg/engine/exceptions.py | 28 +++++++++++++++++ .../unit/engine/test_build_genesis_bundle.py | 4 +-- .../unit/engine/test_generate_cryptostring.py | 4 +-- tests/unit/engine/test_secrets.py | 12 ++++---- .../unit/engine/util/test_shipyard_helper.py | 12 ++++++++ tests/unit/test_cli.py | 4 +-- 9 files changed, 82 insertions(+), 37 deletions(-) diff --git a/doc/source/images/architecture-pegleg.png b/doc/source/images/architecture-pegleg.png index acdfa920f86ced228cd6e88229af967446ed356b..bc99bc57d3bfb5339870e7b6bd2bb503e1d6c6d0 100644 GIT binary patch delta 525 zcmV+o0`mRjr2^!o0+4TiO>dh(5WTO&e;9GH4_J^oZOXxljNN<`RJSaCKbsyPx(xz0q+Wlxi0uF*hB^C3(L;-M@yhtBV~Z;W~) zFSy|uZ>%)cm{Ox1xnrdpc(5+|DIbgM2(m8OQfnQdWPbtAo4maMi71K+|8Q*|`!Ia~ zSg6D7X(^8$5~6p1p<^X6Hg~f4kUWh|mG{P9FD!RW#dzK;A)gS3^9a1DVEK@j@m za6E>GWdyc9WP_VU42^1aj!=lkXpyys2!`yQ?>PKwY29jn48cZ&Wio_xeoT_4ozO~J zee8afbWL!pOuAL((ENGqO$GgX)T9emNk_WsG=?Pd%gBGcxb(8%lF>?OZiFl$rJsX! z2(gguUpB)h-d5IuToqIfN&PqPip5-(EiXwMN){>7r1w-y*(?0f1l}u~zt8{xfB;EE PK~#9!?7a)KYXW(Y_+$n* delta 525 zcmV+o0`mRjr2^!o0+4TiU2obj6n(dp|8SKjcnAutY;7J;#YevuG&WQAfKcTmx2e^{ zMvlXVrv3L`lR&9dMY4Q9&hfb?_qwve*t#q{=01^HZNQ(-e2<~B((b{(TP_!8A^#bl zMZfv&z3-6EMwCcMOkZDw%6$0eF@W_%D{FMIfnIL}>zT@>!i~p&{+~e>m=~BI0Zjyx z_`b!oiBF+Fwy3xj=_^chxz&nPRY#(+Fo{fsYH2v81}$)!4`ED@09Cd*bbiNuW%WCG zCag$A<)rD#<_6u!4J+LsfOXbR`IIF`kaNkEYUc*?zgRc$bQZxB zw}sx9WNu4^#w|eB^m^=?&cg<=MIn@ZoX4<3Q%S9$-y4Met6tdeaADyF69})MKa9Q% zqVNJ9mlJUHF6-PbVyJX&QiM!a*2tu`M9^gqVkh8NP3u;FVgxSgEazR|vtyDp?WEDl z>0=M8dD8@U+VZWohvu&r!C2D2M?<<`HE&2)jmC&Xe(Qx#gJF>Lhm2Or2rE?%oPG|L z5yVo}f7ukDMO`=tQk_vbBK0@HC5wg1Ymt*SRBWP2li5>^vp3{|1jI-3J52xpfB;EE PK~#9!?7a)KYXW(YW_ks- diff --git a/pegleg/cli.py b/pegleg/cli.py index 52a6f8b7..f7275d35 100644 --- a/pegleg/cli.py +++ b/pegleg/cli.py @@ -434,26 +434,15 @@ def generate_pki(site_name, author): 'to genesis.sh script.') @SITE_REPOSITORY_ARGUMENT def genesis_bundle(*, build_dir, validators, site_name): - prom_encryption_key = os.environ.get("PROMENADE_ENCRYPTION_KEY") - peg_encryption_key = os.environ.get("PEGLEG_PASSPHRASE") - encryption_key = None - if (prom_encryption_key and len(prom_encryption_key) > 24 and - peg_encryption_key and len(peg_encryption_key) > 24): - click.echo("WARNING: PROMENADE_ENCRYPTION_KEY is deprecated, " - "using PEGLEG_PASSPHRASE instead", err=True) - config.set_passphrase(peg_encryption_key) - encryption_key = peg_encryption_key - elif prom_encryption_key and len(prom_encryption_key) > 24: - click.echo("ERROR: PROMENADE_ENCRYPTION_KEY is deprecated, " - "use PEGLEG_PASSPHRASE instead", err=True) - raise click.ClickException("ERROR: PEGLEG_PASSPHRASE must be set " - "and at least 24 characters long.") - elif peg_encryption_key and len(peg_encryption_key) > 24: - config.set_passphrase(peg_encryption_key) - encryption_key = peg_encryption_key - else: - raise click.ClickException("ERROR: PEGLEG_PASSPHRASE must be set " - "and at least 24 characters long.") + passphrase = os.environ.get("PEGLEG_PASSPHRASE") + salt = os.environ.get("PEGLEG_SALT") + encryption_key = passphrase + if passphrase: + passphrase = passphrase.encode() + if salt: + salt = salt.encode() + config.set_passphrase(passphrase) + config.set_salt(salt) PeglegSecretManagement.check_environment() bundle.build_genesis(build_dir, diff --git a/pegleg/config.py b/pegleg/config.py index a481e4f2..84f09814 100644 --- a/pegleg/config.py +++ b/pegleg/config.py @@ -16,6 +16,8 @@ # context passing but will require a somewhat heavy code refactor. See: # http://click.pocoo.org/5/commands/#nested-handling-and-contexts +from pegleg.engine import exceptions + try: if GLOBAL_CONTEXT: pass @@ -28,7 +30,9 @@ except NameError: 'site_rev': None, 'type_path': 'type', 'passphrase': None, - 'salt': None + 'salt': None, + 'salt_min_length': 24, + 'passphrase_min_length': 24 } @@ -151,9 +155,15 @@ def set_rel_type_path(p): GLOBAL_CONTEXT['type_path'] = p -def set_passphrase(p): +def set_passphrase(passphrase): """Set the passphrase for encryption and decryption.""" - GLOBAL_CONTEXT['passphrase'] = p + + if not passphrase: + raise exceptions.PassphraseNotFoundException() + elif len(passphrase) < GLOBAL_CONTEXT['passphrase_min_length']: + raise exceptions.PassphraseInsufficientLengthException() + + GLOBAL_CONTEXT['passphrase'] = passphrase def get_passphrase(): @@ -161,9 +171,15 @@ def get_passphrase(): return GLOBAL_CONTEXT['passphrase'] -def set_salt(p): +def set_salt(salt): """Set the salt for encryption and decryption.""" - GLOBAL_CONTEXT['salt'] = p + + if not salt: + raise exceptions.SaltNotFoundException() + elif len(salt) < GLOBAL_CONTEXT['salt_min_length']: + raise exceptions.SaltInsufficientLengthException() + + GLOBAL_CONTEXT['salt'] = salt def get_salt(): diff --git a/pegleg/engine/exceptions.py b/pegleg/engine/exceptions.py index d96f519d..8473f94a 100644 --- a/pegleg/engine/exceptions.py +++ b/pegleg/engine/exceptions.py @@ -102,3 +102,31 @@ class GenesisBundleGenerateException(PeglegBaseException): """ message = 'Bundle generation failed on deckhand validation.' + + +# +# CREDENTIALS EXCEPTIONS +# + +class PassphraseNotFoundException(PeglegBaseException): + """Exception raised when passphrase is not set.""" + + message = 'PEGLEG_PASSPHRASE must be set' + + +class PassphraseInsufficientLengthException(PeglegBaseException): + """Exception raised when passphrase is too short.""" + + message = 'PEGLEG_PASSPHRASE must be at least 24 characters long.' + + +class SaltNotFoundException(PeglegBaseException): + """Exception raised when salt is not set.""" + + message = 'PEGLEG_SALT must be set' + + +class SaltInsufficientLengthException(PeglegBaseException): + """Exception raised when salt is too short.""" + + message = 'PEGLEG_SALT must be at least 24 characters long.' diff --git a/tests/unit/engine/test_build_genesis_bundle.py b/tests/unit/engine/test_build_genesis_bundle.py index 21baba93..35b96f05 100644 --- a/tests/unit/engine/test_build_genesis_bundle.py +++ b/tests/unit/engine/test_build_genesis_bundle.py @@ -91,7 +91,7 @@ data: ABAgagajajkb839215387 @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt' + ENV_SALT: 'MySecretSalt1234567890][' }) def test_no_encryption_key(temp_path): # Write the test data to temp file @@ -119,7 +119,7 @@ def test_no_encryption_key(temp_path): @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt' + ENV_SALT: 'MySecretSalt1234567890][' }) def test_failed_deckhand_validation(temp_path): # Write the test data to temp file diff --git a/tests/unit/engine/test_generate_cryptostring.py b/tests/unit/engine/test_generate_cryptostring.py index 4e9576f3..d46de908 100644 --- a/tests/unit/engine/test_generate_cryptostring.py +++ b/tests/unit/engine/test_generate_cryptostring.py @@ -166,7 +166,7 @@ def test_cryptostring_long_len(): 'cicd_site_repo/site/cicd/passphrases/passphrase-catalog.yaml', ]) @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt'}) + ENV_SALT: 'MySecretSalt1234567890]['}) def test_generate_passphrases(*_): _dir = tempfile.mkdtemp() os.makedirs(os.path.join(_dir, 'cicd_site_repo'), exist_ok=True) @@ -239,7 +239,7 @@ def test_generate_passphrases_exception(capture): 'cicd_global_repo/site/cicd/passphrases/passphrase-catalog.yaml', ]) @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt'}) + ENV_SALT: 'MySecretSalt1234567890]['}) def test_global_passphrase_catalog(*_): _dir = tempfile.mkdtemp() os.makedirs(os.path.join(_dir, 'cicd_site_repo'), exist_ok=True) diff --git a/tests/unit/engine/test_secrets.py b/tests/unit/engine/test_secrets.py index cfe147c9..703d42d5 100644 --- a/tests/unit/engine/test_secrets.py +++ b/tests/unit/engine/test_secrets.py @@ -74,7 +74,7 @@ def test_encrypt_and_decrypt(): @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'aShortPassphrase', - ENV_SALT: 'MySecretSalt' + ENV_SALT: 'MySecretSalt1234567890][' }) def test_short_passphrase(): with pytest.raises(click.ClickException, @@ -84,7 +84,7 @@ def test_short_passphrase(): @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt'}) + ENV_SALT: 'MySecretSalt1234567890]['}) def test_secret_encrypt_and_decrypt(create_tmp_deployment_files, tmpdir): site_dir = tmpdir.join("deployment_files", "site", "cicd") passphrase_doc = """--- @@ -160,7 +160,7 @@ def test_pegleg_secret_management_constructor_with_invalid_arguments(): @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt' + ENV_SALT: 'MySecretSalt1234567890][' }) def test_encrypt_decrypt_using_file_path(temp_path): # write the test data to temp file @@ -190,7 +190,7 @@ def test_encrypt_decrypt_using_file_path(temp_path): @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt' + ENV_SALT: 'MySecretSalt1234567890][' }) def test_encrypt_decrypt_using_docs(temp_path): # write the test data to temp file @@ -226,7 +226,7 @@ def test_encrypt_decrypt_using_docs(temp_path): reason='cfssl must be installed to execute these tests') @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt' + ENV_SALT: 'MySecretSalt1234567890][' }) def test_generate_pki_using_local_repo_path(create_tmp_deployment_files): """Validates ``generate-pki`` action using local repo path.""" @@ -252,7 +252,7 @@ def test_generate_pki_using_local_repo_path(create_tmp_deployment_files): reason='cfssl must be installed to execute these tests') @mock.patch.dict(os.environ, { ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', - ENV_SALT: 'MySecretSalt' + ENV_SALT: 'MySecretSalt1234567890][' }) def test_check_expiry(create_tmp_deployment_files): """ Validates check_expiry """ diff --git a/tests/unit/engine/util/test_shipyard_helper.py b/tests/unit/engine/util/test_shipyard_helper.py index ac5c3166..9e6ade57 100644 --- a/tests/unit/engine/util/test_shipyard_helper.py +++ b/tests/unit/engine/util/test_shipyard_helper.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os + import json import mock import pytest @@ -20,6 +22,8 @@ from tests.unit import test_utils from mock import ANY from pegleg.engine import util +from pegleg.engine.util.pegleg_secret_management import ENV_PASSPHRASE +from pegleg.engine.util.pegleg_secret_management import ENV_SALT from pegleg.engine.util.shipyard_helper import ShipyardHelper from pegleg.engine.util.shipyard_helper import ShipyardClient @@ -89,6 +93,10 @@ def test_shipyard_helper_init_(): return_value=DATA) @mock.patch.object(ShipyardHelper, 'formatted_response_handler', autospec=True, return_value=None) +@mock.patch.dict(os.environ, { + ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', + ENV_SALT: 'MySecretSalt1234567890][' +}) def test_upload_documents(*args): """ Tests upload document """ # Scenario: @@ -115,6 +123,10 @@ def test_upload_documents(*args): return_value=DATA) @mock.patch.object(ShipyardHelper, 'formatted_response_handler', autospec=True, return_value=None) +@mock.patch.dict(os.environ, { + ENV_PASSPHRASE: 'ytrr89erARAiPE34692iwUMvWqqBvC', + ENV_SALT: 'MySecretSalt1234567890][' +}) def test_upload_documents_fail(*args): """ Tests Document upload error """ # Scenario: diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 10a45cf8..68ab8841 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -459,7 +459,7 @@ class TestSiteSecretsActions(BaseCLIActionTest): super(TestSiteSecretsActions, cls).setup_class() cls.runner = CliRunner(env={ "PEGLEG_PASSPHRASE": 'ytrr89erARAiPE34692iwUMvWqqBvC', - "PEGLEG_SALT": "MySecretSalt" + "PEGLEG_SALT": "MySecretSalt1234567890][" }) def _validate_generate_pki_action(self, result): @@ -514,7 +514,7 @@ class TestSiteSecretsActions(BaseCLIActionTest): reason='cfssl must be installed to execute these tests') @mock.patch.dict(os.environ, { "PEGLEG_PASSPHRASE": "123456789012345678901234567890", - "PEGLEG_SALT": "123456" + "PEGLEG_SALT": "MySecretSalt1234567890][" }) def test_site_secrets_encrypt_and_decrypt_local_repo_path(self): """Validates ``generate-pki`` action using local repo path."""