netdev - Re: Audit vs netlink interaction problem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47DE254A.5070608@openvz.org>
Date:	Mon, 17 Mar 2008 11:01:14 +0300
From:	Pavel Emelyanov <xemul@...nvz.org>
To:	Thomas Graf <tgraf@...g.ch>, David Woodhouse <dwmw2@...radead.org>
CC:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: Audit vs netlink interaction problem

Thomas Graf wrote:
> * Thomas Graf <tgraf@...g.ch> 2008-03-14 19:29
>> * Pavel Emelyanov <xemul@...nvz.org> 2008-03-14 20:05
>>> Hmmm... I'm afraid, that this can break the audit filtering and signal
>>> auditing. I haven't yet looked deep into it, but it compares the 
>>> task->tgid with this audit_pid for different purposes. If audit_pid
>>> changes this code will be broken.
>> OK, then both pids have to be stored. audit_pid remains as-is but is
>> no longer used as destination netlink pid. A second pid is stored and
>> updated whenever a netlink message is received from userspace.
> 
> The following patch represents what I mean. Untested!

Looks great, all the more so I created very similar patch.
David, can we have this in mainline some day?

Thanks,
Pavel

> Index: net-2.6.26/kernel/audit.c
> ===================================================================
> --- net-2.6.26.orig/kernel/audit.c	2008-03-14 19:31:53.000000000 +0100
> +++ net-2.6.26/kernel/audit.c	2008-03-14 19:38:35.000000000 +0100
> @@ -82,6 +82,9 @@
>   * contains the (non-zero) pid. */
>  int		audit_pid;
>  
> +/* Actual netlink pid of the userspace process */
> +static int	audit_nlk_pid;
> +
>  /* If audit_rate_limit is non-zero, limit the rate of sending audit records
>   * to that number per second.  This prevents DoS attacks, but results in
>   * audit records being dropped. */
> @@ -347,12 +350,12 @@
>  		skb = skb_dequeue(&audit_skb_queue);
>  		wake_up(&audit_backlog_wait);
>  		if (skb) {
> -			if (audit_pid) {
> -				int err = netlink_unicast(audit_sock, skb, audit_pid, 0);
> +			if (audit_nlk_pid) {
> +				int err = netlink_unicast(audit_sock, skb, audit_nlk_pid, 0);
>  				if (err < 0) {
>  					BUG_ON(err != -ECONNREFUSED); /* Shoudn't happen */
> -					printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n", audit_pid);
> -					audit_pid = 0;
> +					printk(KERN_ERR "audit: *NO* daemon at audit_nlk_pid=%d\n", audit_nlk_pid);
> +					audit_nlk_pid = 0;
>  				}
>  			} else {
>  				if (printk_ratelimit())
> @@ -623,6 +626,12 @@
>  							sid, 1);
>  
>  			audit_pid = new_pid;
> +
> +			/*
> +			 * Netlink pid is only updated here to avoid overwrites
> +			 * from potential processes only querying the interface.
> +			 */
> +			audit_nlk_pid = NETLINK_CB(skb).pid;
>  		}
>  		if (status_get->mask & AUDIT_STATUS_RATE_LIMIT)
>  			err = audit_set_rate_limit(status_get->rate_limit,
> @@ -1350,7 +1359,7 @@
>  	if (!audit_rate_check()) {
>  		audit_log_lost("rate limit exceeded");
>  	} else {
> -		if (audit_pid) {
> +		if (audit_nlk_pid) {
>  			struct nlmsghdr *nlh = nlmsg_hdr(ab->skb);
>  			nlh->nlmsg_len = ab->skb->len - NLMSG_SPACE(0);
>  			skb_queue_tail(&audit_skb_queue, ab->skb);
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html