Details
-
New Feature
-
Status: Closed (View Workflow)
-
Critical
-
Resolution: Fixed
Description
This needs a change in the client auth plugin API
- client authentication plugins to get a new method hash_password(), the same as in the server plugin
The new authentication will work like this
Client side, when sending client reply packet:
- If SSL is used, and --ssl-verify-server-cert is in force, but
- no --ssl-ca or --ssl-fingerprint is in force, and
- the certificate failed validation as self-signed, and
- client authentication plugin doesn't have hash_password() method, and
- the non-empty password was provided, then
- disconnect, otherwise
- continue (let's call it late certificate validation mode)
Server side, when sending the OK packet after successful authentication:
- if SSL is used, and
- the certificate is ephemeral (after
MDEV-31856), and - the account has non-empty password, then
- calculate SHA2(user's hashed password, scramble, certificate fingerprint), and
- put it in the OK's info field, prefixed by byte 0x01
Client side, when receiving OK packet:
- if in the late certificate validation mode, then
- use hash_password() callback, calculate SHA2(user's hashed password, scramble, certificate fingerprint), compare
Notes
- client plugin versions and the API version have to be incremented
- the server doesn't know if the client is in the late password validation mode, so it might do some unnecessary work just in case
- this could be fixed by a new capability bit, or
- just live with potential unnecessary work on connect — it is assumed that in overwhelming majority of the cases this work will be necessary (almost all setups will use this mode)
Attachments
Issue Links
- blocks
-
-
- Closed
-
- causes
-
MDEV-33639 AuthSwitchResponse info has changed since 11.4
-
- Closed
-
- relates to
-
-
- Closed
-
-
-
- Closed
-
-
-
- Closed
-
-
-
- Open
-
-
-
- Closed
-
-
-
- Open
-
Activity
Neither issue you mentioned above is a TLS attack, and 3 out of four are not even at "attack" as such.
Thank you for confirming that you're less concerned about this particular protocol extension, supposedly it means that there is no step-by-step explanation of how this mechanism could be attacked.
serg, please, make a small changes in test main.ssl_autoverify in cases with "- - no-defaults" option - add option "--character-sets-dir" with value otherwise on some environments I get warnings in this cases:
../mariadb: Warning: Charset id '33' csname 'utf8' trying to replace existing csname 'utf8mb3'
|
../mariadb: Warning: Charset id '83' csname 'utf8' trying to replace existing csname 'utf8mb3'
|
I have already reported several practical TLS attacks against actual production MariaDB code which has existed for many years, e.g.
CONC-648, CONC-654,CONC-656, MDEV-31585. I've proposed fixes for all of these; they remain mostly unfixed.I would describe all these bugs as "pretty obvious": I didn't set out to find TLS bugs in MariaDB, but once I started looking at the relevant code for other reasons, they started to jump out at me all over the place.
I've also explained several times (e.g. comment on CONC-648 from 3 months ago how these vulnerabilities are largely a consequence of improper entanglement between different conceptually separate layers of networking protocols. This is not some very specialized knowledge: it's included in basically every guide to programming with TLS, and in many reports of previous real-world TLS vulnerabilities.
What you have proposed here, serg, is yet another new entanglement between the application-layer authentication (e.g. MariaDB native password auth) and the transport-layer security mechanism (TLS), as I mentioned in a comment right above).
I am less concerned about the particular vulnerability that you are going to introduce if you merge this code, than I am with what appears to be broken or missing processes in MariaDB development.
It appears that there must be little or no review of the security implications of code changes like the one proposed here.