Skip to content

Dns over http2 5773 v1 #9965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
catenacyber wants to merge 11 commits into from
Closed

Dns over http2 5773 v1 #9965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0c908a4
http2: extract dns request
catenacyber Dec 5, 2023
db356b1
doh: pstate flag for layered packet to come
catenacyber Dec 5, 2023
08f4f0a
doh: store layered packet in HTTP2 state
catenacyber Dec 5, 2023
1f79632
doh: get layered on C side, ready to queue
catenacyber Dec 5, 2023
746cf64
doh: enque pseudo packet
catenacyber Dec 5, 2023
2c350df
fixup! doh: enque pseudo packet
catenacyber Dec 5, 2023
e7c66e3
doh: switch proto and state back and forth
catenacyber Dec 5, 2023
f911442
doh: try if dns is parsed
catenacyber Dec 5, 2023
123de0a
fixup! doh: try if dns is parsed
catenacyber Dec 5, 2023
4ca50c9
doh: it works also for responses
catenacyber Dec 5, 2023
083cc8b
fixup format
catenacyber Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rust/src/applayer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ pub const APP_LAYER_PARSER_EOF_TS : u16 = BIT_U16!(5);
pub const APP_LAYER_PARSER_EOF_TC : u16 = BIT_U16!(6);
pub const APP_LAYER_PARSER_TRUNC_TS : u16 = BIT_U16!(7);
pub const APP_LAYER_PARSER_TRUNC_TC : u16 = BIT_U16!(8);
pub const APP_LAYER_PARSER_LAYERED_PACKET : u16 = BIT_U16!(11);
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please suggest me a better name :-p

Copy link
Member

@jasonish jasonish Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stacked? Encapped? Encapsulated? They'll kinda mean the same thing.


pub const APP_LAYER_PARSER_OPT_ACCEPT_GAPS: u32 = BIT_U32!(0);

Expand Down
151 changes: 131 additions & 20 deletions rust/src/http2/http2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ use crate::conf::conf_get;
use crate::core::*;
use crate::filecontainer::*;
use crate::filetracker::*;

use crate::dns::dns::rs_dns_state_new;
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO use rs_dns_state_free


use nom7::Err;
use std;
use std::collections::VecDeque;
Expand Down Expand Up @@ -144,6 +147,8 @@ pub struct HTTP2Transaction {
pub escaped: Vec<Vec<u8>>,
pub req_line: Vec<u8>,
pub resp_line: Vec<u8>,

pub doh: Vec<u8>,
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: add comment to explain this field

Copy link
Member

@jasonish jasonish Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it needs a longer name, I suppose a comment would help.

}

impl Transaction for HTTP2Transaction {
Expand Down Expand Up @@ -175,6 +180,7 @@ impl HTTP2Transaction {
escaped: Vec::with_capacity(16),
req_line: Vec::new(),
resp_line: Vec::new(),
doh: Vec::new(),
}
}

Expand Down Expand Up @@ -202,12 +208,27 @@ impl HTTP2Transaction {
self.tx_data.set_event(event as u8);
}

fn handle_headers(&mut self, blocks: &[parser::HTTP2FrameHeaderBlock], dir: Direction) {
fn handle_headers(&mut self, blocks: &[parser::HTTP2FrameHeaderBlock], dir: Direction) -> Option<Vec<u8>> {
let mut authority = None;
let mut path = None;
let mut doh = false;
let mut host = None;
for block in blocks {
if block.name == b"content-encoding" {
self.decoder.http2_encoding_fromvec(&block.value, dir);
} else if block.name == b"accept" {
//TODO? faster pattern matching
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Victor, is this ok to match on the different header names ?

if block.value == b"application/dns-message" {
doh = true;
}
} else if block.name == b"content-type" {
if block.value == b"application/dns-message" {
// push original 2-bytes DNS/TCP header
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: have a longer comment

self.doh.push(0);
self.doh.push(0);
}
} else if block.name == b":path" {
path = Some(&block.value);
} else if block.name.eq_ignore_ascii_case(b":authority") {
authority = Some(&block.value);
if block.value.iter().any(|&x| x == b'@') {
Expand All @@ -229,6 +250,14 @@ impl HTTP2Transaction {
}
}
}
if doh {
if let Some(p) = path {
if let Ok((_, dns_req)) = parser::doh_extract_request(p) {
return Some(dns_req);
}
}
}
return None;
}

pub fn update_file_flags(&mut self, flow_file_flags: u16) {
Expand All @@ -237,10 +266,9 @@ impl HTTP2Transaction {
}

fn decompress<'a>(
&'a mut self, input: &'a [u8], dir: Direction, sfcm: &'static SuricataFileContext, over: bool, flow: *const Flow,
&'a mut self, input: &'a [u8], output: &'a mut Vec<u8>, dir: Direction, sfcm: &'static SuricataFileContext, over: bool, flow: *const Flow,
) -> io::Result<()> {
let mut output = Vec::with_capacity(decompression::HTTP2_DECOMPRESSION_CHUNK_SIZE);
let decompressed = self.decoder.decompress(input, &mut output, dir)?;
let decompressed = self.decoder.decompress(input, output, dir)?;
let xid: u32 = self.tx_id as u32;
if dir == Direction::ToClient {
self.ft_tc.tx_id = self.tx_id - 1;
Expand Down Expand Up @@ -299,13 +327,17 @@ impl HTTP2Transaction {
&xid,
);
};
if !self.doh.is_empty() {
self.doh.extend_from_slice(decompressed);
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO comment

}
return Ok(());
}

fn handle_frame(
&mut self, header: &parser::HTTP2FrameHeader, data: &HTTP2FrameTypeData, dir: Direction,
) {
) -> Option<Vec<u8>> {
//handle child_stream_id changes
let mut r = None;
match data {
HTTP2FrameTypeData::PUSHPROMISE(hs) => {
if dir == Direction::ToClient {
Expand All @@ -315,21 +347,21 @@ impl HTTP2Transaction {
}
self.state = HTTP2TransactionState::HTTP2StateReserved;
}
self.handle_headers(&hs.blocks, dir);
r = self.handle_headers(&hs.blocks, dir);
}
HTTP2FrameTypeData::CONTINUATION(hs) => {
if dir == Direction::ToClient
&& header.flags & parser::HTTP2_FLAG_HEADER_END_HEADERS != 0
{
self.child_stream_id = 0;
}
self.handle_headers(&hs.blocks, dir);
r = self.handle_headers(&hs.blocks, dir);
}
HTTP2FrameTypeData::HEADERS(hs) => {
if dir == Direction::ToClient {
self.child_stream_id = 0;
}
self.handle_headers(&hs.blocks, dir);
r = self.handle_headers(&hs.blocks, dir);
}
HTTP2FrameTypeData::RSTSTREAM(_) => {
self.child_stream_id = 0;
Expand Down Expand Up @@ -376,6 +408,7 @@ impl HTTP2Transaction {
}
_ => {}
}
return r;
}
}

Expand Down Expand Up @@ -441,6 +474,9 @@ pub struct HTTP2State {
dynamic_headers_tc: HTTP2DynTable,
transactions: VecDeque<HTTP2Transaction>,
progress: HTTP2ConnectionState,
// layered packets contents for DNS over HTTP2
layered: Vec<Vec<u8>>,
dns_state: Option<*mut std::os::raw::c_void>,
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO comment

I am not sure if this should be generic or DNS-specific...

Copy link
Member

@jasonish jasonish Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is DoH a little special in that it doesn't follow the normal PROTO-over-HTTP formats like something like GRPC might do? Where they just use the bodies for their payload?

Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where they just use the bodies for their payload?

Indeed DoH is not exactly like that : it has DNS message in the URI for requests (a base64-encoded parameter), and in bodies for responses.

Why does it matter ?

Copy link
Member

@jasonish jasonish Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where they just use the bodies for their payload?

Indeed DoH is not exactly like that : it has DNS message in the URI for requests (a base64-encoded parameter), and in bodies for responses.

Why does it matter ?

Say we were to support gRPC over HTTP, and Avro over HTTP, and Thrift over HTTP. Those would also make sense to have something generic. As DoH is different, could it be put within the same generic container or does it always need some special handling due to be in the URI. I guess not. But that's the path of thought I was on.

Copy link
Contributor Author

@catenacyber catenacyber Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed I was hesitating for naming dns_state or something more generic...

Copy link
Member

@jasonish jasonish Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK without over thinking making it more generic until we have the need to make it more generic.

}

impl State<HTTP2Transaction> for HTTP2State {
Expand Down Expand Up @@ -473,6 +509,8 @@ impl HTTP2State {
dynamic_headers_tc: HTTP2DynTable::new(),
transactions: VecDeque::new(),
progress: HTTP2ConnectionState::Http2StateInit,
layered: Vec::new(),
dns_state: None,
}
}

Expand Down Expand Up @@ -925,6 +963,7 @@ impl HTTP2State {

fn parse_frames(
&mut self, mut input: &[u8], il: usize, dir: Direction, flow: *const Flow,
pstate: *mut std::os::raw::c_void
) -> AppLayerResult {
while !input.is_empty() {
match parser::http2_parse_frame_header(input) {
Expand Down Expand Up @@ -970,7 +1009,7 @@ impl HTTP2State {
);

let tx = self.find_or_create_tx(&head, &txdata, dir);
tx.handle_frame(&head, &txdata, dir);
let odoh = tx.handle_frame(&head, &txdata, dir);
let over = head.flags & parser::HTTP2_FLAG_HEADER_EOS != 0;
let ftype = head.ftype;
let sid = head.stream_id;
Expand Down Expand Up @@ -1002,19 +1041,58 @@ impl HTTP2State {
if padded && !rem.is_empty() && usize::from(rem[0]) < hlsafe{
dinput = &rem[1..hlsafe - usize::from(rem[0])];
}
if tx_same.decompress(
let mut output = Vec::with_capacity(decompression::HTTP2_DECOMPRESSION_CHUNK_SIZE);
match tx_same.decompress(
dinput,
&mut output,
dir,
sfcm,
over,
flow).is_err() {
self.set_event(HTTP2Event::FailedDecompression);
flow) {
Ok(_) => {
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO helper function for less indent

if !tx_same.doh.is_empty() {
if over {
// fix DNS/TCP header length field
tx_same.doh[0] = ((tx_same.doh.len() - 2) >> 8) as u8;
tx_same.doh[1] = ((tx_same.doh.len() - 2) & 0xFF) as u8;
self.layered.push(tx_same.doh.to_vec());
if self.dns_state.is_none() {
unsafe {
self.dns_state = Some(rs_dns_state_new(std::ptr::null_mut(), ALPROTO_HTTP2));
}
}
unsafe {
AppLayerParserStateSetFlag(
pstate,
APP_LAYER_PARSER_LAYERED_PACKET,
);
}
}
}
}
_ => {
self.set_event(HTTP2Event::FailedDecompression);
}
}
}
}
None => panic!("no SURICATA_HTTP2_FILE_CONFIG"),
}
}
if let Some(doh) = odoh {
self.layered.push(doh);
unsafe {
AppLayerParserStateSetFlag(
pstate,
APP_LAYER_PARSER_LAYERED_PACKET,
);
}
if self.dns_state.is_none() {
unsafe {
self.dns_state = Some(rs_dns_state_new(std::ptr::null_mut(), ALPROTO_HTTP2));
}
}
Copy link
Contributor Author

@catenacyber catenacyber Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ This is a key first step :
we

  • set a flag in pstate (so that AppLayerParserParse knows that it should deque packets)
  • push the to-be pseudo-packet payload in HTTP2 state
  • create a DNS state if there is not one yet

}
input = &rem[hlsafe..];
}
Err(Err::Incomplete(_)) => {
Expand All @@ -1033,7 +1111,7 @@ impl HTTP2State {
return AppLayerResult::ok();
}

fn parse_ts(&mut self, mut input: &[u8], flow: *const Flow) -> AppLayerResult {
fn parse_ts(&mut self, mut input: &[u8], flow: *const Flow, pstate: *mut std::os::raw::c_void) -> AppLayerResult {
//very first : skip magic
let mut magic_consumed = 0;
if self.progress < HTTP2ConnectionState::Http2StateMagicDone {
Expand Down Expand Up @@ -1073,7 +1151,7 @@ impl HTTP2State {
}

//then parse all we can
let r = self.parse_frames(input, il, Direction::ToServer, flow);
let r = self.parse_frames(input, il, Direction::ToServer, flow, pstate);
if r.status == 1 {
//adds bytes consumed by banner to incomplete result
return AppLayerResult::incomplete(r.consumed + magic_consumed as u32, r.needed);
Expand All @@ -1082,7 +1160,7 @@ impl HTTP2State {
}
}

fn parse_tc(&mut self, mut input: &[u8], flow: *const Flow) -> AppLayerResult {
fn parse_tc(&mut self, mut input: &[u8], flow: *const Flow, pstate: *mut std::os::raw::c_void) -> AppLayerResult {
//first consume frame bytes
let il = input.len();
if self.response_frame_size > 0 {
Expand All @@ -1097,10 +1175,43 @@ impl HTTP2State {
}
}
//then parse all we can
return self.parse_frames(input, il, Direction::ToClient, flow);
return self.parse_frames(input, il, Direction::ToClient, flow, pstate);
}
}

#[no_mangle]
pub unsafe extern "C" fn SCHttp2ClearLayered(
s: &mut HTTP2State
) {
s.layered.clear();
}

#[no_mangle]
pub unsafe extern "C" fn SCHttp2GetLayeredState(
s: &HTTP2State
) -> *mut std::os::raw::c_void {
match s.dns_state {
Some(d) => d,
_ => std::ptr::null_mut(),
}
}

#[no_mangle]
pub unsafe extern "C" fn SCHttp2GetLayered(
s: &HTTP2State, buffer: *mut *const u8, buffer_len: *mut u32, i: u32,
) -> bool {
let i = i as usize;
if i < s.layered.len() {
*buffer = s.layered[i].as_ptr();
*buffer_len = s.layered[i].len() as u32;
return true;
}

*buffer = std::ptr::null();
*buffer_len = 0;
return false;
}

// C exports.

export_tx_data_get!(rs_http2_get_tx_data, HTTP2Transaction);
Expand Down Expand Up @@ -1175,22 +1286,22 @@ pub unsafe extern "C" fn rs_http2_state_tx_free(state: *mut std::os::raw::c_void

#[no_mangle]
pub unsafe extern "C" fn rs_http2_parse_ts(
flow: *const Flow, state: *mut std::os::raw::c_void, _pstate: *mut std::os::raw::c_void,
flow: *const Flow, state: *mut std::os::raw::c_void, pstate: *mut std::os::raw::c_void,
stream_slice: StreamSlice, _data: *const std::os::raw::c_void,
) -> AppLayerResult {
let state = cast_pointer!(state, HTTP2State);
let buf = stream_slice.as_slice();
return state.parse_ts(buf, flow);
return state.parse_ts(buf, flow, pstate);
}

#[no_mangle]
pub unsafe extern "C" fn rs_http2_parse_tc(
flow: *const Flow, state: *mut std::os::raw::c_void, _pstate: *mut std::os::raw::c_void,
flow: *const Flow, state: *mut std::os::raw::c_void, pstate: *mut std::os::raw::c_void,
stream_slice: StreamSlice, _data: *const std::os::raw::c_void,
) -> AppLayerResult {
let state = cast_pointer!(state, HTTP2State);
let buf = stream_slice.as_slice();
return state.parse_tc(buf, flow);
return state.parse_tc(buf, flow, pstate);
}

#[no_mangle]
Expand Down
17 changes: 17 additions & 0 deletions rust/src/http2/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use nom7::multi::many0;
use nom7::number::streaming::{be_u16, be_u24, be_u32, be_u8};
use nom7::sequence::tuple;
use nom7::{Err, IResult};
use nom7::bytes::complete::tag;
use std::fmt;
use std::str::FromStr;

Expand Down Expand Up @@ -751,6 +752,22 @@ pub fn http2_parse_frame_settings(i: &[u8]) -> IResult<&[u8], Vec<HTTP2FrameSett
many0(complete(http2_parse_frame_setting))(i)
}

pub fn doh_extract_request(i: &[u8]) -> IResult<&[u8], Vec<u8>> {
let (i, _) = tag("/dns-query?dns=")(i)?;
match base64::decode(i) {
Ok(mut dec) => {
// adds 2 byte tcp header with length
dec.insert(0, (dec.len() & 0xFF) as u8);
dec.insert(0, (dec.len() >> 8) as u8);
// i is unused
return Ok((i, dec));
}
_ => {
return Err(Err::Error(make_error(i, ErrorKind::MapOpt)));
}
}
}

#[cfg(test)]
mod tests {

Expand Down
Loading