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

[CONC-648] Do not trust error packets received from the server prior to TLS handshake completion #223

Merged
merged 3 commits into from
Dec 21, 2023
Merged

[CONC-648] Do not trust error packets received from the server prior to TLS handshake completion #223

merged 3 commits into from
Dec 21, 2023

Commits on Dec 13, 2023

  1. Client should reject CLIENT-only error codes sent by the server 

    Per @vuvova in
mariadb-corporation#223 (comment):

> I don't think the client should accept client-side errors from the server
> at all.

If the server sends an error packet with error codes in the ranges
`CR_{MIN,MAX}_ERROR` (codes [2000, 2999]) or `CER_{MIN,MAX}_ERROR` (codes
[5000, 5999]), we will replace these with `CR_MALFORMED_PACKET`, rather than
propagating them to the client user.
    @dlenski
    dlenski committed Dec 13, 2023
    Configuration menu
    Copy the full SHA
    f5f8032 View commit details
    Browse the repository at this point in the history

  2. [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. The authenticity of the following error cannot be verified:
    1815 - 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.

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 Dec 13, 2023
    Configuration menu
    Copy the full SHA
    8279430 View commit details
    Browse the repository at this point in the history

  3. Remove unreachable code section 

    Based on Sergei Golubchik's question about this code section in
mariadb-corporation#223 (comment),
eventually culminating in the conclusion that it's literally impossible to
reach this code section based on the types and signedess of the variables
involved:
mariadb-corporation#223 (comment)

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 Dec 13, 2023
    Configuration menu
    Copy the full SHA
    ce7fc39 View commit details
    Browse the repository at this point in the history