-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
Support MySQL 8 Authentication #2233
base: master
Are you sure you want to change the base?
Conversation
41f01c5
to
1044a14
Compare
This comment has been minimized.
This comment has been minimized.
This PR came in a bit before I took some time off. I am spending this weekend to get items on this module in place, though. I hope that helps. Now that the original PR has been rebased and CI is all passing, just need to do a quick review and then we can have a new version out here soon. |
An update to what I said above (since there are some thumbs ups): I'm currently dealing with some fallout where GitHub is flagging a module as a security issue when it is not, which is causing many folks to open support requests I need to respond to. I have been trying to get GitHub to correct the detection, but have been unsuccessful in getting any response from them. I may end up having no choice but to spend the time to rework a module to get a dependency updated just to get the support requests to stop if GitHub does not correct their detection. This may end up using all my time this weekend, but I will keep posted here. I wanted to post an update so folks don't think I'm ignoring this pr/repo if I don't get to it, just there is a higher priority issue at hand. |
@dougwilson Thank for your work. I really appreciate it. |
@dougwilson this comes from a place of absolute understanding of how much awesome work you do - would it be helpful if other people tested this PR? |
Yes, that would be awesome! I was also thinking this weekend to spin out an actual beta package to npm with this as well to help even myself get it testing in qa. |
For what it’s worth we have been running a build using this PR fork in our beta branch for a few months now and so far not encountered an issue |
If there's anything we (Oracle/MySQL) can do to help please let us know. |
Oh that would be fab - we (Ghost) would definitely be able to dedicate a bit of time to testing that next week 🙌 |
throw err; | ||
} | ||
|
||
var stage1 = xor((Buffer.from(password + '\0', 'utf8')).toString('binary'), scramble.toString('binary')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be changed to rolling xor, otherwise it'll fail for password longer than 19 characters
See discussion at sidorares/node-mysql2#1044
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I've been working to fix this PR up over the past week and I did notice that long passwords didn't work on the new auth. There are a bunch of other little minor issues I've been finding as well. I didn't intend to list them out and was just intending to push up all the changes here then merge, but let me know if you think I should list them all out in addition to pushing up the fixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that mysql2 is using slightly different api for this. No config required, and if you need to use non-defaults you pass custom configured plugin as authPlugins: { caching_sha2_password: XXXX }
. Should not be a big problems for user migrating both ways mysql<->mysql2
Hi any update to this? |
Hi @ahrib thanks for asking! Just earlier this year I released a minor update with a few things. It may very will be the last 2.x release. This is because I am organizing work for a 3.x line which will include this new auth methods from this pr. Look forward to an alpha or beta releasing soon! I have been getting the things resolved this week. Sorry for the extended "lul" in work here, but I'm committed to full stream to get many of these prs merged and released into a 3.0 early this year! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Where is this at? Ive just started to work on an upgrade from 8.0.16 to 8.0.21 and cannot connect with ER_NOT_SUPPORTED_AUTH_MODE: Client does not support authentication protocol requested by server; consider upgrading MySQL client. I am at a dead standstill and if I cant get a resolution here very quickly I need to look at moving to the MySQL supplied client which is not my preference. |
@drwharris From my end, I don't plan on updating this PR. For my own work, I'm going to be switching to You can keep using |
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as resolved.
This comment was marked as resolved.
Some notes on debug connecting to MySQL8 with The
|
This comment was marked as spam.
This comment was marked as spam.
Incorporated nwoltman's PR#2233 that adds support for authentication using the caching_sha2_password plugin which is the default in MySQL 8 mysqljs#2233
…ail for passwords longer than 19 characters. Thanks to sidorares and normano for their guidance. Refer to: mysqljs#2233 (comment) sidorares/node-mysql2#1044 sidorares/node-mysql2#1045 Updated version to 2.18.3
@1chisensei and others - if you are in a hurry, you can try my fork. It's available on NPM as @vlasky/mysql. I have not only merged @nwoltman's fantastic PR, but I have also implemented the rolling XOR fix suggested by @sidorares that ensures passwords longer than 19 characters will work with the caching_sha2_password plugin. I hope @dougwilson eventually finds the time to continue his excellent work on this package. I forked it only because I urgently needed to add MySQL 8 support and (partial) protocol compression support to my other fork @vlasky/zongji which is a MySQL binlog listener that relies on functionality that currently only exists in this mysql package and not in node-mysql2. |
…anced authorization, which is not yet available in mysql package (see mysqljs/mysql#2233)
This comment was marked as spam.
This comment was marked as spam.
The MySQL database container failed to start. It failed with this error: > unknown variable 'default-authentication-plugin=mysql_native_password' This is due to the untagged `mysql` image being updated to MySQL 8.4. Replacing it with `--mysql-native-password=ON`, as described in the [MySQL 8.4 changelog][changelog], runs into a different issue, where the `mysql` npm package does not support the newer MySQL authentication mechanisms. The `mysql` npm package is de facto deprecated in lieu of the `mysql2` npm package. There has been an outstanding [open PR][pr] for the last five years, which is very unlikely to be merged. This commit locks the MySQL version to 8.3, which is the last version that still supports the now deprecated authentication mechanisms, and therefore the last version that will be compatible with the `mysql` npm package. [skip changeset] [changelog]: https://dev.mysql.com/doc/refman/8.4/en/mysql-nutshell.html [pr]: mysqljs/mysql#2233 Co-authored-by: Noemi Lapresta <noemi@appsignal.com>
The MySQL database container failed to start. It failed with this error: > unknown variable 'default-authentication-plugin=mysql_native_password' This is due to the untagged `mysql` image being updated to MySQL 8.4. Replacing it with `--mysql-native-password=ON`, as described in the [MySQL 8.4 changelog][changelog], runs into a different issue, where the `mysql` npm package does not support the newer MySQL authentication mechanisms. The `mysql` npm package is de facto deprecated in lieu of the `mysql2` npm package. There has been an outstanding [open PR][pr] for the last five years, which is very unlikely to be merged. This commit locks the MySQL version to 8.3, which is the last version that still supports the now deprecated authentication mechanisms, and therefore the last version that will be compatible with the `mysql` npm package. [skip changeset] [changelog]: https://dev.mysql.com/doc/refman/8.4/en/mysql-nutshell.html [pr]: mysqljs/mysql#2233 Co-authored-by: Noemi Lapresta <noemi@appsignal.com>
The MySQL database container failed to start. It failed with this error: > unknown variable 'default-authentication-plugin=mysql_native_password' This is due to the untagged `mysql` image being updated to MySQL 8.4. Replacing it with `--mysql-native-password=ON`, as described in the [MySQL 8.4 changelog][changelog], runs into a different issue, where the `mysql` npm package does not support the newer MySQL authentication mechanisms. The `mysql` npm package is de facto deprecated in lieu of the `mysql2` npm package. There has been an outstanding [open PR][pr] for the last five years, which is very unlikely to be merged. This commit locks the MySQL version to 8.3, which is the last version that still supports the now deprecated authentication mechanisms, and therefore the last version that will be compatible with the `mysql` npm package. [skip changeset] [changelog]: https://dev.mysql.com/doc/refman/8.4/en/mysql-nutshell.html [pr]: mysqljs/mysql#2233 Co-authored-by: Noemi Lapresta <noemi@appsignal.com>
The mysql dockerfile has been pinned at 8.3 to avoid errors in the default authentication plugin. See mysqljs/mysql#2233 for more information. It might be prudent to switch to mysql2 instead of mysqljs for future release. I've renamed all the server model files to have a numeric prefix to allow execution in order. The 99-debug.sql file crashes, but it is not necessary for the functioning of the BHIMA server. You can now type `docker-compose up` and be taken to the BHIMA installation page on localhost. Environmental variables are set in the `.env` file as usual.
This is a rebase + update of @ruiquelhas's PR #1962.
One thing that has been changed from the original PR is that only MySQL 8.0.x has been added to integration tests rather than multiple RC versions of MySQL 8. It's been long enough that people are probably only using GA versions of MySQL 8 now.
Fixes #2002
Original PR Description
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 for8.0.4
or above, the process is now the following:ClientAuthenticationPacket
with a scramble computed using a SHA-256 hashPerformFullAuthenticationPacket
OkPacket
AuthSwitchResponsePacket
to which the server replies with aOkPacket
AuthMoreDataPacket
ClientAuthenticationPacket
with aFastAuthSuccessPacket
(which basically just signals that anOkPacket
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.