Skip to content

Commit ee933e3

Browse files
Antonio Estradaclaude
andcommitted
fix(priority_drop): promote AES_KEY to Critical to prevent silent E2EE breakage
Resolves security-union#714. Under the priority-drop policy introduced in security-union#708, AES_KEY was classified as `Control`, so it was never preemptively dropped but COULD still be dropped silently at the channel level if the relay's outbound channel hit true overflow (100% full). AES_KEY is the symmetric half of the E2EE handshake (paired with RSA_PUB_KEY, which is already Critical) — losing it breaks encrypted communication for the affected peer pair with no `overflow_critical` label and no page-able alert. Both halves of the handshake must be treated symmetrically. Changes: - `OutboundPriority::classify` now classifies `PacketType::AES_KEY` as `Critical` (alongside `SESSION_ASSIGNED`, `CONGESTION`, `RSA_PUB_KEY`, `MEETING`). - Module docstring and enum doc-comments updated to list AES_KEY in the Critical set, with a brief explanation of the E2EE-handshake rationale. - Existing `classify_aes_key_is_control` test flipped to `classify_aes_key_is_critical`. - `spec_critical_set_is_session_congestion_rsa_meeting` renamed to `spec_critical_set_is_session_congestion_rsa_aes_meeting` and extended to assert AES_KEY membership. - `realchannel_critical_admit_even_at_99_percent_fill` extended to cover AES_KEY. - New `aes_key_is_critical_and_never_dropped` test sweeps fill levels (0/50/80/90/95/99%) to lock in the never-preempt invariant. Quality gates: - `cargo fmt -p videocall-api` clean. - `cargo clippy -p videocall-api --all-targets -- -D warnings` clean. - `cargo test -p videocall-api --lib actors::priority_drop` — 37/37 passing (36 previous tests + 1 new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6d54a16 commit ee933e3

1 file changed

Lines changed: 79 additions & 34 deletions

File tree

actix-api/src/actors/priority_drop.rs

Lines changed: 79 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,13 @@
3838
//! counter still fires (with the new `overflow_critical` label so the
3939
//! new policy can be distinguished from the legacy uniform drops).
4040
//! A subset of control packets — `SESSION_ASSIGNED`, `CONGESTION`,
41-
//! `RSA_PUB_KEY`, `MEETING` — are extra-critical to session
42-
//! lifecycle (reconnection, key exchange, host transitions) and
43-
//! *must* still be attempted on overflow even if the policy were
44-
//! later tightened.
41+
//! `RSA_PUB_KEY`, `AES_KEY`, `MEETING` — are extra-critical to
42+
//! session lifecycle (reconnection, E2EE handshake, host
43+
//! transitions) and *must* still be attempted on overflow even if
44+
//! the policy were later tightened. Both halves of the E2EE
45+
//! handshake (RSA_PUB_KEY + AES_KEY) are Critical because dropping
46+
//! either silently breaks encrypted communication for the affected
47+
//! peer pair with no page-able alert.
4548
//!
4649
//! The decision is per-session: no global state is introduced, and the
4750
//! drop policy is identical for the WebTransport and WebSocket transports.
@@ -86,13 +89,13 @@ pub const PRIORITY_DROP_AUDIO_FILL_RATIO: f32 = 0.95;
8689
pub enum OutboundPriority {
8790
/// Critical lifecycle / key-exchange packet that must never be
8891
/// preemptively dropped: `SESSION_ASSIGNED`, `CONGESTION`,
89-
/// `RSA_PUB_KEY`, `MEETING`. These ride the outbound channel like
90-
/// any other packet, but the priority policy will admit them
91-
/// regardless of channel fill.
92+
/// `RSA_PUB_KEY`, `AES_KEY`, `MEETING`. These ride the outbound
93+
/// channel like any other packet, but the priority policy will
94+
/// admit them regardless of channel fill.
9295
Critical,
93-
/// Generic control / non-media packet (AES_KEY, DIAGNOSTICS,
94-
/// HEALTH, KEYFRAME_REQUEST relayed back to senders, …).
95-
/// Not preemptively dropped; only fails on actual channel overflow.
96+
/// Generic control / non-media packet (DIAGNOSTICS, HEALTH,
97+
/// KEYFRAME_REQUEST relayed back to senders, …). Not preemptively
98+
/// dropped; only fails on actual channel overflow.
9699
Control,
97100
/// Audio media frame. Dropped when fill ratio reaches
98101
/// [`PRIORITY_DROP_AUDIO_FILL_RATIO`].
@@ -128,10 +131,16 @@ impl OutboundPriority {
128131
// (the receiver is congested precisely because the sender
129132
// does not yet know to slow down).
130133
PacketType::CONGESTION => OutboundPriority::Critical,
131-
// RSA_PUB_KEY initiates the asymmetric handshake. Without
132-
// it the receiver cannot decrypt subsequent AES_KEY, so we
133-
// never drop it preemptively.
134+
// RSA_PUB_KEY and AES_KEY are both halves of the E2EE
135+
// handshake — RSA_PUB_KEY initiates the asymmetric step and
136+
// AES_KEY delivers the symmetric session key encrypted under
137+
// it. Dropping either one silently breaks encrypted
138+
// communication for the affected peer pair, with no
139+
// page-able label surfaced to operators. They are small and
140+
// infrequent, so the cost of always admitting them is
141+
// negligible, and the cost of losing one is catastrophic.
134142
PacketType::RSA_PUB_KEY => OutboundPriority::Critical,
143+
PacketType::AES_KEY => OutboundPriority::Critical,
135144
// MEETING packets carry server-authoritative events
136145
// (MEETING_STARTED, MEETING_ENDED, PARTICIPANT_LEFT,
137146
// HOST_MUTE_PARTICIPANT, …). Losing them desyncs the
@@ -151,15 +160,10 @@ impl OutboundPriority {
151160
// ever reused on that hot path.)
152161
_ => OutboundPriority::Control,
153162
},
154-
// Remaining wrappers: AES_KEY, CONNECTION, DIAGNOSTICS,
155-
// HEALTH, PACKET_TYPE_UNKNOWN. AES_KEY in particular is
156-
// critical-ish (key exchange) but is small and infrequent,
157-
// and the existing transport does not have a way to attempt
158-
// a "force admit" past a full channel — treating it as
159-
// Control is the same as the prior uniform behaviour for
160-
// these types. We could promote AES_KEY to Critical, but
161-
// doing so changes no current behaviour (Control never
162-
// preemptively drops either).
163+
// Remaining wrappers: CONNECTION, DIAGNOSTICS, HEALTH,
164+
// PACKET_TYPE_UNKNOWN. Treated as Control: never preemptively
165+
// dropped, only fail on actual channel overflow. This matches
166+
// the prior uniform behaviour for these types.
163167
_ => OutboundPriority::Control,
164168
}
165169
}
@@ -356,12 +360,15 @@ mod tests {
356360
}
357361

358362
#[test]
359-
fn classify_aes_key_is_control() {
360-
// AES_KEY is small & valuable but not in the explicit Critical
361-
// set; behaviour matches the prior uniform policy for it.
363+
fn classify_aes_key_is_critical() {
364+
// AES_KEY is the symmetric half of the E2EE handshake — it
365+
// delivers the session key encrypted under RSA_PUB_KEY. Losing
366+
// it silently breaks encrypted communication between the
367+
// affected peer pair with no `overflow_critical` label, so it
368+
// is classified Critical alongside RSA_PUB_KEY.
362369
assert_eq!(
363370
OutboundPriority::classify(true, PacketType::AES_KEY, None),
364-
OutboundPriority::Control,
371+
OutboundPriority::Critical,
365372
);
366373
}
367374

@@ -737,11 +744,15 @@ mod tests {
737744
}
738745

739746
#[test]
740-
fn spec_critical_set_is_session_congestion_rsa_meeting() {
741-
// "preserve SESSION_*, CONGESTION, RSA_PUB_KEY, MEETING_*"
747+
fn spec_critical_set_is_session_congestion_rsa_aes_meeting() {
748+
// "preserve SESSION_*, CONGESTION, RSA_PUB_KEY, AES_KEY,
749+
// MEETING_*"
742750
//
743751
// Lock in the Critical set so a future change can't silently
744-
// demote one of them into the Control bucket.
752+
// demote one of them into the Control bucket. AES_KEY is the
753+
// symmetric half of the E2EE handshake and is paired with
754+
// RSA_PUB_KEY — dropping either silently breaks encryption for
755+
// the affected peer pair.
745756
assert_eq!(
746757
OutboundPriority::classify(true, PacketType::SESSION_ASSIGNED, None),
747758
OutboundPriority::Critical,
@@ -754,6 +765,10 @@ mod tests {
754765
OutboundPriority::classify(true, PacketType::RSA_PUB_KEY, None),
755766
OutboundPriority::Critical,
756767
);
768+
assert_eq!(
769+
OutboundPriority::classify(true, PacketType::AES_KEY, None),
770+
OutboundPriority::Critical,
771+
);
757772
assert_eq!(
758773
OutboundPriority::classify(true, PacketType::MEETING, None),
759774
OutboundPriority::Critical,
@@ -851,11 +866,12 @@ mod tests {
851866

852867
#[test]
853868
fn realchannel_critical_admit_even_at_99_percent_fill() {
854-
// The Critical bucket guards lifecycle packets (SESSION_ASSIGNED,
855-
// CONGESTION, RSA_PUB_KEY, MEETING). Even when the channel is
856-
// 99% full, the policy must admit. The real try_send may then
857-
// succeed (1 slot left) or fail (raced to fill) — the policy
858-
// itself is not the gating layer here.
869+
// The Critical bucket guards lifecycle and E2EE-handshake
870+
// packets (SESSION_ASSIGNED, CONGESTION, RSA_PUB_KEY, AES_KEY,
871+
// MEETING). Even when the channel is 99% full, the policy must
872+
// admit. The real try_send may then succeed (1 slot left) or
873+
// fail (raced to fill) — the policy itself is not the gating
874+
// layer here.
859875
let total = 100;
860876
let used = 99;
861877
let (tx, _rx) = channel_at_fill(total, used);
@@ -865,6 +881,7 @@ mod tests {
865881
PacketType::SESSION_ASSIGNED,
866882
PacketType::CONGESTION,
867883
PacketType::RSA_PUB_KEY,
884+
PacketType::AES_KEY,
868885
PacketType::MEETING,
869886
] {
870887
let priority = OutboundPriority::classify(true, packet_type, None);
@@ -881,6 +898,34 @@ mod tests {
881898
}
882899
}
883900

901+
#[test]
902+
fn aes_key_is_critical_and_never_dropped() {
903+
// AES_KEY symmetry with RSA_PUB_KEY: both halves of the E2EE
904+
// handshake must survive the priority policy at every fill
905+
// level. Dropping AES_KEY silently breaks encrypted
906+
// communication for the affected peer pair with no
907+
// `overflow_critical` label surfaced to operators — exactly the
908+
// failure mode the priority policy is supposed to prevent for
909+
// RSA_PUB_KEY. The two packet types must be treated
910+
// symmetrically.
911+
let priority = OutboundPriority::classify(true, PacketType::AES_KEY, None);
912+
assert_eq!(
913+
priority,
914+
OutboundPriority::Critical,
915+
"AES_KEY must classify as Critical, mirroring RSA_PUB_KEY",
916+
);
917+
918+
let total = 100usize;
919+
for fill_pct in [0, 50, 80, 90, 95, 99] {
920+
let free = total - fill_pct;
921+
assert_eq!(
922+
evaluate(priority, free, total),
923+
PriorityDropDecision::Admit,
924+
"AES_KEY must be admitted at {fill_pct}% fill (no preemptive drop)",
925+
);
926+
}
927+
}
928+
884929
#[test]
885930
fn realchannel_priority_drop_does_not_consume_slot() {
886931
// Critical contract: when the policy decides to drop, it must

0 commit comments

Comments
 (0)