laurel icon indicating copy to clipboard operation
laurel copied to clipboard

Add support for rsyslog lumberjack mechanism

Open leophys opened this issue 2 years ago • 2 comments

Hi! Thanks for this software!

I am opening this PR to add a functionality I am interested in to better integrate the auditd machinery together with rsyslog. The juice of the feature regards the ability for rsyslog to be able to recognize the beginning of a json structure in a log line. This is achieved prepending a cookie (@cee:) before the log line. In the case of laurel as an auditd plugin, this means that such cookie should be present at the beginning of each line. I thought first of implementing a custom serde serializer, but this is impossible as a serializer has no notion of level and cannot tell when an object is at the beginning of a line. The approach I took here is to use a custom Writer (an implementation of std::io::Write) that mimics a BufWriter, buffering in an internal buffer per line instead of using the byte size as measure. To enable this mechanism, I added a new section in the configuration file, with a single boolean option

[out_format]

lumberjack = true

that defaults to false (the current behavior).

You will notice that there are A LOT of lines in the PR diff. This is because I use (neo)vim and rust-analyzer in the language server machinery. It is not feasible for me to work without it and this machinery autoformats the code at each save. I run cargo fmt on the whole code base and saved it as first commit. The remaining commits in the PR implement what I described above. I beg you to accept this not as a disrespectful act, but rather as a way for you and me to foster public collaboration :innocent:

leophys avatar Aug 15 '22 16:08 leophys

Thank you for your pull request. I won't have time to properly review your changes until next week, so please have a little patience.

One question that came to my mind right away: Would it not be easier to add a configurable prefix parameter to the existing code?

hillu avatar Aug 16 '22 16:08 hillu

On August 16, 2022 6:25:52 PM GMT+02:00, Hilko Bengen @.***> wrote:

Thank you for your pull request. I won't have time to properly review your changes until next week, so please have a little patience.

Don't worry! I am in no hurry. Thanks for considering it.

One question that came to my mind right away: Would it not be easier to add a configurable prefix parameter to the existing code?

It would be surely possible, and I can do it if you want. I did not do it because it seemed to me that this feature is only relevant with that specified prefix :) Do you think that an optional lumberjack_prefix parameter in the configuration would do?

-- blallo

leophys avatar Aug 16 '22 17:08 leophys

I think something like

--- a/src/config.rs
+++ b/src/config.rs
@@ -12,6 +12,8 @@ pub struct Logfile {
     pub users: Option<Vec<String>>,
     pub size: Option<u64>,
     pub generations: Option<u64>,
+    #[serde(rename="line-prefix")]
+    pub line_prefix: Option<String>,
 }
 
 #[derive(Clone,Debug,Default,Serialize,Deserialize,PartialEq)]

plus corresponding changes in FileRotate should suffice.

hillu avatar Aug 19 '22 16:08 hillu

Hi, sorry for the long silence!

I think something like

--- a/src/config.rs
+++ b/src/config.rs
@@ -12,6 +12,8 @@ pub struct Logfile {
     pub users: Option<Vec<String>>,
     pub size: Option<u64>,
     pub generations: Option<u64>,
+    #[serde(rename="line-prefix")]
+    pub line_prefix: Option<String>,
 }
 
 #[derive(Clone,Debug,Default,Serialize,Deserialize,PartialEq)]

plus corresponding changes in FileRotate should suffice.

I did not understand your proposed solutions. As a matter of fact, I plugged this capability in the LumberjackWriter only, using the new section of the config I already added. What do you think?

I saw there are a bunch of conflicts, probably because you merged another PR and the cargo fmt is broken. I will try to open a new PR, just for the code formatting and then a second PR for this feature, but I am taking a small leave from the laptop and will be mostly AFK. Meanwhile, would you mind to review and comment the proposed changes here, so that I can backport your observation to the new PR?

Thanks!

EDIT: I just saw that the conflict comes from the fact that you applied cargo fmt yourself (and also added a validation step). I suppose I just have to rebase this PR!

EDIT 2: I rebased without conflicts (:heart:); seems too good to be true. So I guess this is again the main PR.

leophys avatar Aug 25 '22 21:08 leophys

@leophys Sorry if I had not stated clearly enough what I was going for.

Please have a look at https://github.com/hillu/laurel/tree/feature/lumberjack – here the line prefix is just added to the Logger in main.rs, without the need for implementing a separate rotating file backend.

Does this do what you originally intended?

hillu avatar Sep 06 '22 22:09 hillu

Ahahahahah, nice! It is so much cleaner than my current PR! It seems to me that it is exactly what I meant it to do.

leophys avatar Sep 07 '22 19:09 leophys

Ahahahahah, nice! It is so much cleaner than my current PR! It seems to me that it is exactly what I meant it to do.

Great, I'm going to push my change then.

I would like to give credit to you in the commit message and in the changelog for the next release. What name should I use?

hillu avatar Sep 07 '22 19:09 hillu

Ahahahahah, nice! It is so much cleaner than my current PR! It seems to me that it is exactly what I meant it to do.

Great, I'm going to push my change then.

Yes please!

I would like to give credit to you in the commit message and in the changelog for the next release. What name should I use?

Thanks! You can use my github handle :)

leophys avatar Sep 07 '22 19:09 leophys