Project

General

Profile

Actions

Bug #5780

closed

HTTP/2 - FN when matching on multiple http2.header contents

Added by Brandon Murphy almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

It appears that when attempting to combine two different HTTP2 headers into a single rule, no alert is produced.

Consider the following rules and the attached pcap, which contains a single tcp session with a single HTTP2 stream.

alert http2 $HOME_NET any -> any any (msg:"HTTP2 - Single Header - Authority"; flow:established,to_server; http2.header; content:"authority: bugertor"; sid:1;)
alert http2 $HOME_NET any -> any any (msg:"HTTP2 - Single Header - Method"; flow:established,to_server; http2.header; content:"method: GET"; sid:2;)
alert http2 $HOME_NET any -> any any (msg:"HTTP2 - Two Headers - Authority/Method"; flow:established,to_server; http2.header; content:"method: GET"; content:"authority: bugertor.com"; sid:3;)

Current Behavior

Only sid:1 and sid:2 fire

Expected Behavior

All three signatures should fire.

HTTP Keyword Overloading

Once the correct http2 configuration option is enabled (http1-rules), the using the standard http1 keywords (http.method, http.host) the below signature works as expected.

alert http $HOME_NET any -> any any (msg:"HTTP2 - Overload Test"; flow:established,to_server; http.method; content:"GET"; http.host; content:"bugertor.com"; sid:4;)


Files

http2_multiple_headers.pcap (1.7 KB) http2_multiple_headers.pcap Brandon Murphy, 01/10/2023 03:53 PM

Related issues 1 (0 open1 closed)

Related to Suricata - Feature #5784: detect: allow cross buffer inspection on multi-buffer matchesClosedVictor JulienActions
Actions #1

Updated by Brandon Murphy almost 2 years ago

Actions #2

Updated by Brandon Murphy almost 2 years ago

  • Description updated (diff)
Actions #3

Updated by Victor Julien almost 2 years ago

  • Status changed from New to Assigned
  • Assignee changed from OISF Dev to Philippe Antoine
Actions #4

Updated by Philippe Antoine almost 2 years ago

Workaround use http.header instead of http2.header cf alert http2 any any -> any any (msg:"HTTP2 - Two Headers - Authority/Method"; http.header; content:"method: GET"; content:"authority: bugertor.com"; sid:4;)

Actions #5

Updated by Philippe Antoine almost 2 years ago

Not HTTP2 specific, multi-buffer thing

Another reproducer is

alert mqtt any any -> any any (msg:"HTTP2 - Two Headers - Authority/Method"; mqtt.subscribe.topic; content:"topicX"; mqtt.subscribe.topic; content:"topicY"; sid:5;)
alert mqtt any any -> any any (msg:"HTTP2 - Two Headers - Authority/Method"; mqtt.subscribe.topic; content:"topicY"; sid:6;)
alert mqtt any any -> any any (msg:"HTTP2 - Two Headers - Authority/Method"; mqtt.subscribe.topic; content:"topicX"; sid:7;)

on suricata-verify/tests/mqtt-sub-rules/mqtt5_sub_userpass.pcap

Actions #6

Updated by Brandon Murphy almost 2 years ago

Philippe Antoine wrote in #note-4:

Workaround use http.header instead of http2.header cf alert http2 any any -> any any (msg:"HTTP2 - Two Headers - Authority/Method"; http.header; content:"method: GET"; content:"authority: bugertor.com"; sid:4;)

Oh, that's interesting! Do you happen to know if this only works when overloading is enabled (in 6.0.x)?

Actions #7

Updated by Philippe Antoine almost 2 years ago

Do you happen to know if this only works when overloading is enabled (in 6.0.x)?

I tested on master.
And I am not sure what you mean by overloading... What is it ?

By the way, thanks Brandon for this interesting report ;-)

Also, I would not expect http2.header; content:"method: GET"; content:"authority: bugertor.com";. to match because you want both contents in a single header (like http2.header; content:"method"; content:"GET"; does match)
But I would expect http2.header; content:"method: GET"; http2.header; content:"authority: bugertor.com"; to match and it does not...
What do you think about it as a rule writer ?
Do you understand "http header" as a single key value pair, or as all the key value pairs in a HTTP request/response ?

Actions #8

Updated by Brandon Murphy almost 2 years ago

Philippe Antoine wrote in #note-7:

Do you happen to know if this only works when overloading is enabled (in 6.0.x)?

I tested on master.
And I am not sure what you mean by overloading... What is it ?

Enabling the http1-rules option within the http2 app layer configuration item. I stole the term from https://redmine.openinfosecfoundation.org/issues/4067

By the way, thanks Brandon for this interesting report ;-)

Also, I would not expect http2.header; content:"method: GET"; content:"authority: bugertor.com";. to match because you want both contents in a single header (like http2.header; content:"method"; content:"GET"; does match)
But I would expect http2.header; content:"method: GET"; http2.header; content:"authority: bugertor.com"; to match and it does not...
What do you think about it as a rule writer ?

I do not like it. I think this is a large difference from the behavior of http.header which "contains all of the extracted headers in a single buffer".

Do you understand "http header" as a single key value pair, or as all the key value pairs in a HTTP request/response ?

"as all the key value pairs in a HTTP request/response" only because it is this way with http.header

Very often we need the ability to write rules on the unique combination of headers and their values. The inability to combine multiple headers names and their values severally limits the rules that can be created, in this case, specific to HTTP/2, though apparently other protocols as well.

The completion of https://redmine.openinfosecfoundation.org/issues/4067 and the new default configuration in suri 7 which enables http1-rules helps a lot to workaround the current limitations of http2.header, though I see it as a problematic limitation non the less.

Actions #9

Updated by Philippe Antoine almost 2 years ago

So, trying to sump up, there are 2 issues :
- http2.header should match http.header behavior even if I do not like this name, I can rename the current http2.header keyword to http2.single_header or such...
- multi buffer should have a mechanism to match different content on different buffer

How does that sound ?

alert http any any -> any any (msg:"HTTP2 - Two Headers - Authority/Method"; http.header; content:"method: GET"; http.header; content:"authority: bugertor.com"; sid:4;). works on master-6.0.x with http1-rules

Actions #10

Updated by Brandon Murphy almost 2 years ago

Philippe Antoine wrote in #note-9:

So, trying to sump up, there are 2 issues :
- http2.header should match http.header behavior even if I do not like this name, I can rename the current http2.header keyword to http2.single_header or such...

Ok, this seems great. I agree, I've never been a fan of the name and I've always wondered why it wasn't http.headers (plural) but I assume the answer is legacy compatibility.

- multi buffer should have a mechanism to match different content on different buffer

Sorry, I'm not sure I fully understand/follow this, can you provide more detail?

How does that sound ?

alert http any any -> any any (msg:"HTTP2 - Two Headers - Authority/Method"; http.header; content:"method: GET"; http.header; content:"authority: bugertor.com"; sid:4;). works on master-6.0.x with http1-rules

Awesome, I learned something new there!

Actions #11

Updated by Philippe Antoine almost 2 years ago

  • Status changed from Assigned to In Review

Brandon, what do you think about https://github.com/OISF/suricata/pull/8371 ?

I assume the answer is legacy compatibility.

I think it is the answer...

Actions #12

Updated by Philippe Antoine almost 2 years ago

  • Related to Feature #5784: detect: allow cross buffer inspection on multi-buffer matches added
Actions #13

Updated by Brandon Murphy almost 2 years ago

Brandon, what do you think about https://github.com/OISF/suricata/pull/8371 ?

I'll be honest, I am not familiar enough with Suri's code base (or code in general) to make any judgment about how this looks.

I have to rely on the PR/Code Review process to flush this out!

Actions #14

Updated by Philippe Antoine almost 2 years ago

  • Target version changed from TBD to 7.0.0-rc1
Actions #15

Updated by Philippe Antoine almost 2 years ago

  • Target version changed from 7.0.0-rc1 to 7.0.0-rc2
Actions #16

Updated by Victor Julien almost 2 years ago

Thinking about this I agree we need a solution but also agree that the http_header / http.header keyword is poorly named. How about we do something like

http.request_header - matches each individual header (the single_header suggestion)
http.request_headers - http.header behavior limited to requests
http.request_headers.raw - http.header.raw for requests
http.response_header
http.response_headers
http.response_headers.raw

These would all apply to http1 and http2, where they can be limited to a http version through alert http1 or I suppose http.protocol; content:"HTTP/1"

Actions #17

Updated by Philippe Antoine almost 2 years ago

But we keep old http.header for compatibility, right ?
And do we change http2.header behavior ? to match the one from http.header (while creating http.request_header to have the single header feature)

Actions #18

Updated by Victor Julien almost 2 years ago

http_header and http.header need to stay as is.

Wrt http2.header, we should probably remove it. It's not part of a stable release as a non-experimental release yet, so we still have freedom there.

Actions #20

Updated by Brandon Murphy almost 2 years ago

Sorry, i'm a bit late getting caught up here, but regarding a solution to support buffers of individual headers, I'd rather see this feature request implemented

https://redmine.openinfosecfoundation.org/issues/5775

Actions #21

Updated by Philippe Antoine over 1 year ago

  • Status changed from In Review to Resolved
Actions #22

Updated by Philippe Antoine over 1 year ago

https://github.com/OISF/suricata/pull/9013 for adding a warning in master6

Actions #23

Updated by Philippe Antoine over 1 year ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF