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

Enable cleartext plugin #176

Merged
merged 4 commits into from
Feb 13, 2024

Conversation

danschultzer
Copy link
Contributor

@danschultzer danschultzer commented Feb 10, 2024

Took me a minute to figure out why my app kept crashing in ECS 😄

RDS IAM auth requires the generated auth token to be send as a cleartext password with MySQL. I'm replicating how mysql-client enables cleartext password in this PR.

There wasn't a way to integrate the tests with actual MySQL containers, since all plugins that support cleartext are in the enterprise edition, so I'm mocking the responses after introspecting mysql packets.

I'm currently using this branch in AWS.

@danschultzer danschultzer force-pushed the enable-cleartext-plugin branch from 9735e14 to 76eed03 Compare February 10, 2024 17:55
@danschultzer danschultzer marked this pull request as ready for review February 10, 2024 18:30
@danschultzer danschultzer force-pushed the enable-cleartext-plugin branch from 76eed03 to ccbd382 Compare February 11, 2024 00:19
Comment on lines 472 to 476
<<74, 0, 0, 0, 10, 56, 46, 48, 46, 51, 53, 0, 127, 24, 4, 0, 93, 42, 61, 27, 60,
38, 85, 12, 0, 255, 255, 255, 2, 0, 255, 223, 21, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 39, 48, 10, 117, 54, 65, 74, 37, 125, 121, 93, 6, 0, 109, 121, 115, 113,
108, 95, 110, 97, 116, 105, 118, 101, 95, 112, 97, 115, 115, 119, 111, 114,
100, 0>>
Copy link
Member

Choose a reason for hiding this comment

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

this is tough to understand and maintain. Could you break it down to along the lines of:

[
  <<74, 0, 0, 0>>,
  [
    # protocol version, always 0x10
    10,
    # server version
    <<"8.0.35", 0>>,
    # ...
  ]
]

I'm really curious to see where exactly the scramble was supposed to be and is empty.

Copy link
Contributor Author

@danschultzer danschultzer Feb 13, 2024

Choose a reason for hiding this comment

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

Sure, but just FYI this part doesn't matter. It's just priming the connection with a scramble, that we are going to make (knowingly) an unsuccesful authentication with. When that fails we continue with the cleartext password. This came from painfully figuring out why I couldn't just send the cleartext password with the mysql_clear_password attached to it right after the handshake. It can only happen after the auth switch from the server.

Copy link
Contributor Author

@danschultzer danschultzer Feb 13, 2024

Choose a reason for hiding this comment

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

I decided to just add comments to each packet rather than expanding the packets. There's not really much to expand on within the packets, but this will definitely help others understand how this works.

@@ -100,6 +101,8 @@ defmodule MyXQL do
will disconnect the connection. See "Disconnecting on Errors" section below for more
information.

* `:enable_cleartext_plugin` - Set to `true` to send password as cleartext (default: `false`)
Copy link
Member

Choose a reason for hiding this comment

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

I think matching mysql CLI is a great starting point but curious if any other connectors have something like auth_plugin: mysql_native_password | caching_sha2_password | .... I think that'd be preferred as we'd reuse the same option when we support more methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cleartext password is distinct in that it has no server side library: https://dev.mysql.com/doc/refman/8.0/en/cleartext-pluggable-authentication.html

And especially note this:

To make inadvertent use of the mysql_clear_password plugin less likely, MySQL clients must explicitly enable it.

This has been an issue with other mysql connectors: sidorares/node-mysql2#1617

Ruby mysql2 sets the env flag for the mysql client it communicates with (but uses the same enable_cleartext_plugin flag in config): brianmario/mysql2#845

The closest thing there is to decide what auth plugin to use is hinting: https://dev.mysql.com/doc/refman/8.3/en/mysql-command-options.html#option_mysql_default-auth

As I understand it, the way the auth works with cleartext password from the mysql client is that it will always go through the auth switch, and it's after the switch that it's possible to succesfully authenticate. In the case of RDS IAM it will first ask for the mysql_native_password and expects the client to send a response for this format (I tried just sending the token itself, but the server wanted a sha1 size response). After this step fails it asks for mysql_clear_password which is what we actually wanted to send.

So IMO it doesn't really make sense to treat mysql_clear_password the same way as other auth plugin. It's a special case. We could force it on our side by setting something like :auth_plugin, which arguably is more explicit, but this is not how the mysql client works, and I haven't seen other libraries do this.

@danschultzer danschultzer force-pushed the enable-cleartext-plugin branch from 5a0fcc8 to 090d43b Compare February 13, 2024 01:32
@wojtekmach wojtekmach merged commit deb741e into elixir-ecto:master Feb 13, 2024
9 of 10 checks passed
@wojtekmach
Copy link
Member

Thank you!

@danschultzer danschultzer deleted the enable-cleartext-plugin branch February 13, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants