master
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Bright Chen
|
c9778ca1a3 | Refactor NULL with nullptr in test (#3464) | ||
|
rajvarun77
|
77e2907314 |
[mysql] Clean-room auth handshake codec (scramble + handshake + packet, Stage 1a of #2093) (#3310)
* feat(mysql): clean-room auth-handshake codec (scramble + handshake + packet) Picks up the connection-phase authentication layer of MySQL protocol support per the staged plan announced on dev@brpc.apache.org and PR #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. * fix(mysql): handle lenenc NULL (0xFB) and reject oversize auth_response Address review findings on the Stage-1a auth codec (#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> * chore(mysql): add diagnostic error logs on auth-codec failure paths Per review (chenBright on #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> * chore(mysql): demote lenenc-codec diagnostic logs to LOG(WARNING) 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> * fix(mysql): build and run mysql/ unittests in Makefile CI path 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> * fix(test): make AuthCase a C++11 aggregate so push_back{...} compiles 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> * fix(build): compile src/brpc/policy/mysql into libbrpc in the Makefile 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> * test(mysql): move auth unittests from mysql/ subdir into test/ Move the three clean-room auth unittests (handshake, packet, scramble) and their test-plan doc out of test/mysql/ into test/, per review on apache/brpc#3310. The existing brpc_*_unittest glob in the Makefile, CMakeLists.txt and BUILD.bazel now picks them up, so all three build scripts revert to master with no mysql-specific additions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mysql): drop run_tests.sh mysql/ subdir entry after move The auth unittests now live in test/ and match the existing brpc*unittest glob in run_tests.sh, so revert the script to master. The removed mysql/brpc*unittest entry needed `shopt -s nullglob` to skip a missing subdir, but CI runs the script under `sh` (dash), where shopt is absent; nullglob stayed off and the now-empty glob was executed as a literal path, exiting 127. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |