Fixes #24367 - add support for passphrased keys #417
Conversation
|
the rubocop tells me to use |
| --- | ||
| "for.*/artifacts/.*/ssh_key_data:": "#{@passphrase}" | ||
| SECRETS | ||
| File.write(passwords_path, secrets) |
There was a problem hiding this comment.
Could we pass the permissions here so there's not even a slightest amount of time during which the file is world readable?
There was a problem hiding this comment.
In fact this may not even be necessary, the /tmp/d2154..../ is accesible only for the owner, I just wanted to make sure auditors won't report this as an issue. But yeah, I can add the permissions too.
| File.write(passwords_path, '') | ||
| File.chmod(0o0600, passwords_path) | ||
| File.write(passwords_path, secrets) |
There was a problem hiding this comment.
I meant something along the lines of File.write(passwords_path, secrets, perm: 0o600) but this should work as well
| @@ -106,6 +108,22 @@ def write_playbook | |||
| File.write(File.join(@root, 'project', 'playbook.yml'), @playbook) | |||
| end | |||
|
|
|||
| def write_ssh_key | |||
| key_path = File.join(@root, 'env', 'ssh_key') | |||
| File.symlink(ForemanRemoteExecutionCore.settings[:ssh_identity_key_file], key_path) | |||
There was a problem hiding this comment.
Key file can be specified on a per-host basis.
If I'm reading https://ansible-runner.readthedocs.io/en/stable/intro.html#env-ssh-key right, then ansible-runner doesn't support providing more keys through the env directory and putting a key there overrides the per-host settings. That would also mean this would break the different keys for different hosts use case completely, no matter if a passphrase is used or not.
| @@ -14,12 +14,14 @@ def initialize(input, suspended_action:) | |||
| @verbosity_level = action_input[:verbosity_level] | |||
| @rex_command = action_input[:remote_execution_command] | |||
| @check_mode = action_input[:check_mode] | |||
| @passphrase = action_input['secrets']['key_passphrase'] | |||
There was a problem hiding this comment.
Passphrase can be provided on a per-host basis
| passwords_path = File.join(@root, 'env', 'passwords') | ||
| secrets = <<~SECRETS | ||
| --- | ||
| "for.*/artifacts/.*/ssh_key_data:": "#{@passphrase}" |
There was a problem hiding this comment.
Is this supposed to be yaml? Shouldn't we build a hash and YAML.dump it to avoid escaping issues?
| File.write(passwords_path, '') | ||
| File.chmod(0o0600, passwords_path) | ||
| File.write(passwords_path, secrets) |
There was a problem hiding this comment.
You need to create the file with the right permissions, as otherwise an attacker just needs to wait for your create to happen, acquire a file handle (at this moment the permissions might be OK), and wait for you to write data to it.
The chmod won't have any influence on the already open handle.
SSH keys protected by the passphrase is not possible to use with pure Ansible. However ansible-runner provides a way to specify the password through env/passwords file. For that, the ssh key also needs to be placed under env/ssh_key (symlink is enough). We already have a passphrase field in REX Job invocation object, but until now we ignored that in Ansible jobs. This patch starts respecting the passphrase for Ansible jobs too. First it parses the passphrase from the action input. During the ansible-runner directory preparation, it also creates the env directory. It symlinks the foreman-proxy key in there and if a passphrase was specified for the Job invocation, it saves it into the env/passwords file. The ansible-runner limitation of a single key means, we can not support multiple keys with multiple passphrases stored as Host Parameters. The feature supports only the passphrase specified on the job level, the same applies to the SSH key. The ansible-runner secrets is only used, if the passphrase is specified for the job. Ansible-runner has a bug that we can't easily specify the env/passwords entry for the ssh-key. The regular expression is only matched with last 100 characters of the line. We can use '^Enter' in the regexp for that reason. Also the prompt typically contain the full path to the private key, which is always dynamic. Therefore the regular expression only contains words artifacts and ssh_key_data: See ansible/ansible-runner#533 for more details. The functionality is documented at * https://ansible-runner.readthedocs.io/en/stable/intro.html#env-ssh-key * https://ansible-runner.readthedocs.io/en/stable/intro.html#env-passwords
|
As discussed online, we'll support only single passphrase for a single key, the whole logic gets activated only if the passphrase is specified. With the file permissions you're of course right. It's kind of academic, because it's all in the private tmp directory. But I improved the code to be more self-explanatory. |
|
Sure, it's a bit academic, but if you're already setting perms, we can do it right ;) |
|
closing in favor of theforeman/smart_proxy_ansible#35, the code has been migrated to other repo meanwhile |
|
the updated patch is at ares@057617b if anyone needs to apply it to the older foreman_ansible (prior core extraction) |
SSH keys protected by the passphrase is not possible to use with pure
Ansible. However ansible-runner provides a way to specify the password
through env/passwords file. For that, the ssh key also needs to be
placed under env/ssh_key (symlink is enough). We already have a
passphrase field in REX Job invocation object, but until now we ignored
that in Ansible jobs.
This patch starts respecting the passphrase for Ansible jobs too. First
it parses the passphrase from the action input. During the
ansible-runner directory preparation, it also creates the env directory.
It symlinks the foreman-proxy key in there and if a passphrase was
specified for the Job invocation, it saves it into the env/passwords
file.
Ansible-runner has a bug that we can't easily specify the env/passwords
entry for the ssh-key. The regular expression is only matched with last
100 characters of the line. We can use '^Enter' in the regexp for that
reason. Also the prompt typically contain the full path to the private
key, which is always dynamic. Therefore the regular expression only
contains words artifacts and ssh_key_data: See
ansible/ansible-runner#533 for more details.
The functionality is documented at