-
Notifications
You must be signed in to change notification settings - Fork 548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support get_server_public_key option #1377
Support get_server_public_key option #1377
Conversation
…y on server-side. get_server_public_key option enables clients to create secure connection automatically even if connection is not SSL.
@@ -1596,6 +1611,7 @@ void init_mysql2_client() { | |||
rb_define_private_method(cMysql2Client, "default_file=", set_read_default_file, 1); | |||
rb_define_private_method(cMysql2Client, "default_group=", set_read_default_group, 1); | |||
rb_define_private_method(cMysql2Client, "init_command=", set_init_command, 1); | |||
rb_define_private_method(cMysql2Client, "get_server_public_key=", set_get_server_public_key, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method name nm, this is a setter. Weird. Reading more.set_get_...
seems extraneous? Can it be plain get_server_public_key
?
@@ -159,6 +159,7 @@ def add_ssl_defines(header) | |||
have_const('SERVER_QUERY_WAS_SLOW', mysql_h) | |||
have_const('MYSQL_OPTION_MULTI_STATEMENTS_ON', mysql_h) | |||
have_const('MYSQL_OPTION_MULTI_STATEMENTS_OFF', mysql_h) | |||
have_const('MYSQL_OPT_GET_SERVER_PUBLIC_KEY', mysql_h) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not necessary to test for this constant into its own HAVE_ constant, because we can directly test for the constant at time of use (see suggested change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sodabrew Hello, Thank you for reviewing my PR.
MYSQL_OPT_GET_SERVER_PUBLIC_KEY
is defined as enum
. If my understand correctly, ifdef
does not work with enum
.
https://github.com/mysql/mysql-server/blob/8.0/include/mysql.h.pp#L411-L448
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You’re absolutely right!
@@ -996,6 +996,13 @@ static VALUE _mysql_client_options(VALUE self, int opt, VALUE value) { | |||
retval = charval; | |||
break; | |||
|
|||
#ifdef HAVE_CONST_MYSQL_OPT_GET_SERVER_PUBLIC_KEY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#ifdef HAVE_CONST_MYSQL_OPT_GET_SERVER_PUBLIC_KEY | |
#ifdef MYSQL_OPT_GET_SERVER_PUBLIC_KEY |
Thank you for the PR! |
What
This PR fixes below MySQL authentication error with SSL-disabled connection.
Background
This error only happen in case of first authentication under cleartext TCP connection.
caching_sha2_password
authentication requires a secure connection at first authentication for each users. The first authentication means that the cache is not ready yet at server-side. The reason why secure connection is required is that MySQL server need to know user password to create cache. After the cache is created, MySQL server does not request secure connection anymore. Please refer to the MySQL manual to know details.libmysqlclient
provides MYSQL_OPT_GET_SERVER_PUBLIC_KEY option which enables clients to create a requested secure connection automatically during authentication. This PR carries it to mysql2.Sample code