Tighten comments and member ordering

This commit is contained in:
J. Nick Koston
2026-08-31 18:42:28 -04:00
parent 3a0bbc4c17
commit a95bfe0bcb
6 changed files with 34 additions and 55 deletions
+2 -5
View File
@@ -1823,11 +1823,8 @@ bool APIConnection::send_hello_response_(const HelloRequest &msg) {
this->complete_authentication_();
#ifdef USE_API_OUTGOING_CONNECTION
// Only honor the flag once a real key is active. With a PSK set, plaintext
// and the all-zeros provisioning PSK are rejected at the transport, and any
// session that predates key activation is force-closed when the key is
// applied (see update_noise_psk_), so a client reaching this point has
// proven possession of the key.
// With a PSK set only key-verified transports reach hello: plaintext and
// zero-PSK are rejected, and pre-activation sessions are force-closed
if (msg.outgoing_connection_target && !this->flags_.outgoing_connection_target) {
if (this->parent_->get_noise_ctx().has_psk()) {
this->flags_.outgoing_connection_target = true;
@@ -62,8 +62,7 @@ static constexpr size_t API_MAX_LOG_BYTES = 168;
/// Initialize the frame helper, returns OK if successful.
APIError APINoiseFrameHelper::init() {
#ifdef USE_API_OUTGOING_CONNECTION
// set_server_hello_first() marks the outgoing mode in state_; restore the
// state init_common_() expects before running it.
// Outgoing mode is marked in state_; restore what init_common_() expects
const bool outgoing = this->state_ == State::CLIENT_HELLO_OUTGOING;
if (outgoing) {
this->state_ = State::INITIALIZE;
@@ -89,8 +88,7 @@ APIError APINoiseFrameHelper::init() {
state_ = State::CLIENT_HELLO;
#ifdef USE_API_OUTGOING_CONNECTION
if (outgoing) {
// Outgoing connection: the peer needs our name and MAC to pick
// the matching key before it can send its PSK-mixed handshake message.
// The peer needs our name and MAC to pick the key before its first message
state_ = State::CLIENT_HELLO_OUTGOING;
return this->send_server_hello_frame_();
}
@@ -306,7 +304,7 @@ APIError APINoiseFrameHelper::state_action_client_hello_() {
#ifdef USE_API_OUTGOING_CONNECTION
if (this->state_ == State::CLIENT_HELLO_OUTGOING) {
// Server hello already went out in init(); go straight to the handshake.
// Server hello already went out in init()
aerr = init_handshake_();
if (aerr != APIError::OK)
return aerr;
@@ -30,10 +30,9 @@ class APINoiseFrameHelper final : public APIFrameHelper {
APIError init_from_handoff(const uint8_t *header, uint8_t header_len);
#endif
#ifdef USE_API_OUTGOING_CONNECTION
// Outgoing connection: init() sends the server hello immediately so the
// peer can identify this device and select the matching key before it sends
// the PSK-mixed first handshake message. Must be called before init();
// stored in state_ so the helper does not grow.
// init() then sends the server hello first so the peer can pick the key
// before its PSK-mixed message. Call before init(); stored in state_ so
// the helper does not grow.
void set_server_hello_first() { this->state_ = State::CLIENT_HELLO_OUTGOING; }
#endif
APIError loop() override;
@@ -32,21 +32,16 @@ void OutgoingConnectionManager::setup() {
void OutgoingConnectionManager::loop(APIServer *server) {
if (server->has_outgoing_target_client_()) {
// on_target_client() already reset the dial state when this client's
// hello arrived; nothing to do while it stays connected.
return;
return; // on_target_client() already reset the dial state
}
if (this->dialed_conn_ != nullptr) {
// A dialed connection is still open but its peer has not sent a flagged
// hello (yet); dialing again would only burn connection slots. The
// connection's own timeouts remove it eventually if the peer is silent.
// Dialed peer still connected but unproven; its own timeouts free the slot
return;
}
const uint32_t now = App.get_loop_component_start_time();
switch (this->state_) {
case DialState::DIAL_STATE_IDLE:
// The connected target went away; give it the configured delay to
// reconnect on its own before dialing.
// Target went away; give it the configured delay to reconnect first
this->state_ = DialState::DIAL_STATE_WAITING;
this->state_ts_ = now;
this->wait_ = API_OUTGOING_CONNECTION_DELAY;
@@ -121,8 +116,7 @@ void OutgoingConnectionManager::poll_connect_(APIServer *server, uint32_t now) {
this->schedule_retry_(now);
return;
}
// Connect completion is a write event; the main loop select() only watches
// read readiness, so poll it here with a zero timeout.
// Connect completion is a write event; the main loop only selects on reads
fd_set writefds;
FD_ZERO(&writefds);
FD_SET(fd, &writefds);
@@ -154,9 +148,7 @@ void OutgoingConnectionManager::poll_connect_(APIServer *server, uint32_t now) {
this->schedule_retry_(now);
return;
}
// The dial worked; hold without escalating until the peer proves itself
// with a flagged hello (on_target_client() resets to idle) or dies
// unproven (on_client_removed() escalates the backoff).
// Hold unescalated until the peer proves itself or dies unproven
this->schedule_wait_(now, PRECONDITION_RETRY_MS);
}
@@ -21,28 +21,21 @@ class APIServer;
class APIConnection;
struct SavedOutgoingTarget {
// Null-terminated IP string; empty when no peer has been remembered yet.
// Stored as text so the platform-specific v4-mapped-IPv6 normalization in
// the socket component is reused on both ends.
// IP as text so the socket component's v4-mapped-IPv6 normalization is
// reused on both ends; empty = none remembered
char host[socket::SOCKADDR_STR_LEN];
} PACKED; // NOLINT
/// Dials out to Home Assistant when no dial-back target client is connected.
/// The TCP direction flips but the protocol roles do not: this device stays
/// the Noise responder, so both sides still verify each other by the shared
/// key. The target is either a host from YAML or the persisted address of the
/// last client whose hello declared it a dial-back target; the listening
/// socket keeps accepting inbound clients the whole time.
/// Dials out when no dial-back target client is connected. Only the TCP
/// direction flips: the device stays the Noise responder, so both sides
/// still verify by key. Targets the YAML host or the last remembered client.
class OutgoingConnectionManager {
public:
void setup();
void loop(APIServer *server);
/// Called when a key-verified client declares itself a dial-back target;
/// the last such client wins as the remembered address.
/// A key-verified client declared itself a dial-back target; last one wins
void on_target_client(APIConnection *conn);
/// Called for every removed connection so a dialed one stops gating
/// re-dials. A dialed connection dying without ever sending the flagged
/// hello is the unproven-peer case, so the backoff escalates here.
/// Clears the dialed-connection gate; dying unproven escalates the backoff
void on_client_removed(APIConnection *conn);
void on_shutdown() {
this->dial_socket_.reset();
@@ -82,19 +75,18 @@ class OutgoingConnectionManager {
}
static constexpr uint32_t PRECONDITION_RETRY_MS = 5000;
// Pointers first (4 bytes each on 32-bit)
std::unique_ptr<socket::Socket> dial_socket_;
// Connection created by the last successful dial; compared, never
// dereferenced. Cleared by on_client_removed()/on_target_client().
// Compared only, never dereferenced
APIConnection *dialed_conn_{nullptr};
#ifndef API_OUTGOING_CONNECTION_HOST
ESPPreferenceObject target_pref_;
SavedOutgoingTarget saved_{};
#endif
// 4-byte types
uint32_t backoff_{BACKOFF_MIN_MS};
// Boot starts in WAITING, measured from boot: normally the client is
// expected to connect in first, so the full delay applies. A deep sleep
// device wakes for a short window where connecting out IS the wake state,
// so it dials as soon as the network is up.
// Boot waits for the client to connect in first; a deep sleep wake window
// is short, so connecting out immediately is the wake state
#ifdef USE_DEEP_SLEEP
uint32_t wait_{0};
#else
@@ -102,6 +94,11 @@ class OutgoingConnectionManager {
#endif
uint32_t state_ts_{0};
uint32_t last_poll_{0};
// Byte-aligned types last
#ifndef API_OUTGOING_CONNECTION_HOST
SavedOutgoingTarget saved_{};
#endif
DialState state_{DialState::DIAL_STATE_WAITING};
};
+3 -7
View File
@@ -287,10 +287,8 @@ void APIServer::add_client_(APIConnection *conn) {
#ifdef USE_API_OUTGOING_CONNECTION
APIConnection *APIServer::add_outgoing_client_(std::unique_ptr<socket::Socket> sock) {
// Inbound clients may have taken the remaining slots while the dial was in
// flight; re-check at the handoff so add_client_ cannot write past clients_.
// The PSK can also have been cleared mid-dial, and mark_outgoing() requires
// the noise helper the constructor only picks while a PSK is set.
// Re-check at the handoff: inbound clients may have filled the slots and
// the PSK may have been cleared (mark_outgoing() needs the noise helper)
if (this->at_client_limit_() || !this->noise_ctx_.has_psk()) {
ESP_LOGW(TAG, "Dropping outgoing connection (%s)", this->at_client_limit_() ? "max connections" : "no key active");
return nullptr;
@@ -620,9 +618,7 @@ bool APIServer::update_noise_psk_(const SavedNoisePsk &new_psk, const LogString
if (!c->send_message(req)) {
API_LOG_MSG_DROPPED(TAG, "Disconnect request");
}
// Force the disconnect: a session opened before the key was active
// (plaintext or zero-PSK) must not survive activation, or its peer
// could later claim capabilities reserved for key-verified clients.
// Force it: a session from before the key was active must not survive
c->flags_.next_close = true;
}
});