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

Caching sha2 #1232

Merged
merged 3 commits into from
Jun 2, 2021
Merged

Caching sha2 #1232

merged 3 commits into from
Jun 2, 2021

Conversation

dveeden
Copy link
Contributor

@dveeden dveeden commented May 24, 2021

What problem does this PR solve?

For improved compatibility with MySQL 8.0 TiDB needs to support the caching_sha2 authentication method.

This implements the validation of passwords for this authentication format.

Related:

What is changed and how it works?

  • Split out mysql_native_password related code from auth/auth.go
  • Added caching_sha2 code.

Check List

Tests

  • Unit test

Side effects

  • Increased code complexity

@morgo
Copy link
Contributor

morgo commented May 26, 2021

/LGTM

auth/caching_sha2.go Outdated Show resolved Hide resolved
auth/caching_sha2.go Show resolved Hide resolved
auth/caching_sha2.go Outdated Show resolved Hide resolved
auth/caching_sha2.go Outdated Show resolved Hide resolved
auth/caching_sha2.go Show resolved Hide resolved
auth/caching_sha2.go Outdated Show resolved Hide resolved
auth/caching_sha2.go Show resolved Hide resolved
auth/caching_sha2_test.go Outdated Show resolved Hide resolved
auth/caching_sha2_test.go Outdated Show resolved Hide resolved
This allows validating passwords against the `authentication_string`
data that MySQL stores for caching_sha2 passwords.

Related:
- pingcap/tidb#9411
@dveeden dveeden requested a review from tiancaiamao June 1, 2021 11:23
@dveeden
Copy link
Contributor Author

dveeden commented Jun 1, 2021

I think we should clone this PR and continue with #1236 which includes the work from this PR

@tiancaiamao
Copy link
Collaborator

/LGTM

@ti-srebot ti-srebot added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Jun 2, 2021
@tiancaiamao
Copy link
Collaborator

I'd rather merge this on first, 400 lines changes is already quite large.
Generally, a single PR more than 300 lines makes a hard time for the reviewer...
You can rebase master and update #1236 then?

@tiancaiamao
Copy link
Collaborator

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@dveeden merge failed.

@tiancaiamao tiancaiamao merged commit 10b704a into pingcap:master Jun 2, 2021
dveeden added a commit to dveeden/tidb that referenced this pull request Jun 30, 2021
Issue link: pingcap#9411

What this does:
- Check the `plugin` column of the `mysql.user` table.
- Based on the plugin from the user record and the plugin the client
  sent we may need to switch the authentication plugin to match the
  one from the user record
- For accounts with `caching_sha2_password` send the "fast
  authentication failed" response to trigger full authentication.
- call `auth.CheckShaPassword` to validate the user.

Implemented functionality:
- Full authentication with `caching_sha2_password` over TLS
- The `default_authentication_plugin` variable
- `CREATE USER... IDENTIFIED WITH 'caching_sha2_password'...`
- `SET PASSWORD...`
- `ALTER USER ... IDENTIFIED BY...`

Missing functionality:
- Support for the RSA public key request packet & response
- Support for RSA key based secret exchange
- Fast authentication (validate against cached entry)

Related:
- Requires pingcap/parser#1232
- pingcap#24141 makes testing
  of this easier, but this is not required.
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