Merge pull request #166 from artem-sidorenko/alg-update

Use different algorithms depending on the ssh version

Author: atomic111

README.md

@@ -46,6 +46,7 @@ override['ssh-hardening']['ssh']['server']['listen_to'] = node['ipaddress']
 * `['ssh-hardening']['ssh']['client']['remote_hosts']` - `[]` - one or more hosts, to which ssh-client can connect to.
 * `['ssh-hardening']['ssh']['client']['password_authentication']` - `false`. Set to `true` if password authentication should be enabled.
 * `['ssh-hardening']['ssh']['client']['roaming']` - `false`. Set to `true` if experimental client roaming should be enabled. This is known to cause potential issues with secrets being disclosed to malicious servers and defaults to being disabled.
+* `['ssh-hardening']['ssh']['server']['host_key_files']` - `nil` to calculate best hostkey configuration based on server version, otherwise specify an array with file paths (e.g. `/etc/ssh/ssh_host_rsa_key`)
 * `['ssh-hardening']['ssh']['server']['dh_min_prime_size']` - `2048` - Minimal acceptable prime length in bits in `/etc/ssh/moduli`. Primes below this number will get removed. (See [this](https://entropux.net/article/openssh-moduli/) for more information and background)
 * `['ssh-hardening']['ssh']['server']['dh_build_primes']` - `false` - If own primes should be built. This rebuild happens only once and takes a lot of time (~ 1.5 - 2h on the modern hardware for 4096 length).
 * `['ssh-hardening']['ssh']['server']['dh_build_primes_size']` - `4096` - Prime length which should be generated. This option is only valid if `dh_build_primes` is enabled.

attributes/default.rb

@@ -74,7 +74,7 @@
 default['ssh-hardening']['ssh']['server']['dh_min_prime_size']        = 2048
 default['ssh-hardening']['ssh']['server']['dh_build_primes']          = false
 default['ssh-hardening']['ssh']['server']['dh_build_primes_size']     = 4096
-default['ssh-hardening']['ssh']['server']['host_key_files']           = ['/etc/ssh/ssh_host_rsa_key', '/etc/ssh/ssh_host_ecdsa_key']
+default['ssh-hardening']['ssh']['server']['host_key_files']           = nil
 default['ssh-hardening']['ssh']['server']['client_alive_interval']    = 600     # 10min
 default['ssh-hardening']['ssh']['server']['client_alive_count']       = 3       # ~> 3 x interval
 default['ssh-hardening']['ssh']['server']['allow_root_with_key']      = false

libraries/devsec_ssh.rb

@@ -70,6 +70,13 @@ class Ssh # rubocop:disable Metrics/ClassLength
       5.3 => 'yes',
       5.9 => 'sandbox'
     }.freeze
+    # Hostkey algorithms
+    # In the current implementation they are server specific so we need own data hash for it
+    HOSTKEY_ALGORITHMS ||= {
+      5.3 => %w(rsa),
+      6.0 => %w(rsa ecdsa),
+      6.6 => %w(rsa ecdsa ed25519)
+    }.freeze
 
     class << self
       def get_server_privilege_separarion # rubocop:disable Style/AccessorMethodName
@@ -80,6 +87,14 @@ def get_server_privilege_separarion # rubocop:disable Style/AccessorMethodName
         ret
       end
 
+      def get_server_algorithms # rubocop:disable Style/AccessorMethodName
+        Chef::Log.debug('Called get_server_algorithms')
+        found_ssh_version = find_ssh_version(get_ssh_server_version, HOSTKEY_ALGORITHMS.keys)
+        ret = HOSTKEY_ALGORITHMS[found_ssh_version]
+        Chef::Log.debug("Using configuration for ssh version #{found_ssh_version}, value: #{ret}")
+        ret
+      end
+
       def get_client_macs(enable_weak = false)
         get_crypto_data(:macs, :client, enable_weak)
       end
@@ -176,7 +191,7 @@ def get_ssh_version(package)
       end
 
       # Guess the version of ssh via OS matrix
-      def guess_ssh_version
+      def guess_ssh_version # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
         family = node['platform_family']
         platform = node['platform']
         version = node['platform_version'].to_f
@@ -188,11 +203,20 @@ def guess_ssh_version
             return 6.6 if version >= 14.04
           when 'debian'
             return 6.6 if version >= 8
+            return 6.0 if version >= 7
             return 5.3 if version <= 6
           end
         when 'rhel'
           return 6.6 if version >= 7
           return 5.3 if version >= 6
+        when 'fedora'
+          return 7.3 if version >= 25
+          return 7.2 if version >= 24
+        when 'suse'
+          case platform
+          when 'opensuse'
+            return 6.6 if version >= 13.2
+          end
         end
         Chef::Log.info("Unknown platform #{node['platform']} with version #{node['platform_version']} and family #{node['platform_family']}. Assuming ssh version #{FALLBACK_SSH_VERSION}")
         FALLBACK_SSH_VERSION

recipes/server.rb

@@ -170,7 +170,8 @@
     mac:    node['ssh-hardening']['ssh']['server']['mac']    || DevSec::Ssh.get_server_macs(node['ssh-hardening']['ssh']['server']['weak_hmac']),
     kex:    node['ssh-hardening']['ssh']['server']['kex']    || DevSec::Ssh.get_server_kexs(node['ssh-hardening']['ssh']['server']['weak_kex']),
     cipher: node['ssh-hardening']['ssh']['server']['cipher'] || DevSec::Ssh.get_server_ciphers(node['ssh-hardening']['ssh']['server']['cbc_required']),
-    use_priv_sep: node['ssh-hardening']['ssh']['use_privilege_separation'] || DevSec::Ssh.get_server_privilege_separarion
+    use_priv_sep: node['ssh-hardening']['ssh']['use_privilege_separation'] || DevSec::Ssh.get_server_privilege_separarion,
+    hostkeys: node['ssh-hardening']['ssh']['server']['host_key_files'] || DevSec::Ssh.get_server_algorithms.map { |alg| "/etc/ssh/ssh_host_#{alg}_key" }
   )
   notifies :restart, 'service[sshd]'
 end

spec/libraries/devsec_ssh_spec.rb

@@ -109,6 +109,46 @@ def self.debug(*); end
       end
     end
 
+    context 'when running on Fedora 25' do
+      let(:family) { 'fedora' }
+      let(:platform) { 'fedora' }
+      let(:version) { '25' }
+
+      it 'should return ssh version 7.3' do
+        expect(subject.send(:guess_ssh_version)).to eq 7.3
+      end
+    end
+
+    context 'when running on Fedora 24' do
+      let(:family) { 'fedora' }
+      let(:platform) { 'fedora' }
+      let(:version) { '24' }
+
+      it 'should return ssh version 7.2' do
+        expect(subject.send(:guess_ssh_version)).to eq 7.2
+      end
+    end
+
+    context 'when running on Opensuse 13.2' do
+      let(:family) { 'suse' }
+      let(:platform) { 'opensuse' }
+      let(:version) { '13.2' }
+
+      it 'should return ssh version 6.6' do
+        expect(subject.send(:guess_ssh_version)).to eq 6.6
+      end
+    end
+
+    context 'when running on Opensuse 42.1' do
+      let(:family) { 'suse' }
+      let(:platform) { 'opensuse' }
+      let(:version) { '42.1' }
+
+      it 'should return ssh version 6.6' do
+        expect(subject.send(:guess_ssh_version)).to eq 6.6
+      end
+    end
+
     context 'when running on unknown platform' do
       let(:family) { 'unknown' }
       let(:platform) { 'unknown' }
@@ -202,6 +242,27 @@ def self.debug(*); end
     end
   end
 
+  describe 'get_server_algorithms' do
+    DevSec::Ssh::HOSTKEY_ALGORITHMS.each do |openssh_version, conf_data|
+      context "when openssh is >= #{openssh_version}" do
+        before :each do
+          # mock get_ssh_server_version. We test it somewhere else
+          expect(subject).to receive(:get_ssh_server_version) { openssh_version }
+        end
+
+        it "get the config value #{conf_data}" do
+          expect(subject.get_server_algorithms).to eq conf_data
+        end
+      end
+    end
+    context 'when openssh has a totaly unsupported version, e.g. 3.0' do
+      it 'should raise an exception' do
+        expect(subject).to receive(:get_ssh_server_version) { 3.0 }
+        expect { subject.get_server_algorithms }.to raise_exception(/Unsupported ssh version/)
+      end
+    end
+  end
+
   # Here we test the public functions:
   # get_[client|server]_[kexs|macs|ciphers]
   # In order to cover all possible combinations, we need a complex nested loops:-\

templates/default/opensshd.conf.erb

@@ -32,7 +32,7 @@ ListenAddress <%= ssh_ip %>
 <% end %>
 
 # List HostKeys here.
-<% Array(@node['ssh-hardening']['ssh']['server']['host_key_files']).each do |host_key_file| %>
+<% @hostkeys.each do |host_key_file| %>
 HostKey <%= host_key_file %>          # Req 20
 <% end %>