1From 390f09692eb99809c679d3f350c7cc185d163e1a Mon Sep 17 00:00:00 2001 2From: Philippe Antoine <pantoine@oisf.net> 3Date: Wed, 27 Mar 2024 14:33:54 +0100 4Subject: [PATCH] http2: use a reference counter for headers 5 6Ticket: 6892 7 8As HTTP hpack header compression allows one single byte to 9express a previously seen arbitrary-size header block (name+value) 10we should avoid to copy the vectors data, but just point 11to the same data, while reamining memory safe, even in the case 12of later headers eviction from the dybnamic table. 13 14Rust std solution is Rc, and the use of clone, so long as the 15data is accessed by only one thread. 16 17Note: This patch is needed to patch CVE-2024-38535 as it defines Rc. 18Upstream-Status: Backport from [https://github.com/OISF/suricata/commit/390f09692eb99809c679d3f350c7cc185d163e1a] 19Signed-off-by: Siddharth Doshi <sdoshi@mvista.com> 20--- 21 rust/src/http2/detect.rs | 19 +++++++------ 22 rust/src/http2/http2.rs | 2 +- 23 rust/src/http2/parser.rs | 61 +++++++++++++++++++++------------------- 24 3 files changed, 43 insertions(+), 39 deletions(-) 25 26diff --git a/rust/src/http2/detect.rs b/rust/src/http2/detect.rs 27index 9c2f8ab..e068a17 100644 28--- a/rust/src/http2/detect.rs 29+++ b/rust/src/http2/detect.rs 30@@ -23,6 +23,7 @@ use crate::core::Direction; 31 use crate::detect::uint::{detect_match_uint, DetectUintData}; 32 use std::ffi::CStr; 33 use std::str::FromStr; 34+use std::rc::Rc; 35 36 fn http2_tx_has_frametype( 37 tx: &mut HTTP2Transaction, direction: Direction, value: u8, 38@@ -404,7 +405,7 @@ fn http2_frames_get_header_firstvalue<'a>( 39 for frame in frames { 40 if let Some(blocks) = http2_header_blocks(frame) { 41 for block in blocks.iter() { 42- if block.name == name.as_bytes() { 43+ if block.name.as_ref() == name.as_bytes() { 44 return Ok(&block.value); 45 } 46 } 47@@ -428,7 +429,7 @@ pub fn http2_frames_get_header_value_vec( 48 for frame in frames { 49 if let Some(blocks) = http2_header_blocks(frame) { 50 for block in blocks.iter() { 51- if block.name == name.as_bytes() { 52+ if block.name.as_ref() == name.as_bytes() { 53 if found == 0 { 54 vec.extend_from_slice(&block.value); 55 found = 1; 56@@ -465,7 +466,7 @@ fn http2_frames_get_header_value<'a>( 57 for frame in frames { 58 if let Some(blocks) = http2_header_blocks(frame) { 59 for block in blocks.iter() { 60- if block.name == name.as_bytes() { 61+ if block.name.as_ref() == name.as_bytes() { 62 if found == 0 { 63 single = Ok(&block.value); 64 found = 1; 65@@ -905,8 +906,8 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) { 66 }; 67 let mut blocks = Vec::new(); 68 let b = parser::HTTP2FrameHeaderBlock { 69- name: name.to_vec(), 70- value: input.to_vec(), 71+ name: Rc::new(name.to_vec()), 72+ value: Rc::new(input.to_vec()), 73 error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, 74 sizeupdate: 0, 75 }; 76@@ -1061,15 +1062,15 @@ mod tests { 77 }; 78 let mut blocks = Vec::new(); 79 let b = parser::HTTP2FrameHeaderBlock { 80- name: "Host".as_bytes().to_vec(), 81- value: "abc.com".as_bytes().to_vec(), 82+ name: "Host".as_bytes().to_vec().into(), 83+ value: "abc.com".as_bytes().to_vec().into(), 84 error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, 85 sizeupdate: 0, 86 }; 87 blocks.push(b); 88 let b2 = parser::HTTP2FrameHeaderBlock { 89- name: "Host".as_bytes().to_vec(), 90- value: "efg.net".as_bytes().to_vec(), 91+ name: "Host".as_bytes().to_vec().into(), 92+ value: "efg.net".as_bytes().to_vec().into(), 93 error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, 94 sizeupdate: 0, 95 }; 96diff --git a/rust/src/http2/http2.rs b/rust/src/http2/http2.rs 97index 326030f..d14ca06 100644 98--- a/rust/src/http2/http2.rs 99+++ b/rust/src/http2/http2.rs 100@@ -204,7 +204,7 @@ impl HTTP2Transaction { 101 102 fn handle_headers(&mut self, blocks: &[parser::HTTP2FrameHeaderBlock], dir: Direction) { 103 for block in blocks { 104- if block.name == b"content-encoding" { 105+ if block.name.as_ref() == b"content-encoding" { 106 self.decoder.http2_encoding_fromvec(&block.value, dir); 107 } 108 } 109diff --git a/rust/src/http2/parser.rs b/rust/src/http2/parser.rs 110index adabeb2..1a46437 100644 111--- a/rust/src/http2/parser.rs 112+++ b/rust/src/http2/parser.rs 113@@ -30,6 +30,7 @@ use nom7::sequence::tuple; 114 use nom7::{Err, IResult}; 115 use std::fmt; 116 use std::str::FromStr; 117+use std::rc::Rc; 118 119 #[repr(u8)] 120 #[derive(Clone, Copy, PartialEq, Eq, FromPrimitive, Debug)] 121@@ -295,8 +296,8 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option<HTTP 122 }; 123 if !name.is_empty() { 124 return Some(HTTP2FrameHeaderBlock { 125- name: name.as_bytes().to_vec(), 126- value: value.as_bytes().to_vec(), 127+ name: Rc::new(name.as_bytes().to_vec()), 128+ value: Rc::new(value.as_bytes().to_vec()), 129 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, 130 sizeupdate: 0, 131 }); 132@@ -304,23 +305,23 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option<HTTP 133 //use dynamic table 134 if n == 0 { 135 return Some(HTTP2FrameHeaderBlock { 136- name: Vec::new(), 137- value: Vec::new(), 138+ name: Rc::new(Vec::new()), 139+ value: Rc::new(Vec::new()), 140 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIndex0, 141 sizeupdate: 0, 142 }); 143 } else if dyn_headers.table.len() + HTTP2_STATIC_HEADERS_NUMBER < n as usize { 144 return Some(HTTP2FrameHeaderBlock { 145- name: Vec::new(), 146- value: Vec::new(), 147+ name: Rc::new(Vec::new()), 148+ value: Rc::new(Vec::new()), 149 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed, 150 sizeupdate: 0, 151 }); 152 } else { 153 let indyn = dyn_headers.table.len() - (n as usize - HTTP2_STATIC_HEADERS_NUMBER); 154 let headcopy = HTTP2FrameHeaderBlock { 155- name: dyn_headers.table[indyn].name.to_vec(), 156- value: dyn_headers.table[indyn].value.to_vec(), 157+ name: dyn_headers.table[indyn].name.clone(), 158+ value: dyn_headers.table[indyn].value.clone(), 159 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, 160 sizeupdate: 0, 161 }; 162@@ -348,8 +349,10 @@ impl fmt::Display for HTTP2HeaderDecodeStatus { 163 164 #[derive(Clone, Debug)] 165 pub struct HTTP2FrameHeaderBlock { 166- pub name: Vec<u8>, 167- pub value: Vec<u8>, 168+ // Use Rc reference counted so that indexed headers do not get copied. 169+ // Otherwise, this leads to quadratic complexity in memory occupation. 170+ pub name: Rc<Vec<u8>>, 171+ pub value: Rc<Vec<u8>>, 172 pub error: HTTP2HeaderDecodeStatus, 173 pub sizeupdate: u64, 174 } 175@@ -391,7 +394,7 @@ fn http2_parse_headers_block_literal_common<'a>( 176 ) -> IResult<&'a [u8], HTTP2FrameHeaderBlock> { 177 let (i3, name, error) = if index == 0 { 178 match http2_parse_headers_block_string(input) { 179- Ok((r, n)) => Ok((r, n, HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess)), 180+ Ok((r, n)) => Ok((r, Rc::new(n), HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess)), 181 Err(e) => Err(e), 182 } 183 } else { 184@@ -403,7 +406,7 @@ fn http2_parse_headers_block_literal_common<'a>( 185 )), 186 None => Ok(( 187 input, 188- Vec::new(), 189+ Rc::new(Vec::new()), 190 HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed, 191 )), 192 } 193@@ -413,7 +416,7 @@ fn http2_parse_headers_block_literal_common<'a>( 194 i4, 195 HTTP2FrameHeaderBlock { 196 name, 197- value, 198+ value: Rc::new(value), 199 error, 200 sizeupdate: 0, 201 }, 202@@ -435,8 +438,8 @@ fn http2_parse_headers_block_literal_incindex<'a>( 203 match r { 204 Ok((r, head)) => { 205 let headcopy = HTTP2FrameHeaderBlock { 206- name: head.name.to_vec(), 207- value: head.value.to_vec(), 208+ name: head.name.clone(), 209+ value: head.value.clone(), 210 error: head.error, 211 sizeupdate: 0, 212 }; 213@@ -556,8 +559,8 @@ fn http2_parse_headers_block_dynamic_size<'a>( 214 return Ok(( 215 i3, 216 HTTP2FrameHeaderBlock { 217- name: Vec::new(), 218- value: Vec::new(), 219+ name: Rc::new(Vec::new()), 220+ value: Rc::new(Vec::new()), 221 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSizeUpdate, 222 sizeupdate: maxsize2, 223 }, 224@@ -614,8 +617,8 @@ fn http2_parse_headers_blocks<'a>( 225 // if we error from http2_parse_var_uint, we keep the first parsed headers 226 if err.code == ErrorKind::LengthValue { 227 blocks.push(HTTP2FrameHeaderBlock { 228- name: Vec::new(), 229- value: Vec::new(), 230+ name: Rc::new(Vec::new()), 231+ value: Rc::new(Vec::new()), 232 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow, 233 sizeupdate: 0, 234 }); 235@@ -765,8 +768,8 @@ mod tests { 236 match r0 { 237 Ok((remainder, hd)) => { 238 // Check the first message. 239- assert_eq!(hd.name, ":method".as_bytes().to_vec()); 240- assert_eq!(hd.value, "GET".as_bytes().to_vec()); 241+ assert_eq!(hd.name, ":method".as_bytes().to_vec().into()); 242+ assert_eq!(hd.value, "GET".as_bytes().to_vec().into()); 243 // And we should have no bytes left. 244 assert_eq!(remainder.len(), 0); 245 } 246@@ -782,8 +785,8 @@ mod tests { 247 match r1 { 248 Ok((remainder, hd)) => { 249 // Check the first message. 250- assert_eq!(hd.name, "accept".as_bytes().to_vec()); 251- assert_eq!(hd.value, "*/*".as_bytes().to_vec()); 252+ assert_eq!(hd.name, "accept".as_bytes().to_vec().into()); 253+ assert_eq!(hd.value, "*/*".as_bytes().to_vec().into()); 254 // And we should have no bytes left. 255 assert_eq!(remainder.len(), 0); 256 assert_eq!(dynh.table.len(), 1); 257@@ -802,8 +805,8 @@ mod tests { 258 match result { 259 Ok((remainder, hd)) => { 260 // Check the first message. 261- assert_eq!(hd.name, ":authority".as_bytes().to_vec()); 262- assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec()); 263+ assert_eq!(hd.name, ":authority".as_bytes().to_vec().into()); 264+ assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec().into()); 265 // And we should have no bytes left. 266 assert_eq!(remainder.len(), 0); 267 assert_eq!(dynh.table.len(), 2); 268@@ -820,8 +823,8 @@ mod tests { 269 match r3 { 270 Ok((remainder, hd)) => { 271 // same as before 272- assert_eq!(hd.name, ":authority".as_bytes().to_vec()); 273- assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec()); 274+ assert_eq!(hd.name, ":authority".as_bytes().to_vec().into()); 275+ assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec().into()); 276 // And we should have no bytes left. 277 assert_eq!(remainder.len(), 0); 278 assert_eq!(dynh.table.len(), 2); 279@@ -856,8 +859,8 @@ mod tests { 280 match r2 { 281 Ok((remainder, hd)) => { 282 // Check the first message. 283- assert_eq!(hd.name, ":path".as_bytes().to_vec()); 284- assert_eq!(hd.value, "/doc/manual/html/index.html".as_bytes().to_vec()); 285+ assert_eq!(hd.name, ":path".as_bytes().to_vec().into()); 286+ assert_eq!(hd.value, "/doc/manual/html/index.html".as_bytes().to_vec().into()); 287 // And we should have no bytes left. 288 assert_eq!(remainder.len(), 0); 289 assert_eq!(dynh.table.len(), 2); 290-- 2912.44.0 292 293