Fix #78680: mysqlnd pam plugin missing terminating null #6667
Conversation
| while ($row = $res->fetch_assoc()) { | ||
| if (isset($row['Name']) && ('mysql_clear_password' == $row['Name'])) { | ||
| $have_pam = true; | ||
| if (isset($row['Name']) && in_array($row['Name'], array('pam', 'mysql_clear_text', 'auth_pam_compat'))) { |
There was a problem hiding this comment.
Is the change from mysql_clear_password to mysql_clear_text intentional here? As far as I can tell, the plugin is called mysql_clear_password, even though various related options use the "cleartext" terminology instead.
There was a problem hiding this comment.
nope. That was a mistake. Thanks @nikic . Force pushing now.
6543660 to
5b2a74b
Compare
| while ($row = $res->fetch_assoc()) { | ||
| if (isset($row['Name']) && ('mysql_clear_password' == $row['Name'])) { | ||
| $have_pam = true; | ||
| if (isset($row['Name']) && in_array($row['Name'], array('pam', 'mysql_clear_password', 'auth_pam_compat'))) { |
There was a problem hiding this comment.
This line of code appears to be looping through the list of server-side plugins returned by SHOW PLUGINS.
mysql_clear_password is a client-side plugin used for the following server-side plugins:
pam(MariaDB Server and Percona Server)auth_pam_compat(Percona Server)authentication_pam(MySQL Enterprise Server).
As far as I know, Client-side plugins don't show up in the output of SHOW PLUGINS, so I don't think mysql_clear_password will ever show up in this loop. Should mysql_clear_password be removed from this specific check?
There was a problem hiding this comment.
opps. corrected to authentication_pam. Looked at https://dev.mysql.com/doc/refman/8.0/en/pam-pluggable-authentication.html#pam-pluggable-authentication-usage
I have pinged some MySQL community folks to test this their Enterprise side.
5b2a74b to
8940e7a
Compare
The PAM service requires the terminating null to be part of the communication. Tested with MariaDB-10.4(pam) and Percona Server 5.7.32(auth_pam_compat). Also changed MySQL Enterprise test to the server side plugin, authentication_pam as opposed to the client plugin mysql_clear_password. Add additional check for pamtest user and pam service file as all are required for the test. More importantly, test result should actually succeed. Thanks Geoff Montee for bug report.
8940e7a to
c8f0914
Compare
|
Merged as 3646604 (with wrong PR reference...) |
|
Thanks @nikic |
|
So as a master branch fix, not a PHP-7.4? |
|
@grooverdan It's on PHP-7.4, GitHub just has trouble understanding out merge workflow :) Changes are merged up into master, so GitHub only shows that branch. |
The PAM service requires the terminating null to be part
of the communication.
Tested with MariaDB-10.4(pam) and Percona Server 5.7.32(auth_pam_compat).
Test result should actually succeed. Add additional check for pamtest user.
Thanks Geoff Montee for bug report.