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.

Sign up for GitHub

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

Jump to bottom

[MDEV-34009] Client-side implementation of instant failover mechanism (and TLS fixes) #245

Open
wants to merge 7 commits into
base: 3.4
Choose a base branch
from
Open

[MDEV-34009] Client-side implementation of instant failover mechanism (and TLS fixes) #245

wants to merge 7 commits into from

Commits on Apr 29, 2024

  1. [MDEV-28634] Do not allow silent downgrade of connections where SSL i… 

    …s requested

The 2015 commit
mariadb-corporation@23895fbd4#diff-4339ae6506ef1fb201f6f836085257e72c191d2b4498df507d499fc30d891005
was described as "Fixed gnutls support", which is an extremely incomplete
description of what it actually does.

In that commit, the Connector/C client authentication plugin was modified to
abort following the initial server greeting packet if the client has
requested a secure transport (`mariadb --ssl`), but the server does not
advertise support for SSL/TLS.

However, there's a crucial caveat here:

If the client has requested secure transport (`mariadb --ssl`) but has not
requested verification of the server's TLS certificate
(`mariadb --ssl --ssl-verify-server-cert`), then the client will silently
accept an unencrypted connection, without even printing a diagnostic message.

Thus, any such client is susceptible to a trivial downgrade to an
unencrypted connection by an on-path attacker who simply flips the TLS/SSL
capability bit in the advertised server capabilities in the greeting packet.

The entire design of MariaDB's TLS support is severely flawed in terms of
user expectations surrounding secure-by-default connections: the `--ssl`
option SHOULD imply `--ssl-verify-server-cert` BY DEFAULT; if a client
actually wants a TLS connection that's susceptible to a trivial MITM'ed
by any pervasive or on-path attacker, that should be an exceptional case
(e.g. `--insecure-ssl-without-server-cert-verification`) rather than the
default.

Even without resolving the issue of no default verification of server
certificates, the issue of silent downgrade to unencrypted connections can
and should be resolved.

Backwards compatibility: This is an INTENTIONAL backwards-incompatible
change to prevent clients from being silently downgraded to unencrypted
connections, when they've specified `--ssl` and thus clearly indicated that
they do not want unencrypted connections.

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
    dlenski committed Apr 29, 2024
    Configuration menu
    Copy the full SHA
    668abf9 View commit details
    Browse the repository at this point in the history

  2. [MDEV-28634] Move check for server TLS/SSL capability to mthd_my_real… 

    …_connect

Two reasons:

1. Reduction of attack surface

   As soon as the client receives the server's capability flags, it knows
   whether the server supports TLS/SSL.

   If the server does not support TLS/SSL, but the client expects and
   requires it, the client should immediately abort at this point in order
   to truncate any code paths by which it could inadvertently continue to
   communicate without TLS/SSL.

2. Separation of concerns

   Whether or not the server supports TLS/SSL encryption at the transport
   layer (TLS stands for TRANSPORT-layer security) is a logically separate
   issue from what APPLICATION-layer authentication modes the client and
   server support or should use.

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
    dlenski committed Apr 29, 2024
    Configuration menu
    Copy the full SHA
    71b52a0 View commit details
    Browse the repository at this point in the history

  3. Add comments about usage of TLS/SSL in connection establishment 

    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
    dlenski committed Apr 29, 2024
    Configuration menu
    Copy the full SHA
    484a959 View commit details
    Browse the repository at this point in the history

  4. Consider Unix socket, shared memory, and named pipe connections to be… 

    … "as secure as TLS" in terms of encryption

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
    dlenski committed Apr 29, 2024
    Configuration menu
    Copy the full SHA
    188873c View commit details
    Browse the repository at this point in the history

  5. [MDEV-34009] Follow instant failover error packets 

    If the server responds to our initial connection with an error packet having
error number `ER_INSTANT_FAILOVER`, and an error string formatted as
`|Human-readable message|host[:port]`, then redirect accordingly.

This redirection is performed in `mthd_my_real_connect`, which appears to be
the most appropriate location given that it already contains code for
retrying/repeating a new server connection.

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
    dlenski committed Apr 29, 2024
    Configuration menu
    Copy the full SHA
    58a682d View commit details
    Browse the repository at this point in the history

  6. Allow clients to disable server-initiated instant failovers 

    This adds a new boolean option, `follow-instant-failovers` (ENABLED by
default), to allow the client to disable server-initiated instant failover
and simply return an error message.

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
    dlenski committed Apr 29, 2024
    Configuration menu
    Copy the full SHA
    997d245 View commit details
    Browse the repository at this point in the history

  7. Limit number of server-initiated instant failovers followed by client 

    Set the limit to 8 for now. This will prevent the client from getting stuck
in redirect loops.

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
    dlenski committed Apr 29, 2024
    Configuration menu
    Copy the full SHA
    fe2dd63 View commit details
    Browse the repository at this point in the history