Hi, Nikita, On Sep 09, Nikita Malyavin wrote:
Hello Sergei!
This patch has a server-side only effect. You see, the old code was sending *either* the internally-generated scramble, or the plugin-provided payload. But never both.
which is correct, I don't see why it would need to send both
The protocol would interpret both as a scramble.
You wrote:
How can thd->scramble[SCRAMBLE_LENGTH] be not zero at this point?
thd->scramble[SCRAMBLE_LENGTH] is set to 1 in acl_authenticate():
if (command == COM_CHANGE_USER) { ... } else { /* mark the thd as having no scramble yet */ thd->scramble[SCRAMBLE_LENGTH]= 1;
So when THD is just connecting, it has 1 in the (SCRAMBLE_LENGTH+1)-th byte.
yes, sorry, I didn't phrase the question correctly. thd->scramble[SCRAMBLE_LENGTH]= 1; was needed to tell the plugin to generate the scramble. A plugin can be called in the middle of the authentication or in the COM_CHANGE_USER. It doesn't know what happens before it and need to know if the scramble was already generated. The function where you added your condition if (thd->scramble[SCRAMBLE_LENGTH] != 0) it's send_server_handshake_packet(), it can only happen as the first packet during authentication, you know with certainty that the scramble was not yet generated, you don't need a condition for that.
This is true only for the server: libmariadb makes no such marking, so it's harder (I know no way) to distinguish the initial handshake from an extra roundtrip.
Overall, my solution is to use only the first 20 bytes as a nonce used to generate the SSL secret. Quick'n'dirty, maybe, but I think that supporting arbitrary length scrambles can be done on top of it, if necessary.
it won't be easy to add later without breaking the compatibility with existing clients, I suspect. Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org