Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

2014-10-21 Thread Richard Guy Briggs
On 14/10/07, Richard Guy Briggs wrote:
 On 14/10/07, Eric Paris wrote:
  On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
   Log the event when a client attempts to connect to the netlink audit 
   multicast
   socket, requiring CAP_AUDIT_READ capability, binding to the 
   AUDIT_NLGRP_READLOG
   group.  Log the disconnect too.

  super crazy yuck.  audit_log_task_info() ??
 
 I agree.  I already suggested that a while ago.  I'd love to.  sgrubb
 thinks it dumps way too much info.  We still haven't got a definitive
 answer about what is enough and what is too much info for any given type
 of record.
 
 I also thought of moving audit_log_task() from auditsc.c to audit.c
 and using that.  For that matter, both audit_log_task() and
 audit_log_task_info() could use audit_log_session_info(), but they are
 in slightly different order of keywords which will upset sgrubb's
 parser.
 
 What to do?
 
 Another paragraph I'd like to see added to
   http://people.redhat.com/sgrubb/audit/audit-parse.txt
 would be a canonical order of keywords.  However, that discussion went
 nowhere.  Would it be reasonable to suggest only two possible orders
 instead of the almost infinite iterations possible and declare a
 standard order of keywords and gradually move to it?

Steve,

Can we agree to *two* orders (instead of the full set of iterations) for
these keywords so that we can start to sort things in a canonical order?
This random order per type of audit log message is chaos.

 - RGB

- RGB

--
Richard Guy Briggs rbri...@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

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


Re: [PATCH] audit: add Paul Moore to the MAINTAINERS entry

2014-10-21 Thread Eric Paris
On Mon, 2014-10-20 at 12:23 -0400, Paul Moore wrote:
 After a long stint maintaining the audit tree, Eric asked me to step
 in and handle the day-to-day management of the audit tree.  We should
 also update the linux-audit mailing list entry to better reflect
 current usage.
 
 Signed-off-by: Paul Moore pmo...@redhat.com

Acked-by: Eric Paris epa...@redhat.com

 ---
  MAINTAINERS |5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/MAINTAINERS b/MAINTAINERS
 index c2066f4..86c24fd 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -1689,10 +1689,11 @@ S:Supported
  F:   drivers/scsi/esas2r
  
  AUDIT SUBSYSTEM
 +M:   Paul Moore p...@paul-moore.com
  M:   Eric Paris epa...@redhat.com
 -L:   linux-audit@redhat.com (subscribers-only)
 +L:   linux-audit@redhat.com (moderated for non-subscribers)
  W:   http://people.redhat.com/sgrubb/audit/
 -T:   git git://git.infradead.org/users/eparis/audit.git
 +T:   git git://git.infradead.org/users/pcmoore/audit
  S:   Maintained
  F:   include/linux/audit.h
  F:   include/uapi/linux/audit.h
 


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


Changing the audit tree

2014-10-21 Thread Paul Moore
Hi Stephen,

The audit tree has just changed hands and as a result the git repo has 
changed.  The new location is:

 * git://git.infradead.org/users/pcmoore/audit next

Thanks,
-Paul

-- 
paul moore
www.paul-moore.com

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


Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

2014-10-21 Thread Richard Guy Briggs
On 14/10/21, Steve Grubb wrote:
 On Tuesday, October 07, 2014 03:03:14 PM Eric Paris wrote:
  On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
   Log the event when a client attempts to connect to the netlink audit
   multicast socket, requiring CAP_AUDIT_READ capability, binding to the
   AUDIT_NLGRP_READLOG group.  Log the disconnect too.
  
   
  
   Sample output:
   time-Tue Oct  7 14:15:19 2014
   type=UNKNOWN[1348] msg=audit(1412705719.316:117): auid=0 uid=0 gid=0 ses=1
   pid=3552 comm=audit-multicast
   exe=/home/rgb/rgb/git/audit-multicast-listen/audit-multicast-listen
   subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 group=0
   op=connect res=1
   
  
   Signed-off-by: Richard Guy Briggs r...@redhat.com
   ---
   For some reason unbind isn't being called on disconnect.  I suspect
   missing
   plumbing in netlink.  Investigation needed...
  
   
include/uapi/linux/audit.h |1 +
kernel/audit.c |   46
  ++- 2 files changed, 45
  insertions(+), 2 deletions(-)
   
  
   diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
   index 4d100c8..7fa6e8f 100644
   --- a/include/uapi/linux/audit.h
   +++ b/include/uapi/linux/audit.h
   @@ -110,6 +110,7 @@
  
#define AUDIT_SECCOMP1326/* Secure Computing event */
#define AUDIT_PROCTITLE  1327/* Proctitle emit event */
#define AUDIT_FEATURE_CHANGE 1328/* audit log listing feature changes
  */
   +#define AUDIT_EVENT_LISTENER 1348/* task joined multicast read socket
   */

#define AUDIT_AVC1400/* SE Linux avc denial or grant */
#define AUDIT_SELINUX_ERR1401/* Internal SE Linux Errors */
  
   diff --git a/kernel/audit.c b/kernel/audit.c
   index 53bb39b..74c81a7 100644
   --- a/kernel/audit.c
   +++ b/kernel/audit.c
   @@ -1108,13 +1108,54 @@ static void audit_receive(struct sk_buff  *skb)
  
 mutex_unlock(audit_cmd_mutex);
}

  
   +static void audit_log_bind(int group, char *op, int err)
   +{
   + struct audit_buffer *ab;
   + char comm[sizeof(current-comm)];
   + struct mm_struct *mm = current-mm;
   +
   + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
   + if (!ab)
   + return;
   +
   + audit_log_format(ab, auid=%d,
   + from_kuid(init_user_ns,
   audit_get_loginuid(current))); + audit_log_format(ab,  uid=%d,
   +  from_kuid(init_user_ns, current_uid()));
   + audit_log_format(ab,  gid=%d,
   +  from_kgid(init_user_ns, current_gid()));
   + audit_log_format(ab,  ses=%d, audit_get_sessionid(current));
   + audit_log_format(ab,  pid=%d, task_pid_nr(current));
   + audit_log_format(ab,  comm=);
   + audit_log_untrustedstring(ab, get_task_comm(comm, current));
   + if (mm) {
   + down_read(mm-mmap_sem);
   + if (mm-exe_file)
   + audit_log_d_path(ab,  exe=,
   mm-exe_file-f_path);
   + up_read(mm-mmap_sem);
   + } else 
   + audit_log_format(ab,  exe=(null));
   + audit_log_task_context(ab); /* subj= */
  
  super crazy yuck.  audit_log_task_info() ??
 
 audit_log_task_info logs too much information for typical use. There are 
 times 
 when you might want to know everything about what's connecting. But in this 
 case, we don't need anything about groups, saved uids, fsuid, or ppid.
 
 Its a shame we don't have a audit_log_task_info_light function which only 
 records:
 
 pid= auid= uid= subj= comm= exe=  ses= tty=

We already have audit_log_task() which gives:
auid=
uid=
gid=
ses=
subj=
pid=
comm=
exe=
This is missing tty=, but has gid=.  Can we please use that function
instead and add tty=?  And while we are at it, refactor
audit_log_task_info() to call audit_log_task()?

Is this standard set above what should be used for certain classes of
log messages?

Yes, it will be in a different order because we don't have a canonical
order yet.  Can we accept two orders of keywords so we can start
canonicalizing, please?

   + audit_log_format(ab,  group=%d, group);
  
  group seems like too easily confused a name.
 
 nlnk-grp is better if its what I think it is.

Where did you find that name?  That could work and it is shorter, but it
seems awkwardly optimized.  nlnk doesn't appear once in the kernel.
nl is already recognized for netlink, mcgrp is already used for
multicast group(s), so I would suggest nl-mcgrp.

 -Steve

- RGB

--
Richard Guy Briggs rbri...@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

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


Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

2014-10-21 Thread Steve Grubb
On Tuesday, October 21, 2014 05:08:22 PM Richard Guy Briggs wrote:
 On 14/10/21, Steve Grubb wrote:
   super crazy yuck.  audit_log_task_info() ??
  
  audit_log_task_info logs too much information for typical use. There are
  times when you might want to know everything about what's connecting. But
  in this case, we don't need anything about groups, saved uids, fsuid, or
  ppid.
  
  Its a shame we don't have a audit_log_task_info_light function which only
  records:
  
  pid= auid= uid= subj= comm= exe=  ses= tty=
 
 We already have audit_log_task() which gives:
   auid=
   uid=
   gid=
   ses=
   subj=
   pid=
   comm=
   exe=
 This is missing tty=, but has gid=.  Can we please use that function
 instead and add tty=?

gid is important for things that might allow access by file permissions. But 
the syscall logging is going to have that and much more. In this case, access 
is granted by having a posix capability. So, all we want is what's the 
process, who's the user, which session/tty is this coming from to find all 
events that might be related.


 And while we are at it, refactor audit_log_task_info() to call
 audit_log_task()?

That will cause problems at this point.

 Is this standard set above what should be used for certain classes of
 log messages?

Its hard to say if that is sufficient for all cases. When access is granted by 
posix capability, sure. This is probably good for most kernel originating 
events. But there are times extra info is needed.

 Yes, it will be in a different order because we don't have a canonical
 order yet.  Can we accept two orders of keywords so we can start
 canonicalizing, please?

I don't understand what you are getting at.

-Steve

+ audit_log_format(ab,  group=%d, group);
   
   group seems like too easily confused a name.
  
  nlnk-grp is better if its what I think it is.
 
 Where did you find that name?  That could work and it is shorter, but it
 seems awkwardly optimized.  nlnk doesn't appear once in the kernel.
 nl is already recognized for netlink, mcgrp is already used for
 multicast group(s), so I would suggest nl-mcgrp.
 
  -Steve
 
 - RGB
 
 --
 Richard Guy Briggs rbri...@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

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


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

2014-10-21 Thread 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

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


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

2014-10-21 Thread 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

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


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

2014-10-21 Thread 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.


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


Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

2014-10-21 Thread Paul Moore
On Tuesday, October 21, 2014 03:56:10 PM Steve Grubb wrote:
 audit_log_task_info logs too much information for typical use. There are
 times when you might want to know everything about what's connecting. But
 in this case, we don't need anything about groups, saved uids, fsuid, or
 ppid.
 
 Its a shame we don't have a audit_log_task_info_light function which only
 records:
 
 pid= auid= uid= subj= comm= exe=  ses= tty=

This is getting back to my earlier concerns/questions about field ordering, or 
at the very least I'm going to hijack this conversation and steer it towards 
field ordering ;)

Before we go to much farther, I'd really like us to agree that ordering is not 
important, can we do that?  As a follow up, what do we need to do to make that 
happen in the userspace tools?

-- 
paul moore
security and virtualization @ redhat

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


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

2014-10-21 Thread 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

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


Re: Changing the audit tree

2014-10-21 Thread Paul Moore
On Wednesday, October 22, 2014 09:19:10 AM Stephen Rothwell wrote:
 Hi Paul,
 
 On Tue, 21 Oct 2014 17:00:48 -0400 Paul Moore p...@paul-moore.com wrote:
  The audit tree has just changed hands and as a result the git repo has
  
  changed.  The new location is:
   * git://git.infradead.org/users/pcmoore/audit next
 
 OK, I have updated to that and have this address as the (sole) new
 contact.

Great, thank you.

-- 
paul moore
www.paul-moore.com

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


Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

2014-10-21 Thread Richard Guy Briggs
On 14/10/21, Paul Moore wrote:
 On Tuesday, October 21, 2014 03:56:10 PM Steve Grubb wrote:
  audit_log_task_info logs too much information for typical use. There are
  times when you might want to know everything about what's connecting. But
  in this case, we don't need anything about groups, saved uids, fsuid, or
  ppid.
  
  Its a shame we don't have a audit_log_task_info_light function which only
  records:
  
  pid= auid= uid= subj= comm= exe=  ses= tty=
 
 This is getting back to my earlier concerns/questions about field ordering, 
 or 
 at the very least I'm going to hijack this conversation and steer it towards 
 field ordering ;)

Well, I've already been pushing it that way because it interferes with
any sort of refactoring that needs to be done to simplify and clean up
the kernel log code.

 Before we go to much farther, I'd really like us to agree that ordering is 
 not 
 important, can we do that?  As a follow up, what do we need to do to make 
 that 
 happen in the userspace tools?

At the very least, as I've suggested, agree on at least one more order,
a canonical one, that can provide a much more firm guide how to present
the keywords so that we're not stuck with an arbitrary order that turns
out not to make sense for some reason or another.

 paul moore

- RGB

--
Richard Guy Briggs rbri...@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

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