This is an automated email from the ASF dual-hosted git repository. rusackas pushed a commit to branch test/mysql-tls-testcontainers in repository https://gitbox.apache.org/repos/asf/superset.git
commit df67f9447a5c44d34842096819794fd886956547 Author: Evan Rusackas <[email protected]> AuthorDate: Fri Oct 2 14:44:42 2026 -0700 test(mysql): cover require_mysql_tls fail-closed and verified-TLS paths apache/superset#44723 fixed a silent TLS downgrade: mysqlclient maps ssl_mode=REQUIRED to opportunistic TLS when linked against MariaDB Connector/C (the library CI's apt-installed mysqlclient actually links against), so a server offering no TLS at all is silently accepted in cleartext instead of rejected. A mocked cursor can't observe this -- the behavior lives in the C client library's own TLS negotiation at real connection time against a real server's real TLS posture. Two containers cover the two-sided property: a --ssl=0 server must make the connection fail rather than silently downgrade, while the default TLS-capable server must still let a legitimate encrypted connection through (asserted via a non-empty Ssl_cipher, not just a successful handshake) -- a fix that merely refused every connection would pass the first check alone. Co-Authored-By: Evan Rusackas <[email protected]> Co-Authored-By: Claude Sonnet 5 <[email protected]> --- tests/testcontainers/db_engine_specs/test_mysql.py | 159 ++++++++++++++++++--- 1 file changed, 141 insertions(+), 18 deletions(-) diff --git a/tests/testcontainers/db_engine_specs/test_mysql.py b/tests/testcontainers/db_engine_specs/test_mysql.py index cc80c0e493e..ebdbde7721b 100644 --- a/tests/testcontainers/db_engine_specs/test_mysql.py +++ b/tests/testcontainers/db_engine_specs/test_mysql.py @@ -26,9 +26,27 @@ Could not be verified locally in this environment: mysqlclient (MySQLdb) has a pre-existing, unrelated native-library linking issue against this machine's Homebrew-installed libmysqlclient. CI installs it via apt on Linux, where this does not occur. + +Also covers `require_mysql_tls` (apache/superset#44723): mysqlclient maps +`ssl_mode=REQUIRED` to *opportunistic* TLS when linked against MariaDB +Connector/C (the library CI's apt-installed mysqlclient actually links +against), meaning a server that offers no TLS at all is silently accepted +in cleartext rather than rejected -- the exact failure mode the fix exists +to close by substituting `VERIFY_CA` for that client library. A mocked +cursor can't observe this: the behavior lives in the C client library's +own TLS negotiation, decided once, at real connection time, against a real +server's real TLS posture. Two containers are used because the property +under test is two-sided: a `--ssl=0` server must make the connection fail +rather than silently downgrade (`test_require_mysql_tls_fails_closed...`), +while the default, TLS-capable server must still let a legitimate +encrypted connection through (`test_require_mysql_tls_connects_with...`) +-- a fix that merely refused every connection would pass the first check +and hide behind it. """ +import tempfile from collections.abc import Iterator +from typing import Any import pytest from sqlalchemy import ( @@ -38,8 +56,11 @@ from sqlalchemy import ( Integer, MetaData, Table as SATable, + text, ) from sqlalchemy.engine import Engine +from sqlalchemy.engine.url import make_url, URL +from sqlalchemy.exc import OperationalError from superset.db_engine_specs.mysql import MySQLEngineSpec from superset.sql.parse import Table @@ -58,26 +79,37 @@ from ._pagination import ( # noqa: E402 ) +def _tcp_host(container: MySqlContainer) -> str: + """Resolve a host MySQLdb will actually reach over TCP. + + get_connection_url() has no host override and defaults to + get_container_host_ip(), which is the literal string "localhost" on + native Linux Docker (e.g. GitHub Actions runners). MySQLdb (mysqlclient) + treats a "localhost" host specially and attempts a Unix socket + connection instead of TCP, which fails since there's no local MySQL + socket -- the container is reached over the network. Only rewrite that + specific local case to 127.0.0.1; a remote Docker daemon reports its own + real host/IP here, which must be preserved so the suite can still reach + it. + """ + host = container.get_container_host_ip() + return "127.0.0.1" if host == "localhost" else host + + @pytest.fixture(scope="module") -def engine() -> Iterator[Engine]: +def mysql_container() -> Iterator[MySqlContainer]: with MySqlContainer("mysql:8.0") as container: - # get_connection_url() has no host override and defaults to - # get_container_host_ip(), which is the literal string "localhost" - # on native Linux Docker (e.g. GitHub Actions runners). MySQLdb - # (mysqlclient) treats a "localhost" host specially and attempts a - # Unix socket connection instead of TCP, which fails since there's - # no local MySQL socket -- the container is reached over the - # network. Only rewrite that specific local case to 127.0.0.1; a - # remote Docker daemon reports its own real host/IP here, which - # must be preserved so the suite can still reach it. - host = container.get_container_host_ip() - if host == "localhost": - host = "127.0.0.1" - port = container.get_exposed_port(container.port) - yield create_engine( - f"mysql://{container.username}:{container.password}" - f"@{host}:{port}/{container.dbname}" - ) + yield container + + [email protected](scope="module") +def engine(mysql_container: MySqlContainer) -> Engine: + host = _tcp_host(mysql_container) + port = mysql_container.get_exposed_port(mysql_container.port) + return create_engine( + f"mysql://{mysql_container.username}:{mysql_container.password}" + f"@{host}:{port}/{mysql_container.dbname}" + ) def test_paginated_query_returns_correct_rows_in_order(engine: Engine) -> None: @@ -115,3 +147,94 @@ def test_get_columns_maps_native_types(engine: Engine) -> None: assert spec is not None assert spec.generic_type == GenericDataType.NUMERIC assert isinstance(spec.sqla_type, Integer) + + [email protected](scope="module") +def no_tls_container() -> Iterator[MySqlContainer]: + """A server with TLS fully disabled, to test the fail-closed guarantee. + + `--ssl=0` is MySQL's deprecated-but-still-supported spelling for turning + off TLS support entirely (`have_ssl` reports `DISABLED`), verified + directly against this image before writing this fixture. + """ + with MySqlContainer("mysql:8.0", command="--ssl=0") as container: + yield container + + +def _require_tls_connect_args(uri_query: str = "ssl=1") -> tuple[URL, dict[str, Any]]: + """Run a `ssl=1` request through the real `adjust_engine_params` path. + + Goes through `MySQLEngineSpec.adjust_engine_params` rather than calling + `require_mysql_tls` directly, so the test exercises the exact call path + production code takes (including driver-name resolution from a bare + `mysql://` URL), not a shortcut around it. + """ + uri = make_url(f"mysql://root:test@placeholder:3306/test?{uri_query}") + return MySQLEngineSpec.adjust_engine_params(uri, {}) + + +def test_require_mysql_tls_fails_closed_without_server_tls( + no_tls_container: MySqlContainer, +) -> None: + """ + The core guarantee apache/superset#44723 exists to provide: a server + offering no TLS at all must make the connection fail, never succeed + silently in cleartext. mysqlclient linked against MariaDB Connector/C + maps `ssl_mode=REQUIRED` to *opportunistic* TLS -- it degrades to + cleartext instead of refusing when the server can't negotiate TLS -- + which is exactly the silent-downgrade this fix closes by requiring + `VERIFY_CA` instead for that client library. Only a real client library + actually attempting real TLS negotiation against a real non-TLS server + can show this; a mocked cursor never negotiates anything. + """ + host = _tcp_host(no_tls_container) + port = no_tls_container.get_exposed_port(no_tls_container.port) + uri, connect_args = _require_tls_connect_args() + uri = uri.set(host=host, port=int(port), database=no_tls_container.dbname) + engine = create_engine(uri, connect_args=connect_args) + with pytest.raises(OperationalError): + with engine.connect(): + pass + + [email protected](scope="module") +def tls_ca_path(mysql_container: MySqlContainer) -> Iterator[str]: + """Extract the shared `engine` container's auto-generated CA to a file. + + `ssl_ca` resolves (per `SHOW VARIABLES LIKE 'ssl_ca'`, verified directly + against this image) to `ca.pem` under the data directory. There's no + testcontainers API for pulling a single file back out of a running + container, so this shells out via the same `exec()` the rest of this + suite already depends on. Depending on `mysql_container` (not spinning + up a second one) reuses the exact server `engine`'s tests already talk + to, so the extracted CA is guaranteed to match. + """ + result = mysql_container.exec(["cat", "/var/lib/mysql/ca.pem"]) + assert result.exit_code == 0, "could not read the auto-generated CA cert" + with tempfile.NamedTemporaryFile(mode="wb", suffix=".pem", delete=False) as ca_file: + ca_file.write(result.output) + yield ca_file.name + + +def test_require_mysql_tls_connects_with_verified_tls( + engine: Engine, tls_ca_path: str +) -> None: + """ + The other side of the same guarantee as the fail-closed test above: a + fix that merely refused every connection would also pass that test + while breaking every legitimate encrypted connection. The default + `mysql:8.0` image auto-generates a CA/server cert pair with TLS + available out of the box (verified directly: `have_ssl` reports `YES`), + so a request that actually trusts that CA must still succeed, and must + be genuinely encrypted rather than merely accepted -- asserted here via + `Ssl_cipher`, which MySQL reports empty for a plaintext session. + """ + uri, connect_args = _require_tls_connect_args(f"ssl=1&ssl_ca={tls_ca_path}") + uri = uri.set( + host=engine.url.host, port=engine.url.port, database=engine.url.database + ) + tls_engine = create_engine(uri, connect_args=connect_args) + with tls_engine.connect() as conn: + cipher = conn.execute(text("SHOW STATUS LIKE 'Ssl_cipher'")).fetchone() + assert cipher is not None + assert cipher[1], "connection succeeded but is not actually using TLS"
