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

Tentacle register action does not work with invalid certificates are used #123

Open
brentm5 opened this issue Nov 2, 2017 · 5 comments
Open

Comments

@brentm5
Copy link
Member

brentm5 commented Nov 2, 2017

This is in response to #122 where using a self signed certificate did not work correctly because the chef http client enforces valid certificates.

@eelco-de-boer
Copy link

eelco-de-boer commented Nov 3, 2017

Well let's continue the discussion here instead of in the PR.
The problem is not only limited to using self signed certificates. It is also affecting people using certificates from their own internal CA's.
Basically Any solution that doesn't use the trusted root store of Windows itself is not going to work.
And i really don't want to keep a double administration of my internal CA's root certs just to make crappy openssl happpy.
Also making it ignore certificate valdiation is not an option. We are under such regulations where not validating, is a no-go.

So I would still use powershell and if you have a problem (for example the PS 3.0 requirement) with using Invoke-RestMethod, then i would suggest using .net library's System.Net.Webcleint (since Invoke-RestMethod is a wrapper around it anyway).

We are already using the solution i put in in the merge request in our own environment and it works like a charm. Also passes our internal tests that run with Gitlab CI and CAN do tests on windows unlike Travis.

So for me to make this issue solved, it should provide a similar functionality that is in my PR (e.g. check root certs in the Windows Certificate store, not OpenSSL certfile.pem.) Until then we will use my fork with my changes.

@brentm5
Copy link
Member Author

brentm5 commented Nov 4, 2017

Thank you for the description that actually makes a lot more sense TBH. I think this makes sense to add into the main branch of Octopus deploy however because its windows specific (and octopus deploy is starting to become less OS specific) I would like to add it as a flag in the tentacle resource to chose whether to add it or not.

We should simply be able to upgrade the following chunk of code with what you implemented and then pass in an option via the resource properties to use that implementation instead of yours.

https://github.com/cvent/octopus-deploy-cookbook/blob/master/libraries/tentacle.rb#L49-L53

That way we can keep the original implementation which is easily testable but also support your completely valid use case. In the future It probably makes sense to abstract away the idea of the api client and implement a powershell version and the default ruby version.

@brentm5
Copy link
Member Author

brentm5 commented Dec 14, 2017

@eelco-de-boer this might be interesting to look into https://github.com/chef/win32-certstore

@eelco-de-boer
Copy link

Very interesting indeed :)

@aae42
Copy link

aae42 commented Dec 20, 2017

we had to deal with this issue, too...

this seems more like a chef limitation than a limitation of the cookbook, though, any cookbook that makes use of the chef api to hit one of the https sites we're securing with our internal PKI is going to have this issue, we coded around it for linux and windows in our base cookbooks that get applied to all nodes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants