Skip to content

Regression: port parameter dropped from enableSecureConnection() call in ConnectionParameters constructor #101637

@clickgapai

Description

@clickgapai

Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment.

Retrospective finding from a historical scan of PR #74212 (merged 2025-01-06). Confirmed on current codebase — close with a note if already fixed.

Describe what's wrong

clickhouse-client --port 9440 does NOT auto-enable TLS. The client sends plaintext native protocol instead of TLS ClientHello, causing connection failures or data leaks when connecting to a TLS-only port.

Root cause: ConnectionParameters.cpp:70: commit dce8775 ('SSH Authentication') refactored the constructor, renaming parameters and adding StrongTypedef wrappers, but dropped the third argument (port_) from the enableSecureConnection() call. Changed from enableSecureConnection(config, connection_host, connection_port) to enableSecureConnection(config, host_).

Affected locations:

  • src/Client/ConnectionParameters.cpp:70 — enableSecureConnection(config, host_) missing port_ argument

Impact: Users connecting with --port 9440 without --secure get plaintext connections instead of TLS. This defeats the entire purpose of PR #74212's auto-detection feature. Connection to TLS-only servers fails with unhelpful errors, and connections to servers accepting both protocols would transmit credentials in plaintext.

Does it reproduce on most recent release?

Yes — confirmed on current master (commit a56315c84236).

How to reproduce

Run a TCP listener on port 9440, then connect clickhouse client with --port 9440 (no --secure). Inspect first bytes: 0x16 0x03 = TLS (correct), 0x00 0x11 ClickHouse = native protocol (bug).

Expected behavior

Client should send TLS ClientHello (starting with 0x16 0x03) when connecting to port 9440 without explicit --secure flag

Error message and/or stacktrace

Client sends native protocol handshake (bytes: 0011436c69636b486f75736520636c69656e741a) to port 9440 instead of TLS ClientHello (0x16 0x03)

Additional context

Suggested fix: Change line 70 from enableSecureConnection(config, host_) to enableSecureConnection(config, host_, port_)

Analysis details: Confidence HIGH | Severity P1 | Testability: INTEGRATION_TEST

Found during automated review of PR #74212.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions