Project

General

Profile

Actions

Task #4067

closed

Feature #4201: http2: full protocol support

http2: overload existing http keywords to support http/2

Added by Victor Julien over 3 years ago. Updated 10 months ago.

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

Description

Meta tickets. Please create evaluate all existing http keywords and see if we can support them in http/2. For the ones we can, please create a sub-ticket each keyword. For the ones we can't support we need an explanation of why (in this ticket) and a documentation update in the user guide.


Files

glupteba_http2_1_anon_1.pcap (66 KB) glupteba_http2_1_anon_1.pcap Brandon Murphy, 12/29/2022 08:20 PM

Related issues 3 (0 open3 closed)

Related to Suricata - Documentation #5962: documentation: mention the use of http1 in rule protocolClosedPhilippe AntoineActions
Related to Suricata - Feature #5972: rules: "requires" keyword representing the minimum version of suricata to support the ruleClosedJason IshActions
Blocked by Suricata - Bug #6025: detect: allow bsize 0 for existing empty buffersClosedPhilippe AntoineActions
Actions #1

Updated by Victor Julien over 3 years ago

  • Priority changed from Normal to High
Actions #2

Updated by Victor Julien over 3 years ago

  • Subject changed from http/2: overload existing http keywords to support http/2 to http2: overload existing http keywords to support http/2
Actions #3

Updated by Victor Julien over 3 years ago

  • Parent task set to #4201
Actions #4

Updated by Philippe Antoine about 3 years ago

  • Status changed from Assigned to In Review
Actions #5

Updated by Philippe Antoine about 3 years ago

Some keywords are now working. Need to complete with all keywords

Actions #6

Updated by Philippe Antoine almost 3 years ago

One keyword may not be translated :
http.stat_msg has no HTTP2 meaning (besides translating the status code)

Actions #7

Updated by Philippe Antoine almost 3 years ago

http.start does not seem to make sense for HTTP2 in my humble opinion

Actions #8

Updated by Philippe Antoine almost 3 years ago

http.response_body seems to be handled by file_data for HTTP2

Actions #9

Updated by Philippe Antoine almost 3 years ago

There is no such thing as http.response_line in HTTP2

Actions #10

Updated by Philippe Antoine almost 3 years ago

nor http.request_line

There is no http.protocol, or it is always HTTP2

Actions #11

Updated by Philippe Antoine almost 3 years ago

http.request_body should be covered by file_data

Actions #12

Updated by Philippe Antoine almost 3 years ago

Should HTTP2MimicHttp1Request translate headers names ? like Host becomes :authority from HTTP1 to HTTP2

How would we do the http.host normalisation ?

Should we concatenate the values in case there are multiple times the same header (name) in HTTP2 ?

Actions #13

Updated by Philippe Antoine almost 3 years ago

http.cookie calls DetectAppLayerMpmRegister2 with SIG_FLAG_TOCLIENT and HTP_REQUEST_HEADERS, that should rather be HTP_RESPONSE_HEADERS, right ?

Actions #14

Updated by Victor Julien almost 3 years ago

https://github.com/OISF/suricata/pull/6087 was merged towards this ticket. It is not complete as some body keywords are missing as mentioned in the PR:

http.request_body and http.response_body, covered by file_data

Actions #15

Updated by Philippe Antoine almost 3 years ago

What remains to be done :

- http.host : do the same normalization... same for http.header. For http.header.raw it is not raw in HTTP2, we need to concatenate key and value. For http.header_names, we can have linefeeds in HTTP2 header names, should we escape them ?
- Concatenate when we get multiple values for one header name cf https://suricata.readthedocs.io/en/suricata-6.0.0/rules/http-keywords.html#id2 example request with 2 Hosts ?
- Make HTTP2MimicHttp1Request translate header names (Host becomes :authority) ?
- http.request_body and http.response_body, covered by file_data. Should we have these specifically ?
- http.request_line and http.response_line do not exist in HTTP2, should we emulate them ? What about http.start ?
- http.protocol and http.stat_msg are implicit, should we emulate them ?

Actions #16

Updated by Philippe Antoine over 2 years ago

After https://github.com/OISF/suricata/pull/6183
There will be the following questions where we want the opinion of signature writers :
- http.request_body and http.response_body, covered by file_data. Should we have these specifically ?
- http.request_line and http.response_line do not exist in HTTP2, should we emulate them ? What about http.start ?
- http.protocol and http.stat_msg are implicit, should we emulate them ?

Actions #17

Updated by Jason Williams over 2 years ago

There will be the following questions where we want the opinion of signature writers :

- http.request_body and http.response_body, covered by file_data. Should we have these specifically ?
- http.request_line and http.response_line do not exist in HTTP2, should we emulate them ? What about http.start ?
- http.protocol and http.stat_msg are implicit, should we emulate them ?

To retain compatibility of current rules in ET ruleset for example, emulation would likely be necessary of all of these keywords are used fairly heavily

Actions #18

Updated by Philippe Antoine over 2 years ago

Merge of https://github.com/OISF/suricata/pull/6328

Only emulation remaining

Actions #19

Updated by Philippe Antoine over 1 year ago

  • Target version changed from 7.0.0-beta1 to TBD
Actions #20

Updated by Victor Julien over 1 year ago

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

Updated by Philippe Antoine over 1 year ago

Do you know what you want for 7 rc1 ?

That is do you know what should be done for
- http.request_body and http.response_body, covered by file_data. Should we have these specifically ?
- http.request_line and http.response_line do not exist in HTTP2, should we emulate them ? What about http.start ?
- http.protocol and http.stat_msg are implicit, should we emulate them ?

Actions #22

Updated by Philippe Antoine over 1 year ago

  • Status changed from In Review to Feedback
  • Assignee changed from Philippe Antoine to Victor Julien
  • Priority changed from High to Normal
Actions #23

Updated by Brandon Murphy about 1 year ago

Philippe Antoine wrote in #note-21:

Do you know what you want for 7 rc1 ?

I know this questions isn't being asked to me, but I've recently been working on some HTTP2 traffic and finding difficulty in sigging them up with some of these keywords.

That is do you know what should be done for
- http.request_body and http.response_body, covered by file_data. Should we have these specifically ?

The ET ruleset makes use of http.request_body and http.response_body, increasingly so over the past while due to https://redmine.openinfosecfoundation.org/issues/4378. It'd be our preference to emulate these.

- http.request_line and http.response_line do not exist in HTTP2, should we emulate them ? What about http.start ?

We do make use of http.rqeuest_line, specifically when it can provide a better fast_pattern than just the URI alone. If I know it's always the same HTTP method and the URI startswith a specific static content, we'll combine them into a request_line and get a better fast_pattern out of it. We can split the use of this, and discontinue the use of http.request_line if needed, but there would be an unknown performance impact to the rules that currently use http.request_line.

- http.protocol and http.stat_msg are implicit, should we emulate them ?

Today, we have some rules that want to make use of http.protocol to specifically negate HTTP/2 traffic, 2845391 - ETPRO INFO HTTP Request with Lowercase user-agent Header Observed is a good example of that, where this should not alert on HTTP/2 traffic. From what I can tell, there is a slight different in behavior between 6.0.9 and git-master when including http.protocol keyword in a pcap with HTTP2 traffic.

- 6.0.9 seems to disregard the keyword all together, are given the example signature. Including http.protocol; content:"http"; nocase fires, but so does http.protocol; content:!"http"; nocase. I would expect only one of these should alert, but both do.

- git-master seems to not fire at all when including any http.protocol keyword. Including either http.protocol; content:"http"; nocase or http.protocol; content:!"http"; nocase results in no alerts. I would expect at least one of these should alert, but neither do.

To be clear on both versions, this was tested with http2 enabled in app-layer-protos and http1-rules also enabled.

uploaded a pcap just incase that's useful.

Actions #24

Updated by Victor Julien about 1 year ago

  • Target version changed from 7.0.0-rc1 to 8.0.0-beta1
Actions #25

Updated by Victor Julien about 1 year ago

Some conclusions after a recent conversation about it:

http.protocol would contain HTTP/2
http.request_line would contain: <METHOD> <URI> HTTP/2\r\n
http.request_body (http_client_body) same as file.data toserver
http.response_body (http_server_body) same as file.data toclient

Open question is http.stat_msg. One way could be to use a common dict to map code to message here. E.g. 404 -> "Not found", but this feels very artificial. Another would be to make the buffer "empty". This would mean a negated match against it would work, but no positive matches.

@Brandon Murphy any thoughts on the above?

Actions #26

Updated by Brandon Murphy about 1 year ago

The current use of http.stat_msg within the ET ruleset could be replaced by using the stat_code instead. Off the top of my head, I would think the stat_msg would be handy is if it doesn't match the normal stat_code. In looking at the ruleset, it seems to have been used as just another way of matching on the stat_code. Looks like a pretty easy effort to remove the usage of the http.stat_msg keyword within the ET ruleset.

This would allow for the buffer being blank for HTTP/2 traffic and in effect, we'd just depreciate the use of it within the ET ruleset unless combined with a http.protocol keyword that would enforce HTTP/1.

Actions #27

Updated by Philippe Antoine 12 months ago

  • Status changed from Feedback to In Progress
  • Assignee changed from Victor Julien to Philippe Antoine
Actions #28

Updated by Philippe Antoine 12 months ago

  • Target version changed from 8.0.0-beta1 to 7.0.0-rc2
Actions #29

Updated by Philippe Antoine 12 months ago

Today, we have some rules that want to make use of http.protocol to specifically negate HTTP/2 traffic

zoomequipd why do not you use @alert http1 then ? (I think it would be better for perf as it gets pre filtered first)

I do not know for 6.0.9, but I expected git master to have no alerts on http.protocol; content:"http"; nocase or http.protocol; content:!"http"; nocase with HTTP2 traffic since http.protocol implied HTTP1
Using a constant buffer HTTP/2. will make the first one match (and not the second one).
We could also use an empty buffer, which will make the second one match (and not the first one).

Actions #30

Updated by Philippe Antoine 12 months ago

Would you like a linter warning that a rule alert http is in fact alert http1 ? (or alert http2)

Actions #31

Updated by Philippe Antoine 12 months ago

  • Status changed from In Progress to In Review
Actions #32

Updated by Brandon Murphy 12 months ago

Philippe Antoine wrote in #note-29:

Today, we have some rules that want to make use of http.protocol to specifically negate HTTP/2 traffic

zoomequipd why do not you use @alert http1 then ? (I think it would be better for perf as it gets pre filtered first)

Because this was introduced in Suri 6 and we do not currently have a suri 6 specific ruleset to make use of this protocol. Also, it's undocumented (https://suricata.readthedocs.io/en/suricata-6.0.10/rules/intro.html?highlight=protocols#protocol)

I do not know for 6.0.9, but I expected git master to have no alerts on http.protocol; content:"http"; nocase or http.protocol; content:!"http"; nocase with HTTP2 traffic since http.protocol implied HTTP1

You had previously asked if http.protocol should be emulated for HTTP/2. It seems to be for backward compatibility with rules this would be desired.

Using a constant buffer HTTP/2. will make the first one match (and not the second one).

A constant buffer would be my preferred solution, though I'm not sure what the implications are of that as it relates to HTTP/3 or other future versions.

Actions #33

Updated by Brandon Murphy 12 months ago

I was looking through some of the previous commits around HTTP2, was http.stat_msg not included in this merge? Maybe it was subsequently removed?

https://github.com/OISF/suricata/pull/5875/commits/5465e0b154ff7ede3e9c423781cf75156d1d1d70

Edit - Is this the implementation of the "map" that Victor mentioned above or it just empty for HTTP/2?

Actions #34

Updated by Philippe Antoine 12 months ago

  • Related to Documentation #5962: documentation: mention the use of http1 in rule protocol added
Actions #35

Updated by Philippe Antoine 12 months ago

Brandon Murphy wrote in #note-33:

I was looking through some of the previous commits around HTTP2, was http.stat_msg not included in this merge? Maybe it was subsequently removed?

https://github.com/OISF/suricata/pull/5875/commits/5465e0b154ff7ede3e9c423781cf75156d1d1d70

This was a confusion from me at the time between stat-code and stat-msg :-/

Edit - Is this the implementation of the "map" that Victor mentioned above or it just empty for HTTP/2?

In the current PR, I put an empty field for HTTP/2
What do you think about this ?

Because this was introduced in Suri 6 and we do not currently have a suri 6 specific ruleset to make use of this protocol. Also, it's undocumented (https://suricata.readthedocs.io/en/suricata-6.0.10/rules/intro.html?highlight=protocols#protocol)

Indeed, nice catch.
So, I created https://redmine.openinfosecfoundation.org/issues/5962

About this, could a keyword to indicate the minimum suricata version for a rule be helpful ?

You had previously asked if http.protocol should be emulated for HTTP/2. It seems to be for backward compatibility with rules this would be desired.

Sure, but I propose to get master branch to have the correct behavior, then backport/fix master-6

Actions #36

Updated by Brandon Murphy 12 months ago

Philippe Antoine wrote in #note-35:

In the current PR, I put an empty field for HTTP/2
What do you think about this ?

I'd prefer it to be HTTP/2 to mimic the behavior as it its with HTTP/1

About this, could a keyword to indicate the minimum suricata version for a rule be helpful ?

Yes, this would be really interesting! Perhaps the engine could throw a warning on and not load the rule if the version is too low?
Would you like me to make a feature request?

Actions #37

Updated by Philippe Antoine 12 months ago

The empty buffer is http_stat_msg for HTTP2

http.protocol is filled with HTTP/2

Yes, this would be really interesting! Perhaps the engine could throw a warning on and not load the rule if the version is too low?

Would you like me to make a feature request?

Please do so and assign it to me, but check if there exists one already ;-)

Actions #38

Updated by Brandon Murphy 12 months ago

Philippe Antoine wrote in #note-37:

The empty buffer is http_stat_msg for HTTP2

Ahhh, ok that sounds fine. I wonder if it would be worth adding an HTTP/2 Nuances section on each keyword that behaves a bit differently for HTTP/2.

http.protocol is filled with HTTP/2

Perfect.

Yes, this would be really interesting! Perhaps the engine could throw a warning on and not load the rule if the version is too low?

Would you like me to make a feature request?

Please do so and assign it to me, but check if there exists one already ;-)

Will do!

Actions #40

Updated by Philippe Antoine 11 months ago

  • Related to Feature #5972: rules: "requires" keyword representing the minimum version of suricata to support the rule added
Actions #41

Updated by Philippe Antoine 11 months ago

  • Blocked by Bug #6025: detect: allow bsize 0 for existing empty buffers added
Actions #42

Updated by Philippe Antoine 10 months ago

  • Status changed from In Review to Closed
Actions

Also available in: Atom PDF