Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #37803 - Remove hardcoded ProxyCommand #845

Merged

Conversation

adamlazik1
Copy link
Contributor

@adamlazik1 adamlazik1 commented Sep 10, 2024

Previously, we added a hardcoded ProxyCommand=none because ipa-client-install added
ProxyCommand /usr/bin/sss_ssh_knownhostsproxy -p %p %h into /etc/ssh/ssh_config, which caused failure to execute ansible commands on systems without the /sbin/nologin shell. 1 However; this also prevents users from using their own jump host in the ssh configuration since the hardcoded command line arguments always take precedence.

Since this issue was fixed in the ipa tooling 3 years ago (they now use the Match exec true rule)2, I propose we remove the hardcoded ProxyCommand to allow users to specify their own jump hosts. The same is being done for remote execution. 3

Some users who have configured the ipa client before the fix landed in ipa might still report that they are getting errors when trying to run ansible commands because the ProxyCommand specified in etc/ssh/ssh_config is failing to execute. We should suggest these users to remove the ProxyCommand from ssh config, which should fix all of their issues originating from this. This is more of a problem of the old ipa tooling rather than a problem of foreman.

Footnotes

  1. https://projects.theforeman.org/issues/25481

  2. https://pagure.io/freeipa/issue/7676

  3. https://github.com/theforeman/smart_proxy_remote_execution_ssh/pull/117

Previously, we added a hardcoded `ProxyCommand=none` because
ipa-client-install added
`ProxyCommand /usr/bin/sss_ssh_knownhostsproxy -p %p %h` into
`/etc/ssh/ssh_config`, which caused failure to execute ansible commands
on systems without the `/sbin/nologin` shell [1]. However; this also
prevents users from using their own jump host in the ssh configuration
since the hardcoded command line arguments always take precedence.

Since this issue was fixed in the ipa tooling 3 years ago (they now use
the `Match exec true` rule [2]), I propose we remove the hardcoded
ProxyCommand to allow users to specify their own jump hosts. The same is
being done for remote execution [3].

Some users who have configured the ipa client before the fix landed in
ipa might still report that they are getting errors when trying to run
ansible commands because the ProxyCommand specified in
`etc/ssh/ssh_config` is failing to execute. We should suggest these
users to remove the ProxyCommand from ssh config, which should fix all
of their issues originating from this. This is more of a problem of the
old ipa tooling rather than a problem of foreman.

[1] https://projects.theforeman.org/issues/25481
[2] https://pagure.io/freeipa/issue/7676
[3] theforeman/smart_proxy_remote_execution_ssh#117
@adamlazik1 adamlazik1 force-pushed the refs-36456-remove-hardcoded-proxycommand branch from 9239feb to a6eb9e2 Compare September 17, 2024 11:17
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of forgot about this because of PTO and all, but I think we can merge this. It should be noted that it'll need an installer migration for existing installations.

@ekohl ekohl merged commit 51dd80f into theforeman:master Oct 28, 2024
16 of 19 checks passed
@adamlazik1 adamlazik1 deleted the refs-36456-remove-hardcoded-proxycommand branch October 29, 2024 10:09
@ekohl
Copy link
Member

ekohl commented Nov 1, 2024

@adamlazik1 can you take a look at an installer migration? I can provide pointers if needed.

@adamlazik1
Copy link
Contributor Author

@ekohl I can take a look but it will probably take some time.

@ekohl
Copy link
Member

ekohl commented Nov 4, 2024

We have a helper to create the migration files in the right places. https://github.com/theforeman/foreman-installer/blob/develop/config/foreman.migrations/20201224125100_ansible_ssh_args.rb is probably a good place to start from.

You can decide how you want to handle it. One way is to see if -o ProxyCommand=none is included in the command and drop it, another is to only deal with it if it completely matches the default.

@adamruzicka probably also has some thoughts on what we want for our upgrading users.

adamlazik1 added a commit to adamlazik1/foreman-installer that referenced this pull request Nov 6, 2024
adamlazik1 added a commit to adamlazik1/foreman-installer that referenced this pull request Nov 15, 2024
ekohl pushed a commit to theforeman/foreman-installer that referenced this pull request Nov 21, 2024
ekohl pushed a commit to ekohl/foreman-installer that referenced this pull request Nov 21, 2024
ekohl pushed a commit to theforeman/foreman-installer that referenced this pull request Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants