Merge pull request #2081 from bovy89/main

Improve SSLVerify options

Author: sanfrancrisko

manifests/vhost.pp

@@ -1498,7 +1498,7 @@
 #
 # @param ssl_ca
 #   Specifies the SSL certificate authority to be used to verify client certificates used 
-#   for authentication. You must also set `ssl_verify_client` to use this.
+#   for authentication.
 #
 # @param ssl_cert
 #   Specifies the SSL certification.
@@ -1516,8 +1516,7 @@
 #   preferred order.
 #
 # @param ssl_certs_dir
-#   Specifies the location of the SSL certification directory to verify client certs. Will not 
-#   be used unless `ssl_verify_client` is also set (see below).
+#   Specifies the location of the SSL certification directory to verify client certs.
 #
 # @param ssl_chain
 #   Specifies the SSL chain. This default works out of the box, but it must be updated in 
@@ -1749,7 +1748,7 @@
   $ssl_protocol                                                                     = undef,
   $ssl_cipher                                                                       = undef,
   $ssl_honorcipherorder                                                             = undef,
-  $ssl_verify_client                                                                = 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,
   Optional[Integer[0]] $ssl_proxy_verify_depth                                      = undef,

spec/acceptance/apache_ssl_spec.rb

@@ -33,11 +33,11 @@ class { 'apache':
       it { is_expected.to contain 'SSLCertificateFile      "/tmp/ssl_cert"' }
       it { is_expected.to contain 'SSLCertificateKeyFile   "/tmp/ssl_key"' }
       it { is_expected.to contain 'SSLCertificateChainFile "/tmp/ssl_chain"' }
-      it { is_expected.not_to contain 'SSLCACertificateFile    "/tmp/ssl_ca"' }
-      it { is_expected.not_to contain 'SSLCARevocationPath     "/tmp/ssl_crl_path"' }
-      it { is_expected.not_to contain 'SSLCARevocationFile     "/tmp/ssl_crl"' }
+      it { is_expected.to contain 'SSLCACertificateFile    "/tmp/ssl_ca"' }
+      it { is_expected.to contain 'SSLCARevocationPath     "/tmp/ssl_crl_path"' }
+      it { is_expected.to contain 'SSLCARevocationFile     "/tmp/ssl_crl"' }
       if apache_hash['version'] == '2.4'
-        it { is_expected.not_to contain 'SSLCARevocationCheck    "chain"' }
+        it { is_expected.to contain 'SSLCARevocationCheck    chain' }
       else
         it { is_expected.not_to contain 'SSLCARevocationCheck' }
       end
@@ -64,7 +64,7 @@ class { 'apache':
           ssl_protocol         => 'test',
           ssl_cipher           => 'test',
           ssl_honorcipherorder => 'test',
-          ssl_verify_client    => 'test',
+          ssl_verify_client    => 'require',
           ssl_verify_depth     => 'test',
           ssl_options          => ['test', 'test1'],
           ssl_proxyengine      => true,
@@ -88,7 +88,7 @@ class { 'apache':
       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 'SSLVerifyClient         test' }
+      it { is_expected.to contain 'SSLVerifyClient         require' }
       it { is_expected.to contain 'SSLVerifyDepth          test' }
       it { is_expected.to contain 'SSLOptions test test1' }
       if apache_hash['version'] == '2.4'
@@ -111,7 +111,7 @@ class { 'apache':
           ssl_cert             => '/tmp/ssl_cert',
           ssl_key              => '/tmp/ssl_key',
           ssl_ca               => '/tmp/ssl_ca',
-          ssl_verify_client    => 'test',
+          ssl_verify_client    => 'optional',
         }
     MANIFEST
     it 'runs without error' do
@@ -139,7 +139,7 @@ class { 'apache':
           ssl_cert             => '/tmp/ssl_cert',
           ssl_key              => '/tmp/ssl_key',
           ssl_certs_dir        => '/tmp',
-          ssl_verify_client    => 'test',
+          ssl_verify_client    => 'require',
         }
     MANIFEST
     it 'runs without error' do
@@ -151,7 +151,7 @@ class { 'apache':
       it { is_expected.to contain 'SSLCertificateFile      "/tmp/ssl_cert"' }
       it { is_expected.to contain 'SSLCertificateKeyFile   "/tmp/ssl_key"' }
       it { is_expected.to contain 'SSLCACertificatePath    "/tmp"' }
-      it { is_expected.to contain 'SSLVerifyClient         test' }
+      it { is_expected.to contain 'SSLVerifyClient         require' }
       it { is_expected.not_to contain 'SSLCACertificateFile' }
     end
   end

spec/defines/vhost_spec.rb

@@ -252,6 +252,13 @@
                     'sslonly'   => 'Off',
                   },
                 },
+                {
+                  'path'              => '/private_1',
+                  'provider'          => 'location',
+                  'ssl_options'       => ['+ExportCertData', '+StdEnvVars'],
+                  'ssl_verify_client' => 'optional',
+                  'ssl_verify_depth'  => '10',
+                },
               ],
               'error_log'                   => false,
               'error_log_file'              => 'httpd_error_log',
@@ -941,6 +948,16 @@
               content: %r{^\s+GssapiLocalName\sOn$},
             )
           }
+          it {
+            is_expected.to contain_concat__fragment('rspec.example.com-directories').with(
+              content: %r{^\s+SSLVerifyClient\soptional$},
+            )
+          }
+          it {
+            is_expected.to contain_concat__fragment('rspec.example.com-directories').with(
+              content: %r{^\s+SSLVerifyDepth\s10$},
+            )
+          }
           it { is_expected.to contain_concat__fragment('rspec.example.com-additional_includes') }
           it { is_expected.to contain_concat__fragment('rspec.example.com-logging') }
           it {

templates/vhost/_directories.erb

@@ -339,6 +339,12 @@
     <%- if directory['ssl_options'] -%>
     SSLOptions <%= Array(directory['ssl_options']).join(' ') %>
     <%- end -%>
+    <%- if directory['ssl_verify_client'] and directory['ssl_verify_client'].match('(none|optional|require|optional_no_ca)') -%>
+    SSLVerifyClient <%= directory['ssl_verify_client'] %>
+    <%- if directory['ssl_verify_depth'] -%>
+    SSLVerifyDepth <%= directory['ssl_verify_depth'] %>
+    <%- end -%>
+    <%- end -%>
     <%- if directory['suphp'] and @suphp_engine == 'on' -%>
     suPHP_UserGroup <%= directory['suphp']['user'] %> <%= directory['suphp']['group'] %>
     <%- end -%>

templates/vhost/_ssl.erb

@@ -18,6 +18,10 @@
   <%- end -%>
   <%- if @ssl_verify_client -%>
   SSLVerifyClient         <%= @ssl_verify_client %>
+  <%- if @ssl_verify_depth -%>
+  SSLVerifyDepth          <%= @ssl_verify_depth %>
+  <%- end -%>
+  <%- end -%>
   <%- if @ssl_certs_dir && @ssl_certs_dir != '' -%>
   SSLCACertificatePath    "<%= @ssl_certs_dir %>"
   <%- end -%>
@@ -30,13 +34,9 @@
   <%- if @ssl_crl -%>
   SSLCARevocationFile     "<%= @ssl_crl %>"
   <%- end -%>
-  <%- if @ssl_verify_depth -%>
-  SSLVerifyDepth          <%= @ssl_verify_depth %>
-  <%- end -%>
   <%- if @ssl_crl_check && scope.function_versioncmp([@apache_version, '2.4']) >= 0 -%>
   SSLCARevocationCheck    <%= @ssl_crl_check %>
   <%- end -%>
-  <%- end -%>
   <%- if @ssl_options -%>
   SSLOptions <%= Array(@ssl_options).join(' ') %>
   <%- end -%>