Add new auditdline probe

[gdidot]

This PR add the auditdline probe following the proposal OVAL-Community/OVAL#114.

This implementation can query the live auditd rules, not just the one present in audit.rules file.

This implementation is based on the auditctl code use for the -l option, I would like to know if this can cause issue with the licensing?

[openscap-ci]

Can one of the admins verify this patch?

[openscap-ci]

Can one of the admins verify this patch?

[lgtm-com]

This pull request introduces 1 alert when merging 2567888 into 29679eb - view on LGTM.com

new alerts:

• 1 for Short global name

[evgenyz]

Hey, @gdidot.

Thank you for the contribution!

Before we start the discussion about the implementation details I would like to kindly ask you to do two things (so that we won't be messing with future comments):

• format all newly-written code in accordance with Linux kernel coding style (https://www.kernel.org/doc/html/v4.10/process/coding-style.html), but maintain preexisting formatting in files where you are merely adding some lines. I know that we weren't explicit about the project's code style in the past, but it is the de-facto standard for the project. And I've updated the contribution guidelines just now to make it also a de-jure requirement;
• squash your PR where appropriate. While it is a good practice to use multiple commits to logically structure the PR, the evolution of the initial implementation is not very valuable for the project history.

[evgenyz]

This implementation is based on the auditctl code use for the -l option, I would like to know if this can cause issue with the licensing?

I think it is OK.

[gdidot]

This implementation is based on the auditctl code use for the -l option, I would like to know if this can cause issue with the licensing?

I think it is OK.

I forgot to say that auditctl is GPL, and if I understand right OpenSCAP is LGPL, this will not be an issue ?

[evgenyz]

This implementation is based on the auditctl code use for the -l option, I would like to know if this can cause issue with the licensing?

I think it is OK.

I forgot to say that auditctl is GPL, and if I understand right OpenSCAP is LGPL, this will not be an issue ?

I believe that the code you borrowed is related to the communication with the API that the audit module provides. Usually there is not much of a room for API invocation. It should be okay. But to be precise, we can analyze the code if you let us know what parts exactly were borrowed.

[gdidot]

It's the code from line 79 to 911in the auditdline_probe.c file.

[evgenyz]

It's the code from line 79 to 911in the auditdline_probe.c file.

Err, that's actually a lot. I wonder if maybe @stevegrubb could shed some light on this culprit?

[gdidot]

@evgenyz I use the clang-format tool to reformat auditdline_probe.c and auditdline_probe.h, is the formating good with you ?

[gdidot]

It's the code from line 79 to 911in the auditdline_probe.c file.

Err, that's actually a lot. I wonder if maybe @stevegrubb could shed some light on this culprit?

I know it's a lot but the proposal OVAL-Community/OVAL#114 ask that the auditline entity is to be like the return of the auditctl -k command (I think is more likely the command auditctl -l -k ).

[lgtm-com]

This pull request introduces 1 alert when merging 7aa8f3c into b8c3f2e - view on LGTM.com

new alerts:

• 1 for Short global name

[stevegrubb]

This audit_close should be moved just above the else because that is the only time the audit_fd descriptor is valid.

[stevegrubb]

Well, auditctl is GPL. I can probably re-license that file to LGPL. But when you lift code from another project, it's usually good form to attribute the code to the original author. (My copyright still applies.) This way people have a notice that they may want to check periodically to see if it's still in sync with the original to pick up bugfixes and new capabilities. In looking this code over, I already see something that I should probably fix in auditctl. :-)

[evgenyz]

@openscap-ci ok to test

[evgenyz]

Well, auditctl is GPL. I can probably re-license that file to LGPL. But when you lift code from another project, it's usually good form to attribute the code to the original author. (My copyright still applies.)

@gdidot So, if we choose to take this path (see below), you at least ought to properly attribute the copyright.

This way people have a notice that they may want to check periodically to see if it's still in sync with the original to pick up bugfixes and new capabilities. In looking this code over, I already see something that I should probably fix in auditctl. :-)

This is the initial idea of the OVAL proposal: to take the behavior of the an existing tool and do the same. Its appeal is in apparent simplicity, but this simplicity is treacherous. This path could lead into problems like we have in systemd* probes: #1533.

[gdidot]

Thanks for your response @stevegrubb. I updated the code to properly credit you, sorry about that, let me know if it's ok with you. @evgenyz I understand your concern, but the proposal for auditdline is not yet fix. I think we can change it to specify a version of auditctl and avoid the same problem.

[stevegrubb]

@gdidot looks good to me. I'll change the license in the audit repo.

[evgenyz]

That's a very stale proposal and there is no movement on the OVAL side. I'm closing it for now.