But here we have a concrete use case: a third-party tool, Lilith, needs
to parse log events from our JsonLayout. (Our SocketServer in
log4j-server have the same need, and it uses this class.)
Should we provide a public API in log4j-core for doing so, which Lilith,
our SocketServer, and possibly others (Chainsaw maybe?) can use? Maybe
such public API should be a facade around this which hides the fact that
we use Jackson.
On 2017-07-22 19:04, Gary Gregory wrote:
Hi All,
Just like anything in log4j-core, we try not break binary compatibility
when we can but we do not bend over backwards like we do for log4j-api.
This class is public probably because Jackson requires it to be, otherwise
I would have made it package private. The fact that we use Jackson for JSON
IO is an implementation detail IMO, not an extension point. If anyone wants
to hard-code a reference to log4j-core's guts then I think its fair to day
that this comes with caveats.
Gary
On Sat, Jul 22, 2017 at 7:27 AM, Jörn Huxhorn <jhuxh...@googlemail.com>
wrote:
Well… it says
/**
* A Jackson JSON {@link ObjectMapper} initialized for Log4j.
* <p>
* <em>Consider this class private.</em>
* </p>
*/
so I wanted to holler about how to continue.
I’ll give it a shot.
On 22. July 2017 at 15:06:28, Mikael Ståldal (mi...@apache.org) wrote:
Would it work to use
org.apache.logging.log4j.core.jackson.Log4jJsonObjectMapper, which is
public?
See here how it is used:
https://github.com/apache/logging-log4j-tools/blob/
master/log4j-server/src/main/java/org/apache/logging/log4j/server/
JsonInputStreamLogEventBridge.java
On 2017-07-22 13:47, Jörn Huxhorn wrote:
It seems like I don’t get access to the log4j2 ObjectMapper or Jackson
Module used for
JSON serialization, i.e. I can’t do something like this:
new JacksonFactory.JSON(encodeThreadContextAsList, includeStacktrace,
false)
It would make sense to reuse your ObjectMapper/Module in my code
instead of reimplementing
them to prevent future compatibility issues down the road.
On 19. July 2017 at 23:04:26, Jörn Huxhorn (jhuxh...@googlemail.com)
wrote:
I’ll give this a shot over the next view days.
Thanks!
On 19. July 2017 at 21:33:49, Mikael Ståldal (mi...@apache.org)
wrote:
JsonLayout (and XmlLayout and YamlLayout) now supports 0-byte
termination of log events. Will be part of the upcoming 2.9 release.
On 2017-07-18 11:48, Jörn Huxhorn wrote:
A general event header and footer would probably be overkill.
I’m using the following interface in Lilith:
public interface WriteByteStrategy {
void writeBytes(DataOutputStream dataOutputStream, byte[] bytes)
throws IOException;
}
One implementation writes an int with the length of bytes before
“bytes", the other
implementation writes a 0-byte after “bytes".
The problem with that approach is that it requires the intermediate
“bytes” array
and
is thus probably incompatible with your zero garbage approach.
I didn’t try end-of-line (\r\n) mainly because I first wanted to
discuss this with
you.
I would also have to implement a new receiver type whereas I could
just reuse an existing
one in case of 0-byte termination.
I’d prefer using 0-byte termination since it would be less fragile
than end-of-line.
The end-of-line approach would break if compact=“false” was set
erroneously while
you’d still have the option of using either compact=“true” or
compact=“false” in
case
of 0-byte termination. 0-byte termination is also slightly more
efficient while
reading
since the splitting can operate on bytes instead of chars or Strings
- another reason
why I didn’t just implement it and instead wanted to discuss this
with you first.
Joern
On 18. July 2017 at 03:26:32, Ralph Goers (
ralph.go...@dslextreme.com) wrote:
Right now that is all handled by the specific layouts. For
example, by default the
RFC5424Layout
doesn’t append newlines so when writing to a file they will all be
on the same “line”,
but
it has an option to append one if needed. Doing the same would be
another option for
the
JSONLayout.
I’d be ok with event header and footers but ONLY if they have zero
overhead when not
present.
IOW, when the Layout is initialized it would have to wire in the
appropriate encode
method.
Ralph
On Jul 17, 2017, at 3:06 PM, Gary Gregory wrote:
Do we want a general event header and footer then?
Gary
On Jul 17, 2017 14:43, "Ralph Goers" wrote:
No. A Footer is only used at end of file. He needs to know how
long each
event is or when it is the start of a new event.
Ralph
On Jul 17, 2017, at 12:32 PM, Gary Gregory
wrote:
Can't you use a footer for any terminator you wish?
Gary
On Mon, Jul 17, 2017 at 12:13 PM, Mikael Ståldal
wrote:
Hi.
(Moving this discussion to logging dev mailing list.)
Have you tried to use:
Then each log event will be terminated by end-of-line (\r\n).
I think it would be easy to implement 0-byte terminated log
events in
JsonLayout, and that would make sense since we have
implemented support
for
that in GelfLayout. Created a JIRA issue for it:
https://issues.apache.org/jira/browse/LOG4J2-1981
As for other tools, the only receivers for Log4j 2
SerializedLayout we
know are Log4j's own SocketServer and Lilith. Chainsaw
currently only
support Log4j 1 SerializedLayout. Log4j's own SocketServer
support
JsonLayout and XmlLayout as well, and we are changing it to use
JsonLayout
by default.
On 2017-07-17 15:01, Joern Huxhorn wrote:
Hi Mikael,
I've just taken a look at the JsonLayout and see no way to
parse its
output event by event.
It's semi-fine for files because then it will result in a
well-formed
array at the root level that can be read in one big swoop. It
isn't
really ideal for that use case either since it means that the
whole
array containing all events needs to be read into memory in
one piece,
causing OOM in case of huge files.
But in Lilith, I really need to process events one by one.
This is
absolutely crucial.
The only way to achieve this would essentially require me to
re-implement a JSON parser that can cope with data like this.
I implemented a similar kludge to support reading of log4j
xml files
(see
https://github.com/huxi/lilith/blob/master/log4j/log4j-xml/
src/main/java/de/huxhorn/lilith/log4j/xml/
Log4jImportCallable.java
) but this was way easier since i could simply search for
"" to find the position at which I could split the stream
of events into a valid XML document for every single event .
This was
still kind of nasty but doable, especially since processing an
"offline"
XML file doesn't have the same performance restriction as
handling of
live events.
I wouldn't have the luxury of such a unique split signal in
JSON,
though.
I would need to detect the "}" at the level of the root array
which
would not simply involve counting of opening and closing
brackets but
also ignoring of brackets inside of strings. This would boil
down to
implementing a custom JSON reader and I won't be doing this.
This isn't just me being lazy either. Such an implementation
wouldn't
be
resource (CPU/memory) friendly anyway.
In my own JSON receiver using my own JSON format there are
two ways to
send those events:
- write an int containing the amount of bytes representing
the event,
then the bytes of the event. This type of events also supports
compression.
- write the bytes of the event, followed by a 0-byte. This
works fine
and JavaScript/ActionScript is able to produce events like
that while
they are unable (or were unable 7 years ago) to count the
bytes on
their
own. This type of events only supports plain text like JSON
or XML and
no compression since compressed events could contain the
0-byte while
XML and JSON both won't allow it.
Both are essentially "binary" formats because of either the
raw ints or
the 0-bytes.
I'm not sure why you deprecate SerializedLayout since the
security
issues arise only while deserializing the events, not while
serializing
them. Is this just meant to educate the user about the issue?
I fixed the remote code execution exploit by implementing
https://github.com/huxi/lilith/blob/master/lilith-engine/
src/main/java/de/huxhorn/lilith/engine/impl/eventproduc
er/WhitelistObjectInputStream.java
This still doesn't fix DoS scenarios (endless loops or OOM in
case of
malicious data) but is close enough for me, mainly because
fixing the
DoS issues would really have to be handled by Java, not by
user code.
Manually re-implementing deserialization would likely be
fragile and
error prone, possibly even introducing other security issues
on its
own.
I'd be interested how other tools are tackling this issue. Is
Chainsaw
officially dead or are they implementing receivers for event
streams
like this?
I'm totally fine with implementing a new receiver as a
replacement for
log4j2 SerializedLayout (even though I won't be able to get
rid of the
Serializable receiver anyway) but the current JsonLayout just
isn't up
to the job at the moment.
Feel free to share this mail with your fellow developers. I'm
open for
suggestions.
I hope this doesn't come across in a harsh way. It isn't
meant to be.
Cheers,
Jörn.
