puppetlabs/puppetlabs-mysql

SSL cert/key params in server.cnf break new versions of mariadb. SSL is disabled.

loopiv opened this issue ยท 19 comments

Describe the Bug

With ssl set to false, the module still adds the cert, ca, and key params which break the latest version of mariadb on EL7.

Expected Behavior

Those params should not be present when ssl is disabled.

Steps to Reproduce

Steps to reproduce the behavior:

  1. On EL7, upgrade mariadb to latest version as of 08-Nov-2022.
  2. MariaDB fails to start.

Environment

  • Version = 12.0.1
  • Platform = CentOS 7

Additional Context

We're not enabling ssl so it's set to false as default.

Upgraded to 13.0.1 and the bug is still present. I think it should be safe to set those params to undef for RedHat systems.

This is not only an issue on RedHat systems; it's just the same on Ubuntu.

I agree, that this is a broader issue and needs a broader (in terms of operating systems) fix than the one suggested in #1511.

That being said, I think it calls for a definite decision on whether TLS should be enabled by default or not. Given the changes in MariaDB, it seems a decision to enable TLS by default would entail also generating proper certs/keys by this module.

Since generating the keys properly has problems in itself ("self-signed" is only marginally better than unencrypted traffic) the go-to solution should probably be to disable TLS by default (so set those parameters to undef - but for every operating system in contrast to the PR only changing this for RedHat).

All that being said, one might argue that this is "only a MariaDB issue" since they now introduced this change to abort the daemon startup, but I think this assessment is wrong: the TLS context hasn't been initialised properly on other operating systems, too. The module just didn't care because it had no consequences but from an abstract point of view, this is broken because it does not provide secure encryption by default.

Cheers
Thomas

Side note (and I'm happy to keep this out of the scope of this issue): given that this module plays TLS police in other places I would say that it requires quite a level of resistance to cognitive dissonance, that you would warn in one case, but happily configure invalid encryption in other places just for the sake of "having enabled encryption" (assuming you even have certs/keys in the default locations at all, because otherwise it just silently fails).

vmpr commented

this issue broke my database after an update to the latest version 10.5.18-MariaDB on Debian.
the parameters need to be undef otherwise every puppet run will break our system again.
is there a quick way to fix that until the module is fixed? i can't change the code my self as we pull direct via r10k from puppet forge and there are already opened MRs

We were bit by this as well, and can confirm that explicitly setting the ssl values to undef fixes the issue.
An upstream fix for the default values would be nice so we don't need to explicitly override them.

vmpr commented

We were bit by this as well, and can confirm that explicitly setting the ssl values to undef fixes the issue. An upstream fix for the default values would be nice so we don't need to explicitly override them.

how do you override them to undef? is there a way via hiera? we pull the code direct from puppet forge with r10k.

There's no way with just hiera that I know of, because you can't set a literal undef value via hiera (it will default to the default puppet value instead). You must explicitly set the values in the puppet code with a class resource instead of doing include mysql::server

There's no way with just hiera that I know of, because you can't set a literal undef value via hiera

Oh but you can, after all this is YAML (if you're actually using the YAML backend, that is):

mysql::server::override_options:
  mysqld:
    ssl-disable: false
    ssl-ca: ~
    ssl-cert: ~
    ssl-key: ~

Depending on your hiera structure, you may also want to deep-merge the override_options key - whether you need this mainly depends on how you built your hiera structure:

lookup_options:
  mysql::server::override_options:
    merge: deep

So... now that everybody has a solution I'd pretty much like to focus on the question of how to fix this in the module for good.

Cheers
Thomas

MariaDB 10.3.37 was pushed to Ubuntu 20.04 as a security update today (so gets done by unattended-upgrades) which now also breaks because of this in addition to EL7:
https://bugs.launchpad.net/ubuntu/+source/mariadb-10.3/+bug/1997916

IMHO the correct fix is in the template, which says:

<%     v.sort.map do |ki, vi| -%>
<%       if ki == 'ssl-disable' or (ki =~ /^ssl/ and v['ssl-disable'] == true) -%>
<%         next %>

This should test the value of v['ssl'] as well.

This also appears to affect MariaDB on Ubuntu 20.04. Having ssl = false still has MariaDB fail to start with this:

2022-12-02 16:12:06 0 [ERROR] Failed to setup SSL
2022-12-02 16:12:06 0 [ERROR] SSL error: SSL_CTX_set_default_verify_paths failed
2022-12-02 16:12:06 0 [ERROR] Aborting

Hey, sorry for the delay, we are currently investigating this issue. After a short team discussion, we are currently favouring @kjetilho approach to solving this problem. However, we will have to make sure that this solution does not have unexpected side effects. So it will take a bit longer before we can merge a fix. Sorry for the inconvenience.

Hey @loopiv, a PR by @kjetilho has been merged to address this issue. The current main build of the module here in GitHub should have it. Can you let us know if this fix has resolved the issue and, if so, close the issue?

I would argue that this is a bug in this MySQL patch: https://jira.mariadb.org/browse/MDEV-29811
When ssl is set to false, MySQL should not try to read the certificates. There is another bug report for it here, too: https://jira.mariadb.org/browse/MDEV-30092

I would argue that this is a bug in this MySQL patch: https://jira.mariadb.org/browse/MDEV-29811 When ssl is set to false, MySQL should not try to read the certificates.

yes, I totally agree, but we need to handle what we have, and IMHO it is good to keep the config file clean from these unused directives anyway.

As far as I can tell, #1513 released in v13.1.0 fixed this. Maybe we can close this issue?

This issue seems to have been resolved. Therefore, we will be closing it. If anyone else experiences this issue, feel free to reopen it.

Verified that this fix has resolved the issue and the issue can be closed.

Hey @loopiv, a PR by @kjetilho has been merged to address this issue. The current main build of the module here in GitHub should have it. Can you let us know if this fix has resolved the issue and, if so, close the issue?

Hello all,

I found this bug because on our system with the puppet mysql module 13.1.0 the following options are configured:

ssl = false
ssl-ca = /etc/puppetlabs/puppet/ssl/certs/ca.pem
ssl-cert = /etc/ssl/certs/CERT.pem
ssl-key = /etc/ssl/private/KEY.pem

Now I want to update to version 15.0.0 and the puppet want to remove ssl-XXX options.

On the Debian 11, mysqld Ver 10.5.23-MariaDB-0+deb11u1 our MariaDB can start and even mysqld --verbose --help says it's using SSL:

# mysqld --verbose --help | grep -Pi ^ssl
ssl                                                          TRUE
ssl-ca                                                       /etc/puppetlabs/puppet/ssl/certs/ca.pem
ssl-cert                                                     /etc/ssl/certs/CERT.pem
ssl-key                                                      /etc/ssl/private/KEY.pem

and mysql --verbose --help says:

--ssl               Enable SSL for connection (automatically enabled if an ssl option is used).
--ssl-ca=name       CA file in PEM format (check OpenSSL docs, implies --ssl)
--ssl-capath=name   CA directory (check OpenSSL docs, implies --ssl)
--ssl-cert=name     X509 cert in PEM format (implies --ssl)
--ssl-cipher=name   SSL cipher to use (implies --ssl)
--ssl-crl=name      CRL file in PEM format (check OpenSSL docs, implies --ssl)
--ssl-crlpath=name  CRL directory (check OpenSSL docs, implies --ssl)
--ssl-key=name      X509 key in PEM format (implies --ssl)

so probably even if ssl is set to false, the other ssl-XXX options enable SSL.

So it's the question now, if the condition v['ssl'] == false in the template should stay.

<%  if ki == 'ssl-disable' or (ki =~ /^ssl/ and v['ssl-disable'] == true) or (ki =~ /^ssl-/ and v['ssl'] == false) -%>

For me, it's really logical, if I want to use SSL, I should enable SSL. So I'am going to add ssl: true into override_options now.

But in this case mysqld reaction on these contradictory options is really not clear.

Regards,
Robert.