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.

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

Automatic OpenSSL CA implementation #11588

Closed
wants to merge 1 commit into from
Closed

Automatic OpenSSL CA implementation #11588

wants to merge 1 commit into from

Conversation

mikn
Copy link

@mikn mikn commented Mar 27, 2014

Not ready for merge!

This is a rather crude and in its current state poorly documented CA implementation for Salt. The idea is to enable minions to request certificates which are automatically signed by a central CA which Salt administers.

Right now it requires two state runs to be able to pick up a certificate, which is not ideal. But if we refresh the pillars during the state run after we've sent the CRL from the minion, we should be able to do it in one go.

The reason I'm putting this up as a pull request was to let other people help me finish this up as I'm rather pressed on time, and from the chatter in #salt on IRC, I figured that there's plenty of people that would like something like this.

It has a couple dependencies too, you need to allow minion to master file uploads and you must create the CA yourself.

@ghost
Copy link

ghost commented Mar 27, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/2826/

@techhat
Copy link
Contributor

techhat commented Mar 27, 2014

@mikn, this is cool, and I appreciate your willingness to accept feedback.

My biggest suggestion at this early phase is to get a tutorial in for how to use it. Not only will it be helpful for other users, but it will very likely help you uncover other weaknesses that have crept into your implementation. Once you have finished the tutorial, make sure to walk through every step yourself, to see where things aren't working exactly the way you expect them to.

@thatch45
Copy link
Contributor

@mikn Is this different from the tls module?

@jsplink
Copy link
Contributor

jsplink commented Mar 28, 2014

Awww man, there's a tls module @mikn. It's pretty elegant, too. Spent some of today working on that easyrsa implementation and just finished. Oh well. Guess I'll work the night.

@mikn
Copy link
Author

mikn commented Mar 28, 2014

@thatch45 There are a couple of differences that prompted the creation of this module. First off, I wanted to have the CSR/PK-creation on the machine that it actually concerned to be able to specify things like Subject Alternative Name and fine-grained usage clauses (and for tiny security gains), which from my understanding you cannot specify from command line alone, so you need to create host-specific openssl configuration files in any case.

I also thought that the name of the module "tls" was limiting in scope compared to what you can actually do with openssl. My plan was to expand it into making a full PKI-implementation including private keys for people, automatic code signing, revocation, CRL publication and such.

@techhat I'll try to put together something and there is definitely some flow that needs to be properly thought through. Right now this implementation requires quite a bit of state code as well.

I should probably have made an issue to have the implementation discussion on, but @obimod was asking for an implementation, so I did a pull request out of what I already had. :)

The main reason I made the pull request was to spur interest and see if I can get some help on the full implementation, because I am as mentioned rather pressed on time for now.
If you want, I can close this pull request and issue a new one when I think it's actually ready to land of course!

@thatch45
Copy link
Contributor

thatch45 commented Apr 1, 2014

Hmm, I am thinking that the right thing to do would be to add these capabilities to the tls module, that way we could satisfy your needs without splintering access to openssl, what do you think @mikn ?

@thatch45
Copy link
Contributor

thatch45 commented Apr 1, 2014

Also, that would allow us to add the functions as they are written rather then holding off for everything to be completed

@mikn
Copy link
Author

mikn commented Apr 2, 2014

@thatch45 In reality I think this should be three different modules, an openssl module for implementation specific calls, a cert module for generating CSRs (or self-signed certificates) and a ca module for doing CA specific stuff like CRL generation and signing CSRs. And on top of this, you should have a state for the certificates themselves (like the pk state module) and a pillar for delivering the certificates.

The reason to split it out like this would be to be able to have a clearer separation of concerns and provide an easy interface to swap out the openssl backend to something else like gnutls or proprietary implementations depending on platform. (I think AD on windows can do these things too?)

I would like to improve this on many areas before actually having this accepted. If you want, we could merge this into the tls module. But I would recommend a migration of the tls module into two modules named ca and cert, splitting the responsibilities of CA and certificate requests to make it easier to use.

@thatch45
Copy link
Contributor

thatch45 commented Apr 2, 2014

I think that we should meet halfway, if we start breaking out the tls module it will break many users deployments, so I think that keeping it as is would be wise, but I do agree on the separation for implementation specific calls, so lets keep moving forward on this openssl module.

@mikn
Copy link
Author

mikn commented Apr 2, 2014

As a suggestion, we could create the ca and cert modules, back the tls module with those and mark it as deprecated in the documentation and remove it sometime down the road. I will probably be copying a lot of the code from the tls module into the openssl module, so the work invested in it won't be wasted. People who wants to keep it after removal could put it in their _modules folder. :)

Would you prefer me to close this pull request and open a new one when I'm ready for merging after this discussion is concluded or would you like me to keep it open?

@thatch45
Copy link
Contributor

thatch45 commented Apr 3, 2014

If we are going to write more code lets close this one out for now, and yes, lets do as you have suggested in breaking out the tls module but keeping it around for the forseeable future as a link to ca and cert

@jsplink
Copy link
Contributor

jsplink commented Apr 3, 2014

That's a good point, Thomas. I think that keeping the ca side of things separate is key (pun intended).

@kiorky
Copy link
Contributor

kiorky commented Apr 3, 2014

@mikn pyopenssl let you do whatever you want for the management of ssl artefacts, no need to go back to an openssl cli wrapper,... And aside, we also have m2crypto wich also have an API for that, both are complementary.
You may have noticed that i added recently support for subjectaltname in the tls module.

@mikn
Copy link
Author

mikn commented Apr 4, 2014

@kiorky Yep! I'll probably be reusing the code from the TLS module to fit into the openssl-module rather than the code I put into this pull request.

@thatch45 Thanks, I'll close this now and do another pull request when I have had time to clean it up! :)

@mikn mikn closed this Apr 4, 2014
@mikn mikn deleted the develop branch April 4, 2014 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants