Re: [PATCH] SELinux: define audit permissions for audit tree netlink messages

2009-06-02 Thread James Morris
On Tue, 2 Jun 2009, Eric Paris wrote:

> Audit trees defined 2 new netlink messages but the netlink mapping tables for
> selinux permissions were not set up.  This patch maps these 2 new operations
> to AUDIT_WRITE.
> 
> Signed-off-by: Eric Paris 


Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next

> ---
> 
>  security/selinux/nlmsgtab.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
> index c6875fd..dd7cc6d 100644
> --- a/security/selinux/nlmsgtab.c
> +++ b/security/selinux/nlmsgtab.c
> @@ -112,6 +112,8 @@ static struct nlmsg_perm nlmsg_audit_perms[] =
>   { AUDIT_DEL_RULE,   NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
>   { AUDIT_USER,   NETLINK_AUDIT_SOCKET__NLMSG_RELAY},
>   { AUDIT_SIGNAL_INFO,NETLINK_AUDIT_SOCKET__NLMSG_READ },
> + { AUDIT_TRIM,   NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
> + { AUDIT_MAKE_EQUIV, NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
>   { AUDIT_TTY_GET,NETLINK_AUDIT_SOCKET__NLMSG_READ },
>   { AUDIT_TTY_SET,NETLINK_AUDIT_SOCKET__NLMSG_TTY_AUDIT   },
>  };
> 

-- 
James Morris


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


[PATCH] SELinux: define audit permissions for audit tree netlink messages

2009-06-02 Thread Eric Paris
Audit trees defined 2 new netlink messages but the netlink mapping tables for
selinux permissions were not set up.  This patch maps these 2 new operations
to AUDIT_WRITE.

Signed-off-by: Eric Paris 
---

 security/selinux/nlmsgtab.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
index c6875fd..dd7cc6d 100644
--- a/security/selinux/nlmsgtab.c
+++ b/security/selinux/nlmsgtab.c
@@ -112,6 +112,8 @@ static struct nlmsg_perm nlmsg_audit_perms[] =
{ AUDIT_DEL_RULE,   NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
{ AUDIT_USER,   NETLINK_AUDIT_SOCKET__NLMSG_RELAY},
{ AUDIT_SIGNAL_INFO,NETLINK_AUDIT_SOCKET__NLMSG_READ },
+   { AUDIT_TRIM,   NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
+   { AUDIT_MAKE_EQUIV, NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
{ AUDIT_TTY_GET,NETLINK_AUDIT_SOCKET__NLMSG_READ },
{ AUDIT_TTY_SET,NETLINK_AUDIT_SOCKET__NLMSG_TTY_AUDIT   },
 };

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


Re: [PATCH] Audit: fix audit nlmsg_len from the kernel

2009-06-02 Thread Eric Paris
On Tue, 2009-06-02 at 16:45 -0400, Steve Grubb wrote:
> On Tuesday 02 June 2009 04:38:02 pm Eric Paris wrote:
> > > But lets fix it anyway rather than rely on dumb luck and implementation
> > > details.
> >
> > Self NAK.   Userspace was also written by someone who didn't know
> > netlink.  Since userspace is half using the netlink macros and half
> > depending on this broken nlmsg_len implementation I don't think we can
> > make any changes in the kernel and they wouldn't be backwards
> > compatible...
> 
> The netlink code is the oldest part of the audit code and predates my 
> involvement. We could fix this if userspace had a way of querying the kernel 
> to 
> see what the audit capabilities are. There have been many times that I've 
> wanted a way to ask the kernel if certain things are supported.
> 
> -Steve

Although yes, that lets new userspace work on old and new kernels it's
not the problem I'm thinking about.  I'm more worried about new kernel
breaking old userspace, a huge no no, since the new kernel has no way of
knowing if userspace supports correct netlink semantics or not

In any case, this patch should certainly not be applied.

-Eric

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


Re: [PATCH] Audit: fix audit nlmsg_len from the kernel

2009-06-02 Thread Steve Grubb
On Tuesday 02 June 2009 04:38:02 pm Eric Paris wrote:
> > But lets fix it anyway rather than rely on dumb luck and implementation
> > details.
>
> Self NAK.   Userspace was also written by someone who didn't know
> netlink.  Since userspace is half using the netlink macros and half
> depending on this broken nlmsg_len implementation I don't think we can
> make any changes in the kernel and they wouldn't be backwards
> compatible...

The netlink code is the oldest part of the audit code and predates my 
involvement. We could fix this if userspace had a way of querying the kernel to 
see what the audit capabilities are. There have been many times that I've 
wanted a way to ask the kernel if certain things are supported.

-Steve

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


Re: [PATCH] Audit: fix audit nlmsg_len from the kernel

2009-06-02 Thread Eric Paris
On Thu, 2009-05-28 at 19:17 -0400, Eric Paris wrote:
> Currently audit emits it's netlink record setting the nlmsg_len equal to the
> size of the data in the netlink message rather than the size of the netlink
> header plus the size of the data in the msg.  nlmsg_end() happens to be a
> nice function which calculates and sets nlmsg_len for you, so we use that
> function to make sure we get it right from now on.
> 
> The busted nlmsg_len doesn't hurt anything as long as sizeof(data) >
> sizeof(nlmsg_hdr) and we don't pack mulitple netlink messages into one skb.
> Since sizeof(nlmsg_hdr)=16 and we send (as it turns out by sheer dumb luck) at
> least 16 bytes of audit timestamp into every message we aren't running into
> problems.
> 
> But lets fix it anyway rather than rely on dumb luck and implementation
> details.

Self NAK.   Userspace was also written by someone who didn't know
netlink.  Since userspace is half using the netlink macros and half
depending on this broken nlmsg_len implementation I don't think we can
make any changes in the kernel and they wouldn't be backwards
compatible...

With this patch userspace prints crap on the end of the intended
message.  Not sure how I missed it at first

I'm going to instead rewrite the userspace stuff to completely expect
the broken behaviour rather than half netlink macros and half broken
implementation

uggg

Do not apply this patch.



> 
> Signed-off-by: Eric Paris 
> ---
> 
>  kernel/audit.c |   13 -
>  1 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 9442c35..5c2ccef 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1468,22 +1468,25 @@ void audit_log_end(struct audit_buffer *ab)
>   if (!audit_rate_check()) {
>   audit_log_lost("rate limit exceeded");
>   } else {
> - struct nlmsghdr *nlh = nlmsg_hdr(ab->skb);
> - nlh->nlmsg_len = ab->skb->len - NLMSG_SPACE(0);
> + struct sk_buff *skb = ab->skb;
> + struct nlmsghdr *nlh = nlmsg_hdr(skb);
> +
> + /* correctly account for nlh->nlmsg_len */
> + nlmsg_end(skb, nlh);
>  
>   if (audit_pid) {
> - skb_queue_tail(&audit_skb_queue, ab->skb);
> + skb_queue_tail(&audit_skb_queue, skb);
>   wake_up_interruptible(&kauditd_wait);
>   } else {
>   if (nlh->nlmsg_type != AUDIT_EOE) {
>   if (printk_ratelimit()) {
>   printk(KERN_NOTICE "type=%d %s\n",
>   nlh->nlmsg_type,
> - ab->skb->data + NLMSG_SPACE(0));
> + skb->data + NLMSG_SPACE(0));
>   } else
>   audit_log_lost("printk limit 
> exceeded\n");
>   }
> - audit_hold_skb(ab->skb);
> + audit_hold_skb(skb);
>   }
>   ab->skb = NULL;
>   }
> 

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