-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
MySQL: add the option to force sending the password as plain text #18252
base: master
Are you sure you want to change the base?
Conversation
* This re-uses the pam authentication method * pdo_mysql: added option MYSQL_ATTR_SEND_CLEAR_PASSWORD * mysqlnd: implemented CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA option in order to send auth data longer than 255 bytes
Ideally, a similar option should be added to mysqli, but I'll wait with that until people have a chance to review this and decide whether it's the right approach. |
a0f9e24
to
93e6505
Compare
@@ -68,7 +68,7 @@ mysqlnd_run_authentication( | |||
memcpy(plugin_data, auth_plugin_data.s, plugin_data_len); | |||
plugin_data[plugin_data_len] = '\0'; | |||
|
|||
requested_protocol = mnd_pestrdup(auth_protocol? auth_protocol : MYSQLND_DEFAULT_AUTH_PROTOCOL, FALSE); | |||
requested_protocol = mnd_pestrdup(mysql_flags & CLIENT_SEND_CLEAR_PASSWORD ? MYSQLND_CLEAR_PASSWORD_AUTH_PROTOCOL : (auth_protocol? auth_protocol : MYSQLND_DEFAULT_AUTH_PROTOCOL), FALSE); |
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.
Is this really the correct place to set this? Why not set it where the function is called? auth_protocol
is a parameter.
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.
See my other comment. There is no way to control the value of auth_protocol
that is passed to this function from the PHP layer.
/* | ||
This is a mysqlnd extension. CLIENT_IGNORE_SIGPIPE is not used anyway. We will reuse it for our case and translate it to forcing the mysql_clear_password protocol | ||
*/ | ||
#define CLIENT_SEND_CLEAR_PASSWORD CLIENT_IGNORE_SIGPIPE /* Force plaintext password */ |
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.
What exactly is this for? Can we not use CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA
?
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.
The sensible, default behavior of mysqlnd is to use the protocol requested by the server, which is mysql_native_password
. This protocol is appropriate for authenticating with username & password.
However, AWS AuroraDB also supports sending an IAM token in place of a password, and this must be sent in the clear. The problem is that AuroraDB doesn't know which authentication method the client intends to use, so it always sends mysql_native_password
, and it's up to the client to decide how to send the password data.
Thus, there needs to be a way force mysqlnd to send the password in the clear. That is why I introduced this flag.
The use case is authenticating to AWS RDS with an IAM token, which has to be sent in the clear.
This may solve #10800