[PATCH v2] audit: report audit wait metric in audit status reply

Thu Dec 3 23:10:03 UTC 2020

On 2020-12-03 10:37, Paul Moore wrote:
> On Thu, Dec 3, 2020 at 7:37 AM Richard Guy Briggs <rgb at redhat.com> wrote:
> > On 2020-12-02 23:12, Paul Moore wrote:
> > > On Wed, Dec 2, 2020 at 10:52 PM Steve Grubb <sgrubb at redhat.com> wrote:
> > > > We need this FEATURE_BITMAP to do anything in userspace. Max's instinct was
> > > > right. Anything that changes the user space API needs to have a
> > > > FEATURE_BITMAP so that user space can do the right thing. The lack of this is
> > > > blocking acceptance of the pull request for the user space piece.
> > >
> > > I don't believe you need a new bitmap entry in this case, you should
> > > be able to examine the size of the reply from the AUDIT_GET request
> > > and make a determination from there.
> >
> > The danger I see is if another feature is added to the audit status and
> > that is backported to a distro rather than this one.  It would be
> > impossible to determine which feature it was from the size alone.
> > Keying off specific fields in the kernel should be able to do
> > this at build time if I understood correctly.
> 
> ...
> 
> On Thu, Dec 3, 2020 at 8:31 AM Steve Grubb <sgrubb at redhat.com> wrote:
> > For the upstream kernel, this may be the case. But in the world where people
> > backport patches, how do I know that the size is related to this patch and no
> > other?
> 
> We've discussed this in the past, and most of you should already know
> my answer to this, but I'll repeat my stance on this again.
> 
> My first priority is the upstream kernel, enterprise distribution
> kernels are secondary.  The upstream kernels do not generally backport
> features, backports are limited to fixes.  If an individual or a
> distribution decides to backport kernel features they are responsible
> for making things work; it is not upstream's responsibility to enable,
> or support, arbitrary combinations of patches.  Any assistance we
> provide here is "best effort" and not guaranteed.
> 
> Bringing this back to the case at hand, the feature bitmap is a
> limited resource and it is my opinion that we need to limit its use to
> only those features which can not be determined through other means;

So far there are only seven bits used out of 32, so it does not appear we are
in danger of running out anytime soon.

It was introduced with commit 0288d7183c41c0192d2963d44590f346f4aee917
	Author:     Richard Guy Briggs <rgb at redhat.com>
	AuthorDate: 2014-11-17 15:51:01 -0500
	Commit:     Paul Moore <pmoore at redhat.com>
	CommitDate: 2014-11-17 16:53:51 -0500
	("audit: convert status version to a feature bitmap")
It was introduced specifically to enable distributions to selectively
backport features.  It was converted away from AUDIT_VERSION.

There are other ways to detect the presence of backlog_wait_time_actual
as I mentioned above.

configure.ac already has these:
	AC_CHECK_DECLS([AUDIT_FEATURE_VERSION], [], [], [[#include <linux/audit.h>]])
	AC_CHECK_MEMBERS([struct audit_status.feature_bitmap], [], [], [[#include <linux/audit.h>]])
	AC_CHECK_DECLS([AUDIT_VERSION_BACKLOG_WAIT_TIME], [], [], [[#include <linux/audit.h>]])
	AC_CHECK_DECLS([AUDIT_STATUS_BACKLOG_WAIT_TIME], [], [], [[#include <linux/audit.h>]])

Max' userspace patch already has provisions to check for the existance of the
macro AUDIT_STATUS_BACKLOG_WAIT_TIME_ACTUAL, so while it doesn't actually check
for the struct audit_status member backlog_wait_time_actual, it does check
something that should only be present if that member is present, so I believe
we are fine as it currently stands.  If you want to be really paranoid, then
add a check for HAVE_STRUCT_AUDIT_STATUS_BACKLOG_WAIT_TIME_ACTUAL with:

	AC_CHECK_MEMBERS([struct audit_status.backlog_wait_time_actual], [], [], [[#include <linux/audit.h>]])

Don't look at audit_set_feature() since it has an outstanding issue
filed against it to untangle the unfortunately named FEATURE_VERSION vs
FEATURE_BITMAP.
	https://github.com/linux-audit/audit-userspace/issues/10

> in this case this feature can be determine by the size of the
> AUDIT_GET reply buffer (the audit_status struct).  Of course if more
> care and thought had been put into the audit kernel/userspace API in
> the first place we would not be in this situation, but we are and we
> need to deal with it as best we can.
> 
> paul moore

- RGB

--
Richard Guy Briggs <rgb at redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635