Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix client cert revoke error with easyrsa 3.0 #338

Closed
wants to merge 3 commits into from
Closed

Fix client cert revoke error with easyrsa 3.0 #338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
92cf6cd
Fix client cert revoke error with easyrsa 3.0
chloesoe Mar 22, 2019
d6d7360
Use version number instead of default
chloesoe May 10, 2019
1be53fc
WIP Add new client to test revoke (not working)
chloesoe May 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions manifests/revoke.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,33 @@

$etc_directory = $openvpn::etc_directory

exec { "revoke certificate for ${name} in context of ${server}":
command => ". ./vars && ./revoke-full ${name}; echo \"exit $?\" | grep -qE '(error 23|exit (0|2))' && touch revoked/${name}",
cwd => "${etc_directory}/openvpn/${server}/easy-rsa",
creates => "${etc_directory}/openvpn/${server}/easy-rsa/revoked/${name}",
provider => 'shell',
case $openvpn::easyrsa_version {
'3.0': {
exec { "revoke certificate for ${name} in context of ${server}":
command => ". ./vars && ./easyrsa --batch revoke ${name}; echo \"exit $?\" | grep -qE '(error 23|exit (0|2|))' && touch revoked/${name}",
cwd => "${etc_directory}/openvpn/${server}/easy-rsa",
creates => "${etc_directory}/openvpn/${server}/easy-rsa/revoked/${name}",
provider => 'shell',
}
# `easyrsa gen-crl` does not work, since it will create the crl.pem
# to keys/crl.pem which is a symlinked to crl.pem in the servers etc
# directory
exec { "renew crl.pem for ${name}":
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will renew the CRL on every puppet run if there is a "revoke". Can be fixed like that:

@@ -32,14 +32,16 @@ define openvpn::revoke (
         cwd      => "${etc_directory}/openvpn/${server}/easy-rsa",
         creates  => "${etc_directory}/openvpn/${server}/easy-rsa/revoked/${name}",
         provider => 'shell',
+        notify  => Exec["renew crl.pem for ${name}",],
       }
       # `easyrsa gen-crl` does not work, since it will create the crl.pem
       # to keys/crl.pem which is a symlinked to crl.pem in the servers etc
       # directory
       exec { "renew crl.pem for ${name}":
-        command  => ". ./vars && EASYRSA_REQ_CN='' EASYRSA_REQ_OU='' openssl ca -gencrl -out ../crl.pem -config ./openssl.cnf",
-        cwd      => "${openvpn::etc_directory}/openvpn/${server}/easy-rsa",
-        provider => 'shell',
+        command     => ". ./vars && EASYRSA_REQ_CN='' EASYRSA_REQ_OU='' openssl ca -gencrl -out ../crl.pem -config ./openssl.cnf",
+        cwd         => "${openvpn::etc_directory}/openvpn/${server}/easy-rsa",
+        provider    => 'shell',
+        refreshonly => true,
       }
     }
     '2.0': {

command => ". ./vars && EASYRSA_REQ_CN='' EASYRSA_REQ_OU='' openssl ca -gencrl -out ../crl.pem -config ./openssl.cnf",
cwd => "${openvpn::etc_directory}/openvpn/${server}/easy-rsa",
provider => 'shell',
}
}
'2.0': {
exec { "revoke certificate for ${name} in context of ${server}":
command => ". ./vars && ./revoke-full ${name}; echo \"exit $?\" | grep -qE '(error 23|exit (0|2))' && touch revoked/${name}",
cwd => "${etc_directory}/openvpn/${server}/easy-rsa",
creates => "${etc_directory}/openvpn/${server}/easy-rsa/revoked/${name}",
provider => 'shell',
}
}
default: {
fail("unexepected value for EasyRSA version, got '${openvpn::easyrsa_version}', expect 2.0 or 3.0.")
}
}
}
14 changes: 13 additions & 1 deletion spec/acceptance/openvpn_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
apply_manifest_on(hosts_as('vpnserver'), pp, catch_failures: true)
apply_manifest_on(hosts_as('vpnserver'), pp, catch_changes: true)
end

it 'creates openvpn client certificate idempotently' do
pp = %(
openvpn::server { 'test_openvpn_server':
Expand All @@ -56,7 +57,18 @@
remote_host => $facts['networking']['ip'],
tls_cipher => 'TLS-DHE-RSA-WITH-AES-128-GCM-SHA256:TLS-DHE-RSA-WITH-AES-128-CBC-SHA',
}
)

openvpn::client { 'vpnclientb' :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the moment the test is failing because the revoke process is not idempotent. So you did a good test. It cached idempotency issue. Great !

It is not idempotent because during the second run some modifications are done:

 Info: Applying configuration version '1557908446'
  Notice: /Stage[main]/Main/Openvpn::Server[test_openvpn_server]/Openvpn::Ca[test_openvpn_server]/File[/etc/openvpn/test_openvpn_server/easy-rsa/revoked/vpnclientb]/group: group changed 'root' to 'nogroup'
  Notice: /Stage[main]/Main/Openvpn::Server[test_openvpn_server]/Openvpn::Ca[test_openvpn_server]/File[/etc/openvpn/test_openvpn_server/easy-rsa/revoked/vpnclientb]/mode: mode changed '0644' to '0750'

And so the puppet code of revoke needs an update to become idempotent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The permission change comes from here: https://github.com/voxpupuli/puppet-openvpn/blob/master/manifests/ca.pp#L75
I would argue this is wrong in the first place. Why does this need to be executable? I'd propose to fix this to be root / 640.

You would then need to ensure that the revoked file is created with the correct permission, you could amend the touch (https://github.com/voxpupuli/puppet-openvpn/blob/master/manifests/revoke.pp#L29) with a chmod to ensure the proper permissions. (You could also replace this with install install -b -m 640 /dev/null revoked/${name} however I'm not sure how portable the install command is.

server => 'test_openvpn_server',
require => Openvpn::Server['test_openvpn_server'],
remote_host => $facts['networking']['ip'],
tls_cipher => 'TLS-DHE-RSA-WITH-AES-128-GCM-SHA256:TLS-DHE-RSA-WITH-AES-128-CBC-SHA',
}

openvpn::revoke { 'vpnclientb' :
server => 'test_openvpn_server',
}
)
apply_manifest_on(hosts_as('vpnserver'), pp, catch_failures: true)
apply_manifest_on(hosts_as('vpnserver'), pp, catch_changes: true)
end
Expand Down
31 changes: 26 additions & 5 deletions spec/defines/openvpn_revoke_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,33 @@
let(:params) { { 'server' => 'test_server' } }

it { is_expected.to compile.with_all_deps }
context 'easyrsa version 2.0' do
let(:facts) do
super().merge('easyrsa' => '2.0')
end

it {
is_expected.to contain_exec('revoke certificate for test_client in context of test_server').with(
'command' => ". ./vars && ./revoke-full test_client; echo \"exit $?\" | grep -qE '(error 23|exit (0|2))' && touch revoked/test_client"
)
}
it {
is_expected.to contain_exec('revoke certificate for test_client in context of test_server').with(
'command' => ". ./vars && ./revoke-full test_client; echo \"exit $?\" | grep -qE '(error 23|exit (0|2))' && touch revoked/test_client"
)
}
end
context 'easyrsa version 3.0' do
let(:facts) do
super().merge('easyrsa' => '3.0')
end

it {
is_expected.to contain_exec('revoke certificate for test_client in context of test_server').with(
'command' => ". ./vars && ./easyrsa --batch revoke test_client; echo \"exit $?\" | grep -qE '(error 23|exit (0|2|))' && touch revoked/test_client"
)
}
it {
is_expected.to contain_exec('renew crl.pem for test_client').with(
'command' => ". ./vars && EASYRSA_REQ_CN='' EASYRSA_REQ_OU='' openssl ca -gencrl -out ../crl.pem -config ./openssl.cnf"
)
}
end
end
end
end