[MDEV-34009] Introduce server-initiated instant failover mechanism (and TLS fixes) #3224
Conversation
|
|
|
Hi @dlenski. Some thoughts...
|
These first two bullet points are closely related… in fact, essentially all of my TLS-related PRs (the accepted ones, the stalled ones, and the closed ones) grew out of TLS-related discoveries made during the initial implementation of this feature. 😬
From my vantage point, there is one very clear use case for this feature as implemented here (gracefully redirecting clients to an alternate server in preparation to shutdown this one for maintenance), and it's easily extensible for other use cases such as load balancing (redirecting some-but-not-all new clients away). In this comment, @ottok nicely explains how this solution here is a very simple one for redirecting clients, and extremely time-tested in other kinds of application protocols including HTTP in particular. In this comment, I expressed my confusion about why an alternative solution was chosen for MDEV-15935, given that "As you noted yourself on the mailing list, you don't know of any actual use cases for advisory-only redirection. By contrast, we are aware of one major use case for "forceful" redirection, graceful server shutdown, as discussed here and on the mailing list."
I'm not sure what "do it client side" means. This PR does indeed involve a change in Connector/C, so that the client can automatically and seamlessly . As for "use a proxy [server]", that means adding another layer of complexity.
Ack. Created MDEV-34009 and rewrote the PR and commits to reference this new JIRA. |
Change `--disable-performance-schema` to `--loose-disable-performance-schema`. Sergei Golubchik broke this in 2bc940f. See MariaDB@2bc940f7c940#r140575671. It was subsequently fixed in MariaDB@354e97c, but that was never merged to 11.3 / 11.4 / 11.5. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
dlenski
changed the title
dlenski
force-pushed
the
MDEV-15935_instant_failover_mechanism
branch
3 times, most recently
from
46c629f
to
46f8cdb
Compare
Because MariaDB simply echoes most TLS-related error messages as they are produced by the TLS library, MTR tests that result in TLS error need to have their output normalized to account for changes in the literal text of TLS-related error messages. In the 2019 commit 576e85a, Sergei Golubchik noted that the error message produced by OpenSSL for a revoked certificate had changed in OpenSSL 1.1.1a. As of OpenSSL 3.2.0, it has changed again ("sslv3 alert" → "ssl/tls alert" in openssl/openssl@81b741f). This change normalizes the test to the output of OpenSSL 3.2.0+, and uses `--replace_regex` to ensure that it will keep working with older versions of OpenSSL as well. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
This adds the system variables `INSTANT_FAILOVER_TARGET` and
`INSTANT_FAILOVER_MODE` (`OFF`/`ON`/`ALL`), and the error code
`ER_INSTANT_FAILOVER`.
When `INSTANT_FAILOVER_MODE=ON`, the server will immediately respond
to newly-connected clients with an error packet containing the error code
4196 (`ER_INSTANT_FAILOVER`) and a specially-formatted message:
|Arbitary human-readable message|value of INSTANT_FAILOVER_TARGET
For example:
|Server is directing clients to the alternative server 'other-mariadb-server.company.com:3307'.|other-mariadb-server.company.com:3307
Updated and compatible clients can parse this message and redirect
appropriately, or display the human-readable message if they do not wish to
follow this redirection. Older clients will display the message in its
entirety, and end users should at least have an idea of what's going on.
In my earliest implementation, the sending of the `ER_INSTANT_FAILOVER`
error packet by the MariaDB server depended on the exploitation of the
client vulnerability https://jira.mariadb.org/browse/CONC-648 (“Client
improperly accepts error packets prior to TLS handshake”), which I
discovered during its implementation.
The server should obviously be able to redirect clients which don't suffer
from this severe vulnerability.
In order to do this, we need to move the redirection handling into
`native_password_authentication()`. This awkward arrangement is
necessitated by the total entanglement of the APPLICATION-layer
authentication code (e.g. username+password for "native" authentication)
with the TRANSPORT-layer security mechanism (TLS literally stands for
Transport Layer Security).
An unfortunate consequence of this is that the redirection mechanism will
not work unless the client is using the "native" authentication plugin, or
until other authentication plugins are similarly updated similarly.
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.
This additional logging is intended to demonstrate exactly when/where/how the switch from a plaintext TCP/IP socket to a TLS/SSL-wrapped socket occurs. The short summary is that the TLS-ification of the connection is completely entangled with the authentication process: - The mechanism for ensuring confidentiality and authenticity of the client/server communications at the TRANSPORT-layer is TLS (which literally stands for Transport Layer Security) - The APPLICATION-layer authentication mechanisms involve traffic exchanged above the transport layer, and support multiple plugins, such as the default "native" authentication plugin using usernames and passwords. - The transport-layer security mechanism is logically distinct from the application-layer authentication mechanism, but in the MariaDB server codebase these are thoroughly entangled and interdependent. This is a network-mislayering design problem with significant consequences for code complexity and flexibility. It leads to several potential and actual security vulnerabilities whereby information is improperly transmitted and accepted in plaintext prior to the TLS handshake. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
MariaDB servers can optionally listen for client connections on an additional TCP port (configured with the system variable `EXTRA_PORT`) in addition to the "normal" TCP port (default 3306). See https://mariadb.com/kb/en/thread-pool-in-mariadb/#configuring-the-extra-port for more information. This `EXTRA_PORT` is intended for emergency and/or administrative use, and we should therefore include it from redirection in `INSTANT_FAILOVER_MODE=ON`, just as we do for non-network based connections (Unix sockets and named pipes). The code required to check for connections to the EXTRA_PORT is greatly complicated by the fact that the `thd->net->vio->mysql_socket` and `thd->net->vio->local` data structures are not reliably or fully populated at the point where they are passed into `parse_client_handshake_packet. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
The updated libmariadb parses the `ER_INSTANT_FAILOVER` error packets (with the message component formatted as `|Human-readable message|value of INSTANT_FAILOVER_TARGET system variable`) and reconnects accordingly. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
…llback. In addition to handling server-initiated failover packets (https://jira.mariadb.org/browse/MDEV-34009), the update to the `libmariadb` submodule in the previous commit also includes fixes for: - https://jira.mariadb.org/browse/CONC-648 (Merged upstream as of mariadb-corporation/mariadb-connector-c@ebcb9ec) - https://jira.mariadb.org/browse/MDEV-28634 (mariadb-corporation/mariadb-connector-c#224, which was REJECTED upstream). This issue is indeed a Connector/C issue, not a server issue, despite its categorization in MDEV rather than CONC (see my long-ago request to move it in https://jira.mariadb.org/browse/MDEV-28634?focusedCommentId=261605#comment-261605). Due to the fix for https://jira.mariadb.org/browse/MDEV-28634, and related fixes that have already been merge dupstream, the Connector/C library and thus the `mariadb` CLI now (correctly!) fail to connect… - When `mariadb --ssl` is specified, but the server doesn't support SSL. - When `mariadb --ssl-verify-server-cert` is specified, but a self-signed cert is found in the certificate chain and it's *not* in the `--ssl-ca` list. The following tests need to be updated: - `main.mysql`: Trusted root certificate(s) must be specified (with `--ssl-ca`) in order for `--ssl-verify-server-cert` not to fail (correctly!) with an error about an untrusted self-signed cert. - `main.ssl_fp`: The combination of `--ssl-fp` (specifying an otherwise-untrusted cert's fingerprint) with `--ssl-verify-server-cert` does not make any logical sense, and indeed it correctly fails with an error about an untrusted self-signed cert. The purpose of `--ssl-verify-server-cert` is to verify a server's cert based on a set of trusted self-signed root certificates. The purpose of `--ssl-fp` is to accept a server's cert based on its specific fingerprint. These two are in conflict. - `main.ssl_7937`: Depended on insecure TLS-to-plaintext fallback. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
This option is ENABLED by default. The `main.mysqld--help` MTR test is updated as well, to reflect the addition of this option to the `mysqld --help` output. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
This MTR test verifies the basic functionality of the
INSTANT_FAILOVER_{MODE,TARGET} system variables, including:
1. `INSTANT_FAILOVER_MODE=ON` as well as `ALL`
2. The handling of the extra port and local-socket connections (redirected
in mode `ALL`, but not in mode `ON`)
3. The behavior of the client both with `--follow-instant-failovers` (the
default) and with `--disable-follow-instant-failovers`
4. The client's handling of redirect loops (>=8 redirections causes
`ER_INSTANT_FAILOVER`)
5. The ability to turn `INSTANT_FAILOVER_MODE` back to `OFF`, and to
resume connections without redirection.
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.
When MTR tests use 'NOSSL' and think they are connecting *with SSL forbidden*, they may actually connecting *with SSL required*. This is a garbage-in, garbage-out situation: all of the tests that use 'NOSSL', and all of the results, may be wrong. Adding a prominent FIXME comment here, and otherwise ignoring because this is too unrelated to the main point of the changes in this branch. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
See comments by dlenski@ on https://jira.mariadb.org/browse/MDEV-31855; almost everything about the "feature" AND the test are irredeemably wrong, and open new security holes. Changes in the `libmariadb` submodule in this server branch prevent the test from "passing"; the fact that the test "passes" does not indicate that its behavior is secure, safe, or correct. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
dlenski
force-pushed
the
MDEV-15935_instant_failover_mechanism
branch
from
46f8cdb
to
1970eb4
Compare
My intention was that these two aren't quite related. If you do not have TLS enabled, for whatever reason, this seems vulnerable to me. Or can this feature only be used when TLS is enabled? (I didn't spot that). As for the MDEV-33822 fix, if that is a bug fix for a 11.4 thing, it should probably go to 11.4, as this is an LTS release there will be more versions of it. It shouldn't be in this PR. Especially since new features won't end up landing in 11.5 any more. There is at least one other commit in this PR that will conflict soon as it appears to fix something that hasn't merged all the way up yet.
OK, I'll rephrase my point here. Why wouldn't you issue it from the proxy? In those use cases you normally have a proxy so that the web application would always connect to the correct DB server anyway. Is there citation for where users are asking for this explicitly to be in the DB server? Or are other DB servers doing this?
I meant in the end user application.
Many thanks. |
|
Thanks for the review @LinuxJedi!
No it does not, but it does ensure that if TLS is used, the redirect happens after TLS connection has been setup. MariaDB already has variable
Feel free to git cherry-pick that commit to another branch. This PR also includes other commits that have already been submitted separately, or which could be included separately but haven't been submitted separately due to low success rate for those previous separate submissions.
Yes, anyone not using a proxy setup or special clustering software would benefit from this. We are not aware of other DB servers doing this (yet), but a lot of HTTP servers are doing this. |
Exactly. And because https://jira.mariadb.org/browse/CONC-648 is now fixed in Connector/C (in mariadb-corporation/mariadb-connector-c@ebcb9ec), a MariaDB Connector/C client using TLS will not accept the failover/redirect packet until after authentication and TLS setup is complete. |
|
I've pared down this PR (and the corresponding client-side PR) to remove all of the commits which address pre-existing TLS issues which were discovered or highlighted during its development:
In other words, this ☝️ pair of PRs only introduces the instant failover feature and adds tests for it and doesn't touch anything else. |
dlenski
changed the title
But this feature seems at odds with why you would want a redirect. So, if high-availability is a goal, a stack would have multiple DB servers fronted by a proxy which would handle the fail over. Because not all maintenance is planned or happens during working hours. This is why I'm struggling to see a use case. In HTTP stacks that have similar high-availability requirements, it is typically the proxy that handles it. Sure, PHP's FastCGI or whatever it is today can probably do it, but you would likely do it in NGINX. |
|
The HTTP 302 redirect is not used in High Availability setups. Usually in
HTTP you have multiple IP addresses for one single hostname and then
multiple HTTP servers responding at those IP addresses, and various
layering of those HTTP servers with reverse proxies etc.
This is *not* mimicking that at all. This is for instant planned failover
just like HTTP 302 redirects, so you can retire one server and replace it
with a new one in a simple way.
|
The last sentence of this is the start of what I'm looking for. Now if you can expand that into a full user story / use case in the Jira, we can look into it. I don't think 302 is a great analogy for this. What you are describing appears to be something above the web server redirecting temporarily from one to the other, a web server under maintenance wouldn't be issuing 302s because you don't know how reliable it will be during the maintenance. It might be a better analogy for something like FederatedX. But at least I finally understand a bit of your thinking behind it. |
|
If you have a connection and are tracking the session_track_system_variables=redirect_url variable you'll get a server instigated redirect notice in the protocol on a result. It seems server shutdown could force connections to receive a protocol message as a result. I haven't followed the discussion in numerous places on this. Just looking at solving a use case without new protocol changes. |
|
I've updated https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/78 to have the latest version of this and I intend to work on this once we have all vector stuff done first. |
cvicentiu
added
the
External Contribution
This PR is a replacement for #2681, which proposed a very similar feature under the name of "server redirect"
Upstream developers chose a different approach (see #1472 and mariadb-corporation/mariadb-connector-c#137) for that feature. See MDEV-15935 ("connection redirection") for further discussion.
I maintain that a simple and obligatory server-directed failover mechanism will be a very useful feature for many users, so I'm reintroducing it here under the alternative name of "instant failover."
Description
This PR proposes a new feature in the MariaDB client-server protocol: a server-initiated failover which can redirect new client connections to an alternate endpoint at the application layer.
It also provides an initial server-side implementation of that feature; the corresponding client-side implementation is in mariadb-corporation/mariadb-connector-c#245.
We want the MariaDB server to be able to tell clients connecting to it, “Sorry, this server is unavailable. Connect to an alternate server instead.” This mechanism is inspired by HTTP 302 (“temporary redirect”) mechanism familiar to all developers of web applications, and is intended to have similar semantics and security properties, since these have now been widely deployed and tested for decades.
How can this PR be tested?
Automated testing
main.instant_failoverMTR test functionally verifies the instant failover mechanism.Manual testing
sql/mariadbd --instant-failover-mode=ON --instant-failover-target='some-other-mariadb.server.com:3306' --port=3306 --extra-port=3307 --socket=/tmp/mysql.sockEXTRA_PORTand to the Unix socket are not redirected:client/mariadb --host 127.0.0.1 --port=3306 -uUSERNAME -pPASSWORD→ redirected toINSTANT_FAILOVER_TARGET-client/mariadb --host 127.0.0.1 --port=3307 -uUSERNAME -pPASSWORD→ not redirected (EXTRA PORT)client/mariadb --socket=/tmp/mysql.sock -uUSERNAME -pPASSWORD→ not redirected (local/non-network-based connection)Basing the PR against the correct MariaDB version
PR quality check