Auth refactoring and bug fixes #807
Conversation
Updates #795
julienschmidt
force-pushed
the
auth_refactoring
branch
from
0dcbff2 to
8985303
Compare
|
@arnehormann @methane I know this is a huge changeset to review, but it would be great if you would find time to do so soon. Much of it is just moved code and very similar tests anyway. This PR and the follow-up PR #808 should make the auth system much more stable and fix several currently existing bugs. |
| return message1 | ||
| } | ||
|
|
||
| func (mc *mysqlConn) auth(authData []byte, plugin string) ([]byte, bool, error) { |
There was a problem hiding this comment.
The actual changes start here. The code above was moved from utils.go
packets.go
Outdated
| return readAuthSwitch(data) | ||
| if len(data) > 1 { | ||
| pluginEndIndex := bytes.IndexByte(data, 0x00) | ||
| plugin := string(data[1:pluginEndIndex]) |
There was a problem hiding this comment.
if pluginEndIndex < 0 {
return nil, "", errors.New("invalid AuthSwitchRequest packet")
}| copy(b[:], cipher) | ||
| return b[:], pluginName, nil | ||
| copy(b[:], authData) | ||
| return b[:], plugin, nil |
There was a problem hiding this comment.
b instead of b[:]. (While old code was same...)
There was a problem hiding this comment.
copy requires slices as parameters but b is an array and not a slice.
|
Just FYI. I've used master and this PR in one of our repos and some tests on master failed with |
* log missing auth plugin name * refactor auth handling * auth: fix AllowNativePasswords * auth: remove plugin name print * packets: attempt to fix writePublicKeyAuthPacket * packets: do not NUL-terminate auth switch packets * move handleAuthResult to auth * add old_password auth tests * auth: add empty old_password test * auth: add cleartext auth tests * auth: add native auth tests * auth: add caching_sha2 tests * rename init and auth packets to documented names * auth: fix plugin name for switched auth methods * buffer: optimize default branches * auth: add tests for switch to caching sha2 * auth: add tests for switch to cleartext password * auth: add tests for switch to native password * auth: sync NUL termination with official connectors * packets: handle missing NUL bytes in AuthSwitchRequests Updates #795
Description
This PR refactors the existing auth code and separates it from other code (currently the auth code is spread over
driver.go,utils.goandpackets.go. This refactoring also serves as a preparation for adding more auth plugins, such assha256_password(#625) ordialog(#803) and an exported interface for adding custom auth plugins, as proposed in #552.It further fixes many bugs: the following new tests fail when backported to the old code (see https://travis-ci.org/go-sql-driver/mysql/jobs/383270883):
This PR is partially based on the work done in #552.
Fixes #806
Checklist