diff --git a/pyisy/connection.py b/pyisy/connection.py index 9bfacda..9fd33b8 100644 --- a/pyisy/connection.py +++ b/pyisy/connection.py @@ -57,6 +57,13 @@ EMPTY_XML_RESPONSE = '' +# ``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 diff --git a/tests/test_connection_extras.py b/tests/test_connection_extras.py index 9304f93..517bf49 100644 --- a/tests/test_connection_extras.py +++ b/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="") + result = await https_conn.request(url) + + assert result == "" + 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