Suricata

This seems to be the same issue as #1275. Libhtp merges the 2 records:

 0000  53 75 72 69 2D 43 75 73  74 6F 6D                  Suri-Cus tom
 0000  41 41 41 41 2C 20 42 42  42 42                     AAAA, BB BB

So it sort of functions as intended, at least intended by libhtp originally. I do think it's probably not correct for Suricata, as ppl have an expectation about what http_header, http_user_agent, http_cookie, http_host inspect.

Yeah, I forgot to update this ticket after looking in to the issue further the other week. You are correct, when there are multiples of the same header, the values will be concatenated, in the order seen from top to bottom, with a comma and space (", ") between each of them. This is especially worth noting for the normalized individual http buffers - 'http_user_agent', 'http_host', and 'http_cookie'.

I forgot to add to my last update that this isn't necessarily a bug, just something to be aware of when writing rules.

Here is a small reminder. The RFC 7230 states :

If a message is received that has multiple Content-Length header
fields with field-values consisting of the same decimal value , or a
single Content-Length header field with a field value containing a
list of identical decimal values (e.g., "Content-Length: 42, 42"),
indicating that duplicate Content-Length header fields have been
generated or combined by an upstream message processor, then the
recipient MUST either reject the message as invalid or replace the
duplicated field-values with a single valid Content-Length field
containing that decimal value prior to determining the message body
length or forwarding the message.

and

If a message is received without Transfer-Encoding and with
either multiple Content-Length header fields having differing
field-values or a single Content-Length header field having an
invalid value, then the message framing is invalid and the
recipient MUST treat it as an unrecoverable error.

A pull request is available here :
https://github.com/OISF/libhtp/pull/187

We log only a warning so as to keep parsing.
In case of different values, we take the latest one (same behavior as Wireshark)
Apache returns code 400 (bas request) for a request with two Content-Length headers, even with the same value, and ends the connection.

There should be a patch as well in Suricata :

diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c
index d01f420fd..6b331d92f 100644
--- a/src/app-layer-htp.c
+++ b/src/app-layer-htp.c
@@ -511,6 +511,7 @@ struct {
     { "C-E gzip has abnormal value", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER},
     { "C-E deflate has abnormal value", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER},
     { "C-E unknown setting", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER},
+    { "Ambiguous C-L value", HTP_REQUEST_INVALID_C_L},

But I am not sure if this needs a bigger update as a comment states `updated up to libhtp 0.5.7` and we are now at libhtp 0.5.29

It should now be ok with :
https://github.com/OISF/libhtp/pull/189
https://github.com/OISF/suricata/pull/3700
https://github.com/OISF/suricata-verify/pull/21

Except that there seems to be a bigger bug : we can bypass the alert by using request pipelining.
My solution would be to make libhtp assign the log->tx variable so that suricata in app-layer-htp.c can assign the app-layer-event to the right transaction id.
But I would prefer someone with a better overview to give a direction.

By the way, why is it the right model to associate an app-layer-event only with the transaction id, and rely on the rules to filter on direction ?

Maybe we should wait for https://redmine.openinfosecfoundation.org/issues/1656 as we do not want to break other cases

Pull request for libhtp should now be the bigger one
https://github.com/OISF/libhtp/pull/194

Other pull requests (suricata and suricata-verify) should go unchanged