mirromutth / r2dbc-mysql

R2DBC MySQL Implementation
Apache License 2.0
656 stars 100 forks source link

Should check server SSL capability #33

Closed mirromutth closed 5 years ago

mirromutth commented 5 years ago

It is also MySQL 5.6 compatibility problem for #32 .

MySQL 5.6.x Community Edition does not support SSL, so if the user has enabled SSL and the MySQL server is 5.6.x Community Edition, the user will receive an error message containing a handshake failure.

In this case, checking the MySQL server capabilities in HandshakeV10Message will be more useful than checking the MySQL server version.

My personal opinion is that for MySQL, SSL cannot support only enable/disable, and should support the following 3 modes:

R2DBC ConnectionFactoryOptions.SSL is a boolean option, I try treat true same as OPTIONAL, and append a SSL option Option.<Boolean>valueOf("sslRequired") for enable REQUIRED mode.

@mp911de Should I support optional SSL mode for MySQL 5.6 compatibility?

Another plan is just throw an exception and close the connection when the user has enabled SSL and the MySQL server does not support SSL. Then r2dbc-mysql still only has two modes, ENABLED or DISABLED. (Equivalent to ENABLED same as REQUIRED, and the OPTIONAL mode is removed)

mp911de commented 5 years ago

What does the MySQL JDBC do? In doubt, R2DBC should try to find answers in JDBC.

mirromutth commented 5 years ago

@mp911de mysql-connector-j use two options for choice SSL mode, useSSL and requireSSL.

As long as the server is correctly configured to use SSL, there is no need to configure anything on the Connector/J client to use encrypted connections (the exception is when Connector/J is connecting to very old server versions like 5.6.25 and earlier or 5.7.5 and earlier, in which case the client must set the connection property useSSL=true in order to use encrypted connections). The client can demand SSL to be used by setting the connection property requireSSL=true; the connection then fails if the server is not configured to use SSL. Without requireSSL=true, the connection just falls back to non-encrypted mode if the server is not configured to use SSL.

Reference from document: https://dev.mysql.com/doc/connector-j/5.1/en/connector-j-reference-using-ssl.html

Note: mysql-connector-j 5.1.x target MySQL server version is 5.5.0 - 5.7.x

More intuitive:

mp911de commented 5 years ago

Thanks for clarifying. Let’s keep a single flag. In SQL Server we have a similar arrangement. The server can not support SSL, support SSL and demand SSL.

I would keep it the way: if the server wants ssl, then enable SSL. If the client wants ssl we should require SSL and fail if SSL is not available.

mirromutth commented 5 years ago

I am not sure if I understand the meanings. I tried to clarify: if the user sets ConnectionFactoryOptions.SSL to true, it means SSL required. If it is set to false, it means SSL disabled. If neither is configured, choose whether according to the SSL support of the MySQL server, it is also means SSL optional.

mp911de commented 5 years ago

Not setting SSL to true means SSL config is undefined. We had a similar discussion at https://github.com/r2dbc/r2dbc-postgresql/pull/104

mirromutth commented 5 years ago

MySQL has the similar arrangement as PostgreSQL in client version 5.7.23 or higher, connector-j version 8.0.13 or higher.

Agree your suggestion, let's keep single flag and support Option.valueOf("sslMode") configuration.

mirromutth commented 5 years ago

SSL_MODE has supported yet, and single flag SSL would been keep.