lhotari commented on PR #25576:
URL: https://github.com/apache/pulsar/pull/25576#issuecomment-4314065557
> Should I close this PR if so,@lhotari?
@PavelZeger Yes, I think this should be closed.
It looks like log4j2 already supports [`%enc{%msg}` and
`%enc{%X}`](https://logging.apache.org/log4j/2.3.x/manual/layouts.html#:~:text=enc%7Bpattern%7D%0Aencode%7Bpattern%7D)
to prevent such attacks.
One option would be to add a value `text-escaped` to `pulsar.log.format` so
that someone who is concerned about log forging could enable it by setting the
environment variable `PULSAR_LOG_FORMAT=text-escaped`.
In the default log configuration, the handling of `text-escaped` would be
added here:
https://github.com/apache/pulsar/blob/ae2b8c76cec9dfc240fcbf6ae323f42d0ccbcc28/conf/log4j2.yaml#L78-L86
According to Claude, Log4j's PatternLayout will output the exception after
the log line unless the pattern explicitly contains `%ex` or `%throwable`.
Since exception messages could also contain input, the exception would have to
be escaped as well if there's a need to prevent any log forging issues.
Something like this would encode the log message, the MDC context and the
exception stacktrace (including exception messages):
`%d{ISO8601_OFFSET_DATE_TIME_HHMM} [%t] %-5level %logger{36} - %enc{%msg}
%enc{%X} %enc{%throwable}%n`
The `%enc` feature in Log4j should be a sufficient way to handle [OWASP Log
Injection/Forging](https://owasp.org/www-community/attacks/Log_Injection) and
[CWE-117: Improper Output Neutralization for
Logs](https://cwe.mitre.org/data/definitions/117.html). Since it comes with the
tradeoff that log readability suffers, it shouldn't be the default. For Pulsar
5.0, we can tackle this by adding the escaping feature to slog.
Would you like to create a separate PR for the `text-escaped` format? It
would be useful to test it whether it works as expected.
In addition, it would be helpful to document this somewhere in the Pulsar
documentation, which is maintained at https://github.com/apache/pulsar-site.
The challenge might be finding a suitable location in the existing
documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
