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

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

Draft

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

Draft

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

Refs #38478 - Introduce SSH CA certificate support

01350ad

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

Draft

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

Are you sure you want to change the base?

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adamruzicka Jul 11, 2025 •

edited

Loading

adamlazik1 Jul 14, 2025 •

edited

Loading

adamlazik1 Jul 11, 2025 •

edited

Loading