Fixes #39657 - drop sshkey gem and replace with ssh-keygen cmd - #11185
Fixes #39657 - drop sshkey gem and replace with ssh-keygen cmd#11185lzap wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Nit:
In config/initializers/inflections.rb is the following comment:
# Causes an overlap between ::SSHKey and the SshKey model
# inflect.acronym 'SSH' # Secure SHell
Is that still relevant, or can we remove it?
There was a problem hiding this comment.
It is still relevant, I thought about either renaming the package/namespace or leaving the inflection as-is.
| def self.generate(type: 'rsa', comment: '', bits: nil) | ||
| Dir.mktmpdir('foreman-ssh-key') do |dir| | ||
| path = File.join(dir, 'key') | ||
| args = ['ssh-keygen', '-t', type.to_s, '-N', '', '-C', comment.to_s, '-f', path, '-q'] |
There was a problem hiding this comment.
Idea: What if we include -v verbose mode when Rails is running in development?
There was a problem hiding this comment.
Sure, actually I quite like the opposite because keygen is quite verbose by default so adding a quite flag for PROD.
| # ssh-keygen command line tool so no third party gem (and its own crypto | ||
| # implementation) is needed, which keeps the behaviour aligned with OpenSSH and | ||
| # friendly to FIPS/PQC requirements. | ||
| class Foreman::Provision::SshKey |
There was a problem hiding this comment.
If this class handles only public keys, should we name it Foreman::Provision::PublicSshKey?
There was a problem hiding this comment.
Well, there is the generate method that generates both public and private keys. So it is not entirely true statement.
Pass -v to ssh-keygen when Rails runs in development so key generation and fingerprinting are easier to debug locally. Co-authored-by: Cursor <cursoragent@cursor.com>
|
This is now ready for re-review. Relevant packaging PRs: |
4 participants
For FIPS/PQC, we want to remove all SSH related rubygems that do not provide all required features. More generally, we would like to simplify security stack in Foreman and leverage OpenSSH library of the system.
This is a multi-commit PR that does the following:
ssh-keygeninstead and add generate methodAs you can see, there is one functional change that comes with the refactoring. The original rubygem returned key lengths in total number of bits, whereas
ssh-keygenreturns number of bits of elyptic curves (or something like that - not important). Therefore, length do change and now they reflect what OpenSSH returns which is for the good. Those lengths are not used for anything other than showing this in the UI/API.Additionally, there was no generate method that would allow for safe and resilient SSH key generation and Foreman plugins do use this gem. This patch provides that method and tests so plugins can use the new
generatemethod. It is also used in test factories where we generate about one dozen of keys each run. I double checked that this is safe on modern kernels and tests would not get stuck in case of low entropy.Example use: https://github.com/theforeman/foreman_google/blob/3e6a3612056539bbb46c897354e1cdf0c835bace/app/models/foreman_google/gce.rb#L101