Implement `AuthenticationHandler` for custom auth mechanisms #1072

ramnes · 2025-12-06T00:22:56Z

Hey team,

Here is some work around #1069 and #1071 to allow a discussion based on an actual implementation.

I think the changes here are pretty obvious:

Renames;
A Credential can have multiple passwords now;
Added OnAuthSuccess and OnAuthFailure to the new AuthenticationHandler.

One side effect of the multiple passwords per user is that we can't have Xor update inplace anymore, because otherwise we would XOR the password we received every time we compare it to a new hash in the list of hashes to try.

About hashes – I moved the Credential passwords hash at comparison-time to avoid paying the full price for every connection. Let's say you have 1000 passwords to match against but match after 5 comparisons: it's better to hash only 5 passwords, not 1000.

lance6716 · 2025-12-06T09:32:40Z

(This PR is a bit large than the other one you opened, I'll take a look later)

ramnes · 2025-12-06T10:02:50Z

Yeah, the PR is a bit too large for my taste too. :)

I didn’t mean for it to be merged as-is, more as a starting point for discussion around the issues I opened. Happy to split it up once we have a clearer idea of what we want to do for each one!

lance6716

6 / 11 viewed, will review later

server/authentication_handler.go

lance6716 · 2025-12-10T06:47:04Z

server/authentication_handler.go

+		authPluginName = optionalAuthPluginName[0]
+	}
+
+	if !isAuthMethodSupported(authPluginName) {


seems in the old code we also support AUTH_CLEAR_PASSWORD. @dveeden Should we change isAuthMethodSupported to add that?

I can also remove this check entirely if it doesn't make sense in that context, as there was no check before. Feel free to tell me.

I think it's better to remove the check. And let's wait @dveeden until all comments are resolved to understand if isAuthMethodSupported should also be updated

I think we probably should support AUTH_CLEAR_PASSWORD.

Note that this is a client side plugin.

The usecase for this is to have the client send the cleartext password to the server which then allows the server to use this to authenticate against LDAP (via binding, a hash stored in ldap doesn't need this) or via PAM or anything else that is custom.

Note that on the client one needs to use --enable-cleartext-plugin or use set it via an option in their apps.

The risk with cleartext is obvious. It should be used only over TLS connections.

Note that caching_sha2_password has two options:

RSA keypair

TLS connection.

With a RSA keypair the public key has to be specified or you have to enable an option to fetch it from the server (which is insecure). I don't think this should be used.

With caching_sha2_password and an empty cache the password is transmitted in cleartext over a TLS connection. We should make sure we don't accept it over a non-TLS connection (except for UNIX domain socket)

lance6716

rest lgtm

lance6716 · 2025-12-13T01:39:23Z

server/auth.go

 		return err
 	}
-	if !found {
+	if !found || len(credential.Passwords) == 0 {


We should add comments to Credential type that, if a user has no password, empty password, or one authentication method is empty password, Credential.Passwords should be []string{"", ...} rather than zero length slice

lance6716 · 2025-12-13T01:41:06Z

server/auth.go

-func (c *Conn) acquirePassword() error {
-	if c.credential.Password != "" {
+func (c *Conn) acquireCredential() error {
+	if len(c.credential.Passwords) > 0 {


Seems we can also use hasEmptyPassword here?

lance6716 · 2025-12-13T01:50:43Z

server/authentication_handler.go

+		authPluginName = optionalAuthPluginName[0]
+	}
+
+	if !isAuthMethodSupported(authPluginName) {


I think it's better to remove the check. And let's wait @dveeden until all comments are resolved to understand if isAuthMethodSupported should also be updated

dveeden

Seems ok on first glance.

dveeden · 2025-12-15T08:01:41Z

server/authentication_handler.go

+		authPluginName = optionalAuthPluginName[0]
+	}
+
+	if !isAuthMethodSupported(authPluginName) {


I think we probably should support AUTH_CLEAR_PASSWORD.

Note that this is a client side plugin.

The usecase for this is to have the client send the cleartext password to the server which then allows the server to use this to authenticate against LDAP (via binding, a hash stored in ldap doesn't need this) or via PAM or anything else that is custom.

Note that on the client one needs to use --enable-cleartext-plugin or use set it via an option in their apps.

The risk with cleartext is obvious. It should be used only over TLS connections.

Note that caching_sha2_password has two options:

RSA keypair

TLS connection.

With a RSA keypair the public key has to be specified or you have to enable an option to fetch it from the server (which is insecure). I don't think this should be used.

With caching_sha2_password and an empty cache the password is transmitted in cleartext over a TLS connection. We should make sure we don't accept it over a non-TLS connection (except for UNIX domain socket)

dveeden · 2025-12-15T08:05:19Z

Could we mix authentication methods? E.g. a user having two authentication methods:

Caching_sha2_password with hash abcdefg
Auth_socket linked to username jdoe

This could be useful for migrating from one to another. Or when a authmethod has limitations (like auth_socket). Or it could be used for MFA

https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_multi_factor_authentication_methods.html

ramnes force-pushed the ramnes/credentials-passwords branch from 6ec779a to fb6703a Compare December 6, 2025 00:26

ramnes changed the title ~~Replace CredentialProvider with a more powerful AuthHandler~~ Implement AuthenticationHandler for custom auth mechanisms Dec 6, 2025

ramnes force-pushed the ramnes/credentials-passwords branch from fb6703a to 55d7dfb Compare December 6, 2025 00:30

ramnes mentioned this pull request Dec 6, 2025

Multiple valid passwords per user in CredentialProvider #1069

Open

ramnes force-pushed the ramnes/credentials-passwords branch from c650a68 to 8c72c7d Compare December 6, 2025 20:03

lance6716 reviewed Dec 10, 2025

View reviewed changes

ramnes force-pushed the ramnes/credentials-passwords branch from 4a52850 to ca22086 Compare December 12, 2025 16:03

ramnes added 4 commits December 12, 2025 17:12

Implement AuthenticationHandler for custom auth mechanisms

b2813fa

Fix xor

84e9fea

Fall through when the cached password doesn't match

4e09810

Run golangci-lint

63d5f38

ramnes force-pushed the ramnes/credentials-passwords branch from ca22086 to 63d5f38 Compare December 12, 2025 16:12

Leverage slices.Contains

dba2caf

ramnes force-pushed the ramnes/credentials-passwords branch from fa64da6 to 9720dc2 Compare December 12, 2025 16:22

Unexport Credential methods and add comment

12a0543

ramnes force-pushed the ramnes/credentials-passwords branch from 9720dc2 to 12a0543 Compare December 12, 2025 16:23

lance6716 reviewed Dec 13, 2025

View reviewed changes

dveeden self-requested a review December 15, 2025 07:41

dveeden reviewed Dec 15, 2025

View reviewed changes

Implement AuthenticationHandler for custom auth mechanisms #1072

Are you sure you want to change the base?

Implement AuthenticationHandler for custom auth mechanisms #1072

Uh oh!

Conversation

ramnes commented Dec 6, 2025

Uh oh!

lance6716 commented Dec 6, 2025

Uh oh!

ramnes commented Dec 6, 2025

Uh oh!

lance6716 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lance6716 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dveeden left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dveeden commented Dec 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implement `AuthenticationHandler` for custom auth mechanisms #1072

Implement `AuthenticationHandler` for custom auth mechanisms #1072