Merge pull request #2152 from davidc/main

(MODULES-11068) Allow apache::vhost ssl_honorcipherorder to take boolean parameter
puppetlabs · May 18, 2021 · e14ec09 · e14ec09
2 parents 367a7c0 + 763154f
commit e14ec09
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 6 deletions.
diff --git a/manifests/vhost.pp b/manifests/vhost.pp
@@ -1765,7 +1765,7 @@
   $ssl_certs_dir                                                                    = $apache::params::ssl_certs_dir,
   $ssl_protocol                                                                     = undef,
   $ssl_cipher                                                                       = undef,
-  $ssl_honorcipherorder                                                             = undef,
+  Variant[Boolean, Enum['on', 'On', 'off', 'Off'], Undef] $ssl_honorcipherorder     = undef,
   Optional[Enum['none', 'optional', 'require', 'optional_no_ca']] $ssl_verify_client = undef,
   $ssl_verify_depth                                                                 = undef,
   Optional[Enum['none', 'optional', 'require', 'optional_no_ca']] $ssl_proxy_verify = undef,
@@ -2029,6 +2029,18 @@
     include apache::mod::mime
   }
 
+  if $ssl_honorcipherorder =~ Boolean or $ssl_honorcipherorder == undef {
+    $_ssl_honorcipherorder = $ssl_honorcipherorder
+  } else {
+    $_ssl_honorcipherorder = $ssl_honorcipherorder ? {
+      'on'    => true,
+      'On'    => true,
+      'off'   => false,
+      'Off'   => false,
+      default => true,
+    }
+  }
+
   if $auth_kerb and $ensure == 'present' {
     include apache::mod::auth_kerb
   }
@@ -2688,7 +2700,7 @@
   # - $ssl_crl_check
   # - $ssl_protocol
   # - $ssl_cipher
-  # - $ssl_honorcipherorder
+  # - $_ssl_honorcipherorder
   # - $ssl_verify_client
   # - $ssl_verify_depth
   # - $ssl_options

diff --git a/spec/acceptance/apache_ssl_spec.rb b/spec/acceptance/apache_ssl_spec.rb
@@ -65,7 +65,7 @@ class { 'apache':
           ssl_certs_dir        => '/tmp',
           ssl_protocol         => 'test',
           ssl_cipher           => 'test',
-          ssl_honorcipherorder => 'test',
+          ssl_honorcipherorder => true,
           ssl_verify_client    => 'require',
           ssl_verify_depth     => 'test',
           ssl_options          => ['test', 'test1'],
@@ -89,7 +89,7 @@ class { 'apache':
       it { is_expected.to contain 'SSLProxyEngine On' }
       it { is_expected.to contain 'SSLProtocol             test' }
       it { is_expected.to contain 'SSLCipherSuite          test' }
-      it { is_expected.to contain 'SSLHonorCipherOrder     test' }
+      it { is_expected.to contain 'SSLHonorCipherOrder     On' }
       it { is_expected.to contain 'SSLVerifyClient         require' }
       it { is_expected.to contain 'SSLVerifyDepth          test' }
       it { is_expected.to contain 'SSLOptions test test1' }

diff --git a/spec/defines/vhost_spec.rb b/spec/defines/vhost_spec.rb
@@ -2082,6 +2082,52 @@
           it { is_expected.to contain_concat__fragment('rspec.example.com-ssl') }
           it { is_expected.not_to contain_concat__fragment('rspec.example.com-sslproxy') }
         end
+        context 'ssl_honorcipherorder' do
+          let :params do
+            {
+              'docroot'            => '/rspec/docroot',
+              'ssl'                => true,
+            }
+          end
+
+          context 'ssl_honorcipherorder default' do
+            it { is_expected.to compile }
+            it { is_expected.to contain_concat__fragment('rspec.example.com-ssl').without_content(%r{^\s*SSLHonorCipherOrder}i) }
+          end
+
+          context 'ssl_honorcipherorder on' do
+            let :params do
+              super().merge({ 'ssl_honorcipherorder' => 'on' })
+            end
+
+            it { is_expected.to compile }
+            it { is_expected.to contain_concat__fragment('rspec.example.com-ssl').with_content(%r{^\s*SSLHonorCipherOrder\s+On$}) }
+          end
+          context 'ssl_honorcipherorder true' do
+            let :params do
+              super().merge({ 'ssl_honorcipherorder' => true })
+            end
+
+            it { is_expected.to compile }
+            it { is_expected.to contain_concat__fragment('rspec.example.com-ssl').with_content(%r{^\s*SSLHonorCipherOrder\s+On$}) }
+          end
+          context 'ssl_honorcipherorder off' do
+            let :params do
+              super().merge({ 'ssl_honorcipherorder' => 'off' })
+            end
+
+            it { is_expected.to compile }
+            it { is_expected.to contain_concat__fragment('rspec.example.com-ssl').with_content(%r{^\s*SSLHonorCipherOrder\s+Off$}) }
+          end
+          context 'ssl_honorcipherorder false' do
+            let :params do
+              super().merge({ 'ssl_honorcipherorder' => false })
+            end
+
+            it { is_expected.to compile }
+            it { is_expected.to contain_concat__fragment('rspec.example.com-ssl').with_content(%r{^\s*SSLHonorCipherOrder\s+Off$}) }
+          end
+        end
         describe 'access logs' do
           context 'single log file' do
             let(:params) do

diff --git a/templates/vhost/_ssl.erb b/templates/vhost/_ssl.erb
@@ -15,8 +15,8 @@
   <%- if @ssl_cipher -%>
   SSLCipherSuite          <%= @ssl_cipher %>
   <%- end -%>
-  <%- if @ssl_honorcipherorder -%>
-  SSLHonorCipherOrder     <%= @ssl_honorcipherorder %>
+  <%- if not @ssl_honorcipherorder.nil? -%>
+  SSLHonorCipherOrder     <%= scope.call_function('apache::bool2httpd', [@_ssl_honorcipherorder]) %>
   <%- end -%>
   <%- if @ssl_verify_client -%>
   SSLVerifyClient         <%= @ssl_verify_client %>