Skip to content

Commit 8b27b76

Browse files
committed
set system.clients empty columns
Depends on scylladb/seastar#2651 Missing columns have been present since probably forever - they were added to the schema but never assigned any value: ``` cqlsh> select * from system.clients; ------------------+------------------------ ... hostname | null ... ssl_cipher_suite | null ssl_enabled | null ssl_protocol | null ... ``` This patch sets values of these columns: - `hostname` is always filled in, - with TLS connection, remaining 3 TSL-related fields are filled in, - without TLS, `ssl_enabled` is set to `false` and other columns are `null`. Fixes: scylladb#9216
1 parent aa8c27b commit 8b27b76

File tree

4 files changed

+72
-7
lines changed

4 files changed

+72
-7
lines changed

generic_server.hh

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public:
4343
execute_under_tenant_type _execute_under_current_tenant = no_tenant();
4444
protected:
4545
server& _server;
46-
connected_socket _fd;
46+
mutable connected_socket _fd;
4747
input_stream<char> _read_buf;
4848
output_stream<char> _write_buf;
4949
future<> _ready_to_respond = make_ready_future<>();

test/cqlpy/test_ssl.py

+41-6
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,22 @@
1212

1313
import ssl
1414
import cassandra.cluster
15+
import re
16+
17+
18+
# This function normalizes the SSL cipher suite name, which we need because:
19+
# - python's library, which tests' driver uses,
20+
# - C's library, which scylla server uses,
21+
# namings conventions are different, and we cannot simply compare them for equality.
22+
def normalize_cipher(cipher_name: str) -> str:
23+
if cipher_name.startswith("TLS_"):
24+
cipher_name = cipher_name[len("TLS_"):] # Remove leading "TLS_" if present.
25+
cipher_name = cipher_name.replace("_WITH_", "-")
26+
cipher_name = cipher_name.replace("_", "-")
27+
# Remove hyphen between letters and digits: e.g. convert "AES-256" to "AES256"
28+
cipher_name = re.sub(r'([A-Z]+)-(\d+)', r'\1\2', cipher_name)
29+
return cipher_name
30+
1531

1632
# Test that TLS 1.2 is supported (because this is what "cqlsh --ssl" uses
1733
# by default), and that other TLS version are either supported - or if
@@ -27,8 +43,8 @@ def test_tls_versions(cql):
2743
# TLS v1.2 must be supported, because this is the default version that
2844
# "cqlsh --ssl" uses. If this fact changes in the future, we may need
2945
# to reconsider this test.
30-
try_connect(cql.cluster, ssl.TLSVersion.TLSv1_2)
31-
print(f"{ssl.TLSVersion.TLSv1_2} supported")
46+
with try_connect(cql.cluster, ssl.TLSVersion.TLSv1_2):
47+
print(f"{ssl.TLSVersion.TLSv1_2} supported")
3248

3349
# All other protocol versions should either work (if Scylla is configured
3450
# to allow them) or fail with the expected error message.
@@ -37,15 +53,31 @@ def test_tls_versions(cql):
3753
ssl.TLSVersion.TLSv1,
3854
ssl.TLSVersion.SSLv3]:
3955
try:
40-
try_connect(cql.cluster, ssl_version)
41-
print(f"{ssl_version} supported")
56+
with try_connect(cql.cluster, ssl_version) as session:
57+
print(f"{ssl_version} supported")
58+
table_result = session.execute(f"SELECT * FROM system.clients")
59+
for row in table_result:
60+
assert row.ssl_enabled == True # ' == True` is redundant, but makes it clear
61+
assert row.hostname == '127.0.0.1' # is that always the case?
62+
# Even if we request to connect with a specific SSL version,
63+
# the connection may end up negotiating a different version,
64+
# thus we can't assert for a specific match.
65+
assert row.ssl_protocol in ["TLS1", "TLS1.0", "TLS1.1", "TLS1.2", "TLS1.3", "SSL3"]
66+
# The session object returned by the Cassandra driver's connect() method
67+
# does not expose the low‑level TLS session details, such as the negotiated cipher suite,
68+
# so we need to, again, assert that it matches one of the expected values.
69+
expected_ciphers = [normalize_cipher(cipher['name']) for cipher in ssl.create_default_context().get_ciphers()]
70+
actual_cipher = normalize_cipher(row.ssl_cipher_suite)
71+
assert actual_cipher in expected_ciphers
4272
except cassandra.cluster.NoHostAvailable as e:
4373
# For the various TLS versions, we get the new TLS alert
4474
# "protocol version". But for SSL, we get the older
4575
# "no protocols available" error.
4676
assert 'protocol version' in str(e) or 'no protocols available' in str(e)
4777
print(f"{ssl_version} not supported")
4878

79+
80+
@contextmanager
4981
def try_connect(orig_cluster, ssl_version):
5082
ssl_context=ssl.SSLContext(ssl.PROTOCOL_TLS)
5183
ssl_context.minimum_version = ssl_version
@@ -63,8 +95,11 @@ def try_connect(orig_cluster, ssl_version):
6395
# so let's increase them to 60 seconds. See issue #11289.
6496
connect_timeout = 60,
6597
control_connection_timeout = 60)
66-
cluster.connect()
67-
cluster.shutdown()
98+
try:
99+
session = cluster.connect()
100+
yield session
101+
finally:
102+
cluster.shutdown()
68103

69104
# Test that if we try to connect to an SSL port with *unencrypted* CQL,
70105
# it doesn't work.

test/cqlpy/test_system.py

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Copyright 2025-present ScyllaDB
2+
#
3+
# SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0
4+
5+
#############################################################################
6+
# Tests asserting correct values are kept in system keyspaces
7+
#############################################################################
8+
9+
import pytest
10+
11+
# By default, tests run CQL over unencrypted sockets,
12+
# so no extra SSL config is required in this testcase.
13+
# The opposite testcase is test_ssl.py::test_tls_versions
14+
def test_system_clients_keeps_correct_tls_entries_when_tls_disabled(cql):
15+
table_result = cql.execute("SELECT * FROM system.clients")
16+
for row in table_result:
17+
assert row.hostname == '127.0.0.1' # is that always the case?
18+
assert row.ssl_enabled == False
19+
assert row.ssl_protocol is None
20+
assert row.ssl_cipher_suite is None

transport/server.cc

+10
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,16 @@ client_data cql_server::connection::make_client_data() const {
650650
cd.connection_stage = client_connection_stage::authenticating;
651651
}
652652
cd.scheduling_group_name = _current_scheduling_group.name();
653+
std::stringstream ss;
654+
ss << _client_state.get_remote_address().addr().as_ipv4_address();
655+
cd.hostname = ss.str();
656+
try {
657+
cd.ssl_enabled = tls::is_operational(_fd);
658+
cd.ssl_protocol = tls::get_protocol_version(_fd);
659+
cd.ssl_cipher_suite = tls::get_cipher_suite(_fd);
660+
} catch (const std::invalid_argument&) {
661+
cd.ssl_enabled = false;
662+
}
653663
return cd;
654664
}
655665

0 commit comments

Comments
 (0)