Fix client cert revoke error with easyrsa 3.0 #338

chloesoe · 2019-05-09T12:03:22Z

In easyrsa 3.0 (used in CentOS) the command has changed. Now there is
only a single binary to run the scripts. Further the generation of CRL
also has changed; now a new crl.pem file is created in keys/crl.pem
which overrides the symlink there. So the revocation check did not work
anymore, because the crl.pem in the base directory was not checked when
a client connected.

Resolves: VSHNOPS-1537

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Dan33l · 2019-05-09T13:05:28Z

Hi @chloesoe can you patch acceptance tests to tackle this issue and ensure that revoke process works ?

manifests/revoke.pp

Dan33l · 2019-05-09T15:09:25Z

hum ... it looks that we have an other PR about same topic #332

bastelfreak · 2019-05-11T15:58:23Z

@chloesoe can you take a look at the failing travis jobs?

Edit: One travis job run into a timeout, I restarted it.

chloesoe · 2019-05-13T21:33:37Z

Hi @chloesoe can you patch acceptance tests to tackle this issue and ensure that revoke process works ?

@Dan33l: As I'm new in Puppet I was not able to write working acceptance tests in a reasonable time. I pushed my work I did so far. Perhaps I will have couple of hours in a month to try to get it running.
With commit b848cb9 I got a step further so a revoke test could possibly work, because so we don't get a deprecation warning and tests start (but failing yet).

andrekeller · 2019-05-14T17:16:16Z

@chloesoe a3c5458 should not be fixed in this PR, the proposed solution has the same issues as #312 (making key files world readable which was not the case before). PR should not address multiple issues.

chloesoe · 2019-05-15T07:09:24Z

@andrekeller That sounds right, but acceptance tests for revoke are failing immediately without this change. And this PR only will be approved if there are acceptance tests. So should I create a separate PR for a3c5458, wait for merging, and then pull and rebase this PR so this change is applied?

Dan33l

Thank you for the job already done @chloesoe .

Please have a look to inline comments.

manifests/ca.pp

Dan33l · 2019-05-15T08:49:03Z

spec/acceptance/openvpn_spec.rb

@@ -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' :


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.

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.

andrekeller · 2019-05-15T09:27:33Z

@andrekeller That sounds right, but acceptance tests for revoke are failing immediately without this change. And this PR only will be approved if there are acceptance tests. So should I create a separate PR for a3c5458, wait for merging, and then pull and rebase this PR so this change is applied?

I'm not a maintainer, but I would drop this commit from this PR and wait for #312 to be merged into master. If you feel you can help with #312 even better. But it is definitely not in scope of this PR.

In easyrsa 3.0 (used in CentOS) the command has changed. Now there is only a single binary to run the scripts. Further the generation of CRL also has changed; now a new crl.pem file is created in keys/crl.pem which overrides the symlink there. So the revocation check did not work anymore, because the crl.pem in the base directory was not checked when a client connected. Resolves: VSHNOPS-1537

As requested in the pull request.

Add a new client with an additional revoke test. Unfortunately I was not able to get the tests working. Command to start the test is: `PUPPET_INSTALL_TYPE=agent BEAKER_IS_PE=no BEAKER_PUPPET_COLLECTION=puppet5 BEAKER_debug=true BEAKER_setfile=ubuntu1804-64vpnserver.ma{hostname=vpnserver}-ubuntu1804-64vpnclienta.a{hostname=vpnclienta} BEAKER_HYPERVISOR=docker LANG=C LC_ALL=C bundle exec rake beaker` It looks like, there weren't any revoke tests yet. So as I'm new to puppet I was not able to create revoking tests from scratch in a reasonable time.

erik-frontify · 2019-08-19T15:16:21Z

manifests/revoke.pp

+      # `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}":


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': {

vox-pupuli-tasks · 2019-12-15T14:15:57Z

Dear @chloesoe, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

vox-pupuli-tasks · 2019-12-15T14:15:57Z

Dear @chloesoe, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Rubueno · 2019-12-15T18:48:01Z

@bastelfreak This can be closed.

vox-pupuli-tasks · 2020-01-05T13:50:13Z

Dear @chloesoe, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

vox-pupuli-tasks · 2020-01-05T13:54:18Z

Dear @chloesoe, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Dan33l requested changes May 9, 2019

View reviewed changes

manifests/revoke.pp Outdated Show resolved Hide resolved

Dan33l added bug Something isn't working needs-tests labels May 9, 2019

Dan33l mentioned this pull request May 9, 2019

Revoke command missing on easy-rsa 3.0 #331 #332

Closed

chloesoe force-pushed the chloesoe/VSHNOPS-1537 branch from 689734c to ada7f05 Compare May 10, 2019 06:27

bastelfreak added the tests-fail label May 11, 2019

andrekeller force-pushed the chloesoe/VSHNOPS-1537 branch from a070186 to ae17255 Compare May 14, 2019 17:05

Dan33l requested changes May 15, 2019

View reviewed changes

Dan33l added the needs-work not ready to merge just yet label May 15, 2019

chloesoe force-pushed the chloesoe/VSHNOPS-1537 branch 3 times, most recently from 849441f to 3122dd1 Compare May 27, 2019 11:14

Dan33l mentioned this pull request Jun 10, 2019

Revoke command missing on easy-rsa 3.0 #331

Closed

chloesoe added 3 commits August 2, 2019 15:44

Use version number instead of default

d6d7360

As requested in the pull request.

chloesoe force-pushed the chloesoe/VSHNOPS-1537 branch from 3122dd1 to 1be53fc Compare August 2, 2019 13:57

erik-frontify reviewed Aug 19, 2019

View reviewed changes

This was referenced Dec 12, 2019

Enable revocation when easyrsa version 3.0 is used #368

Closed

Enable revocation when easyrsa version 3.0 is used Rubueno/puppet-openvpn#1

Closed

Enable revocation when easyrsa version 3.0 is used #369

Merged

vox-pupuli-tasks bot added the merge-conflicts label Dec 15, 2019

dhoppe removed the merge-conflicts label Jan 5, 2020

vox-pupuli-tasks bot added the merge-conflicts label Jan 5, 2020

dhoppe removed the merge-conflicts label Jan 5, 2020

vox-pupuli-tasks bot added the merge-conflicts label Jan 5, 2020

root-expert closed this Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix client cert revoke error with easyrsa 3.0 #338

Fix client cert revoke error with easyrsa 3.0 #338

chloesoe commented May 9, 2019

Dan33l commented May 9, 2019

Dan33l commented May 9, 2019

bastelfreak commented May 11, 2019 •

edited

Loading

chloesoe commented May 13, 2019

andrekeller commented May 14, 2019

chloesoe commented May 15, 2019

Dan33l left a comment

Dan33l May 15, 2019

andrekeller May 15, 2019

andrekeller commented May 15, 2019

erik-frontify Aug 19, 2019

vox-pupuli-tasks bot commented Dec 15, 2019

vox-pupuli-tasks bot commented Dec 15, 2019

Rubueno commented Dec 15, 2019

vox-pupuli-tasks bot commented Jan 5, 2020

vox-pupuli-tasks bot commented Jan 5, 2020

Fix client cert revoke error with easyrsa 3.0 #338

Fix client cert revoke error with easyrsa 3.0 #338

Conversation

chloesoe commented May 9, 2019

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Dan33l commented May 9, 2019

Dan33l commented May 9, 2019

bastelfreak commented May 11, 2019 • edited Loading

chloesoe commented May 13, 2019

andrekeller commented May 14, 2019

chloesoe commented May 15, 2019

Dan33l left a comment

Choose a reason for hiding this comment

Dan33l May 15, 2019

Choose a reason for hiding this comment

andrekeller May 15, 2019

Choose a reason for hiding this comment

andrekeller commented May 15, 2019

erik-frontify Aug 19, 2019

Choose a reason for hiding this comment

vox-pupuli-tasks bot commented Dec 15, 2019

vox-pupuli-tasks bot commented Dec 15, 2019

Rubueno commented Dec 15, 2019

vox-pupuli-tasks bot commented Jan 5, 2020

vox-pupuli-tasks bot commented Jan 5, 2020

bastelfreak commented May 11, 2019 •

edited

Loading