Task #4067
closedFeature #4201: http2: full protocol support
http2: overload existing http keywords to support http/2
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
Updated by Victor Julien about 4 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
Updated by Philippe Antoine almost 4 years ago
- Status changed from Assigned to In Review
Updated by Philippe Antoine almost 4 years ago
Some keywords are now working. Need to complete with all keywords
Updated by Philippe Antoine over 3 years ago
One keyword may not be translated :http.stat_msg
has no HTTP2 meaning (besides translating the status code)
Updated by Philippe Antoine over 3 years ago
http.start
does not seem to make sense for HTTP2 in my humble opinion
Updated by Philippe Antoine over 3 years ago
http.response_body
seems to be handled by file_data for HTTP2
Updated by Philippe Antoine over 3 years ago
There is no such thing as http.response_line
in HTTP2
Updated by Philippe Antoine over 3 years ago
nor http.request_line
There is no http.protocol
, or it is always HTTP2
Updated by Philippe Antoine over 3 years ago
http.request_body
should be covered by file_data
Updated by Philippe Antoine over 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 ?
Updated by Philippe Antoine over 3 years ago
http.cookie
calls DetectAppLayerMpmRegister2
with SIG_FLAG_TOCLIENT
and HTP_REQUEST_HEADERS
, that should rather be HTP_RESPONSE_HEADERS
, right ?
Updated by Victor Julien over 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
Updated by Philippe Antoine over 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 ?
Updated by Philippe Antoine over 3 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 ?
Updated by Jason Williams over 3 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
Updated by Philippe Antoine over 3 years ago
Merge of https://github.com/OISF/suricata/pull/6328
Only emulation remaining
Updated by Philippe Antoine over 2 years ago
- Target version changed from 7.0.0-beta1 to TBD
Updated by Victor Julien about 2 years ago
- Target version changed from TBD to 7.0.0-rc1
Updated by Philippe Antoine about 2 years 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 ?
Updated by Philippe Antoine about 2 years ago
- Status changed from In Review to Feedback
- Assignee changed from Philippe Antoine to Victor Julien
- Priority changed from High to Normal
Updated by Brandon Murphy almost 2 years 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.
Updated by Victor Julien almost 2 years ago
- Target version changed from 7.0.0-rc1 to 8.0.0-beta1
Updated by Victor Julien over 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 toserverhttp.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?
Updated by Brandon Murphy over 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.
Updated by Philippe Antoine over 1 year ago
- Status changed from Feedback to In Progress
- Assignee changed from Victor Julien to Philippe Antoine
Updated by Philippe Antoine over 1 year ago
- Target version changed from 8.0.0-beta1 to 7.0.0-rc2
Updated by Philippe Antoine over 1 year 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).
Updated by Philippe Antoine over 1 year ago
Would you like a linter warning that a rule alert http
is in fact alert http1
? (or alert http2
)
Updated by Philippe Antoine over 1 year ago
- Status changed from In Progress to In Review
Updated by Brandon Murphy over 1 year 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 sincehttp.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.
Updated by Brandon Murphy over 1 year 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?
Updated by Philippe Antoine over 1 year ago
- Related to Documentation #5962: documentation: mention the use of http1 in rule protocol added
Updated by Philippe Antoine over 1 year 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
Updated by Brandon Murphy over 1 year 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?
Updated by Philippe Antoine over 1 year 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 ;-)
Updated by Brandon Murphy over 1 year 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!
Updated by Philippe Antoine over 1 year ago
https://github.com/OISF/suricata/pull/8687 by the way
Updated by Philippe Antoine over 1 year ago
- Related to Feature #5972: rules: "requires" keyword representing the minimum version of suricata to support the rule added
Updated by Philippe Antoine over 1 year ago
- Blocked by Bug #6025: detect: allow bsize 0 for existing empty buffers added
Updated by Philippe Antoine over 1 year ago
- Status changed from In Review to Closed