Support caching sha2 password #794
Conversation
|
Thanks for your contribution. There is #552 which implements auth plugin support (without caching_sha2 implementation). @julienschmidt How do you think about next version? MySQL 8.0 is GA already.
|
|
Thank you for this pull-request! @methane I changed the scheduled release date for v1.4.0 to 1 week from now. We should get it out ASAP. My preference would be option 3. I'll look into it later today, to see how feasible that is. Otherwise we'll fall back to option 2. Making a release without MySQL 8 support would be a bad joke. |
| @@ -1842,7 +1842,7 @@ func TestSQLInjection(t *testing.T) { | |||
|
|
|||
| dsns := []string{ | |||
| dsn, | |||
| dsn + "&sql_mode='NO_BACKSLASH_ESCAPES,NO_AUTO_CREATE_USER'", | |||
There was a problem hiding this comment.
is this a required change?
There was a problem hiding this comment.
NO_AUTO_CREATE_USER SQL mode seems removed at MySQL 8.0
https://dev.mysql.com/doc/refman/8.0/en/mysql-nutshell.html
My linux box show that message
--- FAIL: TestSQLInjection (0.92s)
driver_test.go:161: error on exec CREATE TABLE test (v INTEGER): Error 1231: Variable 'sql_mode' can't be set to the value of 'NO_AUTO_CREATE_USER'
There was a problem hiding this comment.
@methane let's do it the other way round. As we will merge this PR anyway in some form, let's merge it as it is first and then implement the general auth plugin support. The benefit would be that we hopefully already get some feedback if this breaks something before we make the release.
packets.go
Outdated
| plain := make([]byte, 20) | ||
| for k, v := range []byte(mc.cfg.Passwd) { | ||
| plain[k] = byte(v) | ||
| } |
There was a problem hiding this comment.
When len(mc.cfg.Passwd) > 20, this code may overflow.
Could we just use copy(plain, mc.cfg.Passwd) here?
packets.go
Outdated
| plain[k] = byte(v) | ||
| } | ||
| for i := range plain { | ||
| plain[i] ^= cipher[i] |
There was a problem hiding this comment.
len(plain) == 20, but how about len(cipher)?
Shouldn't we do cipher[i % len(cipher)]?
There was a problem hiding this comment.
Parameter cipher is Nonce https://dev.mysql.com/doc/dev/mysql-server/latest/page_caching_sha2_authentication_exchanges.html
from server and always 20 bytes
There was a problem hiding this comment.
I see. Please fix only above. (len(mc.cfg.Passwd) > 20 case).
There was a problem hiding this comment.
Sorry, There is no limitation about password length (< 20) when caching_sha2_password.
Now fix it 84f6018
|
@nakagami Would you add MySQL 8.0 test to |
Sorry, it is difficult for me. |
|
@nakagami please gofmt your code: https://travis-ci.org/go-sql-driver/mysql/jobs/380186488#L738 |
|
Sorry, gofmt & push now |
|
@methane any other requested changes or is this PR ready for merge now? |
|
I want to add MySQL 8 test on Travis, using caching_sha2 auth. |
|
See https://github.com/go-sql-driver/mysql/tree/travis-mysql8 / https://travis-ci.org/go-sql-driver/mysql/jobs/381084434. I'll submit a separate PR which also fixes the |
|
Thanks a lot @nakagami! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Description
Support chaching_sha2_password authentication plugin. It's MySQL 8.0 default plugin.
Related Issue #785
Checklist