rusackas commented on code in PR #45576:
URL: https://github.com/apache/superset/pull/45576#discussion_r4369388502
##########
tests/testcontainers/db_engine_specs/test_mariadb.py:
##########
@@ -115,3 +143,188 @@ 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.
+
+ `--skip-ssl` turns off TLS support entirely on this image (`have_ssl`
+ reports `DISABLED`), verified directly against this image before
+ writing this fixture.
+ """
+ with MySqlContainer("mariadb:11", command="--skip-ssl") 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 `MariaDBEngineSpec.adjust_engine_params` (inherited from
+ `MySQLEngineSpec`, but exercised here as the actual class Superset
+ dispatches to for a `mariadb` connection) rather than calling
+ `require_mysql_tls` directly, so the test exercises the exact call path
+ production code takes for this engine.
+ """
+ uri = make_url(f"mysql://root:test@placeholder:3306/test?{uri_query}")
+ return MariaDBEngineSpec.adjust_engine_params(uri, {})
+
+
+def test_require_mysql_tls_fails_closed_without_server_tls(
+ no_tls_container: MySqlContainer,
+) -> None:
+ """
+ Same guarantee as test_mysql.py's version of this test
(apache/superset#44723):
+ a server offering no TLS at all must make the connection fail, never
+ succeed silently in cleartext. 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.
+ """
+ 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)
+ # Match the TLS refusal itself: an unrelated auth failure (e.g. 1045 or
+ # 2061) also raises OperationalError and would pass with the fix reverted.
+ with pytest.raises(OperationalError, match="SSL is required"):
+ with engine.connect():
+ pass
+
+
+def _write_self_signed_cert(cert_dir: Path) -> None:
+ """Write a throwaway CA + server cert/key into `cert_dir`.
+
+ Unlike the `mysql:8.0` image, `mariadb`'s image does not auto-generate a
+ CA/cert pair when TLS is enabled: `have_ssl` reports `YES` out of the
+ box, but `ssl_ca`/`ssl_cert`/`ssl_key` all report empty and no cert
+ files exist anywhere under `/var/lib/mysql` (verified directly against
+ this image) -- so there's nothing to extract back out the way
+ test_mysql.py's `tls_ca_path` fixture does for `mysql:8.0`. Generate a
+ throwaway one instead and mount it into a dedicated container via
+ `--ssl-cert`/`--ssl-key`/`--ssl-ca`, verified directly to work against
+ this image.
+ """
+ now = datetime.now(timezone.utc)
+
+ ca_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
+ ca_name = x509.Name(
+ [x509.NameAttribute(NameOID.COMMON_NAME, "superset-testcontainers-ca")]
+ )
+ ca_cert = (
+ x509.CertificateBuilder()
+ .subject_name(ca_name)
+ .issuer_name(ca_name)
+ .public_key(ca_key.public_key())
+ .serial_number(x509.random_serial_number())
+ .not_valid_before(now - timedelta(minutes=5))
+ .not_valid_after(now + timedelta(hours=1))
+ .add_extension(x509.BasicConstraints(ca=True, path_length=None),
critical=True)
+ .sign(ca_key, hashes.SHA256())
+ )
+
+ server_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
+ server_name = x509.Name(
+ [x509.NameAttribute(NameOID.COMMON_NAME, "mariadb-testcontainer")]
+ )
+ server_cert = (
+ x509.CertificateBuilder()
+ .subject_name(server_name)
+ .issuer_name(ca_name)
+ .public_key(server_key.public_key())
+ .serial_number(x509.random_serial_number())
+ .not_valid_before(now - timedelta(minutes=5))
+ .not_valid_after(now + timedelta(hours=1))
+ .sign(ca_key, hashes.SHA256())
+ )
+
+ (cert_dir /
"ca.pem").write_bytes(ca_cert.public_bytes(serialization.Encoding.PEM))
+ (cert_dir / "server-cert.pem").write_bytes(
+ server_cert.public_bytes(serialization.Encoding.PEM)
+ )
+ (cert_dir / "server-key.pem").write_bytes(
+ server_key.private_bytes(
+ encoding=serialization.Encoding.PEM,
+ format=serialization.PrivateFormat.TraditionalOpenSSL,
+ encryption_algorithm=serialization.NoEncryption(),
+ )
+ )
+ # Readable by the container's mysql user once bind-mounted read-only.
+ for name in ("ca.pem", "server-cert.pem", "server-key.pem"):
+ os.chmod(cert_dir / name, 0o644)
+
+
[email protected](scope="module")
+def tls_container() -> Iterator[tuple[MySqlContainer, str]]:
+ """A MariaDB server with a throwaway, explicitly-configured TLS cert.
+
+ Needs its own container rather than reusing the shared `engine`
+ container the way test_mysql.py's `tls_ca_path` fixture does: the cert
+ has to be mounted in and pointed to via command-line flags at container
+ start time, and there's no running-container equivalent of "extract the
+ cert the image already auto-generated" the way there is for `mysql:8.0`.
+ """
+ with tempfile.TemporaryDirectory() as cert_dir_str:
+ cert_dir = Path(cert_dir_str)
+ _write_self_signed_cert(cert_dir)
+ # TemporaryDirectory is 0700; the container's mysql user must be able
+ # to traverse the bind-mounted directory on Linux.
+ cert_dir.chmod(0o755)
+ container = MySqlContainer(
+ "mariadb:11",
+ command=(
+ "--ssl-cert=/certs/server-cert.pem "
+ "--ssl-key=/certs/server-key.pem "
+ "--ssl-ca=/certs/ca.pem"
+ ),
+ ).with_volume_mapping(cert_dir_str, "/certs", "ro")
+ with container as started:
+ yield started, str(cert_dir / "ca.pem")
+
+
+def test_require_mysql_tls_rejects_untrusted_ca(
+ tls_container: tuple[MySqlContainer, str], tmp_path: Path
+) -> None:
+ """
+ Guards the verification itself: a CA that did not sign the server's cert
+ must be rejected. `Ssl_cipher` alone only shows the session is encrypted,
+ so this fails if `ssl_ca` is dropped or ignored. `VERIFY_CA` is requested
+ explicitly so the outcome does not depend on which client library
+ mysqlclient links (plain `REQUIRED` only demands encryption on Oracle's).
+ """
+ container, _ = tls_container
+ _write_self_signed_cert(tmp_path)
+ wrong_ca = tmp_path / "ca.pem"
+ host = _tcp_host(container)
+ port = container.get_exposed_port(container.port)
+ uri, connect_args = _require_tls_connect_args(
+ f"ssl=1&ssl_mode=VERIFY_CA&ssl_ca={wrong_ca}"
+ )
+ uri = uri.set(host=host, port=int(port), database=container.dbname)
+ tls_engine = create_engine(uri, connect_args=connect_args)
+ with pytest.raises(OperationalError):
Review Comment:
Good catch @sadpandajoe. The untrusted-CA test now matches the verification
failure (`verif|self.signed|issuer|untrusted`), and the verified-TLS test
requests `ssl_mode=VERIFY_CA`, so dropping `ssl_ca` fails it. I could not run
the Docker tests locally, so the exact error text is unconfirmed until CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
