Handle duplicate/invalid entries in migration SSH inbound addresses

An error (e.g a typo) in a custom tripleo-heat-templates environment
file could lead to an invalid match block in /etc/ssh/sshd_config.
SSH fails-safe and refuses all logins in this case.

This change validates the migration_ssh_localaddrs parameter is an
array of IP addresses and removes and duplicate entries.

Change-Id: Ibcf144d960fe52f0eab0d5015bd30cf7c1e37e25
Closes-Bug: #1688308

diff --git a/manifests/profile/base/nova.pp b/manifests/profile/base/nova.pp
index 6065e62..d786940 100644 (file)
--- a/manifests/profile/base/nova.pp
+++ b/manifests/profile/base/nova.pp
@@ -129,6 +129,10 @@ class tripleo::profile::base::nova (
     $memcache_servers = suffix(hiera('memcached_node_ips'), ':11211')
   }
 
+  validate_array($migration_ssh_localaddrs)
+  $migration_ssh_localaddrs.each |$x| { validate_ip_address($x) }
+  $migration_ssh_localaddrs_real = unique($migration_ssh_localaddrs)
+
   if $step >= 4 or ($step >= 3 and $sync_db) {
     $oslomsg_use_ssl_real = sprintf('%s', bool2num(str2bool($oslomsg_use_ssl)))
     include ::nova::config
@@ -183,10 +187,10 @@ class tripleo::profile::base::nova (
         # Nova SSH tunnel setup (cold-migration)
 
         # Server side
-        if !empty($migration_ssh_localaddrs) {
-          $allow_type = sprintf('LocalAddress %s User', join($migration_ssh_localaddrs,','))
+        if !empty($migration_ssh_localaddrs_real) {
+          $allow_type = sprintf('LocalAddress %s User', join($migration_ssh_localaddrs_real,','))
           $deny_type = 'LocalAddress'
-          $deny_name = sprintf('!%s', join($migration_ssh_localaddrs,',!'))
+          $deny_name = sprintf('!%s', join($migration_ssh_localaddrs_real,',!'))
 
           ssh::server::match_block { 'nova_migration deny':
             name    => $deny_name,

diff --git a/spec/classes/tripleo_profile_base_nova_spec.rb b/spec/classes/tripleo_profile_base_nova_spec.rb
index a48c94f..a7f1cce 100644 (file)
--- a/spec/classes/tripleo_profile_base_nova_spec.rb
+++ b/spec/classes/tripleo_profile_base_nova_spec.rb
@@ -350,6 +350,108 @@ describe 'tripleo::profile::base::nova' do
       }
     end
 
+    context 'with step 4 with libvirt and migration ssh key and invalid migration_ssh_localaddrs' do
+      let(:pre_condition) do
+        <<-eof
+        include ::nova::compute::libvirt::services
+        class { '::ssh::server':
+          storeconfigs_enabled => false,
+          options              => {}
+        }
+        eof
+      end
+      let(:params) { {
+        :step           => 4,
+        :libvirt_enabled => true,
+        :manage_migration => true,
+        :nova_compute_enabled => true,
+        :bootstrap_node  => 'node.example.com',
+        :oslomsg_rpc_hosts => [ 'localhost' ],
+        :oslomsg_rpc_password => 'foo',
+        :migration_ssh_key => { 'private_key' => 'foo', 'public_key' => 'ssh-rsa bar'},
+        :migration_ssh_localaddrs => ['127.0.0.1', '']
+      } }
+
+      it { is_expected.to_not compile }
+    end
+
+    context 'with step 4 with libvirt and migration ssh key and duplicate migration_ssh_localaddrs' do
+      let(:pre_condition) do
+        <<-eof
+        include ::nova::compute::libvirt::services
+        class { '::ssh::server':
+          storeconfigs_enabled => false,
+          options              => {}
+        }
+        eof
+      end
+      let(:params) { {
+        :step           => 4,
+        :libvirt_enabled => true,
+        :manage_migration => true,
+        :nova_compute_enabled => true,
+        :bootstrap_node  => 'node.example.com',
+        :oslomsg_rpc_hosts => [ 'localhost' ],
+        :oslomsg_rpc_password => 'foo',
+        :migration_ssh_key => { 'private_key' => 'foo', 'public_key' => 'ssh-rsa bar'},
+        :migration_ssh_localaddrs => ['127.0.0.1', '127.0.0.1']
+      } }
+
+      it {
+        is_expected.to contain_class('tripleo::profile::base::nova')
+        is_expected.to contain_class('nova').with(
+          :default_transport_url => /.+/,
+          :notification_transport_url => /.+/,
+          :nova_public_key  => nil,
+          :nova_private_key => nil,
+        )
+        is_expected.to contain_class('nova::config')
+        is_expected.to contain_class('nova::placement')
+        is_expected.to contain_class('nova::cache')
+        is_expected.to contain_class('nova::migration::libvirt').with(
+          :transport         => 'ssh',
+          :configure_libvirt => params[:libvirt_enabled],
+          :configure_nova    => params[:nova_compute_enabled]
+        )
+        is_expected.to contain_ssh__server__match_block('nova_migration allow').with(
+          :type  => 'LocalAddress 127.0.0.1 User',
+          :name  => 'nova_migration',
+          :options => {
+            'ForceCommand'           => '/bin/nova-migration-wrapper',
+            'PasswordAuthentication' => 'no',
+            'AllowTcpForwarding'     => 'no',
+            'X11Forwarding'          => 'no',
+            'AuthorizedKeysFile'     => '/etc/nova/migration/authorized_keys'
+          }
+        )
+        is_expected.to contain_ssh__server__match_block('nova_migration deny').with(
+          :type  => 'LocalAddress',
+          :name  => '!127.0.0.1',
+          :options => {
+            'DenyUsers' => 'nova_migration'
+          }
+        )
+        is_expected.to contain_package('openstack-nova-migration').with(
+          :ensure => 'present'
+        )
+        is_expected.to contain_file('/etc/nova/migration/authorized_keys').with(
+          :content => 'ssh-rsa bar',
+          :mode => '0640',
+          :owner => 'root',
+          :group => 'nova_migration',
+        )
+        is_expected.to contain_file('/etc/nova/migration/identity').with(
+          :content => 'foo',
+          :mode => '0600',
+          :owner => 'nova',
+          :group => 'nova',
+        )
+        is_expected.to contain_user('nova_migration').with(
+          :shell => '/bin/bash'
+        )
+      }
+    end
+
     context 'with step 4 with libvirt TLS and migration ssh key' do
       let(:pre_condition) do
         <<-eof