Merge pull request #500 from yakatz/ensure_revoke

Ensure EasyRSA certificates can be properly revoked

Author: yakatz

README.md

@@ -69,7 +69,7 @@ openvpn::client_specific_configs:
     server: 'winterthur'
     ifconfig: '10.200.200.50 10.200.200.51'
 
-openvpn::revokes:
+openvpn::revoke:
   'client3':
     server: 'winterthur'
 ```

REFERENCE.md

@@ -16,7 +16,7 @@
 * [`openvpn::ca`](#openvpn--ca): This define creates the openvpn ca and ssl certificates
 * [`openvpn::client`](#openvpn--client): This define creates client certs for a specified server as well as a tarball that can be directly imported into clients
 * [`openvpn::client_specific_config`](#openvpn--client_specific_config): This feature is explained here: http://openvpn.net/index.php/open-source/documentation/howto.html#policy All the parameters are explained in 
-* [`openvpn::revoke`](#openvpn--revoke): This define creates a revocation on a certificate for a specified server.
+* [`openvpn::revoke`](#openvpn--revoke): This define creates a revocation on a certificate for a specified server. There may not be an openvpn::client resource with the same name.
 * [`openvpn::server`](#openvpn--server): This define creates the openvpn server instance which can run in server or client mode.
 
 ## Classes
@@ -875,7 +875,7 @@ Default value: `true`
 
 ### <a name="openvpn--revoke"></a>`openvpn::revoke`
 
-This define creates a revocation on a certificate for a specified server.
+This define creates a revocation on a certificate for a specified server. There may not be an openvpn::client resource with the same name.
 
 #### Examples
 
@@ -901,8 +901,17 @@ openvpn::revoke {
 
 The following parameters are available in the `openvpn::revoke` defined type:
 
+* [`ensure`](#-openvpn--revoke--ensure)
 * [`server`](#-openvpn--revoke--server)
 
+##### <a name="-openvpn--revoke--ensure"></a>`ensure`
+
+Data type: `Enum['present', 'absent']`
+
+Revoke certificate or allow certificate to be reissued
+
+Default value: `present`
+
 ##### <a name="-openvpn--revoke--server"></a>`server`
 
 Data type: `String`

manifests/client.pp

@@ -83,6 +83,10 @@
     warning('Using $pam is deprecated. Use $authuserpass instead!')
   }
 
+  if defined(Openvpn::Revoke[$name]) {
+    fail("Can't create an Openvpn::Client configuration for client '${name}' while there is an Openvpn::Revoke configuration.")
+  }
+
   Openvpn::Server[$server]
   -> Openvpn::Client[$name]
 
@@ -96,6 +100,11 @@
 
   $server_directory = $openvpn::server_directory
 
+  # If this certificate was previously revoked, remove that file.
+  file { "${server_directory}/${server}/easy-rsa/revoked/${name}":
+    ensure => absent,
+  }
+
   if $expire {
     if is_integer($expire) {
       case $openvpn::easyrsa_version {

manifests/revoke.pp

@@ -1,6 +1,7 @@
 #
-# @summary This define creates a revocation on a certificate for a specified server.
+# @summary This define creates a revocation on a certificate for a specified server. There may not be an openvpn::client resource with the same name.
 #
+# @param ensure Revoke certificate or allow certificate to be reissued
 # @param server Name of the corresponding openvpn endpoint
 # @example
 #   openvpn::client {
@@ -15,11 +16,13 @@
 #
 define openvpn::revoke (
   String $server,
+  Enum['present', 'absent'] $ensure = present,
 ) {
-  Openvpn::Server[$server]
-  -> Openvpn::Revoke[$name]
+  if defined(Openvpn::Client[$name]) and $ensure == 'present' {
+    fail("Can't revoke certificate for client '${name}' while there is still an Openvpn::Client configuration.")
+  }
 
-  Openvpn::Client[$name]
+  Openvpn::Server[$server]
   -> Openvpn::Revoke[$name]
 
   $server_directory = $openvpn::server_directory
@@ -34,32 +37,38 @@
     default => fail("unexepected value for EasyRSA version, got '${openvpn::easyrsa_version}', expect 3.0."),
   }
 
-  file { "${server_directory}/${server}/easy-rsa/revoked/${name}":
-    ensure  => file,
-    require => Exec["revoke certificate for ${name} in context of ${server}"],
-  }
-
-  exec { "revoke certificate for ${name} in context of ${server}":
-    command  => $revocation_command,
-    cwd      => "${server_directory}/${server}/easy-rsa",
-    provider => 'shell',
-    notify   => Exec["renew crl.pem on ${server} because of revocation of ${name}"],
-    creates  => "${server_directory}/${server}/easy-rsa/revoked/${name}",
-  }
+  if $ensure == 'absent' {
+    file { "${server_directory}/${server}/easy-rsa/revoked/${name}":
+      ensure => absent,
+    }
+  } else {
+    file { "${server_directory}/${server}/easy-rsa/revoked/${name}":
+      ensure  => file,
+      require => Exec["revoke certificate for ${name} in context of ${server}"],
+    }
 
-  exec { "renew crl.pem on ${server} because of revocation of ${name}":
-    command     => $renew_command,
-    cwd         => "${server_directory}/${server}/easy-rsa",
-    provider    => 'shell',
-    refreshonly => true,
-  }
+    exec { "revoke certificate for ${name} in context of ${server}":
+      command  => $revocation_command,
+      cwd      => "${server_directory}/${server}/easy-rsa",
+      provider => 'shell',
+      notify   => Exec["renew crl.pem on ${server} because of revocation of ${name}"],
+      creates  => "${server_directory}/${server}/easy-rsa/revoked/${name}",
+    }
 
-  if ($openvpn::easyrsa_version == '3.0') {
-    exec { "copy renewed crl.pem to ${server} keys directory because of revocation of ${name}":
-      command     => "cp ${server_directory}/${server}/easy-rsa/keys/crl.pem ${server_directory}/${server}/crl.pem",
-      subscribe   => Exec["renew crl.pem on ${server} because of revocation of ${name}"],
+    exec { "renew crl.pem on ${server} because of revocation of ${name}":
+      command     => $renew_command,
+      cwd         => "${server_directory}/${server}/easy-rsa",
       provider    => 'shell',
       refreshonly => true,
     }
+
+    if ($openvpn::easyrsa_version == '3.0') {
+      exec { "copy renewed crl.pem to ${server} keys directory because of revocation of ${name}":
+        command     => "cp ${server_directory}/${server}/easy-rsa/keys/crl.pem ${server_directory}/${server}/crl.pem",
+        subscribe   => Exec["renew crl.pem on ${server} because of revocation of ${name}"],
+        provider    => 'shell',
+        refreshonly => true,
+      }
+    }
   }
 }

spec/acceptance/openvpn_spec.rb

@@ -93,7 +93,7 @@
             email        => '[email protected]',
             server       => '10.0.0.0 255.255.255.0',
           }
-          openvpn::client { ['vpnclienta','vpnclientb'] :
+          openvpn::client { ['vpnclienta'] :
             server      => 'test_openvpn_server',
             require     => Openvpn::Server['test_openvpn_server'],
           }

spec/defines/openvpn_client_spec.rb

@@ -93,6 +93,11 @@
             'target' => "#{server_directory}/test_server/easy-rsa/keys/ca.crt"
           )
         }
+
+        it {
+          is_expected.to contain_file("#{server_directory}/test_server/easy-rsa/revoked/test_client").
+            with_ensure('absent')
+        }
       end
 
       context 'with remote_host' do

spec/defines/openvpn_revoke_spec.rb

@@ -30,9 +30,6 @@
             city          => "Some City",
             organization  => "example.org",
             email         => "[email protected]"
-          }',
-            'openvpn::client { "test_client":
-            server => "test_server"
           }'
           ].join
         end
@@ -60,6 +57,30 @@
           is_expected.to contain_exec('copy renewed crl.pem to test_server keys directory because of revocation of test_client').
             with_command("cp #{server_directory}/test_server/easy-rsa/keys/crl.pem #{server_directory}/test_server/crl.pem")
         }
+
+        context 'with conflicting client' do
+          let(:pre_condition) do
+            [
+              'openvpn::client { "test_client":
+              server => "test_server"
+             }'
+            ].join
+          end
+
+          it {
+            is_expected.to compile.and_raise_error(%r{Can't create an Openvpn::Client configuration for client 'test_client' while there is an Openvpn::Revoke configuration.})
+          }
+        end
+
+        context 'remove revocation' do
+          let(:title) { 'test_client' }
+          let(:params) { { 'ensure' => 'absent', 'server' => 'test_server' } }
+
+          it {
+            is_expected.to contain_file("#{server_directory}/test_server/easy-rsa/revoked/test_client").
+              with_ensure('absent')
+          }
+        end
       end
     end
   end