Avoid protocol error on empty password auth switch

[composerinteralia]

Prior to this commit test_connection_error, which creates a connection with a username that doesn't exist and an empty password, was flaky on mysql 8. Sometimes it passed with the expected BaseConnectionError, but often it failed with a TRILOGY_PROTOCOL_VIOLATION.

The source of the TRILOGY_PROTOCOL_VIOLATION was read_auth_switch_packet, which only handles mysql_native_password and caching_sha2_password, whereas in the case of this failure the attempted auth switch was to sha256_password.

I'm not entirely sure why we get an auth switch packet sometimes and an "access denied" error packet other times (I'm guessing it has something to do with mysql 8 changing the default auth plugin to the caching plugin, but that's really just a guess). But in the case of an empty password it seems like we can handle the switch just fine. We've done a similar password_length check in #21.

With this commit test_connection_error passes consistently.

[composerinteralia]

I'm not entirely sure why we get an auth switch packet sometimes and an "access denied" error packet other times (I'm guessing it has something to do with mysql 8 changing the default auth plugin to a caching plugin, but that's really just a guess).

It's probably worth exploring that a bit more, but in the meantime maybe this change is fine 🤷🏻‍♂️

The same test with a password provided would probably still break though... I might be talking myself out of this change. I'll leave the PR open anyways because it contains information that might help debug further.

[adrianna-chang-shopify]

Hm, okay so if I'm understanding you correctly:

• The test passes when trilogy_auth_recv reads an err packet and correctly returns TRILOGY_ERR, because this leads to the expected connection error
• The test fails if trilogy_auth_recv reads an auth switch packet, and the attempted auth switch is to sha256_password, because that auth plugin is unsupported
• We can fix the test by skipping the check of the auth plugin in read_auth_switch_packet if the password len is 0 -- this fixes the test because we won't raise the TRILOGY_PROTOCOL_VIOLATION
• We're unsure about WHY sometimes we're reading an auth switch packet vs an error packet.. It's potentially related to the auth plugin default changing between test runs, and sometimes being the unsupported sha256_password plugin?

It seems strange that MySQL 8.0 would change the default -- supposedly it's caching_sha2_password. I wonder if explicitly setting default_authentication_plugin to caching_sha2_password would help?

The real root of this problem seems to be that we're ending up with an unsupported sha256_password auth plugin (as you've mentioned, seems like we'd expect the same test with a non-empty password to still hit the line of code raising the protocol violation error).. I'll admit my knowledge of SQL authentication methods is limited. Any idea where this value for the auth plugin could be coming from?

As an aside: Even with the CI docker files I've yet to get the test to fail locally on my machine, which is certainly odd 🤔 (I've run it probably 20+ times)

[composerinteralia]

Yup, all of that matches my understanding.

It seems strange that MySQL 8.0 would change the default

There are more details about that change in https://dev.mysql.com/doc/refman/8.0/en/caching-sha2-pluggable-authentication.html and https://dev.mysql.com/doc/refman/8.0/en/upgrading-from-previous-series.html#upgrade-caching-sha2-password

Oops, I just clicked through your link. You've probably already seen those.

my knowledge of SQL authentication methods is limited

Saaaame.

The fact that it's a caching plugin had me wondering whether there might be a different path based on whether it's a cache hit or miss, but based on https://dev.mysql.com/doc/dev/mysql-server/latest/page_caching_sha2_authentication_exchanges.html#sect_caching_sha2_info it seems like we should go down the same quick path every time if the password is empty. 🤷🏻‍♂️

I wonder if explicitly setting default_authentication_plugin to caching_sha2_password would help?

I would think we'd go the other way, setting it to native_mysql_password so that our 8.0 tests work like the 5.7 tests. That might be OK, since I don't think we really support caching_sha2_password yet anyway (e.g. #26) except maybe for empty passwords.

Even with the CI docker files I've yet to get the test to fail locally on my machine, which is certainly odd

Interesting!

[adrianna-chang-shopify]

I would think we'd go the other way, setting it to native_mysql_password so that our 8.0 tests work like the 5.7 tests. That might be OK, since I don't think we really support caching_sha2_password yet anyway (e.g. #26) except maybe for empty passwords.

Yeah maybe that's a sensible fix in the meantime, to revert back to the 5.7 default. I didn't realize there were other ongoing issues with caching_sha2_password. Is anyone from Github looking into that actively?

[composerinteralia]

Closing since I don't think this is the right solution. To be continued in #43