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

Put zero filler into the SSL handshake packet. #1066

Merged
merged 1 commit into from Feb 25, 2020

Conversation

pivanof
Copy link
Contributor

@pivanof pivanof commented Feb 24, 2020

According to the linked documentation at
http://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::SSLRequest
SSLRequest packet should have zero filler similar to the regular handshake request,
but now the driver puts zeros only in the regular request. Luckily vanilla MySQL
doesn't rely on this zero filler and doesn't verify its presence, thus the driver worked
fine so far. But MySQL can change to rely on zeros at any point.

The problem was discovered while testing against a customized MySQL.

Description

Please explain the changes you made here.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

According to the linked documentation at
http://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::SSLRequest
SSLRequest packet should have zero filler similar to the regular handshake request,
but now the driver puts zeros only in the regular request. Luckily vanilla MySQL
doesn't rely on this zero filler and doesn't verify its presence, thus the driver worked
fine so far. But MySQL can change to rely on zeros at any point.

The problem was discovered while testing against a customized MySQL.
@pivanof
Copy link
Contributor Author

pivanof commented Feb 24, 2020

@methane can you merge the PR? I don't have write access to do it myself.

@julienschmidt
Copy link
Member

The copyright holder is Google Inc.?

Ideally also add a simple regression test.

@pivanof
Copy link
Contributor Author

pivanof commented Feb 25, 2020

Yes, the copyright holder is Google.

I've tried to add a test, but turned out it's a hard thing to do due to SSL authentication being a completely different code path. I've also noticed that there are no tests for SSL connections at all, probably because of the same reason. Thus I decided that it should be fine without a test for now.

@julienschmidt julienschmidt merged commit dd9d356 into go-sql-driver:master Feb 25, 2020
tz70s pushed a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
According to the linked documentation at
http://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::SSLRequest
SSLRequest packet should have zero filler similar to the regular handshake request,
but now the driver puts zeros only in the regular request. Luckily vanilla MySQL
doesn't rely on this zero filler and doesn't verify its presence, thus the driver worked
fine so far. But MySQL can change to rely on zeros at any point.

The problem was discovered while testing against a customized MySQL.
tz70s pushed a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
According to the linked documentation at
http://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::SSLRequest
SSLRequest packet should have zero filler similar to the regular handshake request,
but now the driver puts zeros only in the regular request. Luckily vanilla MySQL
doesn't rely on this zero filler and doesn't verify its presence, thus the driver worked
fine so far. But MySQL can change to rely on zeros at any point.

The problem was discovered while testing against a customized MySQL.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants