From ce9b221be4337d66077025236a6623f464e2b7da Mon Sep 17 00:00:00 2001 From: Keith Burzinski Date: Fri, 31 Jul 2026 23:05:06 -0500 Subject: [PATCH] [serial_proxy] Restrict port operations to the subscriber and harden subscription handling (#17796) --- esphome/components/api/api_connection.cpp | 8 ++-- .../components/serial_proxy/serial_proxy.cpp | 47 ++++++++++++++++--- .../components/serial_proxy/serial_proxy.h | 12 +++-- .../components/serial_proxy/serial_proxy.h | 7 +-- 4 files changed, 58 insertions(+), 16 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 9aac7bd7d1..3dc2a06c85 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1543,8 +1543,8 @@ void APIConnection::on_serial_proxy_configure_request(const SerialProxyConfigure static_cast(proxies.size())); return; } - proxies[msg.instance]->configure(msg.baudrate, msg.flow_control, static_cast(msg.parity), msg.stop_bits, - msg.data_size); + proxies[msg.instance]->configure(this, msg.baudrate, msg.flow_control, static_cast(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) { diff --git a/esphome/components/serial_proxy/serial_proxy.cpp b/esphome/components/serial_proxy/serial_proxy.cpp index 04c94e9292..4b3a907416 100644 --- a/esphome/components/serial_proxy/serial_proxy.cpp +++ b/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_); diff --git a/esphome/components/serial_proxy/serial_proxy.h b/esphome/components/serial_proxy/serial_proxy.h index e35fab3d42..268c1b52be 100644 --- a/esphome/components/serial_proxy/serial_proxy.h +++ b/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 diff --git a/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h b/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h index bab27549e7..d8b068fb36 100644 --- a/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h +++ b/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; }