Fix MySQL 8.0.x incompatibilities

[ruiquelhas]

This patch accommodates some breaking changes introduced with MySQL 8.

Closes #1959

In a nutshell, the caching_sha2_password plugin (which is used by default since MySQL 8.0.4) hashes the password using SHA-256 and, after a first successful authentication attempt, saves it in a cache. That first attempt needs to be done under one of two conditions. The client either uses SSL and sends the password as clear text or it encrypts the password using the RSA public key generated by the server. On any subsequent attempt, until the password is somehow removed from the cache or the server shuts down, these rules no longer apply.

The handshake process remains unchanged when connecting to any server with version lower or equal to 8.0.3. Whereas for 8.0.4 or above, the process is now the following:

• the client sends a ClientAuthenticationPacket with a scramble computed using a SHA-256 hash
• if the password is not cached, the server sends back a PerformFullAuthenticationPacket
• if the client uses SSL, the password is sent to the server (as clear text) via a ClearTextPasswordPacket to which the server replies with a OkPacket
• otherwise it uses the server authentication public key compute the scramble, sending a AuthSwitchResponsePacket to which the server replies with a OkPacket
• if the client does not know the server public key (is not provided by the user), it requests it from the server, which sends it back using a AuthMoreDataPacket
• after a first successful authentication attempt, and until the password is cached, the server will reply to the initial ClientAuthenticationPacket with a FastAuthSuccessPacket (which basically just signals that an OkPacket will follow)

If the account is created using the mysql_native_password authentication plugin, the client will just fall back to the "traditional" process during the handshake, keeping compatibility, by default for any previously supported server version.

MySQL 8.0.2 disables the local_infile server variable by default, which breaks a couple of integration tests. The tests were updated to enable the feature by themselves (something that does not have any effect on older server versions and allows the tests to pass with newer versions).

Additionally, one of the integration tests was updated to avoid failing after the first run (using any server version) since it tried to create a table that already existed from the previous runs.

[dougwilson]

This is awesome, thank you soo much! The only real issues I'm seeing with the PR to continue to work on is the following:

• Get support working on Node.js 0.10 and below, or we can decide if this PR should be shelved for the next major release as breaking change to drop those Node.js versions. Being a widely-used base driver we try to support as many Node.js and MySQL versions as possible at once to make it easy to use for everyone.
• There may not be enough coverage of the new code added, as the coverage with the PR is decreasing. There are probably some areas that can use tests.
• MySQL 8 image needs to be stood up on the CI server to test against. This is done in the .travis.yml file similar to our current MySQL / MariaDB matrix.

[dougwilson]

Is this change related specifically to the MySQL 8 support, or just a general fix. Just curious.

[ruiquelhas]

For the new authentication magic to happen in the server, we need to explicitly provide the authentication plugin name in the ClientAuthenticationPacket, and, doing that, we need to be careful of which flags to enable, otherwise the server will, for instance, interpret caching_sha2_password or mysql_native_password as the schema/database name, if none is provided.

I didn't want to mess around with the PacketWriter and this seemed the easiest way to overcome that hurdle. However, we can try to think of something else if it feels weird.

[dougwilson]

Sure, but I was more curious on if this is a general fix or specific to MySQL 8. But even then, users can pass in their own flags list, and this module will enable / disable them unless it's listed here with a flag to force one way or the other. If having it set when it shouldn't is an issue, then it should probably be flagged here so the users cannot pass in the flag and get it set when no db is specified.

[ruiquelhas]

Actually, maybe this specific change is not required after all. The schema name is a null terminated string, so it should not cause that issue I mentioned. This is sort of a leftover from my earlier experiments. I'll try to work around this and update the patch.

Good catch!

[ruiquelhas]

To address your points.

Regarding the first one, I've missed those tests (sorry), but as far as I can tell there is no sane way to use RSA public key encryption on node <=v0.12.0. There are some packages on npm that try to add support through native addons, but I think that might not be a good idea here, since it's pure JavaScript. So the only easy way to work around that is to require users on those older node versions to use SSL when connecting to new MySQL versions. If you are OK with that, we can go that route, otherwise, going in the next major version is probably the best possibility (I guess that's mostly your call).

Missed the coverage as well, I'll try to fix that ASAP.

Again, missed the CI config. Should be fine, at least on Travis. However, besides that, I suggest we also create a test account to run the tests (besides root) to make sure the handshake works fine for both empty and non-empty passwords, which lead to different behaviors. Any issue with that? Other than that, I guess on windows we are out of luck anyway since appveyor only seems to support MySQL 5.7, correct?

[dougwilson]

Regarding the first one, I've missed those tests (sorry), but as far as I can tell there is no sane way to use RSA public key encryption on node <=v0.12.0. There are some packages on npm that try to add support through native addons, but I think that might not be a good idea here, since it's pure JavaScript. So the only easy way to work around that is to require users on those older node versions to use SSL when connecting to new MySQL versions. If you are OK with that, we can go that route, otherwise, going in the next major version is probably the best possibility (I guess that's mostly your call).

Ah, very sad :( Did you do a through search on this (it sounds like it)? Because I can always help if you want to focus on the other parts, but just asking before I duplicate the search for no reason. If we really have to drop supported versions, we can, and it wouldn't particularly delay this, just wanted to make sure it was actually required to do so before saying we should :)

Again, missed the CI config. Should be fine, at least on Travis. However, besides that, I suggest we also create a test account to run the tests (besides root) to make sure the handshake works fine for both empty and non-empty passwords, which lead to different behaviors. Any issue with that? Other than that, I guess on windows we are out of luck anyway since appveyor only seems to support MySQL 5.7, correct?

Yea, so in Travis CI it just uses a Docker image. You can make multiple non-root accounts too; the current setup was just what was needed for testing, so if MySQL 8 requires more elaborate setup, that sounds fine to me. I would ignore AppVeyor.

[ruiquelhas]

I'm using crypto.publicEncrypt() which was added in v0.11.14, and it uses native bindings which I assume were also added around that time. There are some libraries on npm (e.g. ursa and dcrypt) that claim support for earlier node versions, however they seem to implement their own native bindings, which I guess would, sort of, defeat the purpose of this module (or maybe not, you tell me).

There's this node-rsa thingy which apparently is pure JS, but I couldn't make it work and it's pretty slow. I've also seen other approaches like delegating work to openssl via a child process, but that's probably not a good idea either, since it introduces a huge moving part. There's also this forge kit which brings the entire browser kitchen sink with it and I also haven't managed to figure out how it worked.

In any case, most of these things, when they work, it's usually on node >=v0.10, and I guess this module is supposed to support at least v0.6. So, no good news from my side. However, if you can do your own research, that would, of course, be great. I'm not a cryptography expert, but from what I can tell, the reason I have not found anything good is because this RSA thingy in pure JavaScript is still not really feasible (number precision, primes and what not).

My suggestion here is to fail with a client error (the HANDSHAKE_SECURE_TRANSPORT_REQUIRED introduced by this PR) when process.env.version tells us that we are below v0.11.14, which basically would be the default behavior using { ssl: false, secureAuth: false }. It's a pretty naive solution but at least it's not really a breaking change, because existing implementations will keep working as expected with MySQL 5.7 and, let's be honest, people wo don't upgrade from those ancient node versions will, almost for sure, not upgrade MySQL to series 8 as well.

However, if you still feel you are better off bumping the major version, that's fine.

[ruiquelhas]

@dougwilson what's your preference about the issue I mentioned? Do you want me to add the workaround for older node.js versions or do you think it's better to have a major version bump (in that case I guess all we need is to remove the CI setup for older versions)?

[dougwilson]

Sorry I didn't reply. Your comment on a new issue reminded me of this. Yea that works: if the server requires an auth that the client can't do we can error out. We should feature detect that, though, not sniff the version string.

[ruiquelhas]

Ok. Just to be sure, are you talking about sniffing the Node.js version string, MySQL version string, or both? Because we have two different issues right now.

Supporting MySQL 8.0.4 (previous DMR release) which expects the password to be encrypted with the server public key using RSA_PKCS1_PADDING, whereas since 8.0.11 (the new GA release), the encryption should use RSA_PKCS1_OAEP_PADDING. We can always retry the handshake, but the only way to avoid that is by "sniffing" the MySQL server version.

Regarding the Node.js encryption APIs, I guess we can always resort to duck typing to check if they are available (or not) instead of sniffing the version. So, no problem here.

Let me know if there is something else that bothers you about these or other issues. I'll try to push this forward ASAP. Maybe during the weekend.

[dougwilson]

Why do you have to sniff the server version? Shouldn't the server say which one it expects directly? I'm just asking because I think version numbers break down when we start talking about compatible things like MariaDB, right? They may share some version numbers.

I have done Node.js version sniffing before and when it broke the Node.js collaborators just said I should never have been version sniffing their versions and told me feature detect only. So if that is the official direction from there I would go with that.

[johannes]

The case where we might need to sniff the server version is to distinguish MySQL 8.0.10 (pre GA, first one introduced chaching_sha1_passwors) from MySQL 8.0.11 (GA). since 8.0.11 was released yesterday one also could not support 8.0.10, as nobody should be using that RC anyways. jojannes

…

On April 20, 2018 1:38:35 PM GMT+02:00, Douglas Wilson ***@***.***> wrote: Sorry I didn't reply. Your comment on a new issue reminded me of this. Yea that works: if the server requires an auth that the client can't do we can error out. We should feature detect that, though, not sniff the version string.

[ruiquelhas]

Right. However, in this case we would be sort of "whitelisting" MySQL 8.0.4 specifically. I don't think MariaDB has this new authentication plugin (enabled by default or not), but I'm not entirely sure.

In any case, I believe the padding mode was probably a mistake, which was "fixed" for GA. So we can just default to RSA_PKCS1_OAEP_PADDING (the new 8.0.11 GA version) and optionally do a handshake retry for the specific case where RSA_PKCS1_PADDING is needed (MySQL 8.0.4 RC only). Or we can just simply ignore the RC version entirely, like @johannes suggested, by always using RSA_PKCS1_OAEP_PADDING.

This is currently my only open question. Other than that, not doing Node.js version sniffing makes perfect sense, so that is settled.

[ruiquelhas]

By the way, just to be clear, there was a straight jump from 8.0.4 (last RC) to 8.0.11 (GA).

https://mysqlrelease.com/2018/03/mysql-8-0-it-goes-to-11/

[dougwilson]

I like the idea to either (a) just ignore the pre GA padding or (b) retry with the old padding because that would not involve the version sniffing 👍

[ruiquelhas]

@dougwilson I've kept the option for providing a custom key padding, to allow users to exceptionally connect to a MySQL 8.0.4 RC server. In any case, the default value is always RSA_PKCS1_OAEP_PADDING which means it will work, by omission, with the latest 8.0.11 GA version.

I couldn't find a sane way to add an additional Travis CI test matrix for users with non-empty passwords though. I was constantly bumping into an authentication error (I believe it might be related to the mysql docker setup and environment variables mixup), but maybe you can give it a try.

Let me know if there is something else you would like to see in this PR. Otherwise, feel free to merge it! 😉

[dougwilson]

This is awesome, thank you. I've been reviewing it, and sorry I didn't reply that I was in progress. I did find at least one regression just in existing behavior so far, and pushed a test to cover this to master so we have it covered for the future as well. I noticed the change for the ChangeUser sequence no longer passes any of the options down to the Sequence constructor, which differs not from every other sequence. Not sure what the purpose of that change was, but it at least prevents the timeout option from getting passed down.

I'll look at what the fix is and push it up to your PR branch here 👍

[ruiquelhas]

@fan123199 everything should still work if you use the mysql_native_password plugin.

ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_password BY 'YourRootPassword';
-- or
CREATE USER 'foo'@'%' IDENTIFIED WITH mysql_native_password BY 'bar';

[Slyke]

In addition to what @ruiquelhas said, I also had to:

mysql> FLUSH PRIVILEGES;
Query OK, 0 rows affected (0.00 sec)

And then restart the server.
If you are using multiple MySQL users, ensure that you update all of them that are connecting from remote (ie, not localhost). In my case I also had to use '%' instead of 'localhost'.

[kibertoad]

@dougwilson Is this still planned to be merged? It obviously keeps accumulating conflicts while it is not :(

[dougwilson]

This PR regresses behavior and cannot be merged until fixed as I noted above. I volunteered to fix the PR but have not gotten to it. Anyone is welcome to fix the regression in the PR, though, and will merge.

[kibertoad]

@dougwilson Thanks for the update! Not sure I'll get to it in the nearby future personally, though (but it's a possibility).

[dougwilson]

It's np. I did get it mostly rebased and fixed locally but didn't quite finish. I'll bring this close to the top of my current todo list.

[ruiquelhas]

I'll look at what the fix is and push it up to your PR branch here

@dougwilson I was under the impression you would take over from there, and just assumed something else was blocking this. In any case, I'll gladly help to push this forward. Please, let me know what I can do.

[dougwilson]

You are correct. I volunteered to fix the PR. I did get it mostly rebased and fixed locally but didn't quite finish. I'll bring this close to the top of my current todo list.

[ruiquelhas]

Alright, perfect. Let me know if there is something I can help with.