Skip to content

Commit

Permalink
Bug#35737521 connect fails with preferred/as_client and old clients
Browse files Browse the repository at this point in the history
Authenticating via non-ssl connections and using 3rd party clients
fails with:

- Couldn't read RSA public key from server

when using caching-sha2-password's fast-auth support and:

- client_ssl_mode=PREFERRED or PASSTHROUGH
- server_ssl_mode=AS_CLIENT

The problem does not appear with:

- libmysqlclient based clients
- if SSL is used
- if another client_ssl_mode/server_ssl_mode combination is used.

Root Cause
==========

The error is generated by the client as it interprets the protocol
differently than libmysqlclient based clients.

With those client_ssl_mode/server_ssl_mode combinations the router
wronly assumes that it should ask the client to switch the authentication
method although forwarding the server's response would be the correct
message flow.

It only affects PREFERRED/PASSTHROUGH + AS_CLIENT as they have different
starting protocol flow which does NOT ask the client for the plaintext
password over secure transports.

But the AuthForwarder assumes that the password has already been asked
for and gets out of sync.

Change
======

- track if the client's plaintext password was requested idependently
  from the state of "in-handshake".

Change-Id: I2de5e0d0b3fc2a8eaa193096cb32a681500af78a
  • Loading branch information
weigon committed Aug 24, 2023
1 parent 4c04e51 commit 8d7f686
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 9 deletions.
3 changes: 2 additions & 1 deletion router/src/routing/src/classic_auth_caching_sha2_forwarder.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ class AuthCachingSha2Forwarder : public ForwardingProcessor {
public:
AuthCachingSha2Forwarder(MysqlRoutingClassicConnectionBase *conn,
std::string initial_server_auth_data,
bool in_handshake = true,
bool client_requested_full_auth = false)
: ForwardingProcessor(conn),
initial_server_auth_data_{std::move(initial_server_auth_data)},
client_requested_full_auth_{client_requested_full_auth},
stage_{client_requested_full_auth_ ? Stage::Response : Stage::Init} {}
stage_{in_handshake ? Stage::Response : Stage::Init} {}

enum class Stage {
Init,
Expand Down
12 changes: 7 additions & 5 deletions router/src/routing/src/classic_auth_forwarder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ stdx::expected<Processor::Result, std::error_code> AuthForwarder::init() {
connection(), initial_auth_method_data, true));
} else if (auth_method_name == AuthCachingSha2Password::kName) {
connection()->push_processor(std::make_unique<AuthCachingSha2Forwarder>(
connection(), initial_auth_method_data, true));
connection(), initial_auth_method_data, true,
client_requested_full_auth_));
} else if (auth_method_name == AuthNativePassword::kName) {
connection()->push_processor(std::make_unique<AuthNativeForwarder>(
connection(), initial_auth_method_data, true));
Expand Down Expand Up @@ -368,7 +369,7 @@ AuthForwarder::auth_method_switch() {
connection(), auth_method_data, dst_protocol->password().value()));
} else {
connection()->push_processor(std::make_unique<AuthSha256Forwarder>(
connection(), auth_method_data));
connection(), auth_method_data, false));
}
} else if (auth_method_name == AuthCachingSha2Password::kName) {
dst_protocol->auth_method_name(auth_method_name);
Expand All @@ -378,8 +379,9 @@ AuthForwarder::auth_method_switch() {
connection()->push_processor(std::make_unique<AuthCachingSha2Sender>(
connection(), auth_method_data, dst_protocol->password().value()));
} else {
// trigger a switch
connection()->push_processor(std::make_unique<AuthCachingSha2Forwarder>(
connection(), auth_method_data));
connection(), auth_method_data, false));
}
} else if (auth_method_name == AuthNativePassword::kName) {
if (dst_protocol->password().has_value()) {
Expand All @@ -390,7 +392,7 @@ AuthForwarder::auth_method_switch() {
connection(), auth_method_data, dst_protocol->password().value()));
} else {
connection()->push_processor(std::make_unique<AuthNativeForwarder>(
connection(), auth_method_data));
connection(), auth_method_data, false));
}
} else if (auth_method_name == AuthCleartextPassword::kName) {
dst_protocol->auth_method_name(auth_method_name);
Expand All @@ -401,7 +403,7 @@ AuthForwarder::auth_method_switch() {
connection(), auth_method_data, dst_protocol->password().value()));
} else {
connection()->push_processor(std::make_unique<AuthCleartextForwarder>(
connection(), auth_method_data));
connection(), auth_method_data, false));
}
} else {
dst_protocol->auth_method_name(auth_method_name);
Expand Down
6 changes: 5 additions & 1 deletion router/src/routing/src/classic_auth_forwarder.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@
*/
class AuthForwarder : public ForwardingProcessor {
public:
using ForwardingProcessor::ForwardingProcessor;
AuthForwarder(MysqlRoutingClassicConnectionBase *conn,
bool client_requested_full_auth = false)
: ForwardingProcessor(conn),
client_requested_full_auth_{client_requested_full_auth} {}

enum class Stage {
Init,
Expand Down Expand Up @@ -62,6 +65,7 @@ class AuthForwarder : public ForwardingProcessor {
stdx::expected<Result, std::error_code> ok();

Stage stage_{Stage::Init};
bool client_requested_full_auth_;
};

#endif
14 changes: 12 additions & 2 deletions router/src/routing/src/classic_greeting_forwarder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,12 @@ ServerGreetor::client_greeting_after_tls() {

stdx::expected<Processor::Result, std::error_code>
ServerGreetor::initial_response() {
connection()->push_processor(std::make_unique<AuthForwarder>(connection()));
const auto *src_protocol = connection()->client_protocol();

connection()->push_processor(std::make_unique<AuthForwarder>(
connection(),
// password was requested already.
src_protocol->password() && !src_protocol->password()->empty()));

stage(Stage::FinalResponse);
return Result::Again;
Expand Down Expand Up @@ -1890,7 +1895,12 @@ ServerFirstAuthenticator::client_greeting_after_tls() {

stdx::expected<Processor::Result, std::error_code>
ServerFirstAuthenticator::initial_response() {
connection()->push_processor(std::make_unique<AuthForwarder>(connection()));
const auto *src_protocol = connection()->client_protocol();

connection()->push_processor(std::make_unique<AuthForwarder>(
connection(),
// password was requested already.
src_protocol->password() && !src_protocol->password()->empty()));

stage(Stage::FinalResponse);
return Result::Again;
Expand Down

0 comments on commit 8d7f686

Please sign in to comment.