Skip to content
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

Node-mysql does not support auth-switch request from server. #1396

Closed
twocode opened this issue Apr 25, 2016 · 13 comments
Closed

Node-mysql does not support auth-switch request from server. #1396

twocode opened this issue Apr 25, 2016 · 13 comments

Comments

@twocode
Copy link

@twocode twocode commented Apr 25, 2016

Node-mysql does not support auth-switch request from server, and treats it as an error message.

It is not correct.

@sidorares
Copy link
Member

@sidorares sidorares commented Apr 25, 2016

Are you talking about https://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::AuthSwitchRequest ?

Probably right solution right now would be not to set CLIENT_PLUGIN_AUTH flag ( not sure if it's currently set by default or not ). AuthSwitchRequest support would only make sense if the whole auth handshake sequence is pluggable ( right now client is always trying to connect with Authentication::Native41 )

@dougwilson
Copy link
Member

@dougwilson dougwilson commented Apr 27, 2016

Probably right solution right now would be not to set CLIENT_PLUGIN_AUTH flag ( not sure if it's currently set by default or not ).

We not only do not set the flag, but we also prevent users from setting the flag as well (https://github.com/felixge/node-mysql/blob/master/lib/ConnectionConfig.js#L109).

@sidorares
Copy link
Member

@sidorares sidorares commented Apr 28, 2016

If so then it's something wrong with @twocode server setup. Server must not send AuthSwitchRequest if client's CLIENT_PLUGIN_AUTH flag is not set.

@twocode , could you post what is your server version (and node-mysql version you are using as well)?

@twocode
Copy link
Author

@twocode twocode commented Apr 28, 2016

Thanks @sidorares and @dougwilson .

I am aware that CLIENT_PLUGIN_AUTH bit it not set in node-mysql's capability flags and that node-mysql forces to use Native41 authentication method (mysql_native_password) to do the authentication with servers.

However, this design/implementation is blocking something I am working on, which is some mysql proxy and leverages auth-swith flows to authenticate the user. I have tested other popular client drivers (php, java, perl, .Net, ...) and only node-mysql fails here.

I would like to make a patch that include following changes:

  1. Set the CLIENT_PLUGIN_AUTH bit in the client capability flags.
  2. Implement AuthSwitchRequest and AuthSwitchResponse packets.
  3. Still keeping the only authentication method of mysql_native_password,

In this way, if server would like to auth switch to mysql_native_password, node-mysql will send the token again with the new scramble; otherwise, if server auth swtiches to another authentication method, node-mysql will return error. This follows mysql protocol flow and complies to other mysql client drivers.

Please comment?

Thanks

@twocode
Copy link
Author

@twocode twocode commented May 3, 2016

Hi @sidorares

Glad to see you have opened an issue for this in node-mysql2. Have you got any plans to fix it?

Thx

@sidorares
Copy link
Member

@sidorares sidorares commented May 3, 2016

@twocode potentially, at least in a way that CLIENT_PLUGIN_AUTH flag is not set until we can understand auth-switch request ( and maybe add auth-switch request support later )

@twocode
Copy link
Author

@twocode twocode commented May 6, 2016

@sidorares I see. What do you say that I make the changes based on the propose above? It is vital that node-mysql/node-mysql2 support this mysql protocol feature for us.

@sidorares
Copy link
Member

@sidorares sidorares commented May 6, 2016

@twocode it's relatively specific change that you need (and no one else requested), I'm afraid that the only reliable way for you to make this happen soon is to start implementing feature and submit PR :)

@dougwilson
Copy link
Member

@dougwilson dougwilson commented May 6, 2016

Hi @twocode, yes, please feel free to implement a pull request as you described like @sidorares suggested :) We can probably get this added at some point ourselves, but of course that would not have any good timeline, so providing a pull request is the fastest method.

Please try to ensure that this new auth switch packets and mechanisms are accompanied with tests as well. This module has two types of tests: "unit" tests which run against a fake MySQL server using this module and "integration" tests that run against actual MySQL servers. You can put the tests in which ever you feel is the most appropriate.

@twocode
Copy link
Author

@twocode twocode commented May 11, 2016

Thanks both. Let me prepare a patch first.

@DJ-DJL
Copy link

@DJ-DJL DJ-DJL commented Jan 31, 2017

I'm still getting this error today on node-mysql2 version 1.1.2 installed via npm. Was the fix reverted? or maybe the same symptoms have a different cause?

@dougwilson
Copy link
Member

@dougwilson dougwilson commented Jan 31, 2017

Hi @DJ-DJL the mysql2 module is separate from this one.

@sidorares
Copy link
Member

@sidorares sidorares commented Jan 31, 2017

yes, @DJ-DJL please fill issue at mysql2 repo with as much context as you have. As far as I know AuthSwitch is only supported by mysql2, feel free to help it ported to mysqljs

bgrainger added a commit to bgrainger/mysql that referenced this issue May 14, 2017
Rename UseOldPasswordPacket to AuthenticationMethodSwitchRequestPacket
and implement its two other fields: plugin name & data.

Support mysql_native_password and mysql_old_password as potential
authentication methods (but still require 'insecureAuth:true' for the
latter).
bgrainger added a commit to bgrainger/mysql that referenced this issue May 14, 2017
Rename UseOldPasswordPacket to AuthenticationMethodSwitchRequestPacket
and implement its two other fields: plugin name & data.

Support mysql_native_password and mysql_old_password as potential
authentication methods (but still require 'insecureAuth:true' for the
latter).
bgrainger added a commit to bgrainger/mysql that referenced this issue May 14, 2017
Rename UseOldPasswordPacket to AuthenticationMethodSwitchRequestPacket
and implement its two other fields: plugin name & data.

Support mysql_native_password and mysql_old_password as potential
authentication methods (but still require 'insecureAuth:true' for the
latter).
bgrainger added a commit to bgrainger/mysql that referenced this issue May 14, 2017
Add optional auth plugin name and data to UseOldPasswordPacket. Support
mysql_native_password and mysql_old_password as potential authentication
methods (but still require 'insecureAuth:true' for the latter).
bgrainger added a commit to bgrainger/mysql that referenced this issue May 15, 2017
Add optional auth plugin name and data to UseOldPasswordPacket. Support
mysql_native_password and mysql_old_password as potential authentication
methods (but still require 'insecureAuth:true' for the latter).
bgrainger added a commit to bgrainger/mysql that referenced this issue May 15, 2017
Add optional auth plugin name and data to UseOldPasswordPacket. Support
mysql_native_password and mysql_old_password as potential authentication
methods (but still require 'insecureAuth:true' for the latter).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants