From f0c23bbe1bb2e4364ff9e5eab377b68e0f12bd46 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Sat, 2 May 2026 20:48:53 -0400 Subject: [PATCH] Serialize short HTTPS calls; tighten mbedtls per-session heap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The metrics work in PR #10 added a third concurrent TLS client (metrics → monitoring.googleapis.com) on top of cloud_log → logging and OTA → ghcr. Three concurrent handshakes pinned ~90 KB of mbedtls context and pushed `min_free_heap` into the single-digit-KB range, producing intermittent ESP_ERR_HTTP_CONNECT failures (mbedtls ALLOC_FAILED at -0x7F00). Serialize via a shared `Arc>` constructed in main and held **at the call sites** in each sender — std `Mutex` isn't reentrant and `auth.get_or_refresh()` mints a token (an HTTPS POST to oauth2) that must run inside the same critical section. Lock granularity: - cloud_log: token-refresh + entries:write under one lock window. - metrics: token-refresh + timeSeries.create under one lock window. - ota::poll_once: token + manifest under lock; release for the signature bundle fetch's lock; release; verify (CPU, no TLS); download_and_apply UNLOCKED. Releasing between phases avoids blocking cloud_log/metrics behind ~2s of pure-Rust X.509/ECDSA work. - ota::download_and_apply: deliberately not in the lock — multi-second blob downloads must not block per-5-s log flushes or per-30-s metrics POSTs. Plus mbedtls heap knobs that compose with the dynamic-buffer config already in sdkconfig: - CONFIG_MBEDTLS_SSL_KEEP_PEER_CERTIFICATE=n: drop the parsed peer cert chain after handshake. We don't reuse sessions or do mutual TLS, so we never need it post-handshake. Saves ~3-5 KB per active session. Known limitation, deferred to a follow-up: cloud_log/metrics handshakes can still collide with the held-open OTA download session and OOM the download mid-stream. The plan there is an `OTA_DOWNLOAD_IN_PROGRESS` atomic that cloud_log + metrics check before locking — they skip the POST and let the queue accumulate until the download finishes. Co-Authored-By: Claude Opus 4.7 (1M context) --- sdkconfig.defaults.in | 27 +++++++++++++++++++++------ src/cloud_log.rs | 17 +++++++++++++++-- src/gcp_auth.rs | 23 ++++++++++++++++++++++- src/main.rs | 17 ++++++++++++++--- src/metrics.rs | 14 ++++++++++++-- src/ota.rs | 38 +++++++++++++++++++++++++++++--------- 6 files changed, 113 insertions(+), 23 deletions(-) diff --git a/sdkconfig.defaults.in b/sdkconfig.defaults.in index 50b00d1..a8e8002 100644 --- a/sdkconfig.defaults.in +++ b/sdkconfig.defaults.in @@ -7,15 +7,30 @@ CONFIG_PTHREAD_TASK_STACK_SIZE_DEFAULT=16384 CONFIG_MBEDTLS_CERTIFICATE_BUNDLE=y CONFIG_MBEDTLS_CERTIFICATE_BUNDLE_DEFAULT_FULL=y -# Allow mbedtls to grow its TLS I/O buffers dynamically per-session and -# free them between handshakes, instead of pinning the default 16+16 KB -# in/out buffers per concurrent session. Recovers ~15-25 KB per active -# session — needed because we run three concurrent TLS clients -# (cloud_log, metrics, OTA) and three full-size buffer pairs blow past -# our heap budget (`min_free_heap` watermark hit ~9 KB). +# mbedtls heap budgeting. We routinely have 2-3 concurrent TLS sessions +# (cloud_log → logging.googleapis.com, metrics → monitoring.googleapis.com, +# OTA → ghcr.io / pkg-containers); without these knobs the per-session +# heap pinned for I/O buffers + cert chain blows our budget and the +# next handshake fails with MBEDTLS_ERR_SSL_ALLOC_FAILED (-0x7F00). +# +# Even with serialization (cloud_log + metrics + OTA short fetches share +# a Mutex; OTA blob download deliberately doesn't), the OTA download +# holds its session open for tens of seconds while metrics/cloud_log +# handshakes need to fit alongside it. +# +# - DYNAMIC_BUFFER: I/O buffers grow per-session up to the configured +# max instead of being pinned at 16+16 KB. +# - DYNAMIC_FREE_CONFIG_DATA: free the mbedtls_ssl_config struct after +# handshake. +# - DYNAMIC_FREE_CA_CERT: drop the CA bundle data after handshake (we +# re-attach it from esp_crt_bundle on the next handshake). +# - SSL_KEEP_PEER_CERTIFICATE=n: drop the parsed peer cert chain after +# handshake. We don't reuse sessions or do mutual TLS, so we never +# need it post-handshake. Saves ~3-5 KB per active session. CONFIG_MBEDTLS_DYNAMIC_BUFFER=y CONFIG_MBEDTLS_DYNAMIC_FREE_CONFIG_DATA=y CONFIG_MBEDTLS_DYNAMIC_FREE_CA_CERT=y +CONFIG_MBEDTLS_SSL_KEEP_PEER_CERTIFICATE=n # OTA: custom partition table with two app slots, plus app-rollback support # so a bad OTA can be reverted by the bootloader on next boot. diff --git a/src/cloud_log.rs b/src/cloud_log.rs index 844d19b..2847295 100644 --- a/src/cloud_log.rs +++ b/src/cloud_log.rs @@ -21,7 +21,9 @@ use tracing::{Event, Level, Subscriber}; use tracing_subscriber::layer::Context as LayerContext; use tracing_subscriber::Layer; -use crate::gcp_auth::{device_mac, http_post, now_unix_secs, unix_to_rfc3339, TokenProvider}; +use crate::gcp_auth::{ + device_mac, http_post, now_unix_secs, unix_to_rfc3339, ShortHttpsLock, TokenProvider, +}; const NVS_GCP_NS: &str = "gcp"; const NVS_PROJECT_ID: &str = "project_id"; @@ -317,7 +319,12 @@ impl FieldCapture { /// Sender thread main loop. Drains the queue every `FLUSH_INTERVAL`, /// pulls the bearer from the shared `TokenProvider`, and POSTs batches /// to Cloud Logging. -pub fn run(cfg: GcpConfig, auth: Arc, queue: LogQueue) -> ! { +pub fn run( + cfg: GcpConfig, + auth: Arc, + queue: LogQueue, + short_https: ShortHttpsLock, +) -> ! { crate::metrics::publish_self(&crate::metrics::handles::CLOUD_LOG); tracing::info!( project = %cfg.project_id, @@ -347,6 +354,12 @@ pub fn run(cfg: GcpConfig, auth: Arc, queue: LogQueue) -> ! { continue; } + // Hold the short-https lock from token-refresh through POST. + // `auth.get_or_refresh()` may transparently mint a new token, + // which is itself an HTTPS POST to oauth2.googleapis.com — both + // the mint and the entries:write call need to be inside the + // lock to actually serialise vs metrics + OTA short fetches. + let _lock = short_https.lock().unwrap_or_else(|e| e.into_inner()); let bearer = match auth.get_or_refresh() { Ok(b) => b, Err(e) => { diff --git a/src/gcp_auth.rs b/src/gcp_auth.rs index 356db50..872d2df 100644 --- a/src/gcp_auth.rs +++ b/src/gcp_auth.rs @@ -22,11 +22,32 @@ use rsa::pkcs8::DecodePrivateKey; use rsa::signature::{SignatureEncoding, Signer}; use rsa::RsaPrivateKey; use serde::{Deserialize, Serialize}; -use std::sync::Mutex; +use std::sync::{Arc, Mutex}; use std::time::Duration; use crate::cloud_log::GcpConfig; +/// Serialises short HTTPS calls — cloud_log POSTs, metrics POSTs, OTA +/// manifest + sig-bundle fetches, and the OAuth2 token mint that any +/// of them may trigger. Each TLS handshake allocates ~25-35 KB of +/// mbedtls context (less with `CONFIG_MBEDTLS_DYNAMIC_BUFFER`); three +/// concurrent handshakes blew our heap budget (`min_free_heap` ~9 KB +/// before this lock). +/// +/// Held **at call sites** in the senders, not inside `http_post` +/// itself — std `Mutex` isn't reentrant, so locking inside `http_post` +/// would deadlock when `get_or_refresh()` mints a token while the +/// caller already holds the lock. +/// +/// `ota::download_and_apply` deliberately does **not** take this lock: +/// the multi-second blob download must not block per-5-s cloud_log +/// flushes or per-30-s metrics POSTs. +pub type ShortHttpsLock = Arc>; + +pub fn new_short_https_lock() -> ShortHttpsLock { + Arc::new(Mutex::new(())) +} + /// A bearer token cached until ~5 min before expiry. struct CachedToken { token: String, diff --git a/src/main.rs b/src/main.rs index 7a16fec..4457790 100644 --- a/src/main.rs +++ b/src/main.rs @@ -138,14 +138,23 @@ fn main() -> Result<()> { } } + // One shared lock that serialises the *short* HTTPS calls across + // cloud_log + metrics + OTA's manifest/sig-bundle fetches. The OTA + // download (multi-second blob) deliberately runs without it. Even + // when the [gcp] block is absent we still construct it so OTA's + // signature can be the same; cloud_log/metrics just won't be there + // to contend for it. + let short_https = gcp_auth::new_short_https_lock(); + let ota_nvs = nvs.clone(); let ota_trust = trust.clone(); + let ota_lock = short_https.clone(); std::thread::Builder::new() // HTTPS + JSON + SHA256 is ~32 KB; phase 4a adds X.509 parsing // and ECDSA P-256/P-384 verification on top, which want more. // 48 KB is observed-safe with headroom. .stack_size(48 * 1024) - .spawn(move || ota::run(ota_nvs, FW_VERSION, ota_trust)) + .spawn(move || ota::run(ota_nvs, FW_VERSION, ota_trust, Some(ota_lock))) .expect("spawn ota thread"); if let (Some(cfg), Some(queue)) = (gcp, log_queue) { @@ -169,21 +178,23 @@ fn main() -> Result<()> { let cl_cfg = cfg.clone(); let cl_auth = auth.clone(); let cl_queue = queue.clone(); + let cl_lock = short_https.clone(); std::thread::Builder::new() // RSA signing + HTTPS POST. 32 KB matches the OTA loop budget. .stack_size(32 * 1024) - .spawn(move || cloud_log::run(cl_cfg, cl_auth, cl_queue)) + .spawn(move || cloud_log::run(cl_cfg, cl_auth, cl_queue, cl_lock)) .expect("spawn cloud_log thread"); if cfg.metrics_interval_secs > 0 { let m_cfg = cfg; let m_auth = auth; let m_queue = queue; + let m_lock = short_https; std::thread::Builder::new() // No crypto on hot path (token cached via auth). // Mainly HTTPS POST + serde_json. 16 KB sufficient. .stack_size(16 * 1024) - .spawn(move || metrics::run(m_cfg, m_auth, m_queue)) + .spawn(move || metrics::run(m_cfg, m_auth, m_queue, m_lock)) .expect("spawn metrics thread"); } } diff --git a/src/metrics.rs b/src/metrics.rs index 89f8b32..9fce450 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use std::time::Duration; use crate::cloud_log::{GcpConfig, LogQueue}; -use crate::gcp_auth::{device_mac, http_post, unix_to_rfc3339, TokenProvider}; +use crate::gcp_auth::{device_mac, http_post, unix_to_rfc3339, ShortHttpsLock, TokenProvider}; const METRIC_PREFIX: &str = "custom.googleapis.com/esp32"; @@ -48,7 +48,12 @@ fn read_handle(slot: &AtomicUsize) -> Option { } } -pub fn run(cfg: GcpConfig, auth: Arc, queue: LogQueue) -> ! { +pub fn run( + cfg: GcpConfig, + auth: Arc, + queue: LogQueue, + short_https: ShortHttpsLock, +) -> ! { publish_self(&handles::METRICS); tracing::info!( @@ -76,6 +81,11 @@ pub fn run(cfg: GcpConfig, auth: Arc, queue: LogQueue) -> ! { std::thread::sleep(sleep_for); let snapshot = collect(&queue); + + // Lock spans token refresh + POST so both TLS handshakes + // serialise against cloud_log and OTA short fetches. See the + // matching comment in cloud_log.rs. + let _lock = short_https.lock().unwrap_or_else(|e| e.into_inner()); let bearer = match auth.get_or_refresh() { Ok(b) => b, Err(e) => { diff --git a/src/ota.rs b/src/ota.rs index 54b12db..204a4fd 100644 --- a/src/ota.rs +++ b/src/ota.rs @@ -90,6 +90,7 @@ pub fn run( nvs_partition: EspDefaultNvsPartition, fw_version: &str, trust: crate::trust::TrustConfig, + short_https: Option, ) -> ! { crate::metrics::publish_self(&crate::metrics::handles::OTA); let mut nvs = match EspNvs::new(nvs_partition, NVS_NAMESPACE, true) { @@ -127,7 +128,7 @@ pub fn run( "ota: sleeping", ); std::thread::sleep(sleep_for); - match poll_once(&mut nvs, &cfg, &trust) { + match poll_once(&mut nvs, &cfg, &trust, short_https.as_ref()) { Ok(PollOutcome::NoChange) => { consecutive_failures = 0; tracing::debug!("ota: no change"); @@ -180,12 +181,20 @@ fn poll_once( nvs: &mut EspNvs, cfg: &OtaConfig, trust: &crate::trust::TrustConfig, + short_https: Option<&crate::gcp_auth::ShortHttpsLock>, ) -> Result { - let token = fetch_anon_token(&cfg.repo)?; + // Phase 1 — token + manifest. Two short HTTPS calls; serialise + // against cloud_log + metrics POSTs so we don't pile concurrent + // TLS handshakes on the heap. Held only for these two fetches; the + // ~ms gap between releasing here and re-acquiring before the sig + // bundle fetch is enough for the senders to slip through. + let (token, manifest, manifest_digest_hex) = { + let _l = short_https.map(|m| m.lock().unwrap_or_else(|e| e.into_inner())); + let token = fetch_anon_token(&cfg.repo)?; + let (manifest, mdig) = fetch_manifest(&cfg.repo, &cfg.tag, &token)?; + (token, manifest, mdig) + }; - // Fetch manifest and compute its SHA256 — that's the manifest digest - // cosign signed (and that we'll need for the bundle lookup). - let (manifest, manifest_digest_hex) = fetch_manifest(&cfg.repo, &cfg.tag, &token)?; let layer = manifest .layers .into_iter() @@ -211,14 +220,25 @@ fn poll_once( "ota: new digest, verifying signature before download", ); - // Phase 4a: fetch and verify the cosign signature bundle BEFORE the - // (much larger) firmware download. Refuses unknown signers. - let bundle = fetch_signature_bundle(&cfg.repo, &manifest_digest_hex, &token) - .context("fetch signature bundle")?; + // Phase 2 — sig bundle fetch (3 sub-fetches), then local verify. + // Re-acquire the lock for the network calls; release before the + // CPU-bound `verify_bundle` so cloud_log/metrics aren't blocked + // behind our X.509 + ECDSA work. + let bundle = { + let _l = short_https.map(|m| m.lock().unwrap_or_else(|e| e.into_inner())); + fetch_signature_bundle(&cfg.repo, &manifest_digest_hex, &token) + .context("fetch signature bundle")? + }; crate::sig::verify_bundle(&bundle, &manifest_digest_hex, trust) .context("verify signature bundle")?; tracing::info!("ota: signature verified, proceeding with download"); + // Phase 3 — long blob download. Deliberately UNLOCKED: a multi- + // second download must not block per-5-s cloud_log flushes or + // per-30-s metrics POSTs. Holds its own TLS session for the + // duration; cloud_log + metrics may run a second concurrent + // session, which the heap can absorb (vs the previous worst case + // of three). download_and_apply(&cfg.repo, &layer, &token)?; // Persist as pending; main.rs will promote to last_digest after the