MySQL: add the option to force sending the password as plain text #18252

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

Jump to bottom

Open

jafl wants to merge 4 commits into php:master

from jafl:mysql-aws-iam-token

Open

MySQL: add the option to force sending the password as plain text #18252

jafl wants to merge 4 commits into php:master from jafl:mysql-aws-iam-token

Conversation

jafl

Copy link

@jafl jafl commented Apr 4, 2025 •

edited

Loading

The use case is authenticating to AWS RDS with an IAM token, which has to be sent in the clear.

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

This may solve #10800

@jafl


 MySQL: add the option to force sending the password as plain text

87fe0af

* 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

@jafl jafl requested review from kocsismate, kamil-tekiela, SakiTakamachi and bukka as code owners

April 4, 2025 22:35

@github-actions github-actions bot added Extension: mysqlnd Extension: pdo_mysql ABI break labels

Apr 4, 2025

@jafl

Copy link

Author

jafl commented Apr 4, 2025

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.

@github-actions github-actions bot added the Extension: mysqli label

Apr 5, 2025

@jafl


 update mysqli test with new capability flag

93e6505

@jafl jafl force-pushed the mysql-aws-iam-token branch from a0f9e24 to 93e6505 Compare

April 5, 2025 02:57

kamil-tekiela

kamil-tekiela reviewed

Apr 9, 2025

View reviewed changes

ext/mysqlnd/mysqlnd_auth.c

@@ -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);

Copy link

Member

@kamil-tekiela kamil-tekiela Apr 9, 2025

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.

Copy link

Author

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.

ext/mysqlnd/mysqlnd_enum_n_def.h

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 */

Copy link

Member

@kamil-tekiela kamil-tekiela Apr 9, 2025

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?

Copy link

Author

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.

jafl added 2 commits

April 10, 2025 20:39

@jafl


 log mysqlnd protocol for testing

da49843

@jafl


 print more debugging info

c072f64

Labels

ABI break Extension: mysqli Extension: mysqlnd Extension: pdo_mysql

2 participants

@jafl @kamil-tekiela

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

MySQL: add the option to force sending the password as plain text #18252

Are you sure you want to change the base?

MySQL: add the option to force sending the password as plain text #18252

Uh oh!

Conversation

@jafl jafl commented Apr 4, 2025 •

edited

Loading

Uh oh!

Uh oh!

jafl commented Apr 4, 2025

Uh oh!

@kamil-tekiela kamil-tekiela Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

@jafl jafl Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

@kamil-tekiela kamil-tekiela Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

@jafl jafl Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MySQL: add the option to force sending the password as plain text #18252

Are you sure you want to change the base?

MySQL: add the option to force sending the password as plain text #18252

Uh oh!

Conversation

@jafl jafl commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jafl commented Apr 4, 2025

Uh oh!

@kamil-tekiela kamil-tekiela Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

@jafl jafl Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

@kamil-tekiela kamil-tekiela Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

@jafl jafl Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

@jafl jafl commented Apr 4, 2025 •

edited

Loading