Skip to content

Commit

Permalink
http2: limit number of concurrent transactions
Browse files Browse the repository at this point in the history
Ticket: 6481

Instead of just setting the old transactions to a drop state so
that they get later cleaned up by Suricata, fail creating new ones.

This is because one call to app-layer parsing can create many
transactions, and quadratic complexity could happen in one
single app-layer parsing because of find_or_create_tx

(cherry picked from commit 80abc22)
  • Loading branch information
catenacyber authored and victorjulien committed Feb 7, 2024
1 parent 83c5567 commit b1549e9
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
4 changes: 2 additions & 2 deletions rust/src/http2/detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) {
blocks: blocks,
};
let txdata = HTTP2FrameTypeData::HEADERS(hs);
let tx = state.find_or_create_tx(&head, &txdata, STREAM_TOSERVER);
let tx = state.find_or_create_tx(&head, &txdata, STREAM_TOSERVER).unwrap();
tx.frames_ts.push(HTTP2Frame {
header: head,
data: txdata,
Expand Down Expand Up @@ -1052,7 +1052,7 @@ fn http2_tx_set_settings(state: &mut HTTP2State, input: &[u8]) {
match parser::http2_parse_frame_settings(&dec) {
Ok((_, set)) => {
let txdata = HTTP2FrameTypeData::SETTINGS(set);
let tx = state.find_or_create_tx(&head, &txdata, STREAM_TOSERVER);
let tx = state.find_or_create_tx(&head, &txdata, STREAM_TOSERVER).unwrap();
tx.frames_ts.push(HTTP2Frame {
header: head,
data: txdata,
Expand Down
43 changes: 39 additions & 4 deletions rust/src/http2/http2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const HTTP2_FRAME_WINDOWUPDATE_LEN: usize = 4;
pub const HTTP2_MAX_TABLESIZE: u32 = 0x10000; // 65536
// maximum size of reassembly for header + continuation
static mut HTTP2_MAX_REASS: usize = 102400;
static mut HTTP2_MAX_STREAMS: usize = 4096; // 0x1000

#[repr(u8)]
#[derive(Copy, Clone, PartialOrd, PartialEq, Debug)]
Expand Down Expand Up @@ -117,6 +118,8 @@ pub enum HTTP2TransactionState {
HTTP2StateClosed = 7,
//not a RFC-defined state, used for stream 0 frames appyling to the global connection
HTTP2StateGlobal = 8,
//not a RFC-defined state, dropping this old tx because we have too many
HTTP2StateTodrop = 9,
}

#[derive(Debug)]
Expand Down Expand Up @@ -365,6 +368,7 @@ pub enum HTTP2Event {
AuthorityHostMismatch,
UserinfoInUri,
ReassemblyLimitReached,
TooManyStreams,
}

impl HTTP2Event {
Expand All @@ -382,6 +386,7 @@ impl HTTP2Event {
9 => Some(HTTP2Event::FailedDecompression),
10 => Some(HTTP2Event::AuthorityHostMismatch),
11 => Some(HTTP2Event::UserinfoInUri),
12 => Some(HTTP2Event::TooManyStreams),
_ => None,
}
}
Expand Down Expand Up @@ -523,9 +528,21 @@ impl HTTP2State {

pub fn find_or_create_tx(
&mut self, header: &parser::HTTP2FrameHeader, data: &HTTP2FrameTypeData, dir: u8,
) -> &mut HTTP2Transaction {
) -> Option<&mut HTTP2Transaction> {
if header.stream_id == 0 {
return self.create_global_tx();
if self.transactions.len() >= unsafe { HTTP2_MAX_STREAMS } {
for tx_old in &mut self.transactions {
if tx_old.state == HTTP2TransactionState::HTTP2StateTodrop {
// loop was already run
break;
}
tx_old.set_event(HTTP2Event::TooManyStreams);
// use a distinct state, even if we do not log it
tx_old.state = HTTP2TransactionState::HTTP2StateTodrop;
}
return None;
}
return Some(self.create_global_tx());
}
let sid = match data {
//yes, the right stream_id for Suricata is not the header one
Expand All @@ -551,15 +568,28 @@ impl HTTP2State {
self.set_event(HTTP2Event::StreamIdReuse);
}
}
return &mut self.transactions[index - 1];
return Some(&mut self.transactions[index - 1]);
} else {
// do not use SETTINGS_MAX_CONCURRENT_STREAMS as it can grow too much
if self.transactions.len() >= unsafe { HTTP2_MAX_STREAMS } {
for tx_old in &mut self.transactions {
if tx_old.state == HTTP2TransactionState::HTTP2StateTodrop {
// loop was already run
break;
}
tx_old.set_event(HTTP2Event::TooManyStreams);
// use a distinct state, even if we do not log it
tx_old.state = HTTP2TransactionState::HTTP2StateTodrop;
}
return None;
}
let mut tx = HTTP2Transaction::new();
self.tx_id += 1;
tx.tx_id = self.tx_id;
tx.stream_id = sid;
tx.state = HTTP2TransactionState::HTTP2StateOpen;
self.transactions.push_back(tx);
return self.transactions.back_mut().unwrap();
return Some(self.transactions.back_mut().unwrap());
}
}

Expand Down Expand Up @@ -925,6 +955,10 @@ impl HTTP2State {
);

let tx = self.find_or_create_tx(&head, &txdata, dir);
if tx.is_none() {
return AppLayerResult::err();
}
let tx = tx.unwrap();
if reass_limit_reached {
tx.set_event(HTTP2Event::ReassemblyLimitReached);
}
Expand Down Expand Up @@ -1279,6 +1313,7 @@ pub extern "C" fn rs_http2_state_get_event_info_by_id(
HTTP2Event::AuthorityHostMismatch => "authority_host_mismatch\0",
HTTP2Event::UserinfoInUri => "userinfo_in_uri\0",
HTTP2Event::ReassemblyLimitReached => "reassembly_limit_reached\0",
HTTP2Event::TooManyStreams => "too_many_streams\0",
};
unsafe {
*event_name = estr.as_ptr() as *const std::os::raw::c_char;
Expand Down

0 comments on commit b1549e9

Please sign in to comment.