Project

General

Profile

Bug #2810

enabling add request/response http headers in master

Added by Peter Manev 8 months ago. Updated 5 days ago.

Status:
New
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

I was trying out the new response/request header feature ( https://github.com/OISF/suricata/pull/3639 - many thanks for the contribution !) for login and noticed the following

Using latest master as of the time of posting this issue:

suricata -V
This is Suricata version 5.0.0-dev (rev 6c0ec0b2)

Default in yaml

            # set this value to one among {both, request, response} to dump all
            # http headers for every http request and/or response
            # dump-all-headers: [both, request, response]

Given the default setting above in yaml a user may just try to adjust the dump-all-headers and use only "both" or "request, response" inside the []. In those cases the logging of the response/request headers will not work as intended (at least in my tests). See bellow:

Does not work

            # set this value to one among {both, request, response} to dump all
            # http headers for every http request and/or response
            dump-all-headers: [request, response]

Does not work
            # set this value to one among {both, request, response} to dump all
            # http headers for every http request and/or response
            dump-all-headers: [both]

Does not work
            # set this value to one among {both, request, response} to dump all
            # http headers for every http request and/or response
            dump-all-headers: "response, request" 

Works
            # set this value to one among {both, request, response} to dump all
            # http headers for every http request and/or response
            dump-all-headers: "both" 

Thanks Andreas for the pointer.

History

#1

Updated by Victor Julien 8 months ago

  • Assignee set to Maurizio Abba
#2

Updated by Andreas Herz 4 months ago

  • Target version set to TBD
#3

Updated by Victor Julien 20 days ago

  • Assignee changed from Maurizio Abba to OISF Dev
  • Target version changed from TBD to 5.0.0
#4

Updated by Philippe Antoine 15 days ago

The code responsible for this is

        const char *all_headers = ConfNodeLookupChildValue(
                conf, "dump-all-headers");
        if (all_headers != NULL) {
            if (strcmp(all_headers, "both") == 0) {
                http_ctx->flags |= LOG_HTTP_REQ_HEADERS;
                http_ctx->flags |= LOG_HTTP_RES_HEADERS;
            } else if (strcmp(all_headers, "request") == 0) {
                http_ctx->flags |= LOG_HTTP_REQ_HEADERS;
            } else if (strcmp(all_headers, "response") == 0) {
                http_ctx->flags |= LOG_HTTP_RES_HEADERS;
            }
        }

So, I would change the comment in the yaml.
Maybe set this value to one among could become
set this value to one and only one among

And then # dump-all-headers: none
As I was told that we should put the default value in the comment

#5

Updated by Victor Julien 7 days ago

Right, I think if we put 'none' there it should also be handled in the code (and reject all other values).

#6

Updated by Victor Julien 5 days ago

  • Target version changed from 5.0.0 to 5.0.1

Also available in: Atom PDF