Enforce the pr, pw, ev and wr permissions

pyhap/accessory_driver.py

@@ -36,9 +36,17 @@
 
 from pyhap import util
 from pyhap.accessory import Accessory, Bridge, get_topic
-from pyhap.characteristic import Characteristic, CharacteristicError
+from pyhap.characteristic import (
+    PROP_PERMISSIONS,
+    Characteristic,
+    CharacteristicError,
+    HAPValue,
+)
 from pyhap.const import (
     HAP_PERMISSION_NOTIFY,
+    HAP_PERMISSION_READ,
+    HAP_PERMISSION_WRITE,
+    HAP_PERMISSION_WRITE_RESPONSE,
     HAP_PROTOCOL_SHORT_VERSION,
     HAP_REPR_ACCS,
     HAP_REPR_AID,
@@ -85,10 +93,21 @@
 MAX_PREPARED_WRITE_TTL_SECONDS = 30.0
 
 
-def _wrap_char_setter(char: Any, value: Any, client_addr: Any) -> tuple[int, Any]:
+def _stored_value(char: Characteristic, value: HAPValue) -> HAPValue:
+    """Return the value ``client_update_value`` stores for ``value``."""
+    try:
+        stored: HAPValue = char.to_valid_value(value)
+    except ValueError:
+        return value
+    return stored
+
+
+def _wrap_char_setter(
+    char: Any, value: Any, client_addr: Any, notify: bool = True
+) -> tuple[int, Any]:
     """Process an characteristic setter callback trapping and logging all exceptions."""
     try:
-        response = char.client_update_value(value, client_addr)
+        response = char.client_update_value(value, client_addr, notify=notify)
     except Exception:
         logger.exception(
             "%s: Error while setting characteristic %s to %s",
@@ -335,6 +354,10 @@ def __init__(
         network_tuple = (self._listen_address, self.state.port)
         self.http_server = HAPServer(network_tuple, self)
         self.prepared_writes = {}
+        # Write responses of readable characteristics, per client.
+        self._write_responses: dict[
+            tuple[str, int], dict[Characteristic, HAPValue]
+        ] = {}
 
     def listen_address(self) -> str | list[str]:
         """Return the address, or list of addresses, the HAP server listens on."""
@@ -581,6 +604,8 @@ def connection_lost(self, client: Any) -> None:
         for topic in client_topics:
             self.async_subscribe_client_topic(client, topic, subscribe=False)
         self.prepared_writes.pop(client, None)
+        # Deferred, so a write response that is already on its way is dropped too.
+        self.loop.call_soon(self._write_responses.pop, client, None)
 
     def publish(
         self,
@@ -905,12 +930,18 @@ def get_accessories(self, include_value: bool = True) -> dict[str, Any]:
         logger.debug("Get accessories response: %s", hap_rep)
         return {HAP_REPR_ACCS: hap_rep}
 
-    def get_characteristics(self, char_ids: Any) -> dict[str, Any]:
+    def get_characteristics(
+        self, char_ids: Any, client_addr: tuple[str, int] | None = None
+    ) -> dict[str, Any]:
         """Returns values for the required characteristics.
 
         :param char_ids: A list of characteristic "paths", e.g. "1.2" is aid 1, iid 2.
         :type char_ids: list<str>
 
+        :param client_addr: The client that reads. It reads back its own write
+            responses.
+        :type client_addr: tuple <str, int>
+
         :return: Status success for each required characteristic. For example:
 
         .. code-block:: python
@@ -926,6 +957,9 @@ def get_characteristics(self, char_ids: Any) -> dict[str, Any]:
         :rtype: dict
         """
         assert self.accessory is not None  # noqa: S101
+        write_responses = (
+            self._write_responses.get(client_addr, {}) if client_addr else {}
+        )
         chars = []
         for aid_iid in char_ids:
             aid, iid = (int(i) for i in aid_iid.split("."))
@@ -955,10 +989,20 @@ def get_characteristics(self, char_ids: Any) -> dict[str, Any]:
                         available = False
 
                 # A characteristic IID may resolve to a Service or be unknown;
-                # only a Characteristic can report a value.
-                if available and isinstance(char, Characteristic):
-                    rep[HAP_REPR_VALUE] = char.get_value()
-                    rep[HAP_REPR_STATUS] = HAP_SERVER_STATUS.SUCCESS
+                # only a Characteristic can report a value, and only when it
+                # has the paired read permission.
+                if isinstance(char, Characteristic):
+                    if HAP_PERMISSION_READ not in char.properties[PROP_PERMISSIONS]:
+                        rep[HAP_REPR_STATUS] = (
+                            HAP_SERVER_STATUS.WRITE_ONLY_CHARACTERISTIC
+                        )
+                    elif available:
+                        rep[HAP_REPR_VALUE] = (
+                            write_responses[char]
+                            if char in write_responses
+                            else char.get_value()
+                        )
+                        rep[HAP_REPR_STATUS] = HAP_SERVER_STATUS.SUCCESS
             except CharacteristicError:
                 logger.error(
                     "%s: Error getting value for characteristic %s.",
@@ -1014,11 +1058,9 @@ def set_characteristics(
 
         :type chars_query: dict
         """
-        # TODO: Add support for chars that do no support notifications.
-
         queries: list[dict[str, Any]] = chars_query[HAP_REPR_CHARS]
 
-        self._notify(queries, client_addr)
+        not_notifiable = self._notify(queries, client_addr)
 
         updates_by_accessories_services: dict[
             Accessory, dict[Service, dict[Characteristic, Any]]
@@ -1038,14 +1080,21 @@ def set_characteristics(
         assert primary_accessory is not None  # noqa: S101
         primary_aid = primary_accessory.aid
 
-        for query in queries:
-            if HAP_REPR_VALUE not in query and not expired:
-                continue
+        for aid, iid in not_notifiable:
+            results[aid][iid] = {
+                HAP_REPR_STATUS: HAP_SERVER_STATUS.NOTIFICATION_NOT_SUPPORTED
+            }
 
+        for query in queries:
             aid = query[HAP_REPR_AID]
             iid = query[HAP_REPR_IID]
+            if (aid, iid) in not_notifiable:
+                continue
+            if HAP_REPR_VALUE not in query and not expired:
+                # Listed with status 0 if the response has other results.
+                results[aid][iid] = {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS}
+                continue
             value = query.get(HAP_REPR_VALUE)
-            write_response_requested = query.get(HAP_REPR_WRITE_RESPONSE, False)
 
             if aid == primary_aid:
                 acc = primary_accessory
@@ -1062,16 +1111,55 @@ def set_characteristics(
             if char is None:
                 continue
 
+            if (
+                value is not None
+                and HAP_PERMISSION_WRITE not in char.properties[PROP_PERMISSIONS]
+            ):
+                results[aid][iid] = {
+                    HAP_REPR_STATUS: HAP_SERVER_STATUS.READ_ONLY_CHARACTERISTIC
+                }
+                continue
+
+            # Only characteristics with the write response permission answer
+            # a write with a value, and only when the controller asks for it.
+            write_response = (
+                query.get(HAP_REPR_WRITE_RESPONSE, False)
+                and HAP_PERMISSION_WRITE_RESPONSE in char.properties[PROP_PERMISSIONS]
+            )
+            # A new write replaces a former response of this connection. The
+            # request of a readable write response characteristic is not
+            # shared, whatever the setter does, also not with ev subscribers.
+            if value is not None:
+                self._write_responses.get(client_addr, {}).pop(char, None)
+            private_write = (
+                write_response
+                and value is not None
+                and HAP_PERMISSION_READ in char.properties[PROP_PERMISSIONS]
+            )
+            previous_value = char.value
+
             set_result = HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST
             set_result_value = None
 
             if value is not None:
                 set_result, set_result_value = _wrap_char_setter(
-                    char, value, client_addr
+                    char, value, client_addr, notify=not private_write
                 )
-
-            if set_result_value is not None and write_response_requested:
-                result = {HAP_REPR_STATUS: set_result, HAP_REPR_VALUE: set_result_value}
+            # A write with ``r`` to a readable characteristic is a request,
+            # e.g. a Setup Endpoints offer with the writer's SRTP keys. Only
+            # the writer reads the answer back, see _store_write_response.
+            # client_update_value stored the request as the value, so put
+            # back the former one unless the setter set a value of its own.
+            if private_write and char.value == _stored_value(char, value):
+                char.value = previous_value
+
+            if set_result_value is not None and write_response:
+                result = {
+                    HAP_REPR_STATUS: set_result,
+                    HAP_REPR_VALUE: self._store_write_response(
+                        char, set_result_value, client_addr
+                    ),
+                }
             else:
                 result = {HAP_REPR_STATUS: set_result}
 
@@ -1120,6 +1208,40 @@ def set_characteristics(
 
         return {HAP_REPR_CHARS: characteristics} if nonempty_results_exist else None
 
+    def _store_write_response(
+        self,
+        char: Characteristic,
+        response: HAPValue,
+        client_addr: tuple[str, int] | None,
+    ) -> HAPValue:
+        """Keep the write response and return what to send.
+
+        The written value of a write response characteristic, such as an SDP
+        offer, is a request. What the accessory answered is its state. If the
+        characteristic is readable, only the connection that wrote reads the
+        response back, until it closes, and it wins over the getter and a later
+        ``set_value``. Other connections read the characteristic's own value,
+        which the write leaves as it was. A write without ``r`` stays readable
+        for everyone, and a characteristic that also has ``ev`` notifies the
+        written value as any write does.
+        """
+        readable = HAP_PERMISSION_READ in char.properties[PROP_PERMISSIONS]
+        try:
+            valid: HAPValue = char.to_valid_value(response)
+        except ValueError:
+            logger.debug(
+                "%s: not keeping the write response, it is invalid: %s",
+                char.display_name,
+                response,
+            )
+            return response
+        if readable:
+            if client_addr is not None:
+                self._write_responses.setdefault(client_addr, {})[char] = valid
+        else:
+            char.value = valid
+        return valid
+
     def prepare(
         self, prepare_query: dict[str, Any], client_addr: tuple[str, int]
     ) -> dict[str, int]:
@@ -1177,16 +1299,32 @@ def signal_handler(self, _signal: Any, _frame: Any) -> None:
 
     def _notify(
         self, queries: list[dict[str, Any]], client_addr: tuple[str, int]
-    ) -> None:
-        """Notify the driver that the client has subscribed or unsubscribed."""
+    ) -> set[tuple[int, int]]:
+        """Notify the driver that the client has subscribed or unsubscribed.
+
+        Characteristics without the notify permission are left alone; their
+        ``(aid, iid)`` pairs are returned so the request can report
+        -70406 for them.
+        """
+        assert self.accessory is not None  # noqa: S101
+        not_notifiable: set[tuple[int, int]] = set()
         for query in queries:
             if HAP_PERMISSION_NOTIFY not in query:
                 continue
             aid = query[HAP_REPR_AID]
             iid = query[HAP_REPR_IID]
             ev = query[HAP_PERMISSION_NOTIFY]
 
+            char = self.accessory.get_characteristic(aid, iid)
+            if (
+                char is not None
+                and HAP_PERMISSION_NOTIFY not in char.properties[PROP_PERMISSIONS]
+            ):
+                not_notifiable.add((aid, iid))
+                continue
+
             char_topic = get_topic(aid, iid)
             action = "Subscribed" if ev else "Unsubscribed"
             logger.debug("%s client %s to topic %s", action, client_addr, char_topic)
             self.async_subscribe_client_topic(client_addr, char_topic, ev)
+        return not_notifiable

pyhap/characteristic.py

@@ -47,6 +47,9 @@ def _setter_wants_client_addr(setter: Callable[..., Any]) -> bool:
 
 
 # ### HAP Format ###
+# A characteristic value as HAP JSON carries it.
+HAPValue = bool | int | float | str | None
+
 HAP_FORMAT_BOOL = "bool"
 HAP_FORMAT_INT = "int"
 HAP_FORMAT_FLOAT = "float"
@@ -201,7 +204,9 @@ def __init__(
         self._value = self._get_default_value()
         self.getter_callback: Callable[[], Any] | None = None
         # Setters may take just the value or (value, sender_client_addr); see
-        # ``_setter_wants_addr``. The return value is used for write responses.
+        # ``_setter_wants_addr``. The return value is sent back as the write
+        # response when the characteristic has the "wr" permission and the
+        # controller asked for one.
         self.setter_callback: Callable[..., Any] | None = None
         # Cache of whether ``setter_callback`` opts into the client address,
         # recomputed only when the setter object itself changes.
@@ -392,11 +397,17 @@ def set_value(self, value: Any, should_notify: bool = True) -> None:
             self.value = None
 
     def client_update_value(
-        self, value: Any, sender_client_addr: tuple[str, int] | None = None
+        self,
+        value: Any,
+        sender_client_addr: tuple[str, int] | None = None,
+        *,
+        notify: bool = True,
     ) -> Any:
         """Called from broker for value change in Home app.
 
-        Change self.value to value and call callback.
+        Change self.value to value and call callback. With ``notify`` False,
+        subscribers are not told about the written value, e.g. for a request
+        that only the writing connection may read back.
         """
         original_value = value
         if not self._always_null or original_value is not None:
@@ -419,7 +430,7 @@ def client_update_value(
             else:
                 response = self.setter_callback(value)
         changed = self._value != previous_value
-        if changed:
+        if changed and notify:
             self.notify(sender_client_addr)
         if self._always_null:
             self.value = None

pyhap/hap_handler.py

@@ -679,7 +679,7 @@ def handle_get_characteristics(self) -> None:
         assert self.parsed_url is not None  # noqa: S101
         params = parse_qs(self.parsed_url.query)
         response = self.accessory_handler.get_characteristics(
-            params["id"][0].split(",")
+            params["id"][0].split(","), cast("tuple[str, int]", self.client_address)
         )
         chars = response[HAP_REPR_CHARS]