sighupio / furyagent

Apache License 2.0
9 stars 2 forks source link

OpenVPN CRL implementation and config generation #8

Closed lnovara closed 4 years ago

lnovara commented 4 years ago

The aim of this PR is to implement Certificate Revocation List in furyagent to address #3.

Summary of changes:

lzecca78 commented 4 years ago

Amazing work @lnovara ! What do you think about to consolidate all the client openvpn related operation with a wrapper script (in go, why not?) that allow to keep versioning of the user created/revoked from a yaml file ? This allow to handle vpn user seamlessly from everyone without been synced from previous actions.

lnovara commented 4 years ago

Thanks for your feedback @lzecca78!

I don't know if having something to version the created/revoked users from furyagent would be a good idea, in fact I don't think that we should maintain a list of users inside furyagent config file either.

I would like user creation/revocation to be like furyagent configure openvpn-client [--revoke] user@example.com (maybe with OpenVPN config with in-line certificates as output).

What I was thinking about was to just have an "index" (or the certificate themselves) saved on furyagent storage backend and some UI (like furyagent configure openvpn-client --list, for example) to list them since the creation/revocation date are informations already available from either the certificate itself or the CRL.

+---------------------+------------+------------+---------+----------------+
|     Client name     | Valid from |  Valid to  | Expired |    Revoked     |
+---------------------+------------+------------+---------+----------------+
| jondoe@example.com  | 2020-01-30 | 2021-01-31 | ✖       | ✖              |
| janedoe@example.com | 2019-01-20 | 2020-01-20 | ✓       | ✖              |
| user@example.net    | 2019-05-18 | 2020-05-18 | ✖       | ✓ (2020-01-12) |
+---------------------+------------+------------+---------+----------------+

What do you think?

lzecca78 commented 4 years ago

Could be a good idea this solution as well! Even if i wasn't thinking to add the openvpn details yaml to the furyagent.yaml but to a dedicated one (for example: openvpn.yml). This could allow to have a quick view from this file without interacting with the cli. I am totally agree that the furyagent.yml must be unaware of these behaviours.

ralgozino commented 4 years ago

+1 to your proposal @lnovara One question though, does this right now still use the furyfile.yaml to get the user list to revoke? It's not clear to me from the code. If that's the case I'd recommend having a different list for revokation than for creation in the file just to avoid revoking all the certificates by mistake :)

lnovara commented 4 years ago

@ralgozino you're right, for revocation I am using the list in furyagent configuration file and that might be dangerous.

If we all agree, I will mark this PR as WIP and remove from furyagent configuration file the logic related to user creation/revocation.

ralgozino commented 4 years ago

An idea came to my mind.. can we use the Kubernetes CA for OpenVPN? I know there's an API endpoint to sign certificates for example: https://kubernetes.io/docs/tasks/tls/managing-tls-in-a-cluster/#create-a-certificate-signing-request

This could enable us to use our Permissions Manager to enable / disable OpenVPN access also (maybe)

What do you think?

lnovara commented 4 years ago

Using the Kubernetes API wouldn't help since it doesn't have a mechanism to revoke certificate, furthermore I would like to keep the two CAs separate since it's not granted that if you have VPN access you should access the Kubernetes API as well.

angelbarrera92 commented 4 years ago

An idea came to my mind.. can we use the Kubernetes CA for OpenVPN? I know there's an API endpoint to sign certificates for example: https://kubernetes.io/docs/tasks/tls/managing-tls-in-a-cluster/#create-a-certificate-signing-request

This could enable us to use our Permissions Manager to enable / disable OpenVPN access also (maybe)

What do you think?

I like the idea, but maybe the approach could be different to avoid chicken-egg problems or what @lnovara said about certificate revocation.

I was thinking in something like a "vault" with our own CA (SIGHUP). Then create intermediate CA for every customer.

Then, I want to test if a certificate created from the root CA can be used across clients.

I don't know, maybe i am thinking louder.

I think is a good idea to discuss/design in a room all together ;)

phisco commented 4 years ago

+1 for our own Vault, this already was discussed a few times and I think it might be time to start thinking about it seriously.

ralgozino commented 4 years ago

Just to let you know. I've created a card on our Product Trello board to keep track of this PR and prioritize when we should work on it: https://trello.com/c/isYppKQe

lnovara commented 4 years ago

Thanks @ralgozino. I know that this was low on our priority list but since this PR was 90% done I pushed some more work and now I consider it complete and ready to be reviewed.

ralgozino commented 4 years ago

No problem at all @lnovara, the idea of the card was to actually keep this PR on the radar and work on it asap :) I think this adds a lot a value, thanks!!

lnovara commented 4 years ago

I've updated the README and added a changelog.

ralgozino commented 4 years ago

+1 to increasing a minor release. I think we should first merge the patch of the version command and then merge this minor. What do you think?

angelbarrera92 commented 4 years ago

+1 to increasing a minor release. I think we should first merge the patch of the version command and then merge this minor. What do you think?

Fully agree

lnovara commented 4 years ago

Totally agree with you. I am bumping the version to 0.1.0 in the changelog.

angelbarrera92 commented 4 years ago

Before merging it, resolve conflicts by rebasing this branch with master one ;) After that, awesome job @lnovara!

lnovara commented 4 years ago

@angelbarrera92 Rebase done, feel free to merge it.

lnovara commented 4 years ago

OpenVPN CRL implementation - furyagent should log the generated openvpn S/N to allow CRLs usage #3

lnovara commented 4 years ago

OpenVPN CRL implementation - furyagent should log the generated openvpn S/N to allow CRLs usage #3