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

Restrict access to the Puppet master by default #215

Merged
merged 1 commit into from
Oct 15, 2015
Merged

Restrict access to the Puppet master by default #215

merged 1 commit into from
Oct 15, 2015

Conversation

michaelweiser
Copy link
Contributor

Set up a certificate whitelist file and configure it in PuppetDB so that
only the Puppet master has access by default.

@@ -12,6 +12,10 @@
$open_ssl_listen_port = undef
$postgres_listen_addresses = 'localhost'

$certificate_whitelist_file = '/etc/puppetdb/certificate-whitelist'
Copy link
Contributor

Choose a reason for hiding this comment

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

This path is no longer what we use for the latest version of the module - that is, be careful to lift the path from other variables, not static like this - this isn't a good location to store it for PuppetDB 3.x for example.

@kbarber
Copy link
Contributor

kbarber commented Oct 14, 2015

@michaelweiser so making this default now, means it will be a breaking change for some. Can you see a world where this option is for now optional and the default behaviour is to not provide a certificate-whitelist? This will allow us to get the feature right, allow users of current module to test it out without breaking etc.

@kbarber kbarber added this to the 5.1.0 (master) milestone Oct 14, 2015
@michaelweiser
Copy link
Contributor Author

@kbarber: I would advocate for making the default enabled since otherwise any root or puppet user on any Puppet client remains able to download the complete catalogs of all other clients from PuppetDB including any and all passwords and other sensitive information contained therein. I can not see how anyone would have a legitimate use case for this behaviour. I would like to see it improved and users made aware of it by the fact that they actively have to open PuppetDB up to connections from more hosts. But if your judgement is that this does not outweigh the hassle of breakage, we can certainly leave it disabled for now.

I just realised, though: There might be a need to more intelligently determine a list of possibly multiple Puppet masters needing access to this one PuppetDB than just looking at $::servername. I have no clear idea where this information might come from, yet. Any thoughts?

BTW: New commit regarding the changed $confdir forthcoming.

@kbarber
Copy link
Contributor

kbarber commented Oct 14, 2015

I would advocate for making the default enabled since otherwise any root or puppet user on any Puppet client remains able to download the complete catalogs of all other clients from PuppetDB including any and all passwords and other sensitive information contained therein.

Its more that we have a behaviour now, breaking it would mean shipping a 6.0 by our versioning rules. I think we could switch this on by default by 6.0, but this can't go out as a 5.1.0 change. My suggestion is to slot the feature in without it enabled today, and we switch the default later in a 6.0. This also means the feature can be 'bedded-in' also.

I just realised, though: There might be a need to more intelligently determine a list of possibly multiple Puppet masters needing access to this one PuppetDB than just looking at $::servername. I have no clear idea where this information might come from, yet. Any thoughts?

I think if you need multiple puppet masters, possibly better to provide a static list. Users could query this information from elsewhere, but its fragile since this can create bad chicken/egg breakages if done incorrectly.

Add the option to set up a certificate whitelist file and configure it
in PuppetDB so that only specific hosts (i.e. the Puppet master(s)) have
access.
@michaelweiser
Copy link
Contributor Author

Here's a new patch with changed default and spec tests even. Yay! :)

kbarber added a commit that referenced this pull request Oct 15, 2015
Restrict access to the Puppet master by default
@kbarber kbarber merged commit ed371ce into puppetlabs:master Oct 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants