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

Support for caching_sha2_password plugin in mysql/client #6716

Merged
merged 6 commits into from
Feb 22, 2021

Conversation

vamc19
Copy link
Contributor

@vamc19 vamc19 commented Sep 14, 2020

Hey folks! I'm working on a MySQL proxy using Vitess and am blocked by the lack of support for caching_sha2_password plugin in mysql/client. This PR adds support for it.

Reference:
https://dev.mysql.com/doc/dev/mysql-server/latest/page_caching_sha2_authentication_exchanges.html
https://insidemysql.com/preparing-your-community-connector-for-mysql-8-part-2-sha256/

During the initial handshake, we are checking if the default auth plugin
of the server is supported by the client. In cases where the server
supports multiple auth plugins, this check is causing the handshake to
fail without giving the server a chance to send an Authentication Method
Switch Request.

This check happens anyway when handling the auth method switch request
and an error is thrown when server requests an unsupported auth method.

Signed-off-by: Vamsi Atluri <vamc19@gmail.com>
caching_sha2_password plugin, unlike mysql_native_password, is a multi
step process. Client hashes the password using SHA2 algorithm and sends
it to the server. Server responds with either an AUTH_MORE_DATA (0x01)
packet or an Error packet.

Error packet is sent when authentication fails during "fast" auth (more
on this below).

The second byte of AUTH_MORE_DATA packet will either be 0x03 or 0x04.

0x03 represents a successful "fast" auth meaning that the server has a
cached hash of the password for the user and it matches the hash sent by
the client. Server will send an OK packet next.

Server sends 0x04 when the hash of the password is not yet cached. In
this case, client has to do a "full" authentication by sending un-hashed
password. If the client is connected using SSL or a Unix socket, client
can write the password in clear text. If this is not the case, client
has to request a public key from the server to encrypt the password.
Client should obfuscate the password using xor operation before
encrypting it with public key and sending it to the server. Server will
respond with an OK packet if autentication is successful or Error packet
otherwise.

Signed-off-by: Vamsi Atluri <vamc19@gmail.com>
@harshit-gangal
Copy link
Member

@vamc19 were you able to write end to end test for this change?

@vamc19
Copy link
Contributor Author

vamc19 commented Oct 14, 2020

@harshit-gangal I did not get much time to work on this in past few weeks. I'm hoping to pick it up this weekend.

This is where I'm stuck: to test this, I have to create a new user with caching_sha2_password.

  1. I cannot find where the user creation in the database is handled for tests
  2. We should also consider that caching_sha2_password may not be available on all versions of MySQL and Percona. So tests should be able to handle this case.

Right now the approach I'm leaning towards is: In end2end/client_test.go I'll fork a new mysql process, check if that particular version of mysql supports caching_sha2_password, create a new user using caching_sha2_password and finally check if I'm able to create a connection with the new credentials.

Let me know if this approach makes sense or if there is an easier/alternate way to test this.

@harshit-gangal
Copy link
Member

That is how you should approach. e2e test in client_test is sufficient to test the functionality.

Signed-off-by: Vamsi Atluri <vamc19@gmail.com>
Signed-off-by: Vamsi Atluri <vamc19@gmail.com>
Signed-off-by: Vamsi Atluri <vamc19@gmail.com>
@vamc19 vamc19 marked this pull request as ready for review February 21, 2021 05:52
@vamc19 vamc19 requested a review from systay as a code owner February 21, 2021 05:52
Copy link
Member

@harshit-gangal harshit-gangal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks good. One small nit.

cc: @aquarapid @deepthi would you like to take a lot at it.

go/mysql/constants.go Outdated Show resolved Hide resolved
Signed-off-by: Vamsi Atluri <vamc19@gmail.com>
@harshit-gangal harshit-gangal added this to the v10.0 milestone Feb 22, 2021
@harshit-gangal harshit-gangal merged commit 08c841d into vitessio:master Feb 22, 2021
@@ -117,8 +119,8 @@ func NewSalt() ([]byte, error) {
return salt, nil
}

// ScramblePassword computes the hash of the password using 4.1+ method.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are changing the interface here. If there is someone out there who built their own plugin auth server, this will break them.
We need to (at least) document this in the 10.0 release notes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see this as interface. If you think it can break someone's build, we can mark this as breaking change.

Copy link
Member

@deepthi deepthi Mar 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, it is not part of the AuthServer interface. My mistake. There should be no impact on anyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants