Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doc/userguide: more eve http upgrade notes #9166

Closed
wants to merge 1 commit into from
Closed

doc/userguide: more eve http upgrade notes #9166

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions doc/userguide/upgrade.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,53 @@ Logging changes
- Protocol values and their names are built into Suricata instead of using the system's ``/etc/protocols`` file. Some names and casing may have changed
in the values ``proto`` in ``eve.json`` log entries and other logs containing protocol names and values.
See https://redmine.openinfosecfoundation.org/issues/4267 for more information.
- Custom logging of HTTP headers via suricata.yaml ``outputs.eve-log.types.http.custom``
is now done in subobjects ``response_headers`` or ``request_headers`` (as for option ``dump-all-headers``)
instead of at the root of the ``http`` json object (to avoid some collisions).
- Logging of additional HTTP headers configured through the EVE
``http.custom`` option will now be logged in the ``request_headers``
and/or ``response_headers`` respectively instead of merged into the
existing ``http`` object. In Suricata 6.0, a configuration like::

http:
custom: [Server]

would result in a log entry like::

"http": {
"hostname": "suricata.io",
"http_method": "GET",
"protocol": "HTTP/1/1",
"server": "nginx",
...
}

This merging of custom headers in the ``http`` object could result
in custom headers overwriting standard fields in the ``http``
object, or a response header overwriting request header.

To prevent the possibility of fields being overwritten, **all**
custom headers are now logged into the ``request_headers`` and
``response_headers`` arrays to avoid any chance of collision, plus
allowing headers that may be present multiple times to have each
occurrence logged. These arrays are not new in Suricata 7.0, but
have only previously been used for the ``dump-all-headers`` option.

As of Suricata 7.0, the above configuration example will now be
logged like::

"http": {
"hostname": "suricata.io",
"http_method": "GET",
"protocol": "HTTP/1/1",
"response_headers": [
{ "name": "Server", "value": "nginx" }
]
}
Comment on lines +100 to +107
Copy link
Member Author

@jasonish jasonish Jul 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this reinforces why we need to do this and I don't see a sane way to maintain compatibility. Lets take an arbitrary header name "Foo" that could exist in the request and response.

If we do:

"http": {
  "foo": "value",
}

was the server or response?

Further, what should we do about a header that appears multiple times. We then end up with:

"http": {
  "foo": ["value"],
}

(Elastic does fine with those arrays)

This may lead to one wanting:

"http": {
  "request_headers": {
    "foo": ["one", "two", "three"]
  }
}

but this conflicts with our existing request_headers and response_headers that are already arrays. We also can't make this objects for dump-all-headers as Elastic doesn't like arbitrary field growth, or certain chars in the keys.

As I mentioned over in https://redmine.openinfosecfoundation.org/issues/6173, we could make sure the "normalized" header names allowed in custom all have distinct names that don't overwrite any existing field name.. We should also prefix them with request_ and response_.

So for the custom: [Server] example I give, this would mean logging like:

"http": {
  "request_server": "nginx",
  ...
}

(could also be "request_server": ["nginx"] to handle multiple occurrences of a header.

This does feel like a bit of a hack, and will still break many reports etc. that will need to be updated for the new field names. But perhaps a little easier to adapt to than the array approach for 7.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure libhtp won't allow key duplication in a single direction:

User-Agent: abc
User-Agent: def

will end up as

User-Agent: abc, def

Where abc, def is a single string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is rather Suricata doing a join(",") on libhtp's headers table

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So duplicate headers would be handled differently between http 1 and 2?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I think http2 has the same join(",") normalization at some point

But that needs to be tested ;-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So looks like detection works on normalized with join(",") but logging logs only the last value 😨


Effectively making the ``custom`` option a subset of the
``dump-all-headers`` option.

If you use the ``custom`` option this may be a breaking
change. However if you do not use it, there is no change in the
output.

Deprecations
~~~~~~~~~~~~
Expand Down
Loading