[Enhancement] Enforce SSL connection from the client when SSL is enabled on the server #56176

rstyp · 2025-02-21T19:21:32Z

Why I'm doing:

Even when SSL is enabled on server, JDBC clients can still connect without SSL by default, as the server allows connections with or without SSL.

What I'm doing:

Introducing a new boolean config flag to enforce secure connections from clients. When flag is set to true, If clients are connecting without SSL, server refuses client connections

Fixes #56175

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
This is a backport pr

Bugfix cherry-pick branch check:

CLAassistant · 2025-02-21T19:21:39Z

All committers have signed the CLA.

rstyp · 2025-02-21T19:22:49Z

fe/fe-core/src/main/java/com/starrocks/common/Config.java

+     * Allow only encrypted connections from clients
+     **/
+    @ConfField
+    public static boolean require_secure_transport = false;


Similar config as in MySql server

stevenzwu · 2025-02-21T19:36:56Z

fe/fe-core/src/main/java/com/starrocks/common/ErrorCode.java

@@ -106,6 +106,7 @@ public enum ErrorCode {
    ERR_NO_SUCH_QUERY(1365, new byte[] {'4', '2', '0', '0', '0'}, "Unknown query id: %s"),

    ERR_CANNOT_USER(1396, new byte[] {'H', 'Y', '0', '0', '0'}, "Operation %s failed for %s"),
+    ERR_SECURE_TRANSPORT_REQUIRED(1403, new byte[] {'0', '8', '0', '0', '4'}, "Server rejected the insecure connection"),


how did we choose the error code 1403?

From the comment in this class, it seems that we should find the corresponding error code for MySQL server/protocol.

// Try our best to compatible with MySQL's

With that, should this be the error code?

Error number: 3159; Symbol: [ER_SECURE_TRANSPORT_REQUIRED](https://dev.mysql.com/doc/mysql-errors/5.7/en/server-error-reference.html#error_er_secure_transport_required); SQLSTATE: HY000 Message: Connections using insecure transport are prohibited while --require_secure_transport=ON. With the [require_secure_transport](https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_require_secure_transport) system variable, clients can connect only using secure transports. Qualifying connections are those using SSL, a Unix socket file, or shared memory.

Thanks, @stevenzwu, for checking. I’ll revisit the error code.

stevenzwu · 2025-02-21T19:41:06Z

fe/fe-core/src/main/java/com/starrocks/mysql/NegotiateState.java

@@ -20,6 +20,7 @@ public enum NegotiateState {
    READ_AUTH_SWITCH_PKG_FAILED("read auth switch package failed"),
    ENABLE_SSL_FAILED("enable ssl failed"),
    READ_SSL_AUTH_PKG_FAILED("read ssl auth package failed"),
+    SERVER_REJECTED_INSECURE_CONNECTION("server rejected the insecure connection"),


nit: maybe remove server for consistency with other state enum msg, as the state is used by server. so it is implicitly assumed

INSECURE_CONNECTION_REJECTED(" insecure connection rejected"),

stevenzwu · 2025-02-21T19:41:36Z

fe/fe-core/src/main/java/com/starrocks/mysql/MysqlProto.java

@@ -139,6 +139,13 @@ public static NegotiateResult negotiate(ConnectContext context) throws IOExcepti
            return new NegotiateResult(null, NegotiateState.READ_FIRST_AUTH_PKG_FAILED);
        }

+        if (Config.require_secure_transport && !authPacket.isSSLConnRequest()) {
+            LOG.debug("server refused insecure client connection");


this should be warn

…server

stevenzwu · 2025-02-21T22:08:25Z

fe/fe-core/src/main/java/com/starrocks/mysql/MysqlProto.java

@@ -139,6 +139,13 @@ public static NegotiateResult negotiate(ConnectContext context) throws IOExcepti
            return new NegotiateResult(null, NegotiateState.READ_FIRST_AUTH_PKG_FAILED);
        }

+        if (Config.require_secure_transport && !authPacket.isSSLConnRequest()) {
+            LOG.warn("Copy string literal text to the clipboard");


error msg should be updated

stevenzwu

left one nit comment for log msg. otherwise, LGTM

sonarqubecloud · 2025-02-22T11:01:59Z

Quality Gate failed

Failed conditions
D Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

github-actions · 2025-02-22T12:21:57Z

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2025-02-22T12:22:31Z

[FE Incremental Coverage Report]

❌ fail : 4 / 8 (50.00%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	com/starrocks/mysql/MysqlProto.java	1	5	20.00%	[143, 144, 145, 146]
🔵	com/starrocks/common/Config.java	1	1	100.00%	[]
🔵	com/starrocks/mysql/NegotiateState.java	1	1	100.00%	[]
🔵	com/starrocks/common/ErrorCode.java	1	1	100.00%	[]

github-actions · 2025-02-22T12:29:32Z

[BE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

rstyp requested review from a team as code owners February 21, 2025 19:21

github-actions bot added the title needs [type] label Feb 21, 2025

mergify bot assigned rstyp Feb 21, 2025

rstyp commented Feb 21, 2025

View reviewed changes

stevenzwu reviewed Feb 21, 2025

View reviewed changes

rstyp changed the title ~~Enforce SSL connection from the client when SSL is enabled on the server~~ [Enhancement] Enforce SSL connection from the client when SSL is enabled on the server Feb 21, 2025

github-actions bot removed the title needs [type] label Feb 21, 2025

Enforce an SSL connection from the client when SSL is enabled on the …

38b429c

…server

rstyp force-pushed the enforce-ssl-oss branch from 62d1017 to 5a5a3c1 Compare February 21, 2025 21:38

stevenzwu reviewed Feb 21, 2025

View reviewed changes

stevenzwu approved these changes Feb 21, 2025

View reviewed changes

Update error code

23c2d98

rstyp force-pushed the enforce-ssl-oss branch from 5a5a3c1 to 23c2d98 Compare February 21, 2025 22:19

kevincai approved these changes Feb 22, 2025

View reviewed changes

gengjun-git self-assigned this Feb 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] Enforce SSL connection from the client when SSL is enabled on the server #56176

[Enhancement] Enforce SSL connection from the client when SSL is enabled on the server #56176

rstyp commented Feb 21, 2025 •

edited by kevincai

Loading

CLAassistant commented Feb 21, 2025 •

edited

Loading

rstyp Feb 21, 2025

stevenzwu Feb 21, 2025 •

edited

Loading

rstyp Feb 21, 2025

stevenzwu Feb 21, 2025 •

edited

Loading

stevenzwu Feb 21, 2025

stevenzwu Feb 21, 2025

stevenzwu left a comment

sonarqubecloud bot commented Feb 22, 2025

github-actions bot commented Feb 22, 2025

github-actions bot commented Feb 22, 2025

github-actions bot commented Feb 22, 2025

[Enhancement] Enforce SSL connection from the client when SSL is enabled on the server #56176

Are you sure you want to change the base?

[Enhancement] Enforce SSL connection from the client when SSL is enabled on the server #56176

Conversation

rstyp commented Feb 21, 2025 • edited by kevincai Loading

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

CLAassistant commented Feb 21, 2025 • edited Loading

rstyp Feb 21, 2025

Choose a reason for hiding this comment

stevenzwu Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

rstyp Feb 21, 2025

Choose a reason for hiding this comment

stevenzwu Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

stevenzwu Feb 21, 2025

Choose a reason for hiding this comment

stevenzwu Feb 21, 2025

Choose a reason for hiding this comment

stevenzwu left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 22, 2025

Quality Gate failed

github-actions bot commented Feb 22, 2025

[Java-Extensions Incremental Coverage Report]

github-actions bot commented Feb 22, 2025

[FE Incremental Coverage Report]

file detail

github-actions bot commented Feb 22, 2025

[BE Incremental Coverage Report]

rstyp commented Feb 21, 2025 •

edited by kevincai

Loading

CLAassistant commented Feb 21, 2025 •

edited

Loading

stevenzwu Feb 21, 2025 •

edited

Loading

stevenzwu Feb 21, 2025 •

edited

Loading