Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions pyisy/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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


Expand Down
85 changes: 85 additions & 0 deletions tests/test_connection_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from pyisy.connection import (
EMPTY_XML_RESPONSE,
OP_LEGACY_SERVER_CONNECT,
Connection,
get_sslcontext,
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading