[DRAFT][mysql] Clean-room auth handshake codec (scramble + handshake + packet, Stage 1a of #2093) by rajvarun77 · Pull Request #3310 · apache/brpc

rajvarun77 · 2026-05-25T18:35:25Z

Clean-room MySQL client connection-phase auth handshake (Stage 1a of #2093): the scramble functions (mysql_native_password + caching_sha2_password fast path and the RSA / SSL-cleartext slow paths), the length-encoded / packet-header wire codec, and the HandshakeV10 / HandshakeResponse41 / AuthSwitchRequest / AuthMoreData handshake codec — all written from the public MySQL spec to replace the GPL-flagged mysql_auth_hash.cpp. Pure functions, no Socket wiring yet. Verified by 75 golden-vector unit tests plus integration tests that bring up a real mysqld (the which-then-spawn pattern from brpc_redis_unittest.cpp) and drive live logins end to end: fast-auth, RSA full-auth over plain TCP, cleartext full-auth over SSL, auth-plugin switch, and cache reuse.

Companion to the policy/mysql_auth/ codec landed in the previous commit. Tables every algorithm reference, every upstream MySQL/ MariaDB/Percona unit test (24 total) and integration test (5 from libmariadb), maps each to its brpc equivalent or marks it server-side-only, and lists the scope limits of the auth-codec CL.

rajvarun77 · 2026-05-27T17:31:58Z

Hi @wwbmmm @chenBright @yanglimingcn — process question on integration testing for this codec, before I write more code.

The pure-codec unit tests (59 across scramble / handshake / packet) are in place. For end-to-end auth verification I'd like to add an integration test that drives the full handshake against a real mysqld or mariadbd.

brpc already has a precedent for this in test/brpc_redis_unittest.cpp (L62–L87) — system("which redis-server") at startup, spawn it with a temp datadir if found, otherwise each test does puts("Skipped due to absence of redis-server"); return;. That gives real-protocol coverage on contributor machines and CI runners that have the binary, while staying green on lean environments.

Two questions before I mirror the pattern:

Is the redis-style system()-spawn approach the one you'd recommend for MySQL, or would you rather something different — Docker-based service, mtr-style harness, mock server only, etc.?
Is CI expected to have mysqld/mariadbd available (so the tests will actually run in Apache CI), or should the test always self-skip in CI and only execute locally?

The upstream MariaDB integration tests I'd want to mirror (test_auth256 cache-hit + cache-miss + RSA pubkey paths, test_conc312 mid-handshake auth-switch) are listed at the bottom of the spec gist. Happy to mirror whatever pattern you'd want long-term.

Also flagging — CI runs on this branch are currently in action_required state (workflow-approval gate for outside contributors). Could either of you click Approve and run on the Checks tab when convenient?

rajvarun77 · 2026-05-27T17:50:47Z

Update — pushed 38a01f33 (force-push, rebased on master at 477fa492).

Two changes since the last comment:

TLS shortcut for caching_sha2_password slow path is now supported. Added CachingSha2PasswordCleartext() + a dispatcher CachingSha2PasswordSlowPath(password, salt, server_pubkey_pem, bool is_ssl = false). Default is_ssl=false preserves the existing RSA-OAEP behavior; callers that have confirmed the channel is secure pass is_ssl=true and skip the 3-round-trip RSA path. The codec layer stays pure-functional — Stage-1c's Socket integration just threads Socket::is_ssl() straight into the trailing argument. Full rationale + the libmysqlclient reference (sql-common/client_authentication.cc:770-787) is in the PR description under "TLS shortcut via is_ssl flag".
Test count: 75 (+11 from the previous push) — 5 new in MysqlCachingSha2CleartextTest + 6 new in MysqlCachingSha2SlowPathTest. PR description has the by-name mapping table to all 28 upstream unit tests in mysql/mysql-server plus the 6 client-relevant MariaDB integration tests (deferred).

Asks:

@wwbmmm @chenBright @yanglimingcn — could you click Approve and run on the Checks tab? The previous 17-job suite all passed on bc95c031; the new commit is gated on the outside-contributor approval workflow.
Still open: the integration-test approach question from the previous comment (redis-style system()-spawn of mysqld vs. Docker vs. mtr — whichever pattern you prefer).

PR ready for first-pass review whenever you have bandwidth.

rajvarun77 · 2026-05-29T20:47:11Z

Hi @wwbmmm @chenBright @yanglimingcn — could one of you take a look at this Stage-1a codec and shepherd it through review? @wwbmmm you're the natural fit, but anyone with bandwidth is welcome. Happy to keep the staged split and turn comments around fast.

wwbmmm · 2026-05-30T03:32:03Z

Hi @wwbmmm @chenBright @yanglimingcn — process question on integration testing for this codec, before I write more code.

The pure-codec unit tests (59 across scramble / handshake / packet) are in place. For end-to-end auth verification I'd like to add an integration test that drives the full handshake against a real mysqld or mariadbd.

brpc already has a precedent for this in test/brpc_redis_unittest.cpp (L62–L87) — system("which redis-server") at startup, spawn it with a temp datadir if found, otherwise each test does puts("Skipped due to absence of redis-server"); return;. That gives real-protocol coverage on contributor machines and CI runners that have the binary, while staying green on lean environments.

Two questions before I mirror the pattern:

Is the redis-style system()-spawn approach the one you'd recommend for MySQL, or would you rather something different — Docker-based service, mtr-style harness, mock server only, etc.?

Is CI expected to have mysqld/mariadbd available (so the tests will actually run in Apache CI), or should the test always self-skip in CI and only execute locally?

The upstream MariaDB integration tests I'd want to mirror (test_auth256 cache-hit + cache-miss + RSA pubkey paths, test_conc312 mid-handshake auth-switch) are listed at the bottom of the spec gist. Happy to mirror whatever pattern you'd want long-term.

Also flagging — CI runs on this branch are currently in action_required state (workflow-approval gate for outside contributors). Could either of you click Approve and run on the Checks tab when convenient?

I think you can follow the brpc_redis_unittest.cpp way for mysql: install a mysqld locally, and connect the mysqld server in the test code.
For future improvement, I think we can create a new CI job, install redis and mysqld in the CI server and run the corresponding test cases.

…packet) Picks up the connection-phase authentication layer of MySQL protocol support per the staged plan announced on dev@brpc.apache.org and PR apache#2093. Three modules under src/brpc/policy/mysql_auth/: - mysql_auth_scramble: mysql_native_password (SHA1 XOR), plus caching_sha2_password fast path (SHA256 XOR) and slow path with both branches: - RSA-OAEP via OpenSSL EVP_PKEY_encrypt + PKCS1_OAEP_PADDING - TLS-cleartext (NUL-terminated password) selected at runtime via CachingSha2PasswordSlowPath(..., bool is_ssl = false). Default is_ssl=false preserves RSA behavior for callers not yet threaded with the connection's TLS state. - mysql_auth_packet: length-encoded int/string, 4-byte packet header, NUL-terminated string. - mysql_auth_handshake: HandshakeV10 parse, HandshakeResponse41 build, AuthSwitchRequest parse, AuthMoreData parse. All written clean-room from MySQL's public protocol documentation; no GPL-licensed source was consulted. SHA-256 and RSA paths use OpenSSL EVP -- works under both OpenSSL and BoringSSL. 75 unit tests across three files under test/mysql_auth/: 36 in brpc_mysql_auth_scramble_unittest.cpp 19 in brpc_mysql_auth_handshake_unittest.cpp 20 in brpc_mysql_auth_packet_unittest.cpp Mirrors every client-relevant case from mysql/mysql-server's GPLv2 unittest/gunit/sha2_password-t.cc + sha256_scramble_t.cc with independently re-derived hex vectors. Server-side cases (cache, storage format, *_verification_*) are out of scope for a client library; full mapping table linked from the PR description. Scope limits in this CL: auth codec only. No PROTOCOL_MYSQL registration, no Socket integration, no compressed packets, no prepared statements, no transactions. All land in the follow-up CL. TLS state plumbing in Stage 1c flows the dispatcher's Socket::is_ssl() into the trailing argument here -- no further API change. Replaces the earlier src/brpc/policy/mysql_auth_hash.{h,cpp} + test/brpc_mysql_auth_hash_unittest.cpp; those files are moved into the new mysql_auth/ subdirectory and renamed.

rajvarun77 · 2026-06-01T20:12:48Z

Hi @wwbmmm — this is ready for review whenever you have a moment. Could you please take a look? Happy to adjust anything. Thanks!

Copilot

Pull request overview

This PR introduces a clean-room implementation of the MySQL client connection-phase authentication handshake (scramble algorithms + wire codecs for packet framing and handshake packets), along with extensive unit and server-integration tests to validate behavior against real mysqld instances (self-spawned or user-provided).

Changes:

Added MySQL auth scramble implementations for mysql_native_password and caching_sha2_password (fast path + RSA/cleartext slow paths).
Added MySQL wire-format helpers (packet header + length-encoded int/string + NUL-terminated string) and handshake packet codec (HandshakeV10, HandshakeResponse41, AuthSwitchRequest, AuthMoreData).
Added golden-vector unit tests and end-to-end integration tests (including SSL upgrade and auth-plugin switch), plus build integration for CMake/Bazel test targets.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/mysql/README.md	Documents how to run the new MySQL handshake integration tests in spawned vs. running-server modes.
test/mysql/brpc_mysql_auth_scramble_unittest.cpp	Golden-vector unit tests for scramble algorithms and RSA/cleartext slow paths.
test/mysql/brpc_mysql_auth_packet_unittest.cpp	Unit tests for length-encoded integer/string and packet header codecs.
test/mysql/brpc_mysql_auth_handshake_unittest.cpp	Unit tests for handshake packet codec plus end-to-end login integration tests (plain + SSL + auth-switch + cache reuse).
test/CMakeLists.txt	Ensures MySQL unit tests under `test/mysql/` are built by the CMake test glob.
test/BUILD.bazel	Adds a Bazel `cc_test` target to build/run the MySQL unit tests.
src/brpc/policy/mysql/mysql_auth_scramble.h	Declares clean-room auth scramble APIs/constants.
src/brpc/policy/mysql/mysql_auth_scramble.cpp	Implements the scramble algorithms and RSA/cleartext slow paths.
src/brpc/policy/mysql/mysql_auth_packet.h	Declares MySQL packet framing and basic wire-type codecs.
src/brpc/policy/mysql/mysql_auth_packet.cpp	Implements MySQL packet framing and basic wire-type codecs.
src/brpc/policy/mysql/mysql_auth_handshake.h	Declares handshake packet structs and parse/build functions.
src/brpc/policy/mysql/mysql_auth_handshake.cpp	Implements greeting parsing, response building, and auth continuation packet parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rajvarun77 · 2026-06-04T22:20:48Z

@chenBright @wwbmmm — re-review request for the clean-room auth handshake codec (Stage 1a of #2093): mysql_native_password + caching_sha2_password scrambles, the HandshakeV10 / HandshakeResponse41 codec, and the length-encoded wire helpers — all derived from the public MySQL protocol documentation (no GPL lineage).

The full MySQL client protocol (text protocol + transactions + prepared statements) built on top of this codec is up as #3330.

wwbmmm · 2026-06-05T03:58:58Z

LGTM

chenBright

The test/Makefile is used by CI, so it also needs to be updated, right?

brpc/test/Makefile

Lines 237 to 243 in 72adb6e

    
           brpc_%_unittest:$(TEST_PROTO_OBJS) brpc_%_unittest.o | libbrpc.dbg.$(SOEXT) 
        
           	@echo "> Linking $@" 
        
           ifeq ($(SYSTEM),Linux) 
        
           	$(CXX) -o $@ $(LIBPATHS) $(SOPATHS) -Xlinker "-(" $^ -Xlinker "-)" $(STATIC_LINKINGS) $(UT_DYNAMIC_LINKINGS) 
        
           else ifeq ($(SYSTEM),Darwin) 
        
           	$(CXX) -o $@ $(LIBPATHS) $(SOPATHS) $^ $(GTEST_STATIC_LINKINGS) $(UT_DYNAMIC_LINKINGS) 
        
           endif

Address review findings on the Stage-1a auth codec (apache#3310): - DecodeLengthEncodedInt/String: 0xFB is the protocol NULL marker, not an error. Decode it as NULL (1 byte consumed) via a new optional bool* is_null out-param instead of folding it into the failure path, so the shared codec can faithfully parse resultsets containing NULL. Define *out/*is_null on every path and use an overflow-safe bounds check. - BuildHandshakeResponse41: the CLIENT_SECURE_CONNECTION 1-byte length prefix cannot represent an auth_response >255 bytes. Fail hard (return bool false, write nothing, LOG(ERROR)) instead of silently truncating, which produced an invalid response and desynchronized the packet stream. Callers with larger payloads must negotiate CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA. Adds unit tests for the NULL marker (int + string) and for the oversize reject / 255-byte boundary cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Per review (chenBright on apache#3310): the handshake/packet parsers returned false/0 silently on malformed, truncated, or unsupported input, giving no clue why a connection failed. Add a LOG(ERROR) at each failure path naming the function and the concrete cause (truncated <field>, pre-4.1 server, reserved 0xFF marker, length mismatch, etc.). Logic unchanged — only logs inserted; the lenenc NULL (0xFB) success path is intentionally not logged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The length-encoded int/string/header decode failures fire during normal resultset parsing and are not catastrophic, so log them at WARNING rather than ERROR. Handshake-parse failures stay at LOG(ERROR) (a malformed handshake is a fatal connection-setup failure). Severity-only change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@chenBright

test/Makefile and test/run_tests.sh drive the make-based CI lane (clang-unittest: `cd test && make` then `sh ./run_tests.sh`). Both used non-recursive globs anchored at test/, so the new tests under test/mysql/ were neither compiled nor executed there — only the CMake and Bazel lanes picked them up. The make lane was silently green without running them. - Makefile: add `$(wildcard mysql/brpc_*unittest.cpp)` to TEST_BRPC_SOURCES, plus a `mysql/brpc_%_unittest:` link rule (the existing `brpc_%_unittest:` pattern anchors on a literal `brpc_` prefix and won't match a `mysql/` target). - run_tests.sh: add `mysql/brpc*unittest` to the executed set and enable `nullglob` so a missing subdir expands to nothing instead of a bogus name. Addresses @chenBright's review note that test/Makefile is used by CI and must be updated for the new MySQL tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CI compiles the unittests with -std=c++0x (C++11). The test helper struct AuthCase had a default member initializer (bool use_ssl = false), which makes a class a non-aggregate under C++11 (the rule was only relaxed in C++14). That broke every g_auth_cases.push_back({label,user,password,ssl}) with "no matching member function for call to 'push_back'", failing the clang-unittest and clang-unittest-asan compile-tests step. Drop the default member initializer; all five init sites already pass use_ssl explicitly, so behavior is unchanged and the struct is again an aggregate that accepts braced-init under C++11. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CI compiles unittests with -std=c++0x (C++11), where a class with a default member initializer is not an aggregate, so g_auth_cases.push_back({...}) fails to compile. Drop the default member initializer on AuthCase::use_ssl; all init sites pass it explicitly. Mirrors the same fix on the apache#3310 codec branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The Makefile builds libbrpc by globbing $(d)/* over BRPC_DIRS, and the glob is non-recursive, so src/brpc/policy/mysql/*.cpp was never compiled into the library. The make-based CI lanes (clang-unittest, clang-unittest-asan) therefore failed to link the mysql unittests with "undefined reference to brpc::policy::mysql::*" for every codec symbol. CMake (file(GLOB_RECURSE src/brpc/*.cpp)) and Bazel already pick the directory up, which is why only the make lanes broke once the mysql tests were wired into that lane. Add src/brpc/policy/mysql to BRPC_DIRS so the codec sources land in libbrpc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rajvarun77 · 2026-06-09T03:11:23Z

@wwbmmm @chenBright — CI is green on the latest revision (all 17 jobs), and every review thread on this PR is resolved: the lenenc NULL (0xFB) handling, the oversize auth_response hard-fail, the namespace rename, and the diagnostic failure logs you asked for.

I also confirmed from the job logs that the codec unit tests actually execute (not just compile) in the make-based lanes — brpc_mysql_auth_handshake_unittest (28 tests), brpc_mysql_auth_packet_unittest (27) and brpc_mysql_auth_scramble_unittest all run and pass under both clang-unittest and clang-unittest-asan.

This Stage-1a clean-room MySQL auth codec is ready to merge whenever you are happy with it — could one of you merge it when you get a chance? Thanks again for the reviews.

chenBright

Please move brpc_mysql_*_unittest.cpp to the test folder. No changes are needed to the build script.

rajvarun77 · 2026-06-09T20:24:36Z

@wwbmmm — friendly follow-up: thanks again for the LGTM. Since CI is green and all review threads are resolved, would you mind submitting a formal Approve and merging when you have a moment? Happy to address anything else if needed.

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from bc95c03 to d8752f1 Compare May 27, 2026 17:06

rajvarun77 changed the title ~~[DRAFT][mysql] Clean-room mysql_native_password scramble (Stage 1a of #2093 takeover)~~ [DRAFT][mysql] Clean-room auth handshake codec (scramble + handshake + packet, Stage 1a of #2093) May 27, 2026

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from 9a44a6d to d8752f1 Compare May 27, 2026 17:30

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from d8752f1 to 38a01f3 Compare May 27, 2026 17:49

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from 38a01f3 to a06f2b9 Compare May 29, 2026 20:42

wwbmmm reviewed May 30, 2026

View reviewed changes

Comment thread src/brpc/policy/mysql_auth/mysql_auth_handshake.h Outdated

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch 2 times, most recently from 8370692 to ae7580e Compare May 31, 2026 19:38

rajvarun77 mentioned this pull request May 31, 2026

[DRAFT][ci] Install mysql-server for MySQL handshake integration tests #3318

Closed

rajvarun77 force-pushed the mysql-auth-hash-clean-room branch from ae7580e to 6254f7a Compare June 1, 2026 20:09

rajvarun77 marked this pull request as ready for review June 1, 2026 20:11

This was referenced Jun 1, 2026

ci(linux): bring up redis-server and mysql-server for unittests rajvarun77/brpc#1

Open

ci(linux): bring up redis-server and mysql-server for unittests #3323

Merged

chenBright requested a review from Copilot June 2, 2026 02:21

Copilot started reviewing on behalf of chenBright June 2, 2026 02:22 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

Comment thread src/brpc/policy/mysql/mysql_auth_packet.cpp

Comment thread src/brpc/policy/mysql/mysql_auth_packet.h Outdated

Comment thread src/brpc/policy/mysql/mysql_auth_handshake.cpp Outdated

Comment thread src/brpc/policy/mysql/mysql_auth_packet.h Outdated

rajvarun77 mentioned this pull request Jun 4, 2026

Add MySQL client protocol (text protocol, transactions, prepared statements) on clean-room auth (#2093) #3330

Open

chenBright reviewed Jun 6, 2026

View reviewed changes

Comment thread src/brpc/policy/mysql/mysql_auth_handshake.cpp

rajvarun77 and others added 3 commits June 8, 2026 20:54

rajvarun77 and others added 2 commits June 8, 2026 21:37

chenBright reviewed Jun 9, 2026

View reviewed changes

	brpc_%_unittest:$(TEST_PROTO_OBJS) brpc_%_unittest.o \| libbrpc.dbg.$(SOEXT)
	@echo "> Linking $@"
	ifeq ($(SYSTEM),Linux)
	$(CXX) -o $@ $(LIBPATHS) $(SOPATHS) -Xlinker "-(" $^ -Xlinker "-)" $(STATIC_LINKINGS) $(UT_DYNAMIC_LINKINGS)
	else ifeq ($(SYSTEM),Darwin)
	$(CXX) -o $@ $(LIBPATHS) $(SOPATHS) $^ $(GTEST_STATIC_LINKINGS) $(UT_DYNAMIC_LINKINGS)
	endif

Conversation

rajvarun77 commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajvarun77 commented May 27, 2026

Uh oh!

rajvarun77 commented May 27, 2026

Uh oh!

rajvarun77 commented May 29, 2026

Uh oh!

wwbmmm commented May 30, 2026

Uh oh!

Uh oh!

rajvarun77 commented Jun 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajvarun77 commented Jun 4, 2026

Uh oh!

wwbmmm commented Jun 5, 2026

Uh oh!

chenBright left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rajvarun77 commented Jun 9, 2026

Uh oh!

chenBright left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajvarun77 commented Jun 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rajvarun77 commented May 25, 2026 •

edited

Loading

chenBright left a comment •

edited

Loading