Merge pull request #296 from jtopjian/enable-ssl-versions-2

Added ssl_versions parameter
redhat-openstack · Jan 20, 2015 · 177e3e9 · 177e3e9
2 parents 52d3557 + 5675b38
commit 177e3e9
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 6 deletions.
diff --git a/README.md b/README.md
@@ -34,7 +34,7 @@ all features against earlier versions.
 * rabbitmq configuration file.
 * rabbitmq service.
 
-###Beginning with rabbitmq  
+###Beginning with rabbitmq
 
 
 ```puppet
@@ -350,6 +350,12 @@ rabbitmq.config SSL verify setting.
 
 rabbitmq.config `fail_if_no_peer_cert` setting.
 
+####`ssl_versions`
+
+Choose which SSL versions to enable. Example: `['tlsv1.2', 'tlsv1.1']`.
+
+Note that it is recommended to disable `sslv3` and `tlsv1` to prevent against POODLE and BEAST attacks. Please see the [RabbitMQ SSL](https://www.rabbitmq.com/ssl.html) documentation for more information.
+
 ####`stomp_port`
 
 The port to use for Stomp.

diff --git a/manifests/config.pp b/manifests/config.pp
@@ -31,6 +31,7 @@
   $ssl_stomp_port             = $rabbitmq::ssl_stomp_port
   $ssl_verify                 = $rabbitmq::ssl_verify
   $ssl_fail_if_no_peer_cert   = $rabbitmq::ssl_fail_if_no_peer_cert
+  $ssl_versions               = $rabbitmq::ssl_versions
   $stomp_port                 = $rabbitmq::stomp_port
   $wipe_db_on_cookie_change   = $rabbitmq::wipe_db_on_cookie_change
   $config_variables           = $rabbitmq::config_variables

diff --git a/manifests/init.pp b/manifests/init.pp
@@ -39,6 +39,7 @@
   $ssl_stomp_port             = $rabbitmq::params::ssl_stomp_port,
   $ssl_verify                 = $rabbitmq::params::ssl_verify,
   $ssl_fail_if_no_peer_cert   = $rabbitmq::params::ssl_fail_if_no_peer_cert,
+  $ssl_versions               = $rabbitmq::params::ssl_versions,
   $stomp_ensure               = $rabbitmq::params::stomp_ensure,
   $ldap_auth                  = $rabbitmq::params::ldap_auth,
   $ldap_server                = $rabbitmq::params::ldap_server,
@@ -117,6 +118,14 @@
     warning('$ssl_stomp_port requires that $ssl => true and will be ignored')
   }
 
+  if $ssl_versions {
+    if $ssl {
+      validate_array($ssl_versions)
+    } else {
+      fail('$ssl_versions requires that $ssl => true')
+    }
+  }
+
   # This needs to happen here instead of params.pp because
   # $package_source needs to override the constructed value in params.pp
   if $package_source { # $package_source was specified by user so use that one

diff --git a/manifests/params.pp b/manifests/params.pp
@@ -73,6 +73,7 @@
   $ssl_stomp_port             = '6164'
   $ssl_verify                 = 'verify_none'
   $ssl_fail_if_no_peer_cert   = false
+  $ssl_versions               = undef
   $stomp_ensure               = false
   $ldap_auth                  = false
   $ldap_server                = 'ldap'

diff --git a/spec/classes/rabbitmq_spec.rb b/spec/classes/rabbitmq_spec.rb
@@ -39,7 +39,7 @@
   context 'on Debian' do
     let(:params) {{ :manage_repos => true }}
     let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }}
-    
+
     it 'includes rabbitmq::repo::apt' do
       should contain_class('rabbitmq::repo::apt')
     end
@@ -69,7 +69,7 @@
   context 'on Debian' do
     let(:params) {{ :repos_ensure => true }}
     let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }}
-    
+
     it 'includes rabbitmq::repo::apt' do
       should contain_class('rabbitmq::repo::apt')
     end
@@ -89,7 +89,7 @@
   context 'on Debian' do
     let(:params) {{ :manage_repos => true, :repos_ensure => false }}
     let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }}
-    
+
     it 'includes rabbitmq::repo::apt' do
       should contain_class('rabbitmq::repo::apt')
     end
@@ -106,7 +106,7 @@
   context 'on Debian' do
     let(:params) {{ :manage_repos => true, :repos_ensure => true }}
     let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }}
-    
+
     it 'includes rabbitmq::repo::apt' do
       should contain_class('rabbitmq::repo::apt')
     end
@@ -503,6 +503,55 @@
         end
       end
 
+      describe 'ssl options with specific ssl versions' do
+        let(:params) {
+          { :ssl => true,
+            :ssl_port => 3141,
+            :ssl_cacert => '/path/to/cacert',
+            :ssl_cert => '/path/to/cert',
+            :ssl_key => '/path/to/key',
+            :ssl_versions => ['tlsv1.2', 'tlsv1.1']
+        } }
+
+        it 'should set ssl options to specified values' do
+          should contain_file('rabbitmq.config').with_content(%r{ssl_listeners, \[3141\]})
+          should contain_file('rabbitmq.config').with_content(%r{ssl_options, \[\{cacertfile,"/path/to/cacert"})
+          should contain_file('rabbitmq.config').with_content(%r{certfile,"/path/to/cert"})
+          should contain_file('rabbitmq.config').with_content(%r{keyfile,"/path/to/key})
+          should contain_file('rabbitmq.config').with_content(%r{ssl, \[\{versions, \['tlsv1.1', 'tlsv1.2'\]\}\]})
+        end
+      end
+
+      describe 'ssl options with invalid ssl_versions type' do
+        let(:params) {
+          { :ssl => true,
+            :ssl_port => 3141,
+            :ssl_cacert => '/path/to/cacert',
+            :ssl_cert => '/path/to/cert',
+            :ssl_key => '/path/to/key',
+            :ssl_versions => 'tlsv1.2, tlsv1.1'
+        } }
+
+        it 'fails' do
+          expect{subject}.to raise_error(/is not an Array/)
+        end
+      end
+
+      describe 'ssl options with ssl_versions and not ssl' do
+        let(:params) {
+          { :ssl => false,
+            :ssl_port => 3141,
+            :ssl_cacert => '/path/to/cacert',
+            :ssl_cert => '/path/to/cert',
+            :ssl_key => '/path/to/key',
+            :ssl_versions => ['tlsv1.2', 'tlsv1.1']
+        } }
+
+        it 'fails' do
+          expect{subject}.to raise_error(/^\$ssl_versions requires that \$ssl => true/)
+        end
+      end
+
       describe 'ssl admin options' do
         let(:params) {
           { :ssl => true,

diff --git a/templates/rabbitmq.config.erb b/templates/rabbitmq.config.erb
@@ -16,12 +16,18 @@
     {tcp_listeners, []},
 <%- end -%>
 <%- if @ssl -%>
+    <%- if @ssl_versions -%>
+      {ssl, [{versions, [<%= @ssl_versions.sort.map { |v| "'#{v}'" }.join(', ') %>]}]},
+    <%- end -%>
     {ssl_listeners, [<%= @ssl_port %>]},
     {ssl_options, [<%- if @ssl_cacert != 'UNSET' -%>{cacertfile,"<%= @ssl_cacert %>"},<%- end -%>
                     {certfile,"<%= @ssl_cert %>"},
                     {keyfile,"<%= @ssl_key %>"},
                     {verify,<%= @ssl_verify %>},
-                    {fail_if_no_peer_cert,<%= @ssl_fail_if_no_peer_cert %>}]},
+                    {fail_if_no_peer_cert,<%= @ssl_fail_if_no_peer_cert %>}
+                    <%- if @ssl_versions -%>
+                      ,{ssl, [{versions, [<%= @ssl_versions.sort.map { |v| "'#{v}'" }.join(', ') %>]}]}
+                    <% end -%>]},
 <%- end -%>
 <% if @config_variables -%>
 <%- @config_variables.keys.sort.each do |key| -%>