Improve Log4j configuration examples

[ppkarwasz]

The current Log4j 2 configuration examples misses some sources of CRLF injection:

• the message of an exception might contain line feeds: the %xEx converter pattern is implicitly added unless alwaysWriteExceptions is set to false.
• the name of a custom level can also contain line feeds.

In general every part of a log event, with the exclusion of the timestamp, is susceptible to CRLF injection: thread names, logger names, etc. While it is very unlikely that a logger name will contain user data, it is a possibility that should not be excluded.

This PR modifies the Log4j examples in the cheat sheet by:

1. Adding an example of JSON Template Layout usage. Since this is a structured layout, it prevents CRLF injection out of the box.
2. Modifying the Pattern Layout example to encode the entire log event, not just the log message. This makes the log events less readable by humans, but guarantees that each log event is on a single line.

[mackowski]

Awesome update. Thank you @ppkarwasz

[vy]

I would strongly avoid

• Sharing CRLF-fixes – Pattern Layout is not intended to be machine-readable. Trying to make it so by patching CRLF issues creates the impression that "Ah! Okay. Now it is safe to use Pattern Layout in production!", but is not! You can argue that "But we recommend CRLF-fixes only for testing and local development", then again, why would anyone need those fixes for testing and local development?
• Using rolling file appender as a starter – The rolling operation itself is pretty sophisticated, subject to many corner cases, and users generally configure it wrong; Log4j issue tracker contains several rolling related bugs and questions. For testing and local development purposes, we should encourage Console Appender. Users who has specific needs can consult to the Log4j manual, which contains extensive documentation on appenders.

My 2 cents... 💰

[ppkarwasz]

@mackowski,

I agree with @vy that the cheat-sheet should not promote the usage of unstructured layouts. No line-based layouts, no CRLF injection!

Obviously this radically changes the content of the sheet.
I can modify the PR to remove examples of line-based layouts in both Log4j Core and Logback (Logback has JsonEncoder). What do you think?

[mackowski]

@vy and @ppkarwasz you are better experts than me in this subject so yes go ahead!
I will just tag @tghosth because I think they are the original author of this CS.

[tghosth]

I will just tag @tghosth because I think they are the original author of this CS.

Thanks for the tag but I took this content from the original injection prevention cheatsheet for Java which was brought in by @righettod at this commit: https://github.com/OWASP/CheatSheetSeries/blob/17817a84b9dc945e6b6279a5e9f9b321ad17a4b5/cheatsheets_to_convert/Injection_Prevention_Cheat_Sheet_in_Java.md

You would have to ask @righettod where it came from to begin with :)

[jmanico]

I think we are ready for PR's @ppkarwasz - could you also fix the problem in https://github.com/OWASP/CheatSheetSeries/blob/17817a84b9dc945e6b6279a5e9f9b321ad17a4b5/cheatsheets_to_convert/Injection_Prevention_Cheat_Sheet_in_Java.md as well, pretty please?

[ppkarwasz]

Sure, I'll be travelling for work in the next week, but after my return I'll update this PR to:

• mention CRLF injection,
• state that structured logging (JSON, RFC5424) solves that problem at the base.
• provide only examples of structured logging.
• I'll double-check our Rfc5424 Layout if CRLF escaping is enabled by default. If it is not, we'll enable it in the next minor release.

[mackowski]

I converted this to draft since we want to make more changes as part of it

[mackowski]

hey @ppkarwasz do you have any updates to this?

[ppkarwasz]

hey @ppkarwasz do you have any updates to this?

Sorry, I'll put it on my TODO list for this weekend.

[ppkarwasz]

@mackowski,

I update both the Log4j Core and Logback examples to use a JSON Layout. The Log4j Core uses a Socket appender, which is common in cloud environment, while the Logback example uses a rolling file appender, since the Logback Socket appender does not support alternative layouts and uses Java serialization (which is difficult to parse securely).

[mackowski]

Looks good for me

[mackowski]

@jmanico if you can take a look :-)

[jmanico]

This is a very positive improvement, thank you!