Merge pull request #163 from artem-sidorenko/dh-moduli

Avoid small primes for DH and allow rebuild of DH primes

Author: atomic111

README.md

@@ -46,6 +46,9 @@ 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']['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.
 * `['ssh-hardening']['ssh']['server']['listen_to']` `#override attribute#` - one or more ip addresses, to which ssh-server should listen to. Default is to listen on all interfaces. It should be configured for security reasons!
 * `['ssh-hardening']['ssh']['server']['allow_root_with_key']` - `false` to disable root login altogether. Set to `true` to allow root to login via key-based mechanism
 * `['ssh-hardening']['ssh']['server']['allow_tcp_forwarding']` - `false`. Set to `true` to allow TCP Forwarding

attributes/default.rb

@@ -70,6 +70,9 @@
 default['ssh-hardening']['ssh']['server']['cbc_required']             = false
 default['ssh-hardening']['ssh']['server']['weak_hmac']                = false
 default['ssh-hardening']['ssh']['server']['weak_kex']                 = false
+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']['client_alive_interval']    = 600     # 10min
 default['ssh-hardening']['ssh']['server']['client_alive_count']       = 3       # ~> 3 x interval

recipes/server.rb

@@ -30,17 +30,24 @@
     ['0.0.0.0']
   end
 
+# some internal definitions
+cache_dir = ::File.join(Chef::Config[:file_cache_path], cookbook_name.to_s)
+dh_moduli_file = '/etc/ssh/moduli'
+
+# create a cache dir for this cookbook
+# we use it for storing of lock files or selinux files
+directory cache_dir
+
 # installs package name
 package 'openssh-server' do
   package_name node['ssh-hardening']['sshserver']['package']
 end
 
 # Handle addional SELinux policy on RHEL/Fedora for different UsePAM options
 if %w(fedora rhel).include?(node['platform_family'])
-  policy_dir = ::File.join(Chef::Config[:file_cache_path], cookbook_name.to_s)
-  policy_file = ::File.join(policy_dir, 'ssh_password.te')
-  module_file = ::File.join(policy_dir, 'ssh_password.mod')
-  package_file = ::File.join(policy_dir, 'ssh_password.pp')
+  policy_file = ::File.join(cache_dir, 'ssh_password.te')
+  module_file = ::File.join(cache_dir, 'ssh_password.mod')
+  package_file = ::File.join(cache_dir, 'ssh_password.pp')
 
   package 'policycoreutils-python'
   # on fedora we need an addtional package for semodule_package
@@ -56,8 +63,6 @@
   else
     # UsePAM no: enable and install the additional SELinux policy
 
-    directory policy_dir
-
     cookbook_file policy_file do
       source 'ssh_password.te'
     end
@@ -73,6 +78,53 @@
   end
 end
 
+# handle Diffie-Hellman moduli
+# build own moduli file if required
+own_primes_lock_file = ::File.join(cache_dir, 'moduli.lock')
+bash 'build own primes for DH' do
+  code <<-EOS
+    set -e
+    tempdir=$(mktemp -d)
+    ssh-keygen -G $tempdir/moduli.all -b #{node['ssh-hardening']['ssh']['server']['dh_build_primes_size']}
+    ssh-keygen -T $tempdir/moduli.safe -f $tempdir/moduli.all
+    cp $tempdir/moduli.safe #{dh_moduli_file}
+    rm -rf $tempdir
+    touch #{own_primes_lock_file}
+  EOS
+  only_if { node['ssh-hardening']['ssh']['server']['dh_build_primes'] }
+  not_if { ::File.exist?(own_primes_lock_file) }
+  notifies :restart, 'service[sshd]'
+end
+
+# remove all small primes
+# https://stribika.github.io/2015/01/04/secure-secure-shell.html
+dh_min_prime_size = node['ssh-hardening']['ssh']['server']['dh_min_prime_size'].to_i - 1 # 4096 is 4095 in the moduli file
+ruby_block 'remove small primes from DH moduli' do # ~FC014
+  block do
+    tmp_file = "#{dh_moduli_file}.tmp"
+    ::File.open(tmp_file, 'w') do |new_file|
+      ::File.readlines(dh_moduli_file).each do |line|
+        unless line_match = line.match(/^(\d+ ){4}(\d+) \d+ \h+$/) # rubocop:disable Lint/AssignmentInCondition
+          # some line without expected data structure, e.g. comment line
+          # write it and go to the next data
+          new_file.write(line)
+          next
+        end
+
+        # lets compare the bits and do not write the lines with small bit size
+        bits = line_match[2]
+        new_file.write(line) unless bits.to_i < dh_min_prime_size
+      end
+    end
+
+    # we use cp&rm to preserve the permissions of existing file
+    FileUtils.cp(tmp_file, dh_moduli_file)
+    FileUtils.rm(tmp_file)
+  end
+  not_if "test $(awk '$5 < #{dh_min_prime_size} && $5 ~ /^[0-9]+$/ { print $5 }' #{dh_moduli_file} | uniq | wc -c) -eq 0"
+  notifies :restart, 'service[sshd]'
+end
+
 # defines the sshd service
 service 'sshd' do
   # use upstart for ubuntu, otherwise chef uses init

spec/recipes/default_spec.rb

@@ -23,6 +23,10 @@
     ChefSpec::ServerRunner.new.converge(described_recipe)
   end
 
+  before do
+    stub_command("test $(awk '$5 < 2047 && $5 ~ /^[0-9]+$/ { print $5 }' /etc/ssh/moduli | uniq | wc -c) -eq 0").and_return(true)
+  end
+
   # check that the recipes are executed
   it 'includes server recipe' do
     expect(chef_run).to include_recipe('ssh-hardening::server')

spec/recipes/server_spec.rb

@@ -21,12 +21,21 @@
 describe 'ssh-hardening::server' do
   let(:helper_lib) { DevSec::Ssh }
   let(:ssh_config_file) { '/etc/ssh/sshd_config' }
+  let(:dh_primes_ok) { true }
 
   # converge
   cached(:chef_run) do
     ChefSpec::ServerRunner.new.converge(described_recipe)
   end
 
+  before do
+    stub_command("test $(awk '$5 < 2047 && $5 ~ /^[0-9]+$/ { print $5 }' /etc/ssh/moduli | uniq | wc -c) -eq 0").and_return(dh_primes_ok)
+  end
+
+  it 'should create cache directory' do
+    expect(chef_run).to create_directory('/tmp/ssh-hardening-file-cache/ssh-hardening')
+  end
+
   it 'installs openssh-server' do
     expect(chef_run).to install_package('openssh-server')
   end
@@ -229,7 +238,6 @@
       let(:version) { '16.04' }
 
       it 'does not invoke any SELinux resources' do
-        expect(chef_run).not_to create_directory('/tmp/ssh-hardening-file-cache/ssh-hardening')
         expect(chef_run).not_to render_file('/tmp/ssh-hardening-file-cache/ssh-hardening/ssh_password.te')
         expect(chef_run).not_to run_execute('remove selinux policy')
         expect(chef_run).not_to run_bash('build selinux package and install it')
@@ -300,10 +308,6 @@
           expect(chef_run).to render_file('/etc/ssh/sshd_config').with_content('UsePAM no')
         end
 
-        it 'should create cache directory for policy files' do
-          expect(chef_run).to create_directory('/tmp/ssh-hardening-file-cache/ssh-hardening')
-        end
-
         it 'should create selinux source policy file' do
           expect(chef_run).to render_file('/tmp/ssh-hardening-file-cache/ssh-hardening/ssh_password.te')
         end
@@ -327,6 +331,32 @@
     end
   end
 
+  it 'should not build own DH primes per default' do
+    expect(chef_run).not_to run_bash('build own primes for DH')
+  end
+
+  describe 'DH primes handling' do
+    let(:chef_run) do
+      ChefSpec::ServerRunner.new.converge(described_recipe)
+    end
+
+    context 'when there are no small primes' do
+      let(:dh_primes_ok) { true }
+
+      it 'should not remove small primes from DH moduli' do
+        expect(chef_run).not_to run_ruby_block('remove small primes from DH moduli')
+      end
+    end
+
+    context 'when there are small primes present' do
+      let(:dh_primes_ok) { false }
+
+      it 'should invoke small primes from DH module' do
+        expect(chef_run).to run_ruby_block('remove small primes from DH moduli')
+      end
+    end
+  end
+
   describe 'debian banner' do
     cached(:chef_run) do
       ChefSpec::ServerRunner.new(platform: 'ubuntu', version: '16.04').converge(described_recipe)