Allow only TLS - Fixes POODLE CVE-2014-3566

[ghoneycutt]

No description provided.

[ghoneycutt]

@kbarber ping

[kbarber]

@ghoneycutt I've been aware of this one, but since we've already patched PuppetDB with the 2.2.2 release to drop SSLv3 from the defaults I'm still considering whether providing the defaults here in the module is a good idea or not. The problem being, defaults in two places will cause natural drift unless one is very attentive. The ability to change it on its own is fine. Thoughts?

On another note I'm getting sick of how static new options are in this module as well. I really want to fix this so we're not having to modify the module every time we change PuppetDB. Just an aside, but it does feel like a lot of extra work for what is ultimately just a mapping exercise.

[kbarber]

@ghoneycutt ping, see my previous comment.

[ghoneycutt]

@kbarber I believe this and any other bit of configuration should be in the module since that is the way to manage your PuppetDB installation. Perhaps you do not always use this module for PuppetDB since you are a developer, but as a consumer, I want this module to be the canonical way to manage every aspect of the software.

There will be some duplication in settings between PuppetDB itself and this module, but this should only be for the default settings such as this one, which is normal behavior when writing modules.

[kbarber]

@kbarber I believe this and any other bit of configuration should be in the module since that is the way to manage your PuppetDB installation.

It is one of many potential ways. We also change defaults on users at times in the core clojure parts of PuppetDB when we know they are no longer meaningful. Not every user uses the module also.

@ghoneycutt I think you're seeing this from a different angle to me, so perhaps I'll explain in another way. The defaults for ssl-protocols are already defined in the latest version of PuppetDB, and are set to TLSv1, TLSv1.1 and TLSv1.2 as of PuppetDB 2.2.2 which was released a few days ago. Right now that default is there and active and it's a reasonable one for the current situation.

The module on the other hand doesn't have to express a duplicate copy of this default, we've made that decision already in the core code. Expressing it there as well means when we change the default in the core code, we have to also change it in the module and release them in lock step with each other, otherwise the effort won't be as effective.

I agree that we should have a way to configure this setting in the module, but what I do not currently agree with is setting a default in the module. I would rather stop doing that to avoid the maintenance going forward. We already have examples of this that are annoying, and they should go away as well.

So right now I'd happily take a patch that changes the setting like you show here, but one that didn't also provide a default, so if no param was provided it simply did nothing.

[ghoneycutt]

Should I set the param to undef and wrap the ini_setting{} resource in an if statement that checks if it is set?

[kbarber]

@ghoneycutt I think that's suitable for now.

[ghoneycutt]

Thanks Ken