-
Notifications
You must be signed in to change notification settings - Fork 256
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
[MDEV-15935] Client-side implementation of connection redirection mechanism #226
Commits on Jun 12, 2023
-
[CONC-648] Do not trust error packets received prior to TLS handshake…
… completion MariaDB Connector/C does not distinguish [application-layer error packets](https://mariadb.com/kb/en/err_packet) that it receives prior to TLS handshake completion from those that it receives immediately after. (A trivially modified server built from dlenski/mariadb-server@demonstration_of_CONC-648_vulnerability can easily be used to demonstrate this.) Pre-TLS error packet received from this trivially modified server. This packet should NOT be trusted to actually originate from the server: $ mariadb --ssl --ssl-verify-server-cert -uUsername -pVerySecretPassword -h CONC-648.vuln.demo.server.com ERROR 1815 (HY000): Internal error: Client will accept this error as genuine even if running with --ssl --ssl-verify-server-cert, and even though this error is sent in plaintext PRIOR TO TLS HANDSHAKE. Post-(TLS handshake) error packet received from a normal MariaDB server upon an attempt to connect with incorrect credentials. This error packet CAN be trusted to actually originate from the server, assuming transitive trust in the TLS protocol implementation and PKI-based certificate validation: $ mariadb --ssl --ssl-verify-server-cert -uUsername -pWrongPassword -h $NORMAL_MARIADB10.6.14_SERVER ERROR 1045 (28000): Access denied for user 'Username'@'A.B.C.D' (using password: YES) This client behavior opens up MariaDB Connector/C clients to an extremely straightforward [downgrade attack](https://en.wikipedia.org/wiki/Downgrade_attack). An on-path or pervasive attacker can inject errors into MariaDB client→server connections that are intended to be protected by TLS, and the client has no clear mechanism to distinguish such errors from errors that actually come from the server. An attacker could easily use this to DOS a client, or even influence its behavior. For example, consider a client application which is configured… 1. To use TLS with server certificate validation (`--ssl --ssl-verify-server-cert`), and 2. To wait for a back-off period and then *retry* connection attempts if the server responds with `ER_CON_COUNT_ERROR` ("Too many connections") from the server, and 3. To give up and shut down if its connection attempts fail with `ER_ACCESS_DENIED_ERROR` ("Access denied for user"), on the assumption that this is due to an incorrect or expired password, and cannot be resolved without human intervention. An attacker could completely disable the retry mechanism of this application by intercepting connection attempts and replying with `ER_ACCESS_DENIED_ERROR` packets. This patch modifies MariaDB Connector/C so that if the client is configured to use TLS, error packets received prior to the completion of the TLS handshake are untrusted, and are changed to a generic `CR_CONNECTION_ERROR`. $ mariadb --ssl --ssl-verify-server-cert -uUsername -pVerySecretPassword -h CONC-648.vuln.demo.server.com ERROR 2002 (HY000): Received error packet before completion of TLS handshake. Suppressing its details so that the client cannot vary its behavior based on this UNTRUSTED input. 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.
Configuration menu - View commit details
-
Copy full SHA for abad64b - Browse repository at this point
Copy the full SHA abad64bView commit details
Commits on Jun 28, 2023
-
[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.
Configuration menu - View commit details
-
Copy full SHA for 165110d - Browse repository at this point
Copy the full SHA 165110dView commit details -
[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.
Configuration menu - View commit details
-
Copy full SHA for 2ebbeb9 - Browse repository at this point
Copy the full SHA 2ebbeb9View commit details -
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.
Configuration menu - View commit details
-
Copy full SHA for b090994 - Browse repository at this point
Copy the full SHA b090994View commit details
Commits on Jul 5, 2023
-
[MDEV-15935] Follow server redirection error packets
If the server responds to our initial connection with an error packet having error number 4196 (`ER_SERVER_REDIRECT`), 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.
Configuration menu - View commit details
-
Copy full SHA for c73f9bd - Browse repository at this point
Copy the full SHA c73f9bdView commit details
Commits on Jul 6, 2023
-
Allow clients to disable following server redirects
This adds a new boolean option, `follow-server-redirects` (ENABLED by default), to allow the client to disable server redirection 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.
Configuration menu - View commit details
-
Copy full SHA for 54886ac - Browse repository at this point
Copy the full SHA 54886acView commit details -
Limit number of server redirects 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.
Configuration menu - View commit details
-
Copy full SHA for 9768c6e - Browse repository at this point
Copy the full SHA 9768c6eView commit details