From 721de9f10a02fd0bd4be73b125cbc02644104db8 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 3 Jan 2017 16:17:52 -0500 Subject: [PATCH 1/3] pass network ssh key via an env var, not ssh-agent --- awx/main/tasks.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 06a5288d6e..dc5b4fb77e 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -374,9 +374,12 @@ class BaseTask(Task): data += '\n' # For credentials used with ssh-add, write to a named pipe which # will be read then closed, instead of leaving the SSH key on disk. - if name in ('credential', 'network_credential', 'scm_credential', 'ad_hoc_credential') and not ssh_too_old: + if name in ('credential', 'scm_credential', 'ad_hoc_credential') and not ssh_too_old: path = os.path.join(kwargs.get('private_data_dir', tempfile.gettempdir()), name) self.open_fifo_write(path, data) + # Ansible network modules do not yet support ssh-agent. + # Instead, ssh private key file is explicitly passed via an + # env variable. else: handle, path = tempfile.mkstemp(dir=kwargs.get('private_data_dir', None)) f = os.fdopen(handle, 'w') @@ -875,6 +878,10 @@ class RunJob(BaseTask): env['ANSIBLE_NET_USERNAME'] = network_cred.username env['ANSIBLE_NET_PASSWORD'] = decrypt_field(network_cred, 'password') + ssh_keyfile = kwargs.get('private_data_files', {}).get('network_credential', '') + if ssh_keyfile: + env['ANSIBLE_NET_SSH_KEYFILE'] = ssh_keyfile + authorize = network_cred.authorize env['ANSIBLE_NET_AUTHORIZE'] = unicode(int(authorize)) if authorize: @@ -1037,8 +1044,15 @@ class RunJob(BaseTask): private_data_files = kwargs.get('private_data_files', {}) if 'credential' in private_data_files: return private_data_files.get('credential') - elif 'network_credential' in private_data_files: - return private_data_files.get('network_credential') + ''' + Note: Don't inject network ssh key data into ssh-agent for network + credentials because the ansible modules no not yet support it. + We will want to add back in support when/if Ansible network modules + support this. + ''' + #elif 'network_credential' in private_data_files: + # return private_data_files.get('network_credential') + return '' def should_use_proot(self, instance, **kwargs): From 57cbf536dd4e167f76b380acd811f7d0c507273b Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Wed, 4 Jan 2017 10:48:09 -0500 Subject: [PATCH 2/3] unit test update for network cred changes * Ensure network ssh key file set as env variable and remove tests that checked for ssh key being added to the keyring * Ensure that network credentials are set in the job run environment as well as saved to the job model. --- .../tests/unit/test_network_credential.py | 98 +++++++++++++------ 1 file changed, 67 insertions(+), 31 deletions(-) diff --git a/awx/main/tests/unit/test_network_credential.py b/awx/main/tests/unit/test_network_credential.py index dd6a953fb9..8cdf720af3 100644 --- a/awx/main/tests/unit/test_network_credential.py +++ b/awx/main/tests/unit/test_network_credential.py @@ -1,3 +1,5 @@ +import pytest + from awx.main.models.credential import Credential from awx.main.models.jobs import Job from awx.main.models.inventory import Inventory @@ -36,50 +38,84 @@ def test_net_cred_parse(mocker): 'password':'test', 'authorize': True, 'authorize_password': 'passwd', + 'ssh_key_data': """-----BEGIN PRIVATE KEY-----\nstuff==\n-----END PRIVATE KEY-----""", + } + private_data_files = { + 'network_credential': '/tmp/this_file_does_not_exist_during_test_but_the_path_is_real', } job.network_credential = Credential(**options) run_job = RunJob() mocker.patch.object(run_job, 'should_use_proot', return_value=False) - env = run_job.build_env(job, private_data_dir='/tmp') + env = run_job.build_env(job, private_data_dir='/tmp', private_data_files=private_data_files) assert env['ANSIBLE_NET_USERNAME'] == options['username'] assert env['ANSIBLE_NET_PASSWORD'] == options['password'] assert env['ANSIBLE_NET_AUTHORIZE'] == '1' assert env['ANSIBLE_NET_AUTH_PASS'] == options['authorize_password'] + assert env['ANSIBLE_NET_SSH_KEYFILE'] == private_data_files['network_credential'] -def test_net_cred_ssh_agent(mocker, get_ssh_version): - with mocker.patch('django.db.ConnectionRouter.db_for_write'): - run_job = RunJob() +@pytest.fixture +def mock_job(mocker): + options = { + 'username':'test', + 'password':'test', + 'ssh_key_data': """-----BEGIN PRIVATE KEY-----\nstuff==\n-----END PRIVATE KEY-----""", + 'authorize': True, + 'authorize_password': 'passwd', + } - options = { - 'username':'test', - 'password':'test', - 'ssh_key_data': """-----BEGIN PRIVATE KEY-----\nstuff==\n-----END PRIVATE KEY-----""", - 'authorize': True, - 'authorize_password': 'passwd', - } - mock_job_attrs = {'forks': False, 'id': 1, 'cancel_flag': False, 'status': 'running', 'job_type': 'normal', - 'credential': None, 'cloud_credential': None, 'network_credential': Credential(**options), - 'become_enabled': False, 'become_method': None, 'become_username': None, - 'inventory': mocker.MagicMock(spec=Inventory, id=2), 'force_handlers': False, - 'limit': None, 'verbosity': None, 'job_tags': None, 'skip_tags': None, - 'start_at_task': None, 'pk': 1, 'launch_type': 'normal', 'job_template':None, - 'created_by': None, 'extra_vars_dict': None, 'project':None, 'playbook': 'test.yml'} - mock_job = mocker.MagicMock(spec=Job, **mock_job_attrs) + mock_job_attrs = {'forks': False, 'id': 1, 'cancel_flag': False, 'status': 'running', 'job_type': 'normal', + 'credential': None, 'cloud_credential': None, 'network_credential': Credential(**options), + 'become_enabled': False, 'become_method': None, 'become_username': None, + 'inventory': mocker.MagicMock(spec=Inventory, id=2), 'force_handlers': False, + 'limit': None, 'verbosity': None, 'job_tags': None, 'skip_tags': None, + 'start_at_task': None, 'pk': 1, 'launch_type': 'normal', 'job_template':None, + 'created_by': None, 'extra_vars_dict': None, 'project':None, 'playbook': 'test.yml'} + mock_job = mocker.MagicMock(spec=Job, **mock_job_attrs) + return mock_job - mocker.patch.object(run_job, 'update_model', return_value=mock_job) - mocker.patch.object(run_job, 'build_cwd', return_value='/tmp') - mocker.patch.object(run_job, 'should_use_proot', return_value=False) - mocker.patch.object(run_job, 'run_pexpect', return_value=('successful', 0)) - mocker.patch.object(run_job, 'open_fifo_write', return_value=None) - mocker.patch.object(run_job, 'post_run_hook', return_value=None) - run_job.run(mock_job.id) - assert run_job.update_model.call_count == 4 +@pytest.fixture +def run_job_net_cred(mocker, get_ssh_version, mock_job): + mocker.patch('django.db.ConnectionRouter.db_for_write') + run_job = RunJob() + + mocker.patch.object(run_job, 'update_model', return_value=mock_job) + mocker.patch.object(run_job, 'build_cwd', return_value='/tmp') + mocker.patch.object(run_job, 'should_use_proot', return_value=False) + mocker.patch.object(run_job, 'run_pexpect', return_value=('successful', 0)) + mocker.patch.object(run_job, 'open_fifo_write', return_value=None) + mocker.patch.object(run_job, 'post_run_hook', return_value=None) + + return run_job + + +@pytest.mark.skip(reason="Note: Ansible network modules don't yet support ssh-agent added keys.") +def test_net_cred_ssh_agent(run_job_net_cred, mock_job): + run_job = run_job_net_cred + run_job.run(mock_job.id) + + assert run_job.update_model.call_count == 4 + + job_args = run_job.update_model.call_args_list[1][1].get('job_args') + assert 'ssh-add' in job_args + assert 'ssh-agent' in job_args + assert 'network_credential' in job_args + + +def test_net_cred_job_model_env(run_job_net_cred, mock_job): + run_job = run_job_net_cred + run_job.run(mock_job.id) + + assert run_job.update_model.call_count == 4 + + job_args = run_job.update_model.call_args_list[1][1].get('job_env') + assert 'ANSIBLE_NET_USERNAME' in job_args + assert 'ANSIBLE_NET_PASSWORD' in job_args + assert 'ANSIBLE_NET_AUTHORIZE' in job_args + assert 'ANSIBLE_NET_AUTH_PASS' in job_args + assert 'ANSIBLE_NET_SSH_KEYFILE' in job_args + - job_args = run_job.update_model.call_args_list[1][1].get('job_args') - assert 'ssh-add' in job_args - assert 'ssh-agent' in job_args - assert 'network_credential' in job_args From 55047a56439cc3e8db9e32ce6d089b114df62518 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Wed, 4 Jan 2017 11:25:39 -0500 Subject: [PATCH 3/3] letters, they matter --- awx/main/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index dc5b4fb77e..02af9239d9 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1046,7 +1046,7 @@ class RunJob(BaseTask): return private_data_files.get('credential') ''' Note: Don't inject network ssh key data into ssh-agent for network - credentials because the ansible modules no not yet support it. + credentials because the ansible modules do not yet support it. We will want to add back in support when/if Ansible network modules support this. '''