fix: drain caching_sha2_password AuthMoreData+OK in switchAuth(), gua…#148
Open
JacobStephens2 wants to merge 2 commits into
Open
Conversation
…rd consume() against short packets (MySQL 8.4)
…uard Two tests for the fix on this branch: 1. tests/Unit/Event/EventConsumeShortPacketTest.php feeds Event::consume() three short responses (empty, a 7-byte OK packet, 19 bytes) and asserts it skips them and dispatches nothing. Without the < 20-byte guard all three error with a TypeError on BinaryDataReader::readUInt16(). 3 cases, no MySQL needed. 2. tests/Integration/CachingSha2PasswordAuthTest.php asserts the binlog handshake completes over caching_sha2_password on MySQL 8+ (the fast-auth AuthMoreData 0x01 -> 0x03 -> OK path that switchAuth() drains), then reads a QueryDTO to confirm the stream is still aligned. Skips below 8.0; exercised by the 8.0 and 8.4 CI legs.
Author
|
Hi @krowinski, Following up on this one. I've added test coverage and filled in the description:
The description now walks the handshake, the two-packet drain, and how to reproduce. 2 files +50/-1 in the fix itself. Whenever you have a moment to review, I'd appreciate it. Thanks, Jacob |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
The binlog client can't complete its handshake against MySQL 8.4 when the account uses caching_sha2_password (the 8.x default).
switchAuth()reads the auth-switch response but never drains the extra packets caching_sha2_password sends, so every latergetResponse()is two packets ahead and the first read reachesEvent::consume()as a malformed event, crashing with a TypeError onreadInt32()/readUInt16()(PHP 8 strict types).The handshake
On the fast-auth path - the account's password is already in the server's auth cache - the server answers the auth switch with an
AuthMoreDatapacket (status byte 0x01) carryingfast_auth_success(0x03), then theOKpacket. The old code drained neither, leaving the stream offset by two packets. Protocol reference: https://dev.mysql.com/doc/dev/mysql-server/latest/page_caching_sha2_authentication_exchanges.htmlThe change (2 files, +50/-1)
BinLogSocketConnect::switchAuth(): after sending the scrambled password, when the switched plugin is caching_sha2_password, drain theAuthMoreDatapacket and then the trailingOK. If the server instead asks forperform_full_authentication(0x04), throw a clearBinLogExceptionrather than hang - full auth needs the password over TLS or a unix socket, which this client doesn't implement, so fast-auth requires the user already be cached server-side.Event::consume(): skip any response shorter than the 20-byte event-header minimum (1 status byte + 19-byte header) instead of parsing a leftover OK packet as an event. Defense in depth for a misaligned stream.Tests
tests/Unit/Event/EventConsumeShortPacketTest.php: feedsconsume()three short responses (empty, a 7-byte OK packet, 19 bytes) and asserts it skips them and dispatches nothing. Without the guard all three error with a TypeError onreadUInt16(). No MySQL needed.tests/Integration/CachingSha2PasswordAuthTest.php: asserts the binlog handshake completes over caching_sha2_password on MySQL 8+ and reads aQueryDTOto confirm the stream is aligned. Skips below 8.0; exercised by the 8.0 and 8.4 CI legs.Reproducing
Point the binlog client at MySQL 8.4 with a caching_sha2_password user whose password is already cached server-side (any prior successful login warms it). Before this change the handshake throws a TypeError; after it, the connection completes and events read normally. I've been running this in production against MySQL 8.4.