Skip to content

Commit f088f11

Browse files
committed
fix: limit number of CONTINUATION frames allowed
Calculate the amount of allowed CONTINUATION frames based on other settings. max_header_list_size / max_frame_size That is about how many CONTINUATION frames would be needed to send headers up to the max allowed size. We then multiply by that by a small amount, to allow for implementations that don't perfectly pack into the minimum frames *needed*. In practice, *much* more than that would be a very inefficient peer, or a peer trying to waste resources. See https://seanmonstar.com/blog/hyper-http2-continuation-flood/ for more info.
1 parent 07fc824 commit f088f11

File tree

2 files changed

+106
-4
lines changed

2 files changed

+106
-4
lines changed

src/codec/framed_read.rs

+59-4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ pub struct FramedRead<T> {
3030

3131
max_header_list_size: usize,
3232

33+
max_continuation_frames: usize,
34+
3335
partial: Option<Partial>,
3436
}
3537

@@ -41,6 +43,8 @@ struct Partial {
4143

4244
/// Partial header payload
4345
buf: BytesMut,
46+
47+
continuation_frames_count: usize,
4448
}
4549

4650
#[derive(Debug)]
@@ -51,10 +55,16 @@ enum Continuable {
5155

5256
impl<T> FramedRead<T> {
5357
pub fn new(inner: InnerFramedRead<T, LengthDelimitedCodec>) -> FramedRead<T> {
58+
let max_header_list_size = DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE;
59+
let max_continuation_frames = calc_max_continuation_frames(
60+
max_header_list_size,
61+
inner.decoder().max_frame_length(),
62+
);
5463
FramedRead {
5564
inner,
5665
hpack: hpack::Decoder::new(DEFAULT_SETTINGS_HEADER_TABLE_SIZE),
57-
max_header_list_size: DEFAULT_SETTINGS_MAX_HEADER_LIST_SIZE,
66+
max_header_list_size,
67+
max_continuation_frames,
5868
partial: None,
5969
}
6070
}
@@ -68,7 +78,6 @@ impl<T> FramedRead<T> {
6878
}
6979

7080
/// Returns the current max frame size setting
71-
#[cfg(feature = "unstable")]
7281
#[inline]
7382
pub fn max_frame_size(&self) -> usize {
7483
self.inner.decoder().max_frame_length()
@@ -80,13 +89,23 @@ impl<T> FramedRead<T> {
8089
#[inline]
8190
pub fn set_max_frame_size(&mut self, val: usize) {
8291
assert!(DEFAULT_MAX_FRAME_SIZE as usize <= val && val <= MAX_MAX_FRAME_SIZE as usize);
83-
self.inner.decoder_mut().set_max_frame_length(val)
92+
self.inner.decoder_mut().set_max_frame_length(val);
93+
// Update max CONTINUATION frames too, since its based on this
94+
self.max_continuation_frames = calc_max_continuation_frames(
95+
self.max_header_list_size,
96+
val,
97+
);
8498
}
8599

86100
/// Update the max header list size setting.
87101
#[inline]
88102
pub fn set_max_header_list_size(&mut self, val: usize) {
89103
self.max_header_list_size = val;
104+
// Update max CONTINUATION frames too, since its based on this
105+
self.max_continuation_frames = calc_max_continuation_frames(
106+
val,
107+
self.max_frame_size(),
108+
);
90109
}
91110

92111
/// Update the header table size setting.
@@ -96,12 +115,22 @@ impl<T> FramedRead<T> {
96115
}
97116
}
98117

118+
fn calc_max_continuation_frames(header_max: usize, frame_max: usize) -> usize {
119+
// At least this many frames needed to use max header list size
120+
let min_frames_for_list = (header_max / frame_max).max(1);
121+
// Some padding for imperfectly packed frames
122+
// 25% without floats
123+
let padding = min_frames_for_list >> 2;
124+
min_frames_for_list.saturating_add(padding).max(5)
125+
}
126+
99127
/// Decodes a frame.
100128
///
101129
/// This method is intentionally de-generified and outlined because it is very large.
102130
fn decode_frame(
103131
hpack: &mut hpack::Decoder,
104132
max_header_list_size: usize,
133+
max_continuation_frames: usize,
105134
partial_inout: &mut Option<Partial>,
106135
mut bytes: BytesMut,
107136
) -> Result<Option<Frame>, Error> {
@@ -169,6 +198,7 @@ fn decode_frame(
169198
*partial_inout = Some(Partial {
170199
frame: Continuable::$frame(frame),
171200
buf: payload,
201+
continuation_frames_count: 0,
172202
});
173203

174204
return Ok(None);
@@ -259,6 +289,7 @@ fn decode_frame(
259289
Kind::Continuation => {
260290
let is_end_headers = (head.flag() & 0x4) == 0x4;
261291

292+
262293
let mut partial = match partial_inout.take() {
263294
Some(partial) => partial,
264295
None => {
@@ -273,6 +304,23 @@ fn decode_frame(
273304
return Err(Error::library_go_away(Reason::PROTOCOL_ERROR));
274305
}
275306

307+
// Check for CONTINUATION flood
308+
if is_end_headers {
309+
partial.continuation_frames_count = 0;
310+
} else {
311+
let cnt = partial.continuation_frames_count + 1;
312+
if cnt > max_continuation_frames {
313+
tracing::debug!("too_many_continuations, max = {}", max_continuation_frames);
314+
return Err(Error::library_go_away_data(
315+
Reason::ENHANCE_YOUR_CALM,
316+
"too_many_continuations",
317+
));
318+
} else {
319+
partial.continuation_frames_count = cnt;
320+
}
321+
}
322+
323+
276324
// Extend the buf
277325
if partial.buf.is_empty() {
278326
partial.buf = bytes.split_off(frame::HEADER_LEN);
@@ -354,9 +402,16 @@ where
354402
ref mut hpack,
355403
max_header_list_size,
356404
ref mut partial,
405+
max_continuation_frames,
357406
..
358407
} = *self;
359-
if let Some(frame) = decode_frame(hpack, max_header_list_size, partial, bytes)? {
408+
if let Some(frame) = decode_frame(
409+
hpack,
410+
max_header_list_size,
411+
max_continuation_frames,
412+
partial,
413+
bytes,
414+
)? {
360415
tracing::debug!(?frame, "received");
361416
return Poll::Ready(Some(Ok(frame)));
362417
}

tests/h2-tests/tests/server.rs

+47
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,53 @@ async fn too_big_headers_sends_reset_after_431_if_not_eos() {
883883
join(client, srv).await;
884884
}
885885

886+
#[tokio::test]
887+
async fn too_many_continuation_frames_sends_goaway() {
888+
h2_support::trace_init!();
889+
let (io, mut client) = mock::new();
890+
891+
let client = async move {
892+
let settings = client.assert_server_handshake().await;
893+
assert_frame_eq(settings, frames::settings().max_header_list_size(1024 * 24));
894+
895+
// the mock impl automatically splits into CONTINUATION frames if the
896+
// headers are too big for one frame. So without a max header list size
897+
// set, we'll send a bunch of headers that will eventually get nuked.
898+
client
899+
.send_frame(
900+
frames::headers(1)
901+
.request("GET", "https://example.com/")
902+
.field("a".repeat(10_000), "b".repeat(10_000))
903+
.field("c".repeat(10_000), "d".repeat(10_000))
904+
.field("e".repeat(10_000), "f".repeat(10_000))
905+
.field("g".repeat(10_000), "h".repeat(10_000))
906+
.field("i".repeat(10_000), "j".repeat(10_000))
907+
.field("k".repeat(10_000), "l".repeat(10_000))
908+
.field("m".repeat(10_000), "n".repeat(10_000))
909+
.field("o".repeat(10_000), "p".repeat(10_000))
910+
.field("y".repeat(10_000), "z".repeat(10_000)),
911+
)
912+
.await;
913+
client.recv_frame(frames::go_away(0).calm().data("too_many_continuations")).await;
914+
};
915+
916+
let srv = async move {
917+
let mut srv = server::Builder::new()
918+
// should mean ~3 continuation
919+
.max_header_list_size(1024 * 24)
920+
.handshake::<_, Bytes>(io)
921+
.await
922+
.expect("handshake");
923+
924+
let err = srv.next().await.unwrap().expect_err("server");
925+
assert!(err.is_go_away());
926+
assert!(err.is_library());
927+
assert_eq!(err.reason(), Some(Reason::ENHANCE_YOUR_CALM));
928+
};
929+
930+
join(client, srv).await;
931+
}
932+
886933
#[tokio::test]
887934
async fn pending_accept_recv_illegal_content_length_data() {
888935
h2_support::trace_init!();

0 commit comments

Comments
 (0)