validate ssl certificates using client password

What security properties of the client-server connection would actually be improved by doing this?

It seems to me that this will greatly complicate the MariaDB client and server code. In my opinion, it is a bad design, because it further entrenches the entangling of two logically-separate concerns:

1. Transport-layer: establishing an encrypted channel between the client and an authenticated server (TLS)
2. Application-layer authentication and authorization

Entangling these is an extremely bad design choice, that has already led to several security vulnerabilities and greatly complicates solving them in a clean and backwards-compatible way, including at least CONC-648, CONC-654, and MDEV-31585. I wrote at greatly length about this in a comment on CONC-648.

So, I would say that it's important to step back and think about what the goal is here, in terms of improving the security properties of the system, and then think about how to achieve that in ways that reduce, rather than exacerbate, existing security vulnerabilities and design flaws.

The goal is to prevent active MitM of course by validating SSL certificates.

And no, the implementation (V1) of this is quite small and simple.

The goal is to prevent active MitM of course by validating SSL certificates.

The title of this JIRA is “validate ssl certificates using client password”.

How is validation of the server’s SSL certificate (that's a transport-layer concern) conceivably related to the client’s MariaDB user password (that's an application-layer concern)?

in bb-11.3-serg branch, commits:

2c293980590 MDEV-31857 enable --ssl-verify-server-cert by default in the internal client

92d1e29f9eb MDEV-31855 validate ssl certificates using client password in the internal client

dc725663522 disable SSL via named pipes in the internal client

fecaa34d58a free mysql->connector_fd correctly in the internal client

d61d5c2b2f5 change how self-signed certs are accepted by internal client

4354452378d cleanup: X509_check_host() in the internal client

a170006cce6 cleanup: ssl handling in the internal rpl client

350cb3f311f MDEV-31857 enable --ssl-verify-server-cert by default

0131af509a9 enable --ssl in the server by default

b3fe66e7299 MDEV-31856 use ephemeral ssl certificates

8d608f6811e wrong error for bare --ssl on the server side

e651ca6497a cleanup

1ccb8bc0d78 test SSL MitM attack

a8895302211 client support for --tls-fp and --tls--fplist

3d6d49641f3 MDEV-31855 validate ssl certificates using client password

88b1c98c2d8 cleanup: unify client's setting of ssl options

6f0d7df2cb4 test.cnf files should !include default_my.cnf

8b573729ba6 clarify CR_OK_HANDSHAKE_COMPLETE

OK to push (after adding coments about format of the protocol extention (SSL info))

sanja wrote:

OK to push (after adding coments about format of the protocol extention (SSL info))

Yikes.

Do you understand that serg is essentially proposing a new TLS key exchange mechanism here?

Have you solicited the help of experts in cryptographic protocol design who can evaluate the security of this mechanism?

If I were to provide a step-by-step explanation of how this mechanism could be attacked and the client's password stolen undetectably, would you change you mind about "okay to push"?

In a comment on MDEV-31856, serg wrote:

yes, sure. the design document is in MDEV-31855. There's a full high-level description of the new protocol extension.
I'd appreciate if you'd try to come up with attacks against it.

Yes, I already have a detailed step-by-step attack against it.

Using this attack, it is possible for a MITM to fool the client into thinking that it has connected to a server, when it has in fact connected to the MITM, and for the MITM to obtain the client's password.

This attack is exacerbated by a (seemingly-unrelated) security flaw in MariaDB which has never been reported, to my knowledge. It is a very good illustration of why it is dangerous to mix authentication at different protocol layers, as I've been explaining over and over.

Do you want me to explain it?

of course, if you were to provide a step-by-step explanation of how this mechanism could be attacked, we won't push this feature in this form.

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).

of course, if you were to provide a step-by-step explanation of how this mechanism could be attacked, we won't push this feature in this form.

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.

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'

Fixed, thanks

Testing done. Ok to push