re: [PATCH V5 0/5] audit by executable name

re: [PATCH V5 0/5] audit by executable name

[Peter Moody]

Did this [1] land? I'm guessing no because the next pull request from
Eric Paris didn't include it and I don't see it referenced in any of
Paul's pull requests. Finally (most tellingly), I don't see anything
in Linus' tree.

Cheers,
peter

[1] https://www.redhat.com/archives/linux-audit/2014-October/msg00024.html

Re: [PATCH V5 0/5] audit by executable name

[Paul Moore]

On Fri, May 29, 2015 at 12:14 PM, Peter Moody <peter@hda3.com> wrote:
> Did this [1] land? I'm guessing no because the next pull request from
> Eric Paris didn't include it and I don't see it referenced in any of
> Paul's pull requests. Finally (most tellingly), I don't see anything
> in Linus' tree.
>
> Cheers,
> peter
>
> [1] https://www.redhat.com/archives/linux-audit/2014-October/msg00024.html

Nope, but Richard is working on it.

-- 
paul moore
www.paul-moore.com

Re: [PATCH V5 0/5] audit by executable name

[Richard Guy Briggs]

On 15/05/29, Peter Moody wrote:
> Did this [1] land? I'm guessing no because the next pull request from
> Eric Paris didn't include it and I don't see it referenced in any of
> Paul's pull requests. Finally (most tellingly), I don't see anything
> in Linus' tree.

Why?  Were your ears burning?  ;-)

> peter
> 
> [1] https://www.redhat.com/archives/linux-audit/2014-October/msg00024.html

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Re: [PATCH V5 0/5] audit by executable name

[Peter Moody]

On Fri, May 29, 2015 at 9:28 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 15/05/29, Peter Moody wrote:
>> Did this [1] land? I'm guessing no because the next pull request from
>> Eric Paris didn't include it and I don't see it referenced in any of
>> Paul's pull requests. Finally (most tellingly), I don't see anything
>> in Linus' tree.
>
> Why?  Were your ears burning?  ;-)

Hah, nope. I just find myself in the situation where having this
functionality would be very useful.

Cheers

>> peter
>>
>> [1] https://www.redhat.com/archives/linux-audit/2014-October/msg00024.html
>
> - RGB
>
> --
> Richard Guy Briggs <rbriggs@redhat.com>
> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

[PATCH V5 0/5] audit by executable name

[Richard Guy Briggs]

This is a part of Peter Moody, my and Eric Paris' work to implement
audit by executable name.

Please see the accompanying userspace patch:
	https://www.redhat.com/archives/linux-audit/2014-May/msg00019.html
The userspace interface is not expected to change appreciably unless something
important has been overlooked.  Setting and deleting rules works as expected.
	
If the path does not exist at rule creation time, it will be re-evaluated every
time there is a change to the parent directory at which point the change in
device and inode will be noted.


Here's a sample run:

# /usr/local/sbin/auditctl -a always,exit -F dir=/tmp -F exe=/bin/touch -F key=touch_tmp
# /usr/local/sbin/ausearch --start recent -k touch_tmp
time->Mon Jun 30 14:15:06 2014
type=CONFIG_CHANGE msg=audit(1404152106.683:149): auid=0 ses=1 subj=unconfined_u :unconfined_r:auditctl_t:s0-s0:c0.c1023 op="add_rule" key="touch_tmp" list=4 res =1

# /usr/local/sbin/auditctl -l
-a always,exit -S all -F dir=/tmp -F exe=/bin/touch -F key=touch_tmp

# touch /tmp/test

# /usr/local/sbin/ausearch --start recent -k touch_tmp
time->Wed Jul  2 12:18:47 2014
type=UNKNOWN[1327] msg=audit(1404317927.319:132): proctitle=746F756368002F746D702F74657374
type=PATH msg=audit(1404317927.319:132): item=1 name="/tmp/test" inode=25997 dev=00:20 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE
type=PATH msg=audit(1404317927.319:132): item=0 name="/tmp/" inode=11144 dev=00:20 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype=PARENT
type=CWD msg=audit(1404317927.319:132):  cwd="/root"
type=SYSCALL msg=audit(1404317927.319:132): arch=c000003e syscall=2 success=yes exit=3 a0=7ffffa403dd5 a1=941 a2=1b6 a3=34b65b2c6c items=2 ppid=4321 pid=6436 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=1 comm="touch" exe="/usr/bin/touch" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="touch_tmp"


Revision history:
v5: Revert patch "Let audit_free_rule() take care of calling
    audit_remove_mark()." since it caused a group mark deadlock.

v4: Re-order and squash down fixups
    Fix audit_dup_exe() to copy pathname string before calling audit_alloc_mark().

v3: Rationalize and rename some function names and clean up get/put and free code.
    Rename several "watch" references to "mark".
    Rename audit_remove_rule() to audit_remove_mark_rule().
    Let audit_free_rule() take care of calling audit_remove_mark().
    Put audit_alloc_mark() arguments in same order as watch, tree and inode.
    Move the access to the entry for audit_match_signal() to the beginning
     of the function in case the entry found is the same one passed in.
     This will enable it to be used by audit_remove_mark_rule().
    https://www.redhat.com/archives/linux-audit/2014-July/msg00000.html

v2: Misguided attempt to add in audit_exe similar to watches
    https://www.redhat.com/archives/linux-audit/2014-June/msg00066.html

v1.5: eparis' switch to fsnotify
    https://www.redhat.com/archives/linux-audit/2014-May/msg00046.html
    https://www.redhat.com/archives/linux-audit/2014-May/msg00066.html

v1: Change to path interface instead of inode
    https://www.redhat.com/archives/linux-audit/2014-May/msg00017.html

v0: Peter Moodie's original patches
    https://www.redhat.com/archives/linux-audit/2012-August/msg00033.html


Next step:
Get full-path notify working.


Eric Paris (3):
  audit: implement audit by executable
  audit: clean simple fsnotify implementation
  audit: convert audit_exe to audit_fsnotify

Richard Guy Briggs (2):
  audit: avoid double copying the audit_exe path string
  Revert "fixup! audit: clean simple fsnotify implementation"

 include/linux/audit.h      |    1 +
 include/uapi/linux/audit.h |    2 +
 kernel/Makefile            |    2 +-
 kernel/audit.h             |   39 +++++++
 kernel/audit_exe.c         |   49 +++++++++
 kernel/audit_fsnotify.c    |  237 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/auditfilter.c       |   52 +++++++++-
 kernel/auditsc.c           |   16 +++
 8 files changed, 395 insertions(+), 3 deletions(-)
 create mode 100644 kernel/audit_exe.c
 create mode 100644 kernel/audit_fsnotify.c

Re: [PATCH V5 0/5] audit by executable name

[Steve Grubb]

On Thursday, October 02, 2014 11:06:51 PM Richard Guy Briggs wrote:
> This is a part of Peter Moody, my and Eric Paris' work to implement
> audit by executable name.

Does this patch set define an AUDIT_VERSION_SOMETHING and then set 
AUDIT_VERSION_LATEST to it? If not, I need one to tell if the kernel supports 
it when issuing commands. Also, if its conceivable that kernels may pick and 
choose what features could be backported to a curated kernel, should 
AUDIT_VERSION_ be a number that is incremented or a bit mask?

-Steve


> Please see the accompanying userspace patch:
> 	https://www.redhat.com/archives/linux-audit/2014-May/msg00019.html
> The userspace interface is not expected to change appreciably unless
> something important has been overlooked.  Setting and deleting rules works
> as expected.
> 
> If the path does not exist at rule creation time, it will be re-evaluated
> every time there is a change to the parent directory at which point the
> change in device and inode will be noted.
> 
> 
> Here's a sample run:
> 
> # /usr/local/sbin/auditctl -a always,exit -F dir=/tmp -F exe=/bin/touch -F
> key=touch_tmp # /usr/local/sbin/ausearch --start recent -k touch_tmp
> time->Mon Jun 30 14:15:06 2014
> type=CONFIG_CHANGE msg=audit(1404152106.683:149): auid=0 ses=1
> subj=unconfined_u :unconfined_r:auditctl_t:s0-s0:c0.c1023 op="add_rule"
> key="touch_tmp" list=4 res =1
> 
> # /usr/local/sbin/auditctl -l
> -a always,exit -S all -F dir=/tmp -F exe=/bin/touch -F key=touch_tmp
> 
> # touch /tmp/test
> 
> # /usr/local/sbin/ausearch --start recent -k touch_tmp
> time->Wed Jul  2 12:18:47 2014
> type=UNKNOWN[1327] msg=audit(1404317927.319:132):
> proctitle=746F756368002F746D702F74657374 type=PATH
> msg=audit(1404317927.319:132): item=1 name="/tmp/test" inode=25997
> dev=00:20 mode=0100644 ouid=0 ogid=0 rdev=00:00
> obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE type=PATH
> msg=audit(1404317927.319:132): item=0 name="/tmp/" inode=11144 dev=00:20
> mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0
> nametype=PARENT type=CWD msg=audit(1404317927.319:132):  cwd="/root"
> type=SYSCALL msg=audit(1404317927.319:132): arch=c000003e syscall=2
> success=yes exit=3 a0=7ffffa403dd5 a1=941 a2=1b6 a3=34b65b2c6c items=2
> ppid=4321 pid=6436 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
> fsgid=0 tty=ttyS0 ses=1 comm="touch" exe="/usr/bin/touch"
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="touch_tmp"
> 
> 
> Revision history:
> v5: Revert patch "Let audit_free_rule() take care of calling
>     audit_remove_mark()." since it caused a group mark deadlock.
> 
> v4: Re-order and squash down fixups
>     Fix audit_dup_exe() to copy pathname string before calling
> audit_alloc_mark().
> 
> v3: Rationalize and rename some function names and clean up get/put and free
> code. Rename several "watch" references to "mark".
>     Rename audit_remove_rule() to audit_remove_mark_rule().
>     Let audit_free_rule() take care of calling audit_remove_mark().
>     Put audit_alloc_mark() arguments in same order as watch, tree and inode.
> Move the access to the entry for audit_match_signal() to the beginning of
> the function in case the entry found is the same one passed in. This will
> enable it to be used by audit_remove_mark_rule().
>     https://www.redhat.com/archives/linux-audit/2014-July/msg00000.html
> 
> v2: Misguided attempt to add in audit_exe similar to watches
>     https://www.redhat.com/archives/linux-audit/2014-June/msg00066.html
> 
> v1.5: eparis' switch to fsnotify
>     https://www.redhat.com/archives/linux-audit/2014-May/msg00046.html
>     https://www.redhat.com/archives/linux-audit/2014-May/msg00066.html
> 
> v1: Change to path interface instead of inode
>     https://www.redhat.com/archives/linux-audit/2014-May/msg00017.html
> 
> v0: Peter Moodie's original patches
>     https://www.redhat.com/archives/linux-audit/2012-August/msg00033.html
> 
> 
> Next step:
> Get full-path notify working.
> 
> 
> Eric Paris (3):
>   audit: implement audit by executable
>   audit: clean simple fsnotify implementation
>   audit: convert audit_exe to audit_fsnotify
> 
> Richard Guy Briggs (2):
>   audit: avoid double copying the audit_exe path string
>   Revert "fixup! audit: clean simple fsnotify implementation"
> 
>  include/linux/audit.h      |    1 +
>  include/uapi/linux/audit.h |    2 +
>  kernel/Makefile            |    2 +-
>  kernel/audit.h             |   39 +++++++
>  kernel/audit_exe.c         |   49 +++++++++
>  kernel/audit_fsnotify.c    |  237
> ++++++++++++++++++++++++++++++++++++++++++++ kernel/auditfilter.c       |  
> 52 +++++++++-
>  kernel/auditsc.c           |   16 +++
>  8 files changed, 395 insertions(+), 3 deletions(-)
>  create mode 100644 kernel/audit_exe.c
>  create mode 100644 kernel/audit_fsnotify.c

Re: [PATCH V5 0/5] audit by executable name

[Eric Paris]

On Mon, 2014-10-20 at 16:25 -0400, Steve Grubb wrote:
> On Thursday, October 02, 2014 11:06:51 PM Richard Guy Briggs wrote:
> > This is a part of Peter Moody, my and Eric Paris' work to implement
> > audit by executable name.
> 
> Does this patch set define an AUDIT_VERSION_SOMETHING and then set 
> AUDIT_VERSION_LATEST to it? If not, I need one to tell if the kernel supports 
> it when issuing commands. Also, if its conceivable that kernels may pick and 
> choose what features could be backported to a curated kernel, should 
> AUDIT_VERSION_ be a number that is incremented or a bit mask?

Right now the value is 2. So this is your last hope if you want to make
it a bitmask. I'll leave that up to paul/richard to (over) design.

Support for by EXEC should probably be noted somehow. Especially since
audit_netlink_ok() sucks and return EINVAL for unknown message types. We
wouldn't need the bump to version if that returned EOPNOTSUP and
userspace could actually tell what was going on...

> 
> -Steve
> 
> 
> > Please see the accompanying userspace patch:
> > 	https://www.redhat.com/archives/linux-audit/2014-May/msg00019.html
> > The userspace interface is not expected to change appreciably unless
> > something important has been overlooked.  Setting and deleting rules works
> > as expected.
> > 
> > If the path does not exist at rule creation time, it will be re-evaluated
> > every time there is a change to the parent directory at which point the
> > change in device and inode will be noted.
> > 
> > 
> > Here's a sample run:
> > 
> > # /usr/local/sbin/auditctl -a always,exit -F dir=/tmp -F exe=/bin/touch -F
> > key=touch_tmp # /usr/local/sbin/ausearch --start recent -k touch_tmp
> > time->Mon Jun 30 14:15:06 2014
> > type=CONFIG_CHANGE msg=audit(1404152106.683:149): auid=0 ses=1
> > subj=unconfined_u :unconfined_r:auditctl_t:s0-s0:c0.c1023 op="add_rule"
> > key="touch_tmp" list=4 res =1
> > 
> > # /usr/local/sbin/auditctl -l
> > -a always,exit -S all -F dir=/tmp -F exe=/bin/touch -F key=touch_tmp
> > 
> > # touch /tmp/test
> > 
> > # /usr/local/sbin/ausearch --start recent -k touch_tmp
> > time->Wed Jul  2 12:18:47 2014
> > type=UNKNOWN[1327] msg=audit(1404317927.319:132):
> > proctitle=746F756368002F746D702F74657374 type=PATH
> > msg=audit(1404317927.319:132): item=1 name="/tmp/test" inode=25997
> > dev=00:20 mode=0100644 ouid=0 ogid=0 rdev=00:00
> > obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE type=PATH
> > msg=audit(1404317927.319:132): item=0 name="/tmp/" inode=11144 dev=00:20
> > mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0
> > nametype=PARENT type=CWD msg=audit(1404317927.319:132):  cwd="/root"
> > type=SYSCALL msg=audit(1404317927.319:132): arch=c000003e syscall=2
> > success=yes exit=3 a0=7ffffa403dd5 a1=941 a2=1b6 a3=34b65b2c6c items=2
> > ppid=4321 pid=6436 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
> > fsgid=0 tty=ttyS0 ses=1 comm="touch" exe="/usr/bin/touch"
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="touch_tmp"
> > 
> > 
> > Revision history:
> > v5: Revert patch "Let audit_free_rule() take care of calling
> >     audit_remove_mark()." since it caused a group mark deadlock.
> > 
> > v4: Re-order and squash down fixups
> >     Fix audit_dup_exe() to copy pathname string before calling
> > audit_alloc_mark().
> > 
> > v3: Rationalize and rename some function names and clean up get/put and free
> > code. Rename several "watch" references to "mark".
> >     Rename audit_remove_rule() to audit_remove_mark_rule().
> >     Let audit_free_rule() take care of calling audit_remove_mark().
> >     Put audit_alloc_mark() arguments in same order as watch, tree and inode.
> > Move the access to the entry for audit_match_signal() to the beginning of
> > the function in case the entry found is the same one passed in. This will
> > enable it to be used by audit_remove_mark_rule().
> >     https://www.redhat.com/archives/linux-audit/2014-July/msg00000.html
> > 
> > v2: Misguided attempt to add in audit_exe similar to watches
> >     https://www.redhat.com/archives/linux-audit/2014-June/msg00066.html
> > 
> > v1.5: eparis' switch to fsnotify
> >     https://www.redhat.com/archives/linux-audit/2014-May/msg00046.html
> >     https://www.redhat.com/archives/linux-audit/2014-May/msg00066.html
> > 
> > v1: Change to path interface instead of inode
> >     https://www.redhat.com/archives/linux-audit/2014-May/msg00017.html
> > 
> > v0: Peter Moodie's original patches
> >     https://www.redhat.com/archives/linux-audit/2012-August/msg00033.html
> > 
> > 
> > Next step:
> > Get full-path notify working.
> > 
> > 
> > Eric Paris (3):
> >   audit: implement audit by executable
> >   audit: clean simple fsnotify implementation
> >   audit: convert audit_exe to audit_fsnotify
> > 
> > Richard Guy Briggs (2):
> >   audit: avoid double copying the audit_exe path string
> >   Revert "fixup! audit: clean simple fsnotify implementation"
> > 
> >  include/linux/audit.h      |    1 +
> >  include/uapi/linux/audit.h |    2 +
> >  kernel/Makefile            |    2 +-
> >  kernel/audit.h             |   39 +++++++
> >  kernel/audit_exe.c         |   49 +++++++++
> >  kernel/audit_fsnotify.c    |  237
> > ++++++++++++++++++++++++++++++++++++++++++++ kernel/auditfilter.c       |  
> > 52 +++++++++-
> >  kernel/auditsc.c           |   16 +++
> >  8 files changed, 395 insertions(+), 3 deletions(-)
> >  create mode 100644 kernel/audit_exe.c
> >  create mode 100644 kernel/audit_fsnotify.c
> 

Re: [PATCH V5 0/5] audit by executable name

[Paul Moore]

On Monday, October 20, 2014 06:47:27 PM Eric Paris wrote:
> On Mon, 2014-10-20 at 16:25 -0400, Steve Grubb wrote:
> > On Thursday, October 02, 2014 11:06:51 PM Richard Guy Briggs wrote:
> > > This is a part of Peter Moody, my and Eric Paris' work to implement
> > > audit by executable name.
> > 
> > Does this patch set define an AUDIT_VERSION_SOMETHING and then set
> > AUDIT_VERSION_LATEST to it? If not, I need one to tell if the kernel
> > supports it when issuing commands. Also, if its conceivable that kernels
> > may pick and choose what features could be backported to a curated
> > kernel, should AUDIT_VERSION_ be a number that is incremented or a bit
> > mask?
> 
> Right now the value is 2. So this is your last hope if you want to make
> it a bitmask. I'll leave that up to paul/richard to (over) design.

Audit is nothing if not over-designed.  I want to make sure we're consistent 
with the previous design methodologies ;)

I've been thinking about this for about the past half-hour while I've been 
going through some other mail and I'm not really enthused about using the 
version number to encode capabilities.  What sort of problems would we have if 
we introduced a new audit netlink command to query the kernel for audit 
capabilities?

-- 
paul moore
security and virtualization @ redhat

Re: [PATCH V5 0/5] audit by executable name

[Steve Grubb]

On Monday, October 20, 2014 07:02:33 PM Paul Moore wrote:
> On Monday, October 20, 2014 06:47:27 PM Eric Paris wrote:
> > On Mon, 2014-10-20 at 16:25 -0400, Steve Grubb wrote:
> > > On Thursday, October 02, 2014 11:06:51 PM Richard Guy Briggs wrote:
> > > > This is a part of Peter Moody, my and Eric Paris' work to implement
> > > > audit by executable name.
> > > 
> > > Does this patch set define an AUDIT_VERSION_SOMETHING and then set
> > > AUDIT_VERSION_LATEST to it? If not, I need one to tell if the kernel
> > > supports it when issuing commands. Also, if its conceivable that kernels
> > > may pick and choose what features could be backported to a curated
> > > kernel, should AUDIT_VERSION_ be a number that is incremented or a bit
> > > mask?
> > 
> > Right now the value is 2. So this is your last hope if you want to make
> > it a bitmask. I'll leave that up to paul/richard to (over) design.
> 
> Audit is nothing if not over-designed.  I want to make sure we're consistent
> with the previous design methodologies ;)
> 
> I've been thinking about this for about the past half-hour while I've been
> going through some other mail and I'm not really enthused about using the
> version number to encode capabilities.  What sort of problems would we have
> if we introduced a new audit netlink command to query the kernel for audit
> capabilities?

I thought that is what we were getting in this patch:
https://www.redhat.com/archives/linux-audit/2014-January/msg00054.html

As I understood it, I send an AUDIT_GET command on netlink and then look in 
status.version to see what we have. I really think that in the mainline 
kernel, there will be a steady increment of capabilities. However, for 
distributions, they may want to pick and choose which capabilities to backport 
to their shipping kernel. Meaning in practice, a bitmap may be better to allow 
cherry picking capabilities and user space being able to make informed 
decisions.

I really don't mind if this is done by a new netlink command (but if we do, 
what happens to status.version?) or if we just keep going with status.version. 
Just tell me which it is.

-Steve

Re: [PATCH V5 0/5] audit by executable name

[Steve Grubb]

On Monday, October 20, 2014 07:33:39 PM Steve Grubb wrote:
> On Monday, October 20, 2014 07:02:33 PM Paul Moore wrote:
> > On Monday, October 20, 2014 06:47:27 PM Eric Paris wrote:
> > > On Mon, 2014-10-20 at 16:25 -0400, Steve Grubb wrote:
> > > > On Thursday, October 02, 2014 11:06:51 PM Richard Guy Briggs wrote:
> > > > > This is a part of Peter Moody, my and Eric Paris' work to implement
> > > > > audit by executable name.
> > > > 
> > > > Does this patch set define an AUDIT_VERSION_SOMETHING and then set
> > > > AUDIT_VERSION_LATEST to it? If not, I need one to tell if the kernel
> > > > supports it when issuing commands. Also, if its conceivable that
> > > > kernels
> > > > may pick and choose what features could be backported to a curated
> > > > kernel, should AUDIT_VERSION_ be a number that is incremented or a bit
> > > > mask?
> > > 
> > > Right now the value is 2. So this is your last hope if you want to make
> > > it a bitmask. I'll leave that up to paul/richard to (over) design.
> > 
> > Audit is nothing if not over-designed.  I want to make sure we're
> > consistent with the previous design methodologies ;)
> > 
> > I've been thinking about this for about the past half-hour while I've been
> > going through some other mail and I'm not really enthused about using the
> > version number to encode capabilities.  What sort of problems would we
> > have
> > if we introduced a new audit netlink command to query the kernel for audit
> > capabilities?
> 
> I thought that is what we were getting in this patch:
> https://www.redhat.com/archives/linux-audit/2014-January/msg00054.html
> 
> As I understood it, I send an AUDIT_GET command on netlink and then look in
> status.version to see what we have. I really think that in the mainline
> kernel, there will be a steady increment of capabilities. However, for
> distributions, they may want to pick and choose which capabilities to
> backport to their shipping kernel. Meaning in practice, a bitmap may be
> better to allow cherry picking capabilities and user space being able to
> make informed decisions.
> 
> I really don't mind if this is done by a new netlink command (but if we do,
> what happens to status.version?) or if we just keep going with
> status.version. Just tell me which it is.

Further to the point of status.version, its declared as a __u32. So if it were 
a bit map, we can have 32 different features userspace needs to make support 
decisions on. I have a feeling that will last many years because I really 
can't see audit gaining too many more capabilities.

-Steve

Re: [PATCH V5 0/5] audit by executable name

[Paul Moore]

On Monday, October 20, 2014 07:33:39 PM Steve Grubb wrote:
> On Monday, October 20, 2014 07:02:33 PM Paul Moore wrote:
> > On Monday, October 20, 2014 06:47:27 PM Eric Paris wrote:
> > > On Mon, 2014-10-20 at 16:25 -0400, Steve Grubb wrote:
> > > > On Thursday, October 02, 2014 11:06:51 PM Richard Guy Briggs wrote:
> > > > > This is a part of Peter Moody, my and Eric Paris' work to implement
> > > > > audit by executable name.
> > > > 
> > > > Does this patch set define an AUDIT_VERSION_SOMETHING and then set
> > > > AUDIT_VERSION_LATEST to it? If not, I need one to tell if the kernel
> > > > supports it when issuing commands. Also, if its conceivable that
> > > > kernels
> > > > may pick and choose what features could be backported to a curated
> > > > kernel, should AUDIT_VERSION_ be a number that is incremented or a bit
> > > > mask?
> > > 
> > > Right now the value is 2. So this is your last hope if you want to make
> > > it a bitmask. I'll leave that up to paul/richard to (over) design.
> > 
> > Audit is nothing if not over-designed.  I want to make sure we're
> > consistent with the previous design methodologies ;)
> > 
> > I've been thinking about this for about the past half-hour while I've been
> > going through some other mail and I'm not really enthused about using the
> > version number to encode capabilities.  What sort of problems would we
> > have if we introduced a new audit netlink command to query the kernel for
> > audit capabilities?
> 
> I thought that is what we were getting in this patch:
> https://www.redhat.com/archives/linux-audit/2014-January/msg00054.html

That patch, and the simple name "version", looks more like a version number 
and not a capabilities bitmap.  However, as Eric previously pointed out, since 
we are only at version 2, all is not lost.

> As I understood it, I send an AUDIT_GET command on netlink and then look in
> status.version to see what we have. I really think that in the mainline
> kernel, there will be a steady increment of capabilities. However, for
> distributions, they may want to pick and choose which capabilities to
> backport to their shipping kernel. Meaning in practice, a bitmap may be
> better to allow cherry picking capabilities and user space being able to
> make informed decisions.

If we are intending to use this as a way of checking for functionality then it 
really should be a bitmap and not a version number.  Regardless of if we are 
talking about an upstream or distribution kernel.

> I really don't mind if this is done by a new netlink command (but if we do,
> what happens to status.version?) or if we just keep going with
> status.version. Just tell me which it is.

No, let's stick with what we have now.  I mistakenly assumed that since the 
struct field and userspace #defines included "version" that the value was 
actually a version number ... silly me, I have no idea why I thought that.

So, with this in mind, I think a couple of small tweaks are in order (sorry 
Richard), in no particular order:

* Change the audit_status.version field comment in include/uapi/linux/audit.h 
to "/* audit functionality bitmap */", or similar.  We can't really change the 
structure now, but the comment is fair game.

* Change AUDIT_VERSION_LATEST to a bitmask instead of a number.  For example, 
it should be 3 given the current code, not 2.  In a perfect world this 
wouldn't even be in the uapi header, but it is so we need to keep it updated.  
Bumping it higher should be backwards compatible.

Can anyone think of anything else that might be affected by this?

-- 
paul moore
security and virtualization @ redhat

Re: [PATCH V5 0/5] audit by executable name

[Steve Grubb]

On Tuesday, October 21, 2014 05:56:36 PM Paul Moore wrote:
> On Monday, October 20, 2014 07:33:39 PM Steve Grubb wrote:
> > On Monday, October 20, 2014 07:02:33 PM Paul Moore wrote:
> > > On Monday, October 20, 2014 06:47:27 PM Eric Paris wrote:
> > > > On Mon, 2014-10-20 at 16:25 -0400, Steve Grubb wrote:
> > > > > On Thursday, October 02, 2014 11:06:51 PM Richard Guy Briggs wrote:
> > > > > > This is a part of Peter Moody, my and Eric Paris' work to
> > > > > > implement
> > > > > > audit by executable name.
> > > > > 
> > > > > Does this patch set define an AUDIT_VERSION_SOMETHING and then set
> > > > > AUDIT_VERSION_LATEST to it? If not, I need one to tell if the kernel
> > > > > supports it when issuing commands. Also, if its conceivable that
> > > > > kernels
> > > > > may pick and choose what features could be backported to a curated
> > > > > kernel, should AUDIT_VERSION_ be a number that is incremented or a
> > > > > bit
> > > > > mask?
> > > > 
> > > > Right now the value is 2. So this is your last hope if you want to
> > > > make
> > > > it a bitmask. I'll leave that up to paul/richard to (over) design.
> > > 
> > > Audit is nothing if not over-designed.  I want to make sure we're
> > > consistent with the previous design methodologies ;)
> > > 
> > > I've been thinking about this for about the past half-hour while I've
> > > been
> > > going through some other mail and I'm not really enthused about using
> > > the
> > > version number to encode capabilities.  What sort of problems would we
> > > have if we introduced a new audit netlink command to query the kernel
> > > for
> > > audit capabilities?
> > 
> > I thought that is what we were getting in this patch:
> > https://www.redhat.com/archives/linux-audit/2014-January/msg00054.html
> 
> That patch, and the simple name "version", looks more like a version number
> and not a capabilities bitmap.  However, as Eric previously pointed out,
> since we are only at version 2, all is not lost.
> 
> > As I understood it, I send an AUDIT_GET command on netlink and then look
> > in
> > status.version to see what we have. I really think that in the mainline
> > kernel, there will be a steady increment of capabilities. However, for
> > distributions, they may want to pick and choose which capabilities to
> > backport to their shipping kernel. Meaning in practice, a bitmap may be
> > better to allow cherry picking capabilities and user space being able to
> > make informed decisions.
> 
> If we are intending to use this as a way of checking for functionality then
> it really should be a bitmap and not a version number.  Regardless of if we
> are talking about an upstream or distribution kernel.
> 
> > I really don't mind if this is done by a new netlink command (but if we
> > do,
> > what happens to status.version?) or if we just keep going with
> > status.version. Just tell me which it is.
> 
> No, let's stick with what we have now.  I mistakenly assumed that since the
> struct field and userspace #defines included "version" that the value was
> actually a version number ... silly me, I have no idea why I thought that.
> 
> So, with this in mind, I think a couple of small tweaks are in order (sorry
> Richard), in no particular order:
> 
> * Change the audit_status.version field comment in
> include/uapi/linux/audit.h to "/* audit functionality bitmap */", or
> similar.  We can't really change the structure now, but the comment is fair
> game.
> 
> * Change AUDIT_VERSION_LATEST to a bitmask instead of a number.  For
> example, it should be 3 given the current code, not 2.  In a perfect world
> this wouldn't even be in the uapi header, but it is so we need to keep it
> updated. Bumping it higher should be backwards compatible.
> 
> Can anyone think of anything else that might be affected by this?

This plan sounds good to me.

Thanks,
-Steve

Re: [PATCH V5 0/5] audit by executable name

[Eric Paris]

On Tue, 2014-10-21 at 17:56 -0400, Paul Moore wrote:

> * Change the audit_status.version field comment in include/uapi/linux/audit.h 
> to "/* audit functionality bitmap */", or similar.  We can't really change the 
> structure now, but the comment is fair game.

Trying to think how to do things with a #define so you can rename,
"version" is pretty darn generic to pre-process.  You could make it a
union, so userspace code and use a sane name....

> 
> * Change AUDIT_VERSION_LATEST to a bitmask instead of a number.  For example, 
> it should be 3 given the current code, not 2.  In a perfect world this 
> wouldn't even be in the uapi header, but it is so we need to keep it updated.  
> Bumping it higher should be backwards compatible.

Getting 1 without 2 is actually hard to accompish as I remember, but
yes, you're right, i missed that.  I should be 3....

> Can anyone think of anything else that might be affected by this?

No one uses this stuff, just change it.

Re: [PATCH V5 0/5] audit by executable name

[Paul Moore]

On Tuesday, October 21, 2014 06:19:52 PM Eric Paris wrote:
> On Tue, 2014-10-21 at 17:56 -0400, Paul Moore wrote:
> > * Change the audit_status.version field comment in
> > include/uapi/linux/audit.h to "/* audit functionality bitmap */", or
> > similar.  We can't really change the structure now, but the comment is
> > fair game.
> 
> Trying to think how to do things with a #define so you can rename,
> "version" is pretty darn generic to pre-process.  You could make it a
> union, so userspace code and use a sane name....

Yeah, I thought about suggesting the #define approach but figured that might 
just be me worrying about the color of the paint ... okay, Richard, why don't 
you go ahead and change the version field name and put in a #define for 
compatibility.

> > Can anyone think of anything else that might be affected by this?
> 
> No one uses this stuff, just change it.

Yes, but I feel like I need to at least ask the question; how much attention I 
pay to the answers is something else ...

-- 
paul moore
security and virtualization @ redhat

Re: [PATCH V5 0/5] audit by executable name

[Richard Guy Briggs]

On 14/10/21, Paul Moore wrote:
> On Tuesday, October 21, 2014 06:19:52 PM Eric Paris wrote:
> > On Tue, 2014-10-21 at 17:56 -0400, Paul Moore wrote:
> > > * Change the audit_status.version field comment in
> > > include/uapi/linux/audit.h to "/* audit functionality bitmap */", or
> > > similar.  We can't really change the structure now, but the comment is
> > > fair game.
> > 
> > Trying to think how to do things with a #define so you can rename,
> > "version" is pretty darn generic to pre-process.  You could make it a
> > union, so userspace code and use a sane name....
> 
> Yeah, I thought about suggesting the #define approach but figured that might 
> just be me worrying about the color of the paint ... okay, Richard, why don't 
> you go ahead and change the version field name and put in a #define for 
> compatibility.

The #define is a nice way to work around backward compatibility.

> > > Can anyone think of anything else that might be affected by this?
> > 
> > No one uses this stuff, just change it.
> 
> Yes, but I feel like I need to at least ask the question; how much attention I 
> pay to the answers is something else ...

I'm still skeptical this won't blow up...  Like the capabilities bitmap
did.  I suspect there isn't agreement on what constitutes a feature.  We
just added a set/get features bitmap a year ago for things to be turned
on/off and locked...  How does this features bitmap fit in with that
features config?

I don't disagree that a bitmap would be more useful for various
distributions to pick and choose that which they choose to support over
a version number that won't tell the whole story.

> paul moore

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Re: [PATCH V5 0/5] audit by executable name

[Steve Grubb]

On Wednesday, October 29, 2014 03:48:40 PM Richard Guy Briggs wrote:
> On 14/10/21, Paul Moore wrote:
> > > > Can anyone think of anything else that might be affected by this?
> > > 
> > > No one uses this stuff, just change it.
> > 
> > Yes, but I feel like I need to at least ask the question; how much
> > attention I pay to the answers is something else ...
> 
> I'm still skeptical this won't blow up...  Like the capabilities bitmap
> did.  I suspect there isn't agreement on what constitutes a feature.

Anything major that user space would have to know about to determine if its 
supported. If you don't know, just ask if we need to add a bit to the bitmap. 
Some examples, adding the object comparison engine, adding the loginuid-
immutable feature, if we added filtering on TTY that would also qualify (not 
asking for that). Otherwise, user space get EINVAL on the netlink operation 
which is not useful in explaining why the command was rejected.


> We just added a set/get features bitmap a year ago for things to be turned
> on/off and locked...  How does this features bitmap fit in with that
> features config?

I think of that as commanding the features, not determining if they exist.

> I don't disagree that a bitmap would be more useful for various
> distributions to pick and choose that which they choose to support over
> a version number that won't tell the whole story.

I also can be used to allow deprecation in a controlled way such that helpful 
messages are given to the system admin.

-Steve

Re: [PATCH V5 0/5] audit by executable name

[Richard Guy Briggs]

On 14/10/29, Steve Grubb wrote:
> On Wednesday, October 29, 2014 03:48:40 PM Richard Guy Briggs wrote:
> > On 14/10/21, Paul Moore wrote:
> > > > > Can anyone think of anything else that might be affected by this?
> > > > 
> > > > No one uses this stuff, just change it.
> > > 
> > > Yes, but I feel like I need to at least ask the question; how much
> > > attention I pay to the answers is something else ...
> > 
> > I'm still skeptical this won't blow up...  Like the capabilities bitmap
> > did.  I suspect there isn't agreement on what constitutes a feature.
> 
> Anything major that user space would have to know about to determine if its 
> supported. If you don't know, just ask if we need to add a bit to the bitmap. 
> Some examples, adding the object comparison engine, adding the loginuid-
> immutable feature, if we added filtering on TTY that would also qualify (not 
> asking for that). Otherwise, user space get EINVAL on the netlink operation 
> which is not useful in explaining why the command was rejected.

Well, I guess this falls under Linus' "thou shalt not break userspace",
but it would certainly be tempting to change some of those to
EOPNOTSUPP.

> > We just added a set/get features bitmap a year ago for things to be turned
> > on/off and locked...  How does this features bitmap fit in with that
> > features config?
> 
> I think of that as commanding the features, not determining if they exist.

Which partly addresses another thing that occured to me which was that
there could be overlap between the two.  status.version will have more
capacity due to only one bit needed per feature.

> > I don't disagree that a bitmap would be more useful for various
> > distributions to pick and choose that which they choose to support over
> > a version number that won't tell the whole story.
> 
> I also can be used to allow deprecation in a controlled way such that helpful 
> messages are given to the system admin.

That would work only for new things added, enabled explicitly with that
bit set in the bitfield.

> -Steve

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Re: [PATCH V5 0/5] audit by executable name

[Eric Paris]

On Wed, 2014-10-29 at 17:54 -0400, Richard Guy Briggs wrote:
> On 14/10/29, Steve Grubb wrote:
> > On Wednesday, October 29, 2014 03:48:40 PM Richard Guy Briggs wrote:
> > > On 14/10/21, Paul Moore wrote:
> > > > > > Can anyone think of anything else that might be affected by this?
> > > > > 
> > > > > No one uses this stuff, just change it.
> > > > 
> > > > Yes, but I feel like I need to at least ask the question; how much
> > > > attention I pay to the answers is something else ...
> > > 
> > > I'm still skeptical this won't blow up...  Like the capabilities bitmap
> > > did.  I suspect there isn't agreement on what constitutes a feature.
> > 
> > Anything major that user space would have to know about to determine if its 
> > supported. If you don't know, just ask if we need to add a bit to the bitmap. 
> > Some examples, adding the object comparison engine, adding the loginuid-
> > immutable feature, if we added filtering on TTY that would also qualify (not 
> > asking for that). Otherwise, user space get EINVAL on the netlink operation 
> > which is not useful in explaining why the command was rejected.
> 
> Well, I guess this falls under Linus' "thou shalt not break userspace",
> but it would certainly be tempting to change some of those to
> EOPNOTSUPP.

You only break userspace if something breaks   :)

Re: [PATCH V5 0/5] audit by executable name

[Richard Guy Briggs]

On 14/10/29, Eric Paris wrote:
> On Wed, 2014-10-29 at 17:54 -0400, Richard Guy Briggs wrote:
> > On 14/10/29, Steve Grubb wrote:
> > > On Wednesday, October 29, 2014 03:48:40 PM Richard Guy Briggs wrote:
> > > > On 14/10/21, Paul Moore wrote:
> > > > > > > Can anyone think of anything else that might be affected by this?
> > > > > > 
> > > > > > No one uses this stuff, just change it.
> > > > > 
> > > > > Yes, but I feel like I need to at least ask the question; how much
> > > > > attention I pay to the answers is something else ...
> > > > 
> > > > I'm still skeptical this won't blow up...  Like the capabilities bitmap
> > > > did.  I suspect there isn't agreement on what constitutes a feature.
> > > 
> > > Anything major that user space would have to know about to determine if its 
> > > supported. If you don't know, just ask if we need to add a bit to the bitmap. 
> > > Some examples, adding the object comparison engine, adding the loginuid-
> > > immutable feature, if we added filtering on TTY that would also qualify (not 
> > > asking for that). Otherwise, user space get EINVAL on the netlink operation 
> > > which is not useful in explaining why the command was rejected.
> > 
> > Well, I guess this falls under Linus' "thou shalt not break userspace",
> > but it would certainly be tempting to change some of those to
> > EOPNOTSUPP.
> 
> You only break userspace if something breaks   :)

So which scratch monkey do we mount before sending it upstream?

We saw how actually allowing CAP_AUDIT_WRITE from non-init PID
namespaces backfired on us in stuff which didn't use audit before...

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545