rajvarun77 commented on code in PR #3310: URL: https://github.com/apache/brpc/pull/3310#discussion_r3377193768
########## src/brpc/policy/mysql/mysql_auth_packet.cpp: ########## @@ -0,0 +1,139 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "brpc/policy/mysql/mysql_auth_packet.h" + +#include <cstring> + +namespace brpc { +namespace policy { +namespace mysql { + +size_t DecodeLengthEncodedInt(const butil::StringPiece& buf, uint64_t* out) { + if (buf.empty()) { + return 0; + } + const unsigned char first = static_cast<unsigned char>(buf[0]); + if (first < 0xfb) { + *out = first; + return 1; + } + if (first == 0xfc) { Review Comment: Fixed. `DecodeLengthEncodedInt` now treats a leading `0xFB` as the protocol NULL marker rather than an error: it consumes the single byte, sets `*out = 0`, and reports NULL through a new optional `bool* is_null` out-param. Resultsets containing NULL columns now parse correctly through this shared codec. Covered by new unit tests. ########## src/brpc/policy/mysql/mysql_auth_packet.h: ########## @@ -0,0 +1,87 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Wire-format helpers for the MySQL client protocol (length-encoded +// integers, length-encoded strings, packet headers) used by the +// authentication-handshake layer. Specification: +// https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_dt_integers.html +// https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_dt_strings.html +// https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_packets.html + +#ifndef BRPC_POLICY_MYSQL_MYSQL_AUTH_PACKET_H +#define BRPC_POLICY_MYSQL_MYSQL_AUTH_PACKET_H + +#include <stdint.h> + +#include <string> + +#include "butil/strings/string_piece.h" + +namespace brpc { +namespace policy { +namespace mysql { + +// MySQL packet header: 3-byte little-endian payload length + 1-byte +// sequence id. +struct PacketHeader { + uint32_t payload_len; // 0 .. (1 << 24) - 1 + uint8_t seq; +}; +static const size_t kPacketHeaderLen = 4; + +// Maximum payload length representable in a single MySQL packet +// (24-bit length field; larger payloads are split across packets). +static const uint32_t kMaxPayloadLen = (1u << 24) - 1; + +// Decodes a length-encoded integer (lenenc-int) from |buf|. +// On success, stores the value in *out and returns the number of +// bytes consumed (1, 3, 4, or 9). Returns 0 on truncation or on the +// reserved 0xff marker. Review Comment: Fixed. The docstring now documents `0xFB` (NULL marker) alongside `0xFF` (reserved/error), and describes the `is_null` out-param semantics and the defined `*out`/`*is_null` values on every return path. ########## src/brpc/policy/mysql/mysql_auth_handshake.cpp: ########## @@ -0,0 +1,229 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "brpc/policy/mysql/mysql_auth_handshake.h" + +#include <cstring> + +#include "brpc/policy/mysql/mysql_auth_packet.h" +#include "brpc/policy/mysql/mysql_auth_scramble.h" + +namespace brpc { +namespace policy { +namespace mysql { + +namespace { + +// MySQL HandshakeV10 fixed-size pieces and constants. +const size_t kAuthPluginDataPart1Len = 8; +const size_t kReservedAfterCapsLen = 10; +const size_t kFillerAfterPart1Len = 1; +const size_t kReservedInResponseLen = 23; + +// Reads N little-endian bytes from |buf| at |off| into |out|. +template <typename T> +bool ReadLE(const butil::StringPiece& buf, size_t off, size_t n, T* out) { + if (off + n > buf.size()) return false; + T v = 0; + for (size_t i = 0; i < n; ++i) { + v |= static_cast<T>(static_cast<unsigned char>(buf[off + i])) << (8 * i); + } + *out = v; + return true; +} + +template <typename T> +void WriteLE(T value, size_t n, std::string* out) { + for (size_t i = 0; i < n; ++i) { + out->push_back(static_cast<char>((value >> (8 * i)) & 0xff)); + } +} + +} // namespace + +bool ParseHandshakeV10(const butil::StringPiece& payload, HandshakeV10* out) { + if (payload.empty()) return false; + + size_t off = 0; + out->protocol_version = static_cast<uint8_t>(payload[off++]); + if (out->protocol_version != kHandshakeV10Tag) { + return false; + } + + // server_version: NUL-terminated string + std::string version; + { + const butil::StringPiece rest(payload.data() + off, + payload.size() - off); + const size_t consumed = DecodeNullTerminatedString(rest, &version); + if (consumed == 0) return false; + off += consumed; + } + out->server_version = std::move(version); + + // connection_id: 4 LE bytes + if (!ReadLE<uint32_t>(payload, off, 4, &out->connection_id)) return false; + off += 4; + + // auth-plugin-data-part-1: 8 bytes + if (off + kAuthPluginDataPart1Len > payload.size()) return false; + std::string salt(payload.data() + off, kAuthPluginDataPart1Len); + off += kAuthPluginDataPart1Len; + + // filler 0x00 + if (off + kFillerAfterPart1Len > payload.size()) return false; + off += kFillerAfterPart1Len; + + // capability flags (lower 2 bytes) + uint16_t caps_lo = 0; + if (!ReadLE<uint16_t>(payload, off, 2, &caps_lo)) return false; + off += 2; + out->capability_flags = caps_lo; + + if (off == payload.size()) { + // Pre-4.1 server. We don't support these — bail. + return false; + } + + // character_set + if (off >= payload.size()) return false; + out->character_set = static_cast<uint8_t>(payload[off++]); + + // status_flags + if (!ReadLE<uint16_t>(payload, off, 2, &out->status_flags)) return false; + off += 2; + + // capability flags upper 2 bytes + uint16_t caps_hi = 0; + if (!ReadLE<uint16_t>(payload, off, 2, &caps_hi)) return false; + off += 2; + out->capability_flags |= static_cast<uint32_t>(caps_hi) << 16; + + // length of auth-plugin-data (or 0x00 when CLIENT_PLUGIN_AUTH is absent) + if (off >= payload.size()) return false; + const uint8_t apd_total_len = static_cast<uint8_t>(payload[off++]); + + // 10 reserved bytes (all 0x00) + if (off + kReservedAfterCapsLen > payload.size()) return false; + off += kReservedAfterCapsLen; + + if (out->capability_flags & CLIENT_SECURE_CONNECTION) { + // auth-plugin-data-part-2: max(13, apd_total_len - 8) bytes. Modern + // servers send 13 (12 salt bytes + 1 NUL filler). + const size_t part2_len = apd_total_len > kAuthPluginDataPart1Len + ? static_cast<size_t>(apd_total_len) - kAuthPluginDataPart1Len + : static_cast<size_t>(13); + const size_t want = part2_len < 13 ? 13 : part2_len; + if (off + want > payload.size()) return false; + // Concat salt parts; trim trailing NUL filler so callers see the + // raw 20-byte salt. + salt.append(payload.data() + off, want); + off += want; + if (!salt.empty() && salt.back() == '\0') { + salt.pop_back(); + } + } + if (salt.size() != kSaltLen) { + return false; + } + out->auth_plugin_data = std::move(salt); + + if (out->capability_flags & CLIENT_PLUGIN_AUTH) { + std::string name; + const butil::StringPiece rest(payload.data() + off, + payload.size() - off); + const size_t consumed = DecodeNullTerminatedString(rest, &name); + // Some servers omit the trailing NUL; tolerate by treating the + // remainder of the payload as the plugin name. + if (consumed == 0) { + out->auth_plugin_name.assign(rest.data(), rest.size()); + } else { + out->auth_plugin_name = std::move(name); + } + } + + return true; +} + +void BuildHandshakeResponse41(const HandshakeResponse41& req, std::string* out) { + WriteLE<uint32_t>(req.capability_flags, 4, out); + WriteLE<uint32_t>(req.max_packet_size, 4, out); + out->push_back(static_cast<char>(req.character_set)); + out->append(kReservedInResponseLen, '\0'); + out->append(req.username); + out->push_back('\0'); + + if (req.capability_flags & CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA) { + EncodeLengthEncodedString(req.auth_response, out); + } else if (req.capability_flags & CLIENT_SECURE_CONNECTION) { + // Auth response length must fit in one byte under this scheme. + // Callers using payloads >255 bytes (e.g., RSA ciphertext) must + // set CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA instead. + const uint8_t len = static_cast<uint8_t>( + req.auth_response.size() > 0xff ? 0xff : req.auth_response.size()); + out->push_back(static_cast<char>(len)); + out->append(req.auth_response.data(), len); Review Comment: Fixed. `BuildHandshakeResponse41` now returns `bool` and fails hard on this case: when `CLIENT_SECURE_CONNECTION` is set without `CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA` and `auth_response` exceeds 255 bytes, it logs an error and returns `false` without writing to `out`, instead of truncating. Larger payloads (e.g. RSA ciphertext) must negotiate `CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA`. New tests cover the oversize-reject and the 255-byte boundary. ########## src/brpc/policy/mysql/mysql_auth_packet.h: ########## @@ -0,0 +1,87 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Wire-format helpers for the MySQL client protocol (length-encoded +// integers, length-encoded strings, packet headers) used by the +// authentication-handshake layer. Specification: +// https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_dt_integers.html +// https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_dt_strings.html +// https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_packets.html + +#ifndef BRPC_POLICY_MYSQL_MYSQL_AUTH_PACKET_H +#define BRPC_POLICY_MYSQL_MYSQL_AUTH_PACKET_H + +#include <stdint.h> + +#include <string> + +#include "butil/strings/string_piece.h" + +namespace brpc { +namespace policy { +namespace mysql { + +// MySQL packet header: 3-byte little-endian payload length + 1-byte +// sequence id. +struct PacketHeader { + uint32_t payload_len; // 0 .. (1 << 24) - 1 + uint8_t seq; +}; +static const size_t kPacketHeaderLen = 4; + +// Maximum payload length representable in a single MySQL packet +// (24-bit length field; larger payloads are split across packets). +static const uint32_t kMaxPayloadLen = (1u << 24) - 1; + +// Decodes a length-encoded integer (lenenc-int) from |buf|. +// On success, stores the value in *out and returns the number of +// bytes consumed (1, 3, 4, or 9). Returns 0 on truncation or on the +// reserved 0xff marker. +size_t DecodeLengthEncodedInt(const butil::StringPiece& buf, uint64_t* out); + +// Appends a length-encoded integer encoding of |value| to |out|. +void EncodeLengthEncodedInt(uint64_t value, std::string* out); + +// Decodes a length-encoded string into |out_value| and returns the +// number of bytes consumed. Returns 0 if the leading lenenc-int is +// invalid or the declared payload is truncated. +size_t DecodeLengthEncodedString(const butil::StringPiece& buf, + std::string* out_value); Review Comment: Fixed. `DecodeLengthEncodedString` now takes an optional `bool* is_null`: a leading `0xFB` clears `*out_value`, sets `*is_null = true`, and consumes only the marker byte, so callers can distinguish NULL from an empty string. New unit test asserts the NULL-vs-empty distinction. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
