Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] fix weakness by logging of broken header by incorect proxy protocol (IDS/IPS/LOG-analysis)

Dipl. Ing. Sergey Brester via nginx-devel
September 28, 2022 07:08AM
Sure, this was also my first intention. Just after all I thought the
whole buffer
could be better in order to provide a possibility to debug for someone
searching
for a bug. But there are another aids that would help, so indeed let it
be so.

As for the rest, well it is surely a subject of different discussion.
However I think
someone who monitoring logs with foreign data is able to ignore wrong
chars
or unsupported surrogates using appropriate encoding facilities, but...
But I would suggest at least to replace a quote-char to provide
opportunity
to bypass the buffer in quotes with something similar this `... header:
"[^"]*" ...`
if parsing of such lines is needed.

So may be this one either:

+ for (p = buf; p != last; p++) {
+ if (*p == '"') {
+ *p = '''; continue;
+ }
+ if (*p == CR || *p == LF) {
+ break;
+ }
+ }

Although I don't believe the safe data (like IP, etc) shall take place
after "unsafe"
(foreign) input (especially of variable length), but it is rather a
matter of common
logging format for the error-log.
I mean normally one would rather expect something like that:

- [error] 13868#6132: *1 broken header: "...unsafe..." while reading
PROXY protocol, client: 127.0.0.1, server: 0.0.0.0:80

+ [error] 13868#6132: while reading PROXY protocol, client: 127.0.0.1,
server: 0.0.0.0:80 - *1 broken header: "...unsafe..."

Unfortunatelly errorlog is not configurable at the moment at all.

28.09.2022 12:02, Roman Arutyunyan wrote:

> Hi Sergey,
>
> Thanks for reporting this. The issue indeed needs to be fixed. Attached is
> a patch similar to yours that does this. I don't think we need to do anything
> beyond just cutting the first line since there's another similar place in
> nginx - ngx_http_log_error_handler(), where exactly that is implemented.
>
> Whether we need to skip special characters when logging to nginx log is
> a topic for a bigger discussion and this will require a much bigger patch.
> I suggest that we only limit user data to the first line now.
>
> [..]
>
> --
> Roman Arutyunyan
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

[PATCH] fix weakness by logging of broken header by incorect proxy protocol (IDS/IPS/LOG-analysis)

Dipl. Ing. Sergey Brester via nginx-devel 342 September 26, 2022 05:18PM

Re: [PATCH] fix weakness by logging of broken header by incorect proxy protocol (IDS/IPS/LOG-analysis)

Maxim Dounin 45 September 27, 2022 05:08PM

Re: [PATCH] fix weakness by logging of broken header by incorect proxy protocol (IDS/IPS/LOG-analysis)

Roman Arutyunyan 57 September 28, 2022 06:04AM

Re: [PATCH] fix weakness by logging of broken header by incorect proxy protocol (IDS/IPS/LOG-analysis)

Dipl. Ing. Sergey Brester via nginx-devel 50 September 28, 2022 07:08AM

Re: [PATCH] fix weakness by logging of broken header by incorect proxy protocol (IDS/IPS/LOG-analysis)

splitice 78 September 28, 2022 09:34AM

Re: [PATCH] fix weakness by logging of broken header by incorect proxy protocol (IDS/IPS/LOG-analysis)

Roman Arutyunyan 82 September 28, 2022 11:50AM

Re: [PATCH] fix weakness by logging of broken header by incorect proxy protocol (IDS/IPS/LOG-analysis)

Maxim Dounin 70 October 09, 2022 05:56PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 56
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready