Suricata

Addtiional Reference: RFC 9110 - https://www.rfc-editor.org/rfc/rfc9110#name-deprecation-of-userinfo-in-

A sender MUST NOT generate the userinfo subcomponent (and its "@" delimiter) when an "http" or "https" URI reference is generated within a message as a target URI or field value.

Before making use of an "http" or "https" URI reference received from an untrusted source, a recipient SHOULD parse for userinfo and treat its presence as an error; it is likely being used to obscure the authority for the sake of phishing attacks.

I'm not sure why this issue shows as "resolved".

I would prefer an event as we can then use that app-layer-event in other rules. Additionally, the RFC states "treat its presence as an error" - which seems like a good match for an app-layer-event.

And normalizing it out of the uri lets us do things like this where we don't want to inspect the values of the user info, but can still use it's presence within the rule logic via the app-layer-event.

alert http2 any any -> any any (msg:"Bad thing that has user info"; flow:established,to_server; app-layer-event:http2.userinfo_in_uri; http.method; content:"POST"; http.uri; bsize:8; content:"/foo.php"; sid:1; rev:1;)

of course, if I wanted to inspect the values of the user info, I can use `http.request_line`.

Heck, given that user info can be in the uri for HTTP/1, maybe there should be a `http.user_info` buffer and it this info gets thrown into it.

idk if i'm reading this right but it looks like maybe the user info is already getting normalized out of the uri?

https://github.com/OISF/suricata/blob/c8a7204b159553d338a6294218e696a72efdb4db/src/app-layer-htp-libhtp.c#L78-L90

I would prefer an event as we can then use that app-layer-event in other rules. Additionally, the RFC states "treat its presence as an error" - which seems like a good match for an app-layer-event.

You can use

http.request_header; content:":authority: "; startswith; content: "@"; pcre: /:authority: ([^:]*:[^@]*)@/,flow:http2_userinfo;

Um...yeah? but that would also happen at the rule level, which I can't imagine is very optimal compared to putting it in C or Rust (though you'd be the one to know). It would also only benefit users with the specific ruleset containing the rule, so in my case only ET users. Whereas as an event would benefit all suricata users.

Additionally, we'd have to include that logic in every rule that we wanted to test for user info, which, i'm guessing negatively impacts performance as more rules contain that logic? (reminds me of the base64 pcre, or the pcre for an ip address that is used in hundreds of rules)

Um...yeah? but that would also happen at the rule level, which I can't imagine is very optimal compared to putting it in C or Rust (though you'd be the one to know).

One problem is that the app-layer-event gets computed every time, even if you do not have rules...

I get the point that this expression is not the simplest :-)

Philippe Antoine wrote in #note-10:

One problem is that the app-layer-event gets computed every time, even if you do not have rules...

But if libhtp is already parsing uri->username and uri->password, can't this be leveraged without introducing much new overhead?

https://github.com/OISF/libhtp/blob/0.5.x/htp/htp_util.c#L787-L808

I get the point that this expression is not the simplest :-)

I guess so, my apologizes because I hadn't expected it to be a complex task.

Ok, so can we focus on the how, with respect to HTTP/2, userinfo currently ends up within http.host? Do you want a different ticket for that?

But if libhtp is already parsing uri->username and uri->password, can't this be leveraged without introducing much new overhead?

libhtp does HTTP1, not HTTP2

I guess so, my apologizes because I hadn't expected it to be a complex task.

The feature is simple as you can see in the PR.
I am not confident into assessing the runtime cost vs other options...

Ok, so can we focus on the how, with respect to HTTP/2, userinfo currently ends up within http.host? Do you want a different ticket for that?

For me this ticket is about this.

If you want a `http.user_info` buffer, this could be a new ticket

Philippe Antoine wrote in #note-13:

But if libhtp is already parsing uri->username and uri->password, can't this be leveraged without introducing much new overhead?

libhtp does HTTP1, not HTTP2

see, this is why i can't be trusted to look at code :-)

If you want a `http.user_info` buffer, this could be a new ticket

k. IMO, this seems like a good choice to have for HTTP/1. I'll try to get one started.

So, I tested the normalized http.host field when userinfo is included in the :authority header. based on the attached pcap, only the username field makes it into the http.host buffer.

Sids 2 and 3 fire on the attached pcap.

alert http any any -> any any (msg:"user info in http.host"; flow:established,to_server; http.host; content:"username|3a|password"; sid:1;)
alert http any any -> any any (msg:"user info in http.host"; flow:established,to_server; http.host; content:"username"; sid:2;)
alert http any any -> any any (msg:"user info in http.host"; flow:established,to_server; http.host; bsize:8; content:"username"; sid:3;)

I shouldn't be trusted to read this, but I think this section of code is extracting anything "up to" the first colon in the authority?

https://github.com/OISF/suricata/blob/c8a7204b159553d338a6294218e696a72efdb4db/rust/src/http2/detect.rs#L617-L624

thus resulting in the "username" value making it to http.host;

Looks like a bug deserving its own ticket, right ?

I guess if you think so.

IMO it fell under the for the information to be normalized out when populating the http.host buffer part of the request because IMO, none of the userinfo should show up in http.host at all.

i'll go ahead and create one.

Brandon Murphy wrote in #note-17:

i'll go ahead and create one.

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

https://github.com/OISF/suricata-verify/pull/1480 left to mark this as closed