[serial_proxy] Restrict port operations to the subscriber and harden subscription handling (#17796)

esphome/components/api/api_connection.cpp

@@ -1543,8 +1543,8 @@ void APIConnection::on_serial_proxy_configure_request(const SerialProxyConfigure
              static_cast<uint32_t>(proxies.size()));
     return;
   }
-  proxies[msg.instance]->configure(msg.baudrate, msg.flow_control, static_cast<uint8_t>(msg.parity), msg.stop_bits,
-                                   msg.data_size);
+  proxies[msg.instance]->configure(this, msg.baudrate, msg.flow_control, static_cast<uint8_t>(msg.parity),
+                                   msg.stop_bits, msg.data_size);
 }
 
 void APIConnection::on_serial_proxy_write_request(const SerialProxyWriteRequest &msg) {
@@ -1553,7 +1553,7 @@ void APIConnection::on_serial_proxy_write_request(const SerialProxyWriteRequest
     ESP_LOGW(TAG, "Serial proxy instance %" PRIu32 " out of range", msg.instance);
     return;
   }
-  proxies[msg.instance]->write_from_client(msg.data, msg.data_len);
+  proxies[msg.instance]->write_from_client(this, msg.data, msg.data_len);
 }
 
 void APIConnection::on_serial_proxy_set_modem_pins_request(const SerialProxySetModemPinsRequest &msg) {
@@ -1562,7 +1562,7 @@ void APIConnection::on_serial_proxy_set_modem_pins_request(const SerialProxySetM
     ESP_LOGW(TAG, "Serial proxy instance %" PRIu32 " out of range", msg.instance);
     return;
   }
-  proxies[msg.instance]->set_modem_pins(msg.line_states);
+  proxies[msg.instance]->set_modem_pins(this, msg.line_states);
 }
 
 void APIConnection::on_serial_proxy_get_modem_pins_request(const SerialProxyGetModemPinsRequest &msg) {

esphome/components/serial_proxy/serial_proxy.cpp

@@ -89,8 +89,14 @@ void SerialProxy::dump_config() {
                 this->dtr_pin_ != nullptr ? "configured" : "not configured");
 }
 
-void SerialProxy::configure(uint32_t baudrate, bool flow_control, uint8_t parity, uint8_t stop_bits,
-                            uint8_t data_size) {
+void SerialProxy::configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity,
+                            uint8_t stop_bits, uint8_t data_size) {
+#ifdef USE_API
+  if (this->port_claimed_by_other_(api_connection)) {
+    ESP_LOGW(TAG, "Ignoring configure request from client without port access [%" PRIu32 "]", this->instance_index_);
+    return;
+  }
+#endif
   ESP_LOGD(TAG,
            "Configuring serial proxy [%" PRIu32 "]: baud=%" PRIu32 ", flow_ctrl=%s, parity=%" PRIu8 ", stop=%" PRIu8
            ", data=%" PRIu8,
@@ -143,13 +149,27 @@ void SerialProxy::configure(uint32_t baudrate, bool flow_control, uint8_t parity
   }
 }
 
-void SerialProxy::write_from_client(const uint8_t *data, size_t len) {
+void SerialProxy::write_from_client(api::APIConnection *api_connection, const uint8_t *data, size_t len) {
+#ifdef USE_API
+  // Bytes from a client other than the live subscriber would interleave with the
+  // subscriber's traffic on the wire
+  if (this->port_claimed_by_other_(api_connection)) {
+    ESP_LOGW(TAG, "Ignoring write from client without port access [%" PRIu32 "]", this->instance_index_);
+    return;
+  }
+#endif
   if (data == nullptr || len == 0)
     return;
   this->write_array(data, len);
 }
 
-void SerialProxy::set_modem_pins(uint32_t line_states) {
+void SerialProxy::set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) {
+#ifdef USE_API
+  if (this->port_claimed_by_other_(api_connection)) {
+    ESP_LOGW(TAG, "Ignoring modem pin request from client without port access [%" PRIu32 "]", this->instance_index_);
+    return;
+  }
+#endif
   const bool rts = (line_states & SERIAL_PROXY_LINE_STATE_FLAG_RTS) != 0;
   const bool dtr = (line_states & SERIAL_PROXY_LINE_STATE_FLAG_DTR) != 0;
   ESP_LOGV(TAG, "Setting modem pins [%" PRIu32 "]: RTS=%s, DTR=%s", this->instance_index_, ONOFF(rts), ONOFF(dtr));
@@ -175,13 +195,28 @@ uart::UARTFlushResult SerialProxy::flush_port() {
 }
 
 #ifdef USE_API
+bool SerialProxy::port_claimed_by_other_(api::APIConnection *api_connection) const {
+  return this->api_connection_ != nullptr && this->api_connection_ != api_connection &&
+         this->api_connection_->is_connection_setup();
+}
+
 void SerialProxy::serial_proxy_request(api::APIConnection *api_connection, api::enums::SerialProxyRequestType type) {
   switch (type) {
     case api::enums::SERIAL_PROXY_REQUEST_TYPE_SUBSCRIBE:
-      if (this->api_connection_ != nullptr) {
-        ESP_LOGE(TAG, "Only one API subscription is allowed at a time");
+      if (this->api_connection_ == api_connection) {
+        ESP_LOGV(TAG, "API connection is already subscribed to serial proxy [%" PRIu32 "]", this->instance_index_);
         return;
       }
+      if (this->api_connection_ != nullptr) {
+        // A living subscriber keeps exclusive access. Its connection may be dead without
+        // loop() having noticed yet (e.g. the client crashed and reconnected quickly);
+        // in that case let the new client take over instead of locking it out.
+        if (this->api_connection_->is_connection_setup()) {
+          ESP_LOGE(TAG, "Only one API subscription is allowed at a time");
+          return;
+        }
+        ESP_LOGW(TAG, "Previous subscriber disconnected; taking over subscription");
+      }
       this->api_connection_ = api_connection;
       this->enable_loop();
       ESP_LOGV(TAG, "API connection subscribed to serial proxy [%" PRIu32 "]", this->instance_index_);

esphome/components/serial_proxy/serial_proxy.h

@@ -67,12 +67,14 @@ class SerialProxy final : public uart::UARTDevice, public Component {
   api::enums::SerialProxyPortType get_port_type() const { return this->port_type_; }
 
   /// Configure UART parameters and apply them
+  /// @param api_connection The API connection requesting the change
   /// @param baudrate Baud rate in bits per second
   /// @param flow_control True to enable hardware flow control
   /// @param parity Parity setting (0=none, 1=even, 2=odd)
   /// @param stop_bits Number of stop bits (1 or 2)
   /// @param data_size Number of data bits (5-8)
-  void configure(uint32_t baudrate, bool flow_control, uint8_t parity, uint8_t stop_bits, uint8_t data_size);
+  void configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity,
+                 uint8_t stop_bits, uint8_t data_size);
 
   /// Get the currently subscribed API connection (nullptr if none)
   api::APIConnection *get_api_connection() { return this->api_connection_; }
@@ -81,12 +83,13 @@ class SerialProxy final : public uart::UARTDevice, public Component {
   void serial_proxy_request(api::APIConnection *api_connection, api::enums::SerialProxyRequestType type);
 
   /// Write data received from an API client to the serial device
+  /// @param api_connection The API connection sending the data
   /// @param data Pointer to data buffer
   /// @param len Number of bytes to write
-  void write_from_client(const uint8_t *data, size_t len);
+  void write_from_client(api::APIConnection *api_connection, const uint8_t *data, size_t len);
 
   /// Set modem pin states from a bitmask of SerialProxyLineStateFlag values
-  void set_modem_pins(uint32_t line_states);
+  void set_modem_pins(api::APIConnection *api_connection, uint32_t line_states);
 
   /// Get current modem pin states as a bitmask of SerialProxyLineStateFlag values
   uint32_t get_modem_pins() const;
@@ -104,6 +107,9 @@ class SerialProxy final : public uart::UARTDevice, public Component {
 #ifdef USE_API
   /// Read from UART and send to API client (slow path with 256-byte stack buffer)
   void read_and_send_(size_t available);
+
+  /// True when a live subscriber other than the given connection holds the port
+  bool port_claimed_by_other_(api::APIConnection *api_connection) const;
 #endif
 
   /// Instance index for identifying this proxy in API messages

tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h

@@ -32,9 +32,10 @@ class SerialProxy {
   api::enums::SerialProxyPortType get_port_type() const { return {}; }
   api::APIConnection *get_api_connection() { return nullptr; }
   void serial_proxy_request(api::APIConnection *conn, api::enums::SerialProxyRequestType type) {}
-  void configure(uint32_t baudrate, bool flow_control, uint8_t parity, uint32_t stop_bits, uint32_t data_size) {}
-  void write_from_client(const uint8_t *data, size_t len) {}
-  void set_modem_pins(uint32_t line_states) {}
+  void configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity,
+                 uint32_t stop_bits, uint32_t data_size) {}
+  void write_from_client(api::APIConnection *api_connection, const uint8_t *data, size_t len) {}
+  void set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) {}
   uint32_t get_modem_pins() const { return 0; }
   uart::UARTFlushResult flush_port() { return uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS; }