fix: enable legacy-renegotiation TLS compat on demand for ISY-994 (#508)

Author: shbatm

pyisy/connection.py

@@ -57,6 +57,13 @@
 
 EMPTY_XML_RESPONSE = '<?xml version="1.0" encoding="UTF-8"?>'
 
+# ``ssl.OP_LEGACY_SERVER_CONNECT`` was added to the stdlib ``ssl``
+# module in Python 3.12; CI still runs on 3.11. The underlying OpenSSL
+# flag ``SSL_OP_LEGACY_SERVER_CONNECT`` has had the stable value
+# ``0x4`` for years, so fall back to the literal when the attribute is
+# missing.
+OP_LEGACY_SERVER_CONNECT = getattr(ssl, "OP_LEGACY_SERVER_CONNECT", 0x4)
+
 
 class Connection:
     """Connection object to manage connection to and interaction with ISY."""
@@ -222,8 +229,28 @@ async def request(
             #     a modern OpenSSL distro with ``MinProtocol=TLSv1.2``).
             #   * ``verify_ssl=True`` against the controller's self-signed
             #     cert (``ClientConnectorCertificateError``).
-            # Always raise — retrying won't recover from a config
-            # mismatch, and callers (HA Core) need a definitive failure
+            #   * ISY-994 firmware (pre-RFC-5746) rejected by OpenSSL 3.x
+            #     with ``UNSAFE_LEGACY_RENEGOTIATION_DISABLED``. We
+            #     identify ISY-994 by the failure itself (the only peer
+            #     class that fails this way) and degrade the SSL context
+            #     once for the lifetime of the ``Connection`` — modern
+            #     peers (eisy/Polisy IoX, ISY-994 firmware that does
+            #     RFC 5746) stay strict.
+            if (
+                self.sslcontext is not None
+                and not (self.sslcontext.options & OP_LEGACY_SERVER_CONNECT)
+                and "UNSAFE_LEGACY_RENEGOTIATION_DISABLED" in str(err)
+            ):
+                _LOGGER.warning(
+                    "Enabling ISY-994 legacy-renegotiation TLS compatibility for "
+                    "this controller; eisy/Polisy IoX peers do not need this. "
+                    "Original error: %s",
+                    err,
+                )
+                self.sslcontext.options |= OP_LEGACY_SERVER_CONNECT
+                return await self.request(url, retries=retries, ok404=ok404, delay=delay, retry404=retry404)
+            # Always raise — retrying a real version/cert mismatch won't
+            # recover, and callers (HA Core) need a definitive failure
             # to translate into ``ConfigEntryNotReady`` rather than a
             # silent ``None`` that looks like a transient miss. The SSL
             # detail rides along in the exception chain.
@@ -434,6 +461,13 @@ def get_sslcontext(
     # Allow older ciphers for original ISY-994 hardware (TLS 1.1/1.2 only;
     # set_ciphers does not affect TLS 1.3 ciphersuites).
     context.set_ciphers("DEFAULT:!aNULL:!eNULL:!MD5:!3DES:!DES:!RC4:!IDEA:!SEED:!aDSS:!SRP:!PSK")
+    # Note: ``OP_LEGACY_SERVER_CONNECT`` (ISY-994 RFC-5746 compat) is
+    # NOT set here. ``Connection.request()`` enables it on demand the
+    # first time the peer rejects the handshake with
+    # ``UNSAFE_LEGACY_RENEGOTIATION_DISABLED`` — that way modern peers
+    # (eisy/Polisy IoX, ISY-994 firmware that honors RFC 5746) keep
+    # strict TLS, and only the controllers that actually need it
+    # degrade.
     return context
 
 

tests/test_connection_extras.py

@@ -15,6 +15,7 @@
 
 from pyisy.connection import (
     EMPTY_XML_RESPONSE,
+    OP_LEGACY_SERVER_CONNECT,
     Connection,
     get_sslcontext,
 )
@@ -62,6 +63,17 @@ def test_get_sslcontext_auto_pins_min_v12_no_max() -> None:
     assert ctx.check_hostname is False
 
 
+def test_get_sslcontext_does_not_preset_legacy_renegotiation() -> None:
+    """``OP_LEGACY_SERVER_CONNECT`` (the ISY-994 RFC-5746 compat flag)
+    must NOT be set by default — modern peers (eisy/Polisy IoX, ISY-994
+    firmware that honors RFC 5746) keep strict TLS. The flag is enabled
+    on demand by ``Connection.request()`` only when the peer rejects
+    the handshake with ``UNSAFE_LEGACY_RENEGOTIATION_DISABLED``."""
+    ctx = get_sslcontext(use_https=True)
+    assert ctx is not None
+    assert not (ctx.options & OP_LEGACY_SERVER_CONNECT)
+
+
 def test_get_sslcontext_verify_ssl_true_flips_cert_verification() -> None:
     """``verify_ssl=True`` is the opt-in for users who installed a
     properly-signed cert on their controller. It flips both
@@ -352,6 +364,79 @@ async def test_request_ssl_error_raises_on_test_connection_path(
             await conn.request(url, retries=None)
 
 
+async def test_request_legacy_reneg_failure_enables_compat_and_retries() -> None:
+    """First handshake fails with ``UNSAFE_LEGACY_RENEGOTIATION_DISABLED``
+    (signature of ISY-994's pre-RFC-5746 TLS stack against modern OpenSSL).
+    ``request()`` must:
+
+    1. Flip ``OP_LEGACY_SERVER_CONNECT`` on the existing SSL context
+       (modern peers — eisy/Polisy IoX, ISY-994 firmware that honors
+       RFC 5746 — never reach this branch and stay strict).
+    2. Log a one-time WARNING explaining the degradation.
+    3. Retry the request and surface the second response normally.
+    """
+    from unittest.mock import MagicMock
+
+    https_conn = Connection(address="h", port=443, username="u", password="p", use_https=True)
+    try:
+        # Sanity: flag is OFF on a fresh connection (verifies the
+        # context-builder default; the request-path retry is what
+        # actually flips it).
+        assert https_conn.sslcontext is not None
+        assert not (https_conn.sslcontext.options & OP_LEGACY_SERVER_CONNECT)
+
+        url = https_conn.compile_url(["config"])
+        reneg_err = aiohttp.ClientConnectorSSLError(
+            MagicMock(),
+            ssl.SSLError(
+                1,
+                "[SSL: UNSAFE_LEGACY_RENEGOTIATION_DISABLED] unsafe legacy renegotiation disabled",
+            ),
+        )
+        with aioresponses() as mocked:
+            # First call: handshake refusal. Second call (after the
+            # retry flips the flag): success.
+            mocked.get(url, exception=reneg_err)
+            mocked.get(url, status=200, body="<configuration/>")
+            result = await https_conn.request(url)
+
+        assert result == "<configuration/>"
+        assert https_conn.sslcontext.options & OP_LEGACY_SERVER_CONNECT
+    finally:
+        await https_conn.close()
+
+
+async def test_request_legacy_reneg_does_not_trigger_for_unrelated_ssl_errors() -> None:
+    """Only the specific ``UNSAFE_LEGACY_RENEGOTIATION_DISABLED``
+    failure flips the flag. A generic protocol error
+    (``UNSUPPORTED_PROTOCOL``, cert verify failure, etc.) must surface
+    as ``ISYConnectionError`` without weakening the SSL context — those
+    are real config mismatches the user needs to fix, not ISY-994
+    legacy compat."""
+    from unittest.mock import MagicMock
+
+    from pyisy.exceptions import ISYConnectionError
+
+    https_conn = Connection(address="h", port=443, username="u", password="p", use_https=True)
+    try:
+        url = https_conn.compile_url(["config"])
+        proto_err = aiohttp.ClientConnectorSSLError(
+            MagicMock(),
+            ssl.SSLError(1, "[SSL: UNSUPPORTED_PROTOCOL] unsupported protocol"),
+        )
+        with aioresponses() as mocked:
+            mocked.get(url, exception=proto_err)
+            with pytest.raises(ISYConnectionError, match="SSL/TLS error"):
+                await https_conn.request(url)
+
+        # Flag must remain OFF — the user's security posture isn't
+        # silently weakened on every SSL failure.
+        assert https_conn.sslcontext is not None
+        assert not (https_conn.sslcontext.options & OP_LEGACY_SERVER_CONNECT)
+    finally:
+        await https_conn.close()
+
+
 async def test_request_non_rest_url_does_not_crash(conn: Connection) -> None:
     """Regression for #488: ``request()`` derives its debug-log endpoint
     from the URL by splitting on ``"rest"``. ``get_description()`` builds