Fixes #38332 - Remove host key from known_hosts before first Ansible job

[nofaralfasi]

Background & Motivation:
Foreman tracks which Smart Proxy executes remote jobs for each host. On first execution, it attempts to remove any existing SSH host key for that host from /usr/share/foreman-proxy/.ssh/known_hosts to avoid key mismatch issues.

However, the cleanup mechanism only runs when the first job uses the script provider.
If the first job uses Ansible, the cleanup is skipped, but the job is still recorded as the host's first execution.

As a result, if a script job is run later against that same host, the SSH connection may fail due to a stale known_hosts entry - since the expected cleanup was already bypassed during the initial Ansible run.

This PR ensures that:
Ansible-based executions properly participate in the host key cleanup mechanism during first execution, aligning the behavior with script-based jobs and preventing subsequent SSH key mismatch errors.

[adamruzicka]

I'd be willing to let the parameter override slide, but we need to handle all the hosts in the inventory.

[adamruzicka]

Haven't tested yet, but it looks reasonable

[adamruzicka]

Would it be possible to use the reassembled ansible inventory as stored in @inventory instead of pulling things out of the raw input? Not a hard requirement though

[nofaralfasi]

I chose to use input.values instead of @inventory because the latter doesn't include the first_execution parameter.
While extracting this value from input.values for each host adds a bit of overhead, it seems necessary in order to access first_execution.
Do you think there’s a cleaner or more efficient way to handle this?

[ofedoren]

Does this mean that other parameters used in prune_known_hosts_on_first_execution are available through @inventory? If so, and just one first_execution param is missing, wouldn't it be logical to include that parameter there instead of rebuilding kind of the same information?

[nofaralfasi]

Yes, the other parameters used in prune_known_hosts_on_first_execution are available through @inventory. However, I’m not sure I fully follow, I’m not really rebuilding anything. I’m simply using the original input.values because @inventory doesn’t include the first_execution field for each host.

[nofaralfasi]

I added the following method to resolve this:
https://github.com/theforeman/smart_proxy_ansible/pull/99/files#diff-65b556454b9717ae33c08487e6ace03409dadb3c66b54f921b1dee1caccfb142R334-R348

[ekohl]

It really helps if the commit message itself and the PR description explain why something is needed. Today we use Redmine, but I think it's a good practice to maintain a good git log.

Security wise, doesn't this mean that on every single job you wipe the known SSH keys? In other words, would this effectively mean we run without any known host entries at all? If so, then why not bypass known hosts entirely and use /dev/null?

[nofaralfasi]

It really helps if the commit message itself and the PR description explain why something is needed. Today we use Redmine, but I think it's a good practice to maintain a good git log.

Updated.

Security wise, doesn't this mean that on every single job you wipe the known SSH keys? In other words, would this effectively mean we run without any known host entries at all? If so, then why not bypass known hosts entirely and use /dev/null?

We only remove the host key from known_hosts on the first execution for a given host and proxy combination. The goal is to prevent connection failures caused by stale SSH host keys, which can occur when a host is reprovisioned and its SSH keys are regenerated.

As for using /dev/null for known_hosts - that's effectively disabling SSH host key verification entirely, which is a much larger security tradeoff. This approach aims to balance safety and usability by avoiding false failures due to expected host key changes, while still maintaining SSH verification for all other cases.

[ekohl]

We only remove the host key from known_hosts on the first execution for a given host and proxy combination. The goal is to prevent connection failures caused by stale SSH host keys, which can occur when a host is reprovisioned and its SSH keys are regenerated.

Is there currently a job or mechanism that removes the known hosts key? For example, after a host is posting the "finish" step you can remove it.

Another mechanism you can consider is to optionally enhance the finish script to upload the known host keys and actively update the entries.

The last option is to implement KnownHostsCommand and have a command that actively queries Foreman. Scalability may be a concern as well as when Foreman is unavailable. Common solutions to that is a cache or actively pushing updates when Foreman changes so the Smart Proxy has a local database. This may conflict with FreeIPA/IdM which already maintains a database of host SSH keys.

[nofaralfasi]

Is there currently a job or mechanism that removes the known hosts key? For example, after a host is posting the "finish" step you can remove it.

Yes, the mechanism I’m using in this PR is here.

It follows the same approach used in the foreman_remote_execution plugin for SSH-based REX jobs — specifically:

• script_runner.rb
• utils.rb

This cleanup runs just before establishing the SSH connection, and only on first execution for a given host/proxy pair, to avoid stale key failures after reprovisioning.

Another mechanism you can consider is to optionally enhance the finish script to upload the known host keys and actively update the entries.

The last option is to implement KnownHostsCommand and have a command that actively queries Foreman. Scalability may be a concern as well as when Foreman is unavailable. Common solutions to that is a cache or actively pushing updates when Foreman changes so the Smart Proxy has a local database. This may conflict with FreeIPA/IdM which already maintains a database of host SSH keys.

Regarding the additional suggestions:

• A mechanism tied to the “finish” template could work, but it may not cover all use cases (e.g. reprovisioning outside of Foreman).
• The KnownHostsCommand approach would be more robust, but it adds complexity, especially with regards to availability, caching, and integration with other tools like FreeIPA/IdM, as you mentioned.

I definitely agree it’s worth exploring long-term improvements to this, but I’d prefer to keep this PR scoped to aligning Ansible-based jobs with the behavior we already have for script-based execution.

[stejskalleos]

@nofaralfasi, can you please respond to the comments from @ekohl

It looks like those are the last two things to settle before the merge, unless @ofedoren and @adamruzicka have other comments to add.

[stejskalleos]

QE: @shubhamsg199

[ekohl]

The inline code suggestions are all very minor. However, theforeman/smart_proxy_remote_execution_ssh@cdc55cd introduced prune_known_hosts! (landed in 0.5.0) so you need to update the dependency in the gemspec.

Other than that I think it looks sensible.

[ekohl]

This still needs to be modified to ~> 0.5.

smart_proxy_ansible/smart_proxy_ansible.gemspec

Line 32 in 2231534

gem.add_runtime_dependency('smart_proxy_remote_execution_ssh', '~> 0.4')

[adamruzicka]

This approach aims to balance safety and usability by avoiding false failures due to expected host key changes, while still maintaining SSH verification for all other cases.

Just to be extra clear, the "all other cases" really means just when using the script. Pull doesn't use ssh keys at all and ansible ignores known hosts altogether by default due to https://github.com/theforeman/puppet-foreman_proxy/blob/master/manifests/plugin/ansible/params.pp#L10

[adamruzicka]

In general looks good.

I'm a bit unhappy about this doing more than what we do in sp-rex-ssh, but in here we have much more information about the host.z

[shubhamsg199]

Tested the changes and is working as expected. A comment is added to the SAT-27377