mysql8 auth compatibility #781
Conversation
# Conflicts: # .github/workflows/ci.yml # client/auth.go
client/auth.go
Outdated
| if max := 13; rest > max { | ||
| rest = max | ||
| } | ||
| if data[pos+rest-1] != 0 { |
There was a problem hiding this comment.
| if data[pos+rest-1] != 0 { | |
| if data[pos+rest] != 0 { |
BTW, im not familiar with this part so double check that why there's NULL after scramble? In the MySQL protocol doc I think it's a fixed length string, rather than a NULL-terminated string.
There was a problem hiding this comment.
I found the origin of it:
n - rest of the plugin provided data (at least 12 bytes)
1 - \0 byte, terminating the second part of a scramble
And sha256_password auth implementation follows this rule:
Native authentication sent 20 bytes + '\0' character = 21 bytes.
This plugin must do the same to stay consistent with historical behavior
Also, alibaba channel contains the same description:
Packet规定最后13个byte是剩下的scrumble, 但实际上最后一个字节是0, 不应该包含在scrumble中.
I used a translator for this.
I can remove this check by \0, but it looks like a historic standard convention.
There was a problem hiding this comment.
What's about 20 bytes len I found this:
the first packet must have at least 20 bytes of a scramble.
if a plugin provided less, we pad it to 20 with zeros
Right now I don't see places where it would be more than 20 bytes, but let it be for the future.
There was a problem hiding this comment.
I checked this logic for mysql 8.0 and 5.7. Unfortunately, compatibility with 5.6- was broken since we added json fields in our tests in 2021.
Ooooops. I saw only 13 and 26 bytes variants before... Co-authored-by: lance6716 <lance6716@gmail.com>
| // auth_data is end with 0x00, so we need to trim 0x00 | ||
| resetOfAuthDataEndPos := pos + maxAuthDataLen - 8 - 1 | ||
| c.salt = append(c.salt, data[pos:resetOfAuthDataEndPos]...) | ||
| if c.capability&CLIENT_SECURE_CONNECTION != 0 { |
There was a problem hiding this comment.
Just remind that this is a deprecated flag https://dev.mysql.com/doc/dev/mysql-server/latest/group__group__cs__capabilities__flags.html#ga8be684cc38eeca913698414efec06933 , we can skip this if-check or leave it because no test is broken.
There was a problem hiding this comment.
Yes, it seems always set.
There was a problem hiding this comment.
What about merging this PR as is? I added an issue.
No description provided.