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

So I have a test like (with custom config since require_secure_transport=ON) #596

Closed
grooverdan opened this issue Jun 24, 2024 · 6 comments

Comments

@grooverdan
Copy link
Member

          So I have a test like (with custom config since `require_secure_transport=ON`)
test: [ "CMD", "healthcheck.sh", "--defaults-file=/etc/mysql/conf.d/my.cnf", "--connect", "--innodb_initialized" ]

And it's spamming a lot of warnings like

2024-06-24 11:44:15 120 [Warning] Aborted connection 120 to db: 'unconnected' user: 'unauthenticated' host: '::1' (This connection closed normally without authentication)

And looks like it's spamming this specifically if there is a --connect argument, even though validation succeeds. I tested this specifically spamming the respective command manually on the container.
I understand that this is "kind of" normal, since --connect is expected to just try to establish TCP connection, not authenticate, but can we somehow suppress the warnings for the check? The settings in manual do suggest using 10s interval, but then it means spamming the false-positive warning every 10 seconds, even when connection is technically established.
Or am I doing something wrong?

Originally posted by @Simbiat in #573 (comment)

@grooverdan
Copy link
Member Author

caused by #594

grooverdan added a commit to grooverdan/mariadb-docker that referenced this issue Jun 25, 2024
require-secure-transport on the server mandates that tls or
unix socket be used. The healthcheck user doesn't have explict
tls credentials, so would have failed. 11.4+ would have
tls negiotated, except in MariaDB#594 it was disabled for people that
didn't configure ssl-ca correctly.

To resolve this _process_sql adds an explict --protocol socket
to get around the default configuration of 'protocol=tcp' in
.my-healthcheck.sh. The protocol=tcp was there to catch people
who put `healthcheck.sh --innodb_initialized` to discover it
checked that in the starting phase of the container, without
a tcp connection being available, it still returned true.

We work around this my making a connection test always
occur in the healthcheck.

Remove the protocol=tcp from the generation of .my-healthcheck.cnf
files.

--connect, as a method that requires to test the connection,
we add a mechanims that examines @@skip_networking and considers
that if false, the connection is viable. We made a unix socket
connection to do the test, which is active the same time as tcp
sockets are.

This alternate --connect method would have only worked the
credentials of the healthcheck user where valid. If it isn't
fall back to looking for "Can't connect".

Closes: MariaDB#596
@grooverdan
Copy link
Member Author

@Simbiat - testing of #597 welcome.

grooverdan added a commit to grooverdan/mariadb-docker that referenced this issue Jun 25, 2024
require-secure-transport on the server mandates that tls or
unix socket be used. The healthcheck user doesn't have explict
tls credentials, so would have failed. 11.4+ would have
tls negiotated, except in MariaDB#594 it was disabled for people that
didn't configure ssl-ca correctly.

To resolve this _process_sql adds an explict --protocol socket
to get around the default configuration of 'protocol=tcp' in
.my-healthcheck.sh. The protocol=tcp was there to catch people
who put `healthcheck.sh --innodb_initialized` to discover it
checked that in the starting phase of the container, without
a tcp connection being available, it still returned true.

We work around this my making a connection test always
occur in the healthcheck.

Remove the protocol=tcp from the generation of .my-healthcheck.cnf
files.

--connect, as a method that requires to test the connection,
we add a mechanims that examines @@skip_networking and considers
that if false, the connection is viable. We made a unix socket
connection to do the test, which is active the same time as tcp
sockets are.

This alternate --connect method would have only worked the
credentials of the healthcheck user where valid. If it isn't
fall back to looking for "Can't connect".

Closes: MariaDB#596
@Simbiat
Copy link

Simbiat commented Jun 25, 2024

I am not sure I fully understand the description of the pull request, but if what it's changing is related to a workaround for incorrect TLS, then I am not sure this is correct approach.
My logic is simple: if require_secure_transport is off - we do not care for TLS and if it fails during --connect, but we still connect - it's fine. If require_secure_transport is on, then failing of TLS handshake essentially means failure of --connect. No, if the server-side keys are not setup correctly - that's on server admin entirely, and if keys are not present, or otherwise fail - we should fail the healthcheck. But if the issue is with incorrect client-side keys - that's something that can be managed: just generate the keys for healthcheck user, if they do not exist. If a custom config is used - that's again on the server admin, so we should fail the check if something is wrong.
As for removing protocol=tcp, I got inconsistent behavior. On test environment - that did seem to help, but on prod environment - no, even after container restart to ensure new config is pulled in (in case there is some caching). But then, i also switched to using custom config in the first place (for the sake of TLS), and custom config does not have protocol in it at all, and I was still getting warnings, when using --connect

@grooverdan
Copy link
Member Author

Am I missing the minimal case of what is the issue:

$ podman run --rm --env MARIADB_ROOT_PASSWORD=1 --name msec  -d  mariadb:11.4 --require-secure-transport=1
$ podman exec msec bash -x -v healthcheck.sh --connect && echo ya

Seem to be healthy? Are you using TLS certs/keys as well?

@Simbiat
Copy link

Simbiat commented Jun 25, 2024

Contents of config are here, security.cnf included in it has only login/password for client and ssl_* files for both server and client (3 for each).
Server launched in Docker container through docker compose (dockerfile).

healthcheck.sh --defaults-file=/etc/mysql/conf.d/my.cnf --connect

In this case it works fine, but generates warnings in logs.

healthcheck.sh --connect

Fails because no TLS is used, and not setup in healthcheck.cnf generated on original container start

@grooverdan
Copy link
Member Author

Thanks for the clarification and I can immediately see that #594 caused this problem for you.

There's a current problem with < 11.4 images which is:

$ podman run --rm --env MARIADB_ROOT_PASSWORD=1 --name msec  -d  mariadb:10.6 --require-secure-transport=1
1a4620f1de4ef21e666dd60f671ca82dd0c1be834682bd71da5fa0dd6777b499

$ podman exec msec healthcheck.sh --connect --innodb_initialized
ERROR 1045 (28000): Access denied for user 'healthcheck'@'::1' (using password: YES)
healthcheck innodb_initialized failed

This probably isn't a new problem, but using --require-secure-transport would make the healthchecks fails and it seems important to fix this at the same time.

Because healthchecks are over tcp and with TLS disabled because of #594, the --require-secure-transport is considering the connection insecure and failing to perform any SQL test.

11.4, similar thing with a better error message:

$ podman run --rm --env MARIADB_ROOT_PASSWORD=1 --name msec  -d  mariadb:11.4 --require-secure-transport=1
1cae8bb80bee3d2583c1dcd68a711e26d2e671c92b558f786ccd2c17905f7733

$ podman exec msec healthcheck.sh --connect --innodb_initialized
ERROR 3159 (08004): Connections using insecure transport are prohibited while --require_secure_transport=ON.
healthcheck innodb_initialized failed

So its these two cases that is making the fix a little more extensive that originally envisioned. The alternate is using tcp and creating tls certs and keys for a local connection.

The unix socket counts as a secure transport, but the determination of --connect without causing warnings required a connection to be successful. This is why the check of SELECT @@skip_networking exists. To ensure that this isn't a install/upgrade instance starting.

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

No branches or pull requests

2 participants