Refs #38478 - Introduce SSH CA certificate support #126

adamlazik1 · 2025-06-05T14:49:26Z

No description provided.

adamlazik1 · 2025-06-05T14:49:40Z

Only drafting stage.

lib/smart_proxy_remote_execution_ssh/multiplexed_ssh_connection.rb

lib/smart_proxy_remote_execution_ssh/plugin.rb

lib/smart_proxy_remote_execution_ssh/multiplexed_ssh_connection.rb

adamruzicka · 2025-06-09T07:44:29Z

lib/smart_proxy_remote_execution_ssh/multiplexed_ssh_connection.rb

+      ssh_options << "-o CertificateFile=#{@client_ca_cert_file}" if @use_ssh_certificates && @client_ca_cert_file
      ssh_options << "-o IdentitiesOnly=yes"
      ssh_options << "-o StrictHostKeyChecking=accept-new"
-      ssh_options << "-o UserKnownHostsFile=#{prepare_known_hosts}" if @host_public_key
+      ssh_options << "-o UserKnownHostsFile=#{@client_ca_known_hosts_file}" if @use_ssh_certificates && @client_ca_known_hosts_file
+      ssh_options << "-o UserKnownHostsFile=#{prepare_known_hosts}" if !@use_ssh_certificates && @host_public_key


If client_ca_cert_file are client_ca_known_hosts_file are set, but the client doesn't accept them, will it fallback to traditional non-ca pubkey authentication?

I will need to test this once I have the ssh cert setup in a functional state.

It appears that the known hosts file does not need to exist at all, but the authentication seems to fail if the client does not accept the certificate. I will do some further testing, but I would say that authentication attempt failing upon not accepting the certificate is the desired outcome in regards to security.

adamlazik1 · 2025-06-19T14:56:39Z

I will keep this in draft because there will be four PRs in total that should get merged at roughly the same time, but I do believe that this is now ready for review.

adamlazik1 · 2025-07-11T06:41:47Z

lib/smart_proxy_remote_execution_ssh.rb

+      def cert_file
+        File.expand_path("#{private_key_file}-cert.pub")
+      end


Should we require this strict naming convention for cert file
similarly as we do for public key file, or should we make this an editable
value in settings, with this format being the possible default?

adamlazik1 · 2025-07-11T06:46:05Z

lib/smart_proxy_remote_execution_ssh/multiplexed_ssh_connection.rb

+      if @host_public_key
+        ssh_options << "-o UserKnownHostsFile=#{prepare_known_hosts}"
+      elsif @client_ca_known_hosts_file
+        ssh_options << "-o UserKnownHostsFile=#{@client_ca_known_hosts_file}"


If we have a known_hosts file with cert authorities listed, should we change StrictHostKeyChecking to yes? To me, it doesn't make much sense to have it otherwise.

IMO yes. Otherwise, we get "if you trust this cert, connect and if you don't, connect anyway".

For host keys, those should be all the combinations that can happen:

Proxy Host Result

No record for host plain SSH key TOFU - success + save key

Record for host plain SSH key, matching record success

Record for host plain SSH key, not matching record failure

CA cert plain SSH key ~~Fallback to rows 1-3~~ failure

CA cert trusted certificate success

CA cert untrusted certificate failure

No record for host, no CA cert untrusted certificate Fallback to row 1

Record for host, no CA cert untrusted certificate Fallback to rows 2-3

Could we agree that this table describes the expected behaviour?

One could argue that proxy expecting a cert and host not providing any (row 4) should fail rather than falling back to traditional pubkey auth.

I too would say that row 4 could fail, but there is currently option to only have one host CA per smart proxy so it would be setting behavior for all hosts that use that proxy for REX. That is probably the only point that I see why it should fallback to plain SSH key instead of fail.

Until we decide on one or the other, I will treat the expected behavior of row 4 to be failure. I updated the StrictHostKeyChecking param accordingly. I will also make the same edits to ansible.

So row 4 should indeed result in a failure, added two more rows to the table describing one other case that we previously missed.

I can confirm that rows 7 and 8 also behave as intended, so the behavior of the whole feature should work as expected.

lhellebr · 2025-07-11T09:47:24Z

lib/smart_proxy_remote_execution_ssh/multiplexed_ssh_connection.rb

+      @client_ca_known_hosts_file = settings.ssh_ca_known_hosts_file
+      @client_ca_public_key_file = settings.ssh_user_ca_public_key_file
+      @client_ca_cert_file = settings.ssh_ca_cert_file
+      @client_cert_file = Proxy::RemoteExecution::Ssh.cert_file


Why do you treat these inconsistently? There is also Proxy::RemoteExecution::Ssh.ca_public_key_file but you don't use it. And for the others, there is no such method.

It relates to my question to the comment above: #126 (comment)

cert_file is, for now, a non-editable value whose name depends on the private key file, so it is not in settings. Personally, I do not see any problem to making the cert file name an editable value, I just wanted to confirm first whether there are any objections against it. For example, public key file name is also a non-editable value and depends on the private key file.

lib/smart_proxy_remote_execution_ssh/multiplexed_ssh_connection.rb

lib/smart_proxy_remote_execution_ssh/plugin.rb

adamlazik1 · 2025-07-21T09:05:31Z

Switching back to draft since the feature got postponed to 3.16

lhellebr · 2025-12-03T11:48:25Z

I have been able to verify the SSH CA cert feature as a complex of the following 4 PRs:
#126
theforeman/foreman#10571
theforeman/foreman_remote_execution#977
theforeman/puppet-foreman_proxy#867

Including the following use cases:
SSH REX
Ansible REX
Pull mode REX
Cockpit

Including hosts created by:
Global registration
Provisioning

Including scenarios:
SSH CA on Satellite side
SSH CA on host side
SSH CA on both sides

Both positive scenarios and negative scenarios, that is incorrect CA, incorrect cert, incorrect principal.

=> ACK to this PR

adamruzicka · 2025-12-17T15:26:55Z

@adamlazik1 could you please sort out rubocop and tests and then undraft?

adamruzicka

one last nitpick, otherwise looking good

adamruzicka · 2025-12-18T14:38:19Z

lib/smart_proxy_remote_execution_ssh/multiplexed_ssh_connection.rb

      ssh_options << "-o User=#{@ssh_user}"
      ssh_options << "-o Port=#{@ssh_port}" if @ssh_port
      ssh_options << "-o IdentityFile=#{@client_private_key_file}" if @client_private_key_file
+      ssh_options << "-o CertificateFile=#{@client_cert_file}" if @client_cert_file


if @client_cert_file

won't this always evaluate to true because Proxy::RemoteExecution::Ssh.cert_file always returns a path, even if doesn't point at an actual file?

That is true, and also it definitely should not work like that. I Fixed the cert_file method to only return path if file is present.

adamruzicka · 2025-12-19T12:20:21Z

lib/smart_proxy_remote_execution_ssh.rb

+          { ca_public_key_file: 'CA public key', cert_file: 'certificate' }.each do |file, label|
+            file_path = public_send(file)
+            unless file_path && File.exist?(file_path)
+              raise "SSH #{label} file '#{file_path}' doesn't exist"
+            end
+          end


With the recent changes to how ca_public_key_file setting is handled (possibly returning nil), you can run into errors like this if you only set ssh_user_ca_public_key_file

2025-12-19T13:17:30 [W] Error details for Couldn't enable 'script': <RuntimeError>: SSH certificate file '' doesn't exist /Users/aruzicka/vcs/foreman/smart-proxy-remote-execution-ssh/lib/smart_proxy_remote_execution_ssh.rb:75:in `block in validate_ssh_settings!'

it is not technically wrong, but it looks odd

Thanks for catching it, I decided to handle client_cert_file differently.

adamruzicka

Looks good

adamruzicka · 2025-12-19T13:09:32Z

Thank you @adamlazik1 & @lhellebr !

pr-processor bot added the Not yet reviewed label Jun 5, 2025

adamruzicka reviewed Jun 9, 2025

View reviewed changes

pr-processor bot removed the Not yet reviewed label Jun 9, 2025

adamlazik1 force-pushed the ssh-cert-support branch 2 times, most recently from 3103fb7 to ca0debd Compare June 10, 2025 10:30

adamlazik1 mentioned this pull request Jun 16, 2025

Fixes #38499 - Introduce SSH cert support theforeman/foreman#10571

Open

adamlazik1 force-pushed the ssh-cert-support branch 3 times, most recently from 10ac419 to dd262da Compare June 19, 2025 14:53

adamlazik1 mentioned this pull request Jun 26, 2025

Introduce SSH cert support theforeman/puppet-foreman_proxy#867

Open

adamlazik1 force-pushed the ssh-cert-support branch 3 times, most recently from 1702658 to d3f27fa Compare July 11, 2025 06:46

adamlazik1 marked this pull request as ready for review July 11, 2025 06:47

adamlazik1 commented Jul 11, 2025

View reviewed changes

lhellebr reviewed Jul 11, 2025

View reviewed changes

lib/smart_proxy_remote_execution_ssh/multiplexed_ssh_connection.rb Outdated Show resolved Hide resolved

lhellebr reviewed Jul 11, 2025

View reviewed changes

lib/smart_proxy_remote_execution_ssh/plugin.rb Show resolved Hide resolved

adamlazik1 force-pushed the ssh-cert-support branch 2 times, most recently from c3b0ef1 to f8928c2 Compare July 14, 2025 14:56

adamlazik1 marked this pull request as draft July 21, 2025 09:05

adamlazik1 force-pushed the ssh-cert-support branch from f8928c2 to 179c7a5 Compare October 22, 2025 11:50

adamlazik1 force-pushed the ssh-cert-support branch from 179c7a5 to 01350ad Compare November 11, 2025 09:55

lhellebr approved these changes Dec 3, 2025

View reviewed changes

lhellebr mentioned this pull request Dec 3, 2025

Fixes #38478 - Introduce SSH CA certificate support theforeman/foreman_remote_execution#977

Open

adamlazik1 force-pushed the ssh-cert-support branch from 01350ad to 56dec0f Compare December 18, 2025 09:36

adamlazik1 marked this pull request as ready for review December 18, 2025 09:37

adamlazik1 force-pushed the ssh-cert-support branch 2 times, most recently from 6580cbf to 99d382c Compare December 18, 2025 14:36

adamruzicka reviewed Dec 18, 2025

View reviewed changes

adamlazik1 force-pushed the ssh-cert-support branch from 99d382c to afc3804 Compare December 18, 2025 15:05

adamruzicka reviewed Dec 19, 2025

View reviewed changes

Refs #38478 - Introduce SSH CA certificate support

f5f5cc4

adamlazik1 force-pushed the ssh-cert-support branch from afc3804 to f5f5cc4 Compare December 19, 2025 12:37

adamruzicka approved these changes Dec 19, 2025

View reviewed changes

adamruzicka merged commit 179bae6 into theforeman:master Dec 19, 2025
11 checks passed

adamlazik1 deleted the ssh-cert-support branch December 19, 2025 13:12

Proxy	Host	Result
No record for host	plain SSH key	TOFU - success + save key
Record for host	plain SSH key, matching record	success
Record for host	plain SSH key, not matching record	failure
CA cert	plain SSH key	~~Fallback to rows 1-3~~ failure
CA cert	trusted certificate	success
CA cert	untrusted certificate	failure
No record for host, no CA cert	untrusted certificate	Fallback to row 1
Record for host, no CA cert	untrusted certificate	Fallback to rows 2-3

Refs #38478 - Introduce SSH CA certificate support #126

Refs #38478 - Introduce SSH CA certificate support #126

Uh oh!

Conversation

adamlazik1 commented Jun 5, 2025

Uh oh!

adamlazik1 commented Jun 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamlazik1 commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamruzicka Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamlazik1 Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamlazik1 Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adamlazik1 commented Jul 21, 2025

Uh oh!

lhellebr commented Dec 3, 2025

Uh oh!

adamruzicka commented Dec 17, 2025

Uh oh!

adamruzicka left a comment

Choose a reason for hiding this comment

Uh oh!

adamruzicka Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamruzicka Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamruzicka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adamruzicka commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

adamruzicka Jul 11, 2025 •

edited

Loading

adamlazik1 Jul 14, 2025 •

edited

Loading

adamlazik1 Jul 11, 2025 •

edited

Loading

adamruzicka Dec 18, 2025 •

edited

Loading

adamruzicka Dec 19, 2025 •

edited

Loading