[PATCH ghak90 V6] fixup! audit: add containerid filtering

[PATCH ghak90 V6] fixup! audit: add containerid filtering

[Richard Guy Briggs]

Remove the BUG() call since we will never have an invalid op value as
audit_data_to_entry()/audit_to_op() ensure that the op value is a a
known good value.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditfilter.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 407b5bb3b4c6..385a114a1254 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1244,7 +1244,6 @@ int audit_comparator64(u64 left, u32 op, u64 right)
 	case Audit_bittest:
 		return ((left & right) == right);
 	default:
-		BUG();
 		return 0;
 	}
 }
-- 
1.8.3.1

Re: [PATCH ghak90 V6] fixup! audit: add containerid filtering

[Paul Moore]

On Fri, May 31, 2019 at 1:54 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> Remove the BUG() call since we will never have an invalid op value as
> audit_data_to_entry()/audit_to_op() ensure that the op value is a a
> known good value.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/auditfilter.c | 1 -
>  1 file changed, 1 deletion(-)

Thanks for sending this out.  However, in light of the discussion in
the patchset's cover letter it looks like we need to better support
nested container orchestrators which is likely going to require some
non-trivial changes to the kernel/userspace API.  Because of this I'm
going to hold off pulling these patches into a "working" branch,
hopefully the next revision of these patches will solve the nested
orchestrator issue enough that we can continue to move forward with
testing.

> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 407b5bb3b4c6..385a114a1254 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1244,7 +1244,6 @@ int audit_comparator64(u64 left, u32 op, u64 right)
>         case Audit_bittest:
>                 return ((left & right) == right);
>         default:
> -               BUG();
>                 return 0;
>         }
>  }
> --
> 1.8.3.1
>


-- 
paul moore
www.paul-moore.com

Re: [PATCH ghak90 V6] fixup! audit: add containerid filtering

[Daniel Walsh]

The need for nested container support is the `Enemy of the good`.  This
idea has been being worked on for years and has always been blocked by
this seldom used feature.

We are working on a project right this summer to allow us to use the
audit system to track the syscalls used by a container and then generate
a seccomp.json file to lock down the container in the future.  Think of
it as Audit2allow for seccomp rules on a container.  The problem is,
other then PID1 inside of the container, it is real difficult to follow
the other processes inside of the container without a containerid. 

Is there a chance we could get a single ID for a container for now, and
then enhance the feature in the future for nested ContainerIDs. 

I fear that this will block the container id for years.


On 6/3/19 6:01 PM, Paul Moore wrote:
> On Fri, May 31, 2019 at 1:54 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>> Remove the BUG() call since we will never have an invalid op value as
>> audit_data_to_entry()/audit_to_op() ensure that the op value is a a
>> known good value.
>>
>> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> ---
>>  kernel/auditfilter.c | 1 -
>>  1 file changed, 1 deletion(-)
> Thanks for sending this out.  However, in light of the discussion in
> the patchset's cover letter it looks like we need to better support
> nested container orchestrators which is likely going to require some
> non-trivial changes to the kernel/userspace API.  Because of this I'm
> going to hold off pulling these patches into a "working" branch,
> hopefully the next revision of these patches will solve the nested
> orchestrator issue enough that we can continue to move forward with
> testing.
>
>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>> index 407b5bb3b4c6..385a114a1254 100644
>> --- a/kernel/auditfilter.c
>> +++ b/kernel/auditfilter.c
>> @@ -1244,7 +1244,6 @@ int audit_comparator64(u64 left, u32 op, u64 right)
>>         case Audit_bittest:
>>                 return ((left & right) == right);
>>         default:
>> -               BUG();
>>                 return 0;
>>         }
>>  }
>> --
>> 1.8.3.1
>>
>

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH ghak90 V6] fixup! audit: add containerid filtering

[Paul Moore]

On Tue, Jun 4, 2019 at 3:55 AM Daniel Walsh <dwalsh@redhat.com> wrote:
> The need for nested container support is the `Enemy of the good`.  This
> idea has been being worked on for years and has always been blocked by
> this seldom used feature.

Speaking with some of the LXC folks, nested orchestrators isn't a
seldom used feature for them, they see bug reports on a regular basis
from users nesting various different orchestrators.  You are correct
that we didn't originally think this was very common, which is
unfortunate, but it's better we fix this now than push something into
the kernel which might require a kernel API change to be useful to a
wider audience.  To be clear, that's where we are at: we need to
figure out what the kernel API would look like to support nested
container orchestrators.  My gut feeling is that this isn't going to
be terribly complicated compared to the rest of the audit container ID
work, but it is going to be some work.  We had a discussion about some
potential solutions in the cover letter and it sounds like Richard is
working up some ideas now, let's wait to see what that looks like.

> We are working on a project right this summer to allow us to use the
> audit system to track the syscalls used by a container and then generate
> a seccomp.json file to lock down the container in the future.  Think of
> it as Audit2allow for seccomp rules on a container.  The problem is,
> other then PID1 inside of the container, it is real difficult to follow
> the other processes inside of the container without a containerid.

That sounds interesting.

> Is there a chance we could get a single ID for a container for now, and
> then enhance the feature in the future for nested ContainerIDs.

The blocker is the kernel/userspace API; I'm not going to merge code
which provides an API which breaks when you try to nest orchestrators.
Whatever we pick we are likely going to have to support f-o-r-e-v-e-r,
so I want to make sure it is useful for the significant use cases we
have today.

> I fear that this will block the container id for years.

It's ready when it's ready.

> On 6/3/19 6:01 PM, Paul Moore wrote:
> > On Fri, May 31, 2019 at 1:54 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> >> Remove the BUG() call since we will never have an invalid op value as
> >> audit_data_to_entry()/audit_to_op() ensure that the op value is a a
> >> known good value.
> >>
> >> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> >> ---
> >>  kernel/auditfilter.c | 1 -
> >>  1 file changed, 1 deletion(-)
> > Thanks for sending this out.  However, in light of the discussion in
> > the patchset's cover letter it looks like we need to better support
> > nested container orchestrators which is likely going to require some
> > non-trivial changes to the kernel/userspace API.  Because of this I'm
> > going to hold off pulling these patches into a "working" branch,
> > hopefully the next revision of these patches will solve the nested
> > orchestrator issue enough that we can continue to move forward with
> > testing.
> >
> >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> >> index 407b5bb3b4c6..385a114a1254 100644
> >> --- a/kernel/auditfilter.c
> >> +++ b/kernel/auditfilter.c
> >> @@ -1244,7 +1244,6 @@ int audit_comparator64(u64 left, u32 op, u64 right)
> >>         case Audit_bittest:
> >>                 return ((left & right) == right);
> >>         default:
> >> -               BUG();
> >>                 return 0;
> >>         }
> >>  }
> >> --
> >> 1.8.3.1
> >>
> >
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit



-- 
paul moore
www.paul-moore.com