Add support for 'mysql_clear_password - continued #2327
Conversation
ZJONSSON
changed the title
| options = options || {}; | ||
|
|
||
| switch (name) { | ||
| case 'mysql_native_password': | ||
| return Auth.token(options.password, data.slice(0, 20)); | ||
| case 'mysql_clear_password': | ||
| if (!isSecure) { | ||
| throw new Error('Authentication method mysql_clear_password not supported on insecure connections'); |
There was a problem hiding this comment.
I get where this is coming from, but is probably going to be limiting to users in the long run. What about just adding an option the user can use to enable / disable this? I'm thinking along the lines of our insecureAuth option (perhaps clearAuth?) and how the mysql command line program requires a flag to enable it (https://dev.mysql.com/doc/refman/8.0/en/mysql-command-options.html#option_mysql_enable-cleartext-plugin).
There was a problem hiding this comment.
It looks like the --enable-cleartext-plugin is just to enable the plugin itself. Are you thinking the insecureAuth would allow clear_password even if we are not on a tls socket?
There was a problem hiding this comment.
I'm thinking to have it behave inline with the mysql command line program: add an option (like perhaps clearAuth?) that that user must enable to use the clear password auth. That would be the only requirement -- the user can choose to use that over SSL or not as their own choice, just like in the mysql command line program.
There was a problem hiding this comment.
I understand. Looking at the python module, if you specify auth_plugin='mysql_clear_password' then cleartext is used whether SSL is on or not. So maybe that would be the expected behavior.
I still think there is a big difference between saying: I want to use cleartext only inside secure channel, vs I want to use cleartext no matter what. For example, some of the GUI Sql clients I tested would bail on cleartext if SSL is not turned on.
Maybe the right way is to use clearAuth = true to enable it for secure channel, and insecureAuth to allow clear to be in an unencrypted socket?
|
Thanks @dougwilson - just to clarify my interest in this - we are doing a proxy mysql server and want to handle the auth using bcrypt, as credentials are shared with other services. The only realistic way to use bcrypt is to required clear_password from a secure channel. Let me take another spin at this, really appreciate all the suggestions you have. |
Originally, would only allow user-set flags that disable a flag enabled by default. Now it will also allow user-set flags that enable a flag that is disabled by default.
Allow insecure cleartext if options.insecureAuth == true
* Specific SSL flag set for clear pwd tests
ZJONSSON
force-pushed
the
master-nmggithub
branch
from
8a1b95c
to
63f6bfe
Compare
ZJONSSON
force-pushed
the
master-nmggithub
branch
from
cbdcae3
to
6753e1b
Compare
|
@jbreckman is it still LGTM? |
Thanks @nmggithub for the mysql_clear_password branch. This PR is a continuation of #2225 with some unit tests added. Also I'm rejecting mysql_clear_password unless it's on a secure socket, which seems to be conventional among database clients. Maybe we should allow an override to that condition, or just drop the check altogether?
@dougwilson I didn't find a simple way to get access to whether the socket is secure or not in a generic fashion. I implemented it in a relatively hacky way here (setting
this._tls = true) for the handshake, but changeUser might still be missing access to whether the socket is secure or not.