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
  • Loading branch information
catenacyber authored and victorjulien committed Feb 6, 2024
1 parent 86de7cf commit 80abc22
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 17 deletions.
4 changes: 2 additions & 2 deletions rust/src/http2/detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) {
blocks,
};
let txdata = HTTP2FrameTypeData::HEADERS(hs);
let tx = state.find_or_create_tx(&head, &txdata, Direction::ToServer);
let tx = state.find_or_create_tx(&head, &txdata, Direction::ToServer).unwrap();
tx.frames_ts.push(HTTP2Frame {
header: head,
data: txdata,
Expand Down Expand Up @@ -975,7 +975,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, Direction::ToServer);
let tx = state.find_or_create_tx(&head, &txdata, Direction::ToServer).unwrap();
tx.frames_ts.push(HTTP2Frame {
header: head,
data: txdata,
Expand Down
47 changes: 32 additions & 15 deletions rust/src/http2/http2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,9 +611,21 @@ impl HTTP2State {

pub fn find_or_create_tx(
&mut self, header: &parser::HTTP2FrameHeader, data: &HTTP2FrameTypeData, dir: Direction,
) -> &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 Down Expand Up @@ -643,30 +655,31 @@ impl HTTP2State {
let tx = &mut self.transactions[index - 1];
tx.tx_data.update_file_flags(self.state_data.file_flags);
tx.update_file_flags(tx.tx_data.file_flags);
return tx;
return Some(tx);
} else {
let mut tx = HTTP2Transaction::new();
self.tx_id += 1;
tx.tx_id = self.tx_id;
tx.stream_id = sid;
tx.state = HTTP2TransactionState::HTTP2StateOpen;
// do not use SETTINGS_MAX_CONCURRENT_STREAMS as it can grow too much
if self.transactions.len() > unsafe { HTTP2_MAX_STREAMS } {
// set at least one another transaction to the drop state
if self.transactions.len() >= unsafe { HTTP2_MAX_STREAMS } {
for tx_old in &mut self.transactions {
if tx_old.state != HTTP2TransactionState::HTTP2StateTodrop {
// use a distinct state, even if we do not log it
tx_old.set_event(HTTP2Event::TooManyStreams);
tx_old.state = HTTP2TransactionState::HTTP2StateTodrop;
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;
tx.tx_data.update_file_flags(self.state_data.file_flags);
tx.update_file_flags(tx.tx_data.file_flags);
tx.tx_data.file_tx = STREAM_TOSERVER|STREAM_TOCLIENT; // might hold files in both directions
self.transactions.push_back(tx);
return self.transactions.back_mut().unwrap();
return Some(self.transactions.back_mut().unwrap());
}
}

Expand Down Expand Up @@ -1038,6 +1051,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.tx_data.set_event(HTTP2Event::ReassemblyLimitReached as u8);
}
Expand Down

0 comments on commit 80abc22

Please sign in to comment.