Implement support for cleartext authentication plugin #168

wildmaples · 2024-03-20T19:54:18Z

Resolves: #157

The cleartext pluggable authentication is a client side plugin that can be set through the --enable-cleartext-plugin flag. We override the auth plugin received from the server inside the client, before building and sending the auth packet.

I'm not sure if people like the variable name enable_cleartext_plugin but I defaulted to that since that's the flag.

wildmaples · 2024-03-20T20:06:38Z

src/client.c

+    } else {
+        auth_plugin = handshake->auth_plugin;
+    }
+
    rc = trilogy_build_auth_packet(&builder, conn->socket->opts.username, conn->socket->opts.password,


We can refactor this function to trilogy_build_auth_packet(&builder, conn->socket->opts, handshake, auth_plugin) in another PR.

gajwani · 2024-03-20T20:28:47Z

src/protocol.c

@@ -568,7 +568,9 @@ int trilogy_build_auth_packet(trilogy_builder_t *builder, const char *user, cons

    if (pass_len > 0) {
        // Fallback to te default unless we have SHA2 requested


nit: I think this comment should likely be moved down to between line 573/4 and another comment should be added here to add context to the new "cleartext" behavior.

Great call, thanks! I didn't add a comment for the cleartext if-block because I think it's pretty straightforward. Happy to add it if maintainers think it's helpful.

contrib/ruby/ext/trilogy-ruby/cext.c

dbussink · 2024-03-22T16:42:43Z

src/client.c

+        auth_plugin = "mysql_clear_password";
+    } else {
+        auth_plugin = handshake->auth_plugin;
+    }


I think there's a difference between enabling the plugin so that it's used of the server switches to it and making it the default advertised by the client.

Is the behavior the same with libmysqlclient that if it's enabled, it also becomes the default that the client advertises? Or does it only allow switching to it there?

Just so I understand correctly, you're asking if the server will accept the cleartext plugin password without the client informing the server of the plugin?

If so, then I believe that to be the case. According to the libmysqlclient source code / documentation, we don't need to preemptively inform the server that we are using the cleartext plugin before the authentication connection. To put it in another way, if cleartext plugin is enabled, it does become the "default" that the client advertises, in a sense where the client can just send it to the server.

What happens if we connect with enable_cleartext_plugin :true but then the server handshake indicates a non-cleartext plugin like caching_sha2_password or whatever? Will this code ignore what the server told us and send the cleartext password anyway? Does that then turn into an auth switch?

(I'm still learning about how these plugins work, so not sure I'm making sense.)

You're right, the first handshake we get back from the server will indicate the server's authentication plugin preference. However, according to the MySQL docs on "Determining Authentication Method", the client does not have to use the server's authentication method.

When including authentication reply in the Protocol::HandshakeResponse:, client is not obligated to use the same authentication method that was used by the server in the Protocol::Handshake packet.

As the code is right now, we will send the cleartext password but the server can still respond with an auth switch request, and we will do so accordingly. We think this is OK because the request from the user to use cleartext password has been fulfilled. If the server is still not happy then it's OK for us to do the auth switch.

Do you have a good way of testing out one of the plugins that needs a cleartext password? I'm curious what plugin is advertised in the handshake for those. Like, if the server wants to use authentication_pam does the handshake advertise the server plugin authentication_pam or the client plugin mysql_clear_password?

By the way, I should have started by saying thank you for working on this!

@HParker and I found a test server plugin that uses mysql_clear_password as the client plugin, and I was able to enable it in a locally with:

INSTALL PLUGIN cleartext_plugin_server SONAME 'auth_test_plugin.so'; CREATE USER 'cleartext_user'@'localhost' IDENTIFIED WITH cleartext_plugin_server BY 'password';

Then I connected via the command line client and looked at the packets:

mysql --protocol=tcp --ssl-mode=disable -p -u cleartext_user --enable-cleartext-plugin

Server sends handshake, with auth plugin caching_sha2_password

Client sends handshake response, still with auth plugin caching_sha2_password

Server sends auth switch request, now with auth plugin mysql_clear_password

Client sends auth switch response with just the cleartext password and nothing else

Server sends OK and we are connected

That's more what I was expecting. The server is telling the client to use mysql_clear_password, and the --enable-cleartext-plugin option only affects step 4. While a client can choose a different plugin than what the server advertised in the handshake, I don't know how the server handles mysql_clear_password in particular since it is a client-side-only plugin.

Trying to connect to this same user with Trilogy using this PR results in:

Trilogy.new(host: '127.0.0.1', username: 'cleartext_user', password: 'password', enable_cleartext_plugin: true)

Server sends handshake, with auth plugin caching_sha2_password

Client sends handshake response, switching to mysql_clear_password

Server responds with "Access denied"

I'm not totally sure why that fails (including with the fix so we send the password). But maybe we should try to behave more like mysql here?

composerinteralia · 2024-04-02T01:59:40Z

src/protocol.c

-        // Fallback to te default unless we have SHA2 requested
-        if (!strcmp("caching_sha2_password", auth_plugin)) {
+        if (!strcmp("mysql_clear_password", auth_plugin)) {
+            memcpy(auth_response, pass, pass_len);


Will this memcpy segfault if pass_len is longer than auth_response?

We're also not setting auth_response_len, which we check below to decide whether to write it at all.

composerinteralia · 2024-04-02T02:12:19Z

test/protocol/building/auth_packet_test.c

+    err = trilogy_build_auth_packet(&builder, user, pass, strlen(pass), NULL, 45, "mysql_clear_password", scramble, 0);
+    ASSERT_OK(err);
+
+    static const uint8_t expected[] = {


Should there be a cleartext password in this expected output? I think it's the same as the same bytes as in test_build_auth_packet_cleartext (although a little difficult to visually diff). The missing auth_response_len would explain it, I think.

composerinteralia · 2024-04-02T02:26:16Z

src/client.c

+        auth_plugin = "mysql_clear_password";
+    } else {
+        auth_plugin = handshake->auth_plugin;
+    }


What happens if we connect with enable_cleartext_plugin :true but then the server handshake indicates a non-cleartext plugin like caching_sha2_password or whatever? Will this code ignore what the server told us and send the cleartext password anyway? Does that then turn into an auth switch?

(I'm still learning about how these plugins work, so not sure I'm making sense.)

composerinteralia · 2024-04-02T02:28:55Z

contrib/ruby/ext/trilogy-ruby/cext.c

@@ -522,6 +522,10 @@ static VALUE rb_trilogy_connect(VALUE self, VALUE encoding, VALUE charset, VALUE
        connopt.flags |= TRILOGY_CAPABILITIES_CONNECT_WITH_DB;
    }

+    if (RTEST(rb_hash_aref(opts, ID2SYM(id_enable_cleartext_plugin)))) {
+        connopt.enable_cleartext_plugin = true;


I wonder if we should warn or refuse to do this if the appropriate ssl options are not set.

wildmaples · 2024-04-03T20:37:21Z

Thanks for the feedback everyone! Chatted with @composerinteralia on Zoom and closing this in favour of: #171

wildmaples added 3 commits March 19, 2024 18:19

Pass cleartext_plugin_enabled as params to build auth packet

e64befe

Implement support for cleartext auth plugin with tests

0197c06

Handle cleartext plugin in the client

c6f3eae

wildmaples force-pushed the cleartext-plugin branch from e2f106c to c6f3eae Compare March 20, 2024 20:04

wildmaples commented Mar 20, 2024

View reviewed changes

gajwani reviewed Mar 20, 2024

View reviewed changes

Move comment to the appropriate location

df780df

paracycle reviewed Mar 21, 2024

View reviewed changes

contrib/ruby/ext/trilogy-ruby/cext.c Outdated Show resolved Hide resolved

Remove unused param

37273ae

dbussink reviewed Mar 22, 2024

View reviewed changes

composerinteralia reviewed Apr 2, 2024

View reviewed changes

wildmaples mentioned this pull request Apr 3, 2024

Implement support for cleartext authentication plugin #171

Merged

wildmaples closed this Apr 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement support for cleartext authentication plugin #168

Implement support for cleartext authentication plugin #168

wildmaples commented Mar 20, 2024 •

edited

Loading

wildmaples Mar 20, 2024

gajwani Mar 20, 2024 •

edited

Loading

wildmaples Mar 21, 2024

dbussink Mar 22, 2024

wildmaples Apr 1, 2024

composerinteralia Apr 2, 2024

wildmaples Apr 2, 2024

composerinteralia Apr 2, 2024

composerinteralia Apr 3, 2024 •

edited

Loading

composerinteralia Apr 2, 2024

composerinteralia Apr 2, 2024

composerinteralia Apr 2, 2024 •

edited

Loading

composerinteralia Apr 2, 2024

composerinteralia Apr 2, 2024 •

edited

Loading

wildmaples commented Apr 3, 2024

		@@ -568,7 +568,9 @@ int trilogy_build_auth_packet(trilogy_builder_t builder, const char user, cons

		if (pass_len > 0) {
		// Fallback to te default unless we have SHA2 requested

Implement support for cleartext authentication plugin #168

Implement support for cleartext authentication plugin #168

Conversation

wildmaples commented Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

gajwani Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

composerinteralia Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

composerinteralia Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

composerinteralia Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

wildmaples commented Apr 3, 2024

wildmaples commented Mar 20, 2024 •

edited

Loading

gajwani Mar 20, 2024 •

edited

Loading

composerinteralia Apr 3, 2024 •

edited

Loading

composerinteralia Apr 2, 2024 •

edited

Loading

composerinteralia Apr 2, 2024 •

edited

Loading