3:32 p.m.
Convert audit from only listening in init_net to use register_pernet_subsys()
to dynamically manage the netlink socket list.
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
kernel/audit.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++---------
kernel/audit.h | 4 +++
2 files changed, 57 insertions(+), 11 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 91e53d0..06e2676 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -64,6 +64,7 @@
#include <linux/freezer.h>
#include <linux/tty.h>
#include <linux/pid_namespace.h>
+#include <net/netns/generic.h>
#include "audit.h"
@@ -122,6 +123,7 @@ static atomic_t audit_lost = ATOMIC_INIT(0);
/* The netlink socket. */
static struct sock *audit_sock;
+int audit_net_id;
/* Hash for inode-based rules */
struct list_head audit_inode_hash[AUDIT_INODE_BUCKETS];
@@ -391,6 +393,7 @@ static void kauditd_send_skb(struct sk_buff *skb)
printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n", audit_pid);
audit_log_lost("auditd disappeared\n");
audit_pid = 0;
+ audit_sock = NULL;
/* we might get lucky and get this in the next auditd */
audit_hold_skb(skb);
} else
@@ -474,13 +477,15 @@ int audit_send_list(void *_dest)
struct audit_netlink_list *dest = _dest;
int pid = dest->pid;
struct sk_buff *skb;
+ struct net *net = get_net_ns_by_pid(pid);
+ struct audit_net *aunet = net_generic(net, audit_net_id);
/* wait for parent to finish and send an ACK */
mutex_lock(&audit_cmd_mutex);
mutex_unlock(&audit_cmd_mutex);
while ((skb = __skb_dequeue(&dest->q)) != NULL)
- netlink_unicast(audit_sock, skb, pid, 0);
+ netlink_unicast(aunet->nlsk, skb, pid, 0);
kfree(dest);
@@ -515,13 +520,15 @@ out_kfree_skb:
static int audit_send_reply_thread(void *arg)
{
struct audit_reply *reply = (struct audit_reply *)arg;
+ struct net *net = get_net_ns_by_pid(reply->pid);
+ struct audit_net *aunet = net_generic(net, audit_net_id);
mutex_lock(&audit_cmd_mutex);
mutex_unlock(&audit_cmd_mutex);
/* Ignore failure. It'll only happen if the sender goes away,
because our timeout is set to infinite. */
- netlink_unicast(audit_sock, reply->skb, reply->pid, 0);
+ netlink_unicast(aunet->nlsk , reply->skb, reply->pid, 0);
kfree(reply);
return 0;
}
@@ -690,6 +697,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr
*nlh)
audit_log_config_change("audit_pid", new_pid, audit_pid, 1);
audit_pid = new_pid;
audit_nlk_portid = NETLINK_CB(skb).portid;
+ audit_sock = NETLINK_CB(skb).sk;
}
if (status_get->mask & AUDIT_STATUS_RATE_LIMIT) {
err = audit_set_rate_limit(status_get->rate_limit);
@@ -886,24 +894,58 @@ static void audit_receive(struct sk_buff *skb)
mutex_unlock(&audit_cmd_mutex);
}
-/* Initialize audit support at boot time. */
-static int __init audit_init(void)
+static int __net_init audit_net_init(struct net *net)
{
- int i;
struct netlink_kernel_cfg cfg = {
.input = audit_receive,
};
+ struct audit_net *aunet = net_generic(net, audit_net_id);
+
+ pr_info("audit: initializing netlink socket in namespace\n");
+
+ aunet->nlsk = netlink_kernel_create(net, NETLINK_AUDIT, &cfg);
+ if (aunet->nlsk == NULL)
+ return -ENOMEM;
+ if (!aunet->nlsk)
+ audit_panic("cannot initialize netlink socket in namespace");
+ else
+ aunet->nlsk->sk_sndtimeo = MAX_SCHEDULE_TIMEOUT;
+ return 0;
+}
+
+static void __net_exit audit_net_exit(struct net *net)
+{
+ struct audit_net *aunet = net_generic(net, audit_net_id);
+ struct sock *sock = aunet->nlsk;
+ if (sock == audit_sock) {
+ audit_pid = 0;
+ audit_sock = NULL;
+ }
+
+ rcu_assign_pointer(aunet->nlsk, NULL);
+ synchronize_net();
+ netlink_kernel_release(sock);
+}
+
+static struct pernet_operations __net_initdata audit_net_ops = {
+ .init = audit_net_init,
+ .exit = audit_net_exit,
+ .id = &audit_net_id,
+ .size = sizeof(struct audit_net),
+};
+
+/* Initialize audit support at boot time. */
+static int __init audit_init(void)
+{
+ int i;
+
if (audit_initialized == AUDIT_DISABLED)
return 0;
- printk(KERN_INFO "audit: initializing netlink socket (%s)\n",
+ pr_info("audit: initializing netlink subsys (%s)\n",
audit_default ? "enabled" : "disabled");
- audit_sock = netlink_kernel_create(&init_net, NETLINK_AUDIT, &cfg);
- if (!audit_sock)
- audit_panic("cannot initialize netlink socket");
- else
- audit_sock->sk_sndtimeo = MAX_SCHEDULE_TIMEOUT;
+ register_pernet_subsys(&audit_net_ops);
skb_queue_head_init(&audit_skb_queue);
skb_queue_head_init(&audit_skb_hold_queue);
diff --git a/kernel/audit.h b/kernel/audit.h
index 123c9b7..b7cc537 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -249,6 +249,10 @@ struct audit_netlink_list {
int audit_send_list(void *);
+struct audit_net {
+ struct sock *nlsk;
+};
+
extern int selinux_audit_rule_update(void);
extern struct mutex audit_filter_mutex;
--
1.7.1
10:54 p.m.
Hi, Richard
On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
Convert audit from only listening in init_net to use
register_pernet_subsys()
to dynamically manage the netlink socket list.
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
Right now audit still can't be used in uninit pid/user namespace,
Consider this, when user in uninit pid/user namespace is allowed
to setup/run audit subsystem, since the kernel thread always runs
in init pid namespace, so we can't get right net namespace through
get_net_ns_by_pid, The audit information will be sent to incorrect
net namespace by kernel thread.
In my opinion, This patch is limited and nonextensile.
Maybe you should check the patchset "[Part1 PATCH 00/22] Add namespace support for
audit"
I sent in 06/19/2013, In my solution, audit kernel side netlink sockets belongs
to user namespace, and the user space audit netlink sockets will find the audit
kernel socket through current_net_ns()->user_ns->audit.sock.
The "[PATCH 04/22] netlink: Add compare function for netlink_table" of this
patchset
has been merged in linux mainline. I think if you look at my patchset, you will find
the [PATCH 03/22] and [PATCH 05/22] will achieve the same aim of your patch.
Thanks!
4:15 p.m.
On Wed, Jul 17, 2013 at 11:54:21AM +0800, Gao feng wrote:
Hi, Richard
On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
> Convert audit from only listening in init_net to use register_pernet_subsys()
> to dynamically manage the netlink socket list.
>
> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> ---
Right now audit still can't be used in uninit pid/user namespace,
Consider this, when user in uninit pid/user namespace is allowed
to setup/run audit subsystem, since the kernel thread always runs
in init pid namespace, so we can't get right net namespace through
get_net_ns_by_pid, The audit information will be sent to incorrect
net namespace by kernel thread.
In my opinion, This patch is limited and nonextensile.
Maybe you should check the patchset "[Part1 PATCH 00/22] Add namespace support for
audit"
I sent in 06/19/2013, In my solution, audit kernel side netlink sockets belongs
to user namespace, and the user space audit netlink sockets will find the audit
kernel socket through current_net_ns()->user_ns->audit.sock.
I already looked at your 48-patch and 22-patch sets and the threads of
comments. The concerns expressed in that thread haven't been fully
addressed yet by you.
The "[PATCH 04/22] netlink: Add compare function for
netlink_table" of this patchset
has been merged in linux mainline. I think if you look at my patchset, you will find
the [PATCH 03/22] and [PATCH 05/22] will achieve the same aim of your patch.
I don't have any specific issues with patch 04/22.
For patch 05/22, I would have just stopped with comparing the two net
namespace pointers.
As for patch 03/22...
The init user namespace doesn't have a one-to-one mapping to network
namespace, so this won't solve the problem I was trying to solve.
In the initial user namespace, I can have as many network namespaces as
I want. I want kaudit to listen in all of them. There is already a
conservative check to make sure that audit won't permit changes from
any non-initial user namespace (or pid space):
kernel/audit.c:583:audit_netlink_ok():
if ((current_user_ns() != &init_user_ns) ||
(task_active_pid_ns(current) != &init_pid_ns))
return -EPERM;
This check needs to be revisited to allow some loosening of this policy,
but it was sound to start off too restrictive.
(https://bugzilla.redhat.com/show_bug.cgi?id=947530)
The certification issues surrounding non-initial user namespaces haven't
been adequately resolved yet, not having yet seen a followup patchset,
so we can combine these ideas once those issues have been addressed.
I agree we will need to be careful how the specific target socket and
portid are selected once we end up in other pid namespaces. For now,
are there specific concerns with this patch or better ways to
future-proof the selection of kaudit sockets and portids?
Thanks!
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545
10:20 p.m.
On 07/20/2013 05:15 AM, Richard Guy Briggs wrote:
On Wed, Jul 17, 2013 at 11:54:21AM +0800, Gao feng wrote:
> Hi, Richard
>
> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
>> Convert audit from only listening in init_net to use register_pernet_subsys()
>> to dynamically manage the netlink socket list.
>>
>> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
>> ---
>
> Right now audit still can't be used in uninit pid/user namespace,
> Consider this, when user in uninit pid/user namespace is allowed
> to setup/run audit subsystem, since the kernel thread always runs
> in init pid namespace, so we can't get right net namespace through
> get_net_ns_by_pid, The audit information will be sent to incorrect
> net namespace by kernel thread.
>
> In my opinion, This patch is limited and nonextensile.
>
> Maybe you should check the patchset "[Part1 PATCH 00/22] Add namespace support
for audit"
> I sent in 06/19/2013, In my solution, audit kernel side netlink sockets belongs
> to user namespace, and the user space audit netlink sockets will find the audit
> kernel socket through current_net_ns()->user_ns->audit.sock.
I already looked at your 48-patch and 22-patch sets and the threads of
comments. The concerns expressed in that thread haven't been fully
addressed yet by you.
Sorry, I think I had addressed all the problems in thar thread, maybe I missed
some, please help me to point it out, fell free to keep on discussing with me
in that thread.
> The "[PATCH 04/22] netlink: Add compare function for
netlink_table" of this patchset
> has been merged in linux mainline. I think if you look at my patchset, you will find
> the [PATCH 03/22] and [PATCH 05/22] will achieve the same aim of your patch.
I don't have any specific issues with patch 04/22.
For patch 05/22, I would have just stopped with comparing the two net
namespace pointers.
As for patch 03/22...
The init user namespace doesn't have a one-to-one mapping to network
namespace, so this won't solve the problem I was trying to solve.
If your problem is auditctl is unavailable in uninit net namespace, I
think my solution can solve this problem, since two audit netlink sockets
can communicate with each other when the net namespaces they belong to are
created by the same user namespace.
Maybe I misunderstand what is your problem here.
In the initial user namespace, I can have as many network namespaces
as
I want. I want kaudit to listen in all of them. There is already a
conservative check to make sure that audit won't permit changes from
any non-initial user namespace (or pid space):
kernel/audit.c:583:audit_netlink_ok():
if ((current_user_ns() != &init_user_ns) ||
(task_active_pid_ns(current) != &init_pid_ns))
return -EPERM;
This check needs to be revisited to allow some loosening of this policy,
but it was sound to start off too restrictive.
(https://bugzilla.redhat.com/show_bug.cgi?id=947530)
Yes, it was too restrictive, but I can't see what the help from this patch to
solve this problem.
The certification issues surrounding non-initial user namespaces
haven't
been adequately resolved yet, not having yet seen a followup patchset,
so we can combine these ideas once those issues have been addressed.
I agree we will need to be careful how the specific target socket and
portid are selected once we end up in other pid namespaces. For now,
are there specific concerns with this patch or better ways to
future-proof the selection of kaudit sockets and portids?
I my solution, even there are many net namespaces belong to the same user namespace,
there will only be one audit kernel side netlink socket, so all of the user space
audit netlink sockets in these net namespaces will find out/communicate with this
kernel audit socket.
and the kaudit sockets, portid belong to the user namespace,they are the one and only
in each user namespace.
Thanks
12:22 p.m.
On Mon, Jul 22, 2013 at 11:20:57AM +0800, Gao feng wrote:
On 07/20/2013 05:15 AM, Richard Guy Briggs wrote:
> On Wed, Jul 17, 2013 at 11:54:21AM +0800, Gao feng wrote:
>> Hi, Richard
>>
>> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
>>> Convert audit from only listening in init_net to use
register_pernet_subsys()
>>> to dynamically manage the netlink socket list.
>>>
>>> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
>>> ---
>>
>> Right now audit still can't be used in uninit pid/user namespace,
>> Consider this, when user in uninit pid/user namespace is allowed
>> to setup/run audit subsystem, since the kernel thread always runs
>> in init pid namespace, so we can't get right net namespace through
>> get_net_ns_by_pid, The audit information will be sent to incorrect
>> net namespace by kernel thread.
>>
>> In my opinion, This patch is limited and nonextensile.
>>
>> Maybe you should check the patchset "[Part1 PATCH 00/22] Add namespace
support for audit"
>> I sent in 06/19/2013, In my solution, audit kernel side netlink sockets belongs
>> to user namespace, and the user space audit netlink sockets will find the audit
>> kernel socket through current_net_ns()->user_ns->audit.sock.
>
> I already looked at your 48-patch and 22-patch sets and the threads of
> comments. The concerns expressed in that thread haven't been fully
> addressed yet by you.
>
Sorry, I think I had addressed all the problems in thar thread, maybe I missed
some, please help me to point it out, fell free to keep on discussing with me
in that thread.
There are several branches to that thread that went unresolved. I
haven't seen a followup patchset that attempts to address them:
https://www.redhat.com/archives/linux-audit/2013-June/msg00046.html
https://www.redhat.com/archives/linux-audit/2013-June/msg00056.html
https://www.redhat.com/archives/linux-audit/2013-June/msg00048.html
https://www.redhat.com/archives/linux-audit/2013-June/msg00050.html
But coming back to Eric Paris' original response and subsequent example,
neither have been addressed adequately:
https://www.redhat.com/archives/linux-audit/2013-June/msg00035.html
https://www.redhat.com/archives/linux-audit/2013-June/msg00039.html
and neither has the concern about making LSPP certification impossible.
>> The "[PATCH 04/22] netlink: Add compare function for
netlink_table" of this patchset
>> has been merged in linux mainline. I think if you look at my patchset, you will
find
>> the [PATCH 03/22] and [PATCH 05/22] will achieve the same aim of your patch.
>
> I don't have any specific issues with patch 04/22.
>
> For patch 05/22, I would have just stopped with comparing the two net
> namespace pointers.
>
> As for patch 03/22...
>
> The init user namespace doesn't have a one-to-one mapping to network
> namespace, so this won't solve the problem I was trying to solve.
If your problem is auditctl is unavailable in uninit net namespace, I
think my solution can solve this problem, since two audit netlink sockets
can communicate with each other when the net namespaces they belong to are
created by the same user namespace.
I don't follow how this is possible.
Maybe I misunderstand what is your problem here.
> In the initial user namespace, I can have as many network namespaces as
> I want. I want kaudit to listen in all of them. There is already a
> conservative check to make sure that audit won't permit changes from
> any non-initial user namespace (or pid space):
> kernel/audit.c:583:audit_netlink_ok():
> if ((current_user_ns() != &init_user_ns) ||
> (task_active_pid_ns(current) != &init_pid_ns))
> return -EPERM;
> This check needs to be revisited to allow some loosening of this policy,
> but it was sound to start off too restrictive.
> (https://bugzilla.redhat.com/show_bug.cgi?id=947530)
Yes, it was too restrictive, but I can't see what the help from this patch to
solve this problem.
It hasn't been solved yet. It is one of the next in line.
> The certification issues surrounding non-initial user namespaces
haven't
> been adequately resolved yet, not having yet seen a followup patchset,
> so we can combine these ideas once those issues have been addressed.
>
> I agree we will need to be careful how the specific target socket and
> portid are selected once we end up in other pid namespaces. For now,
> are there specific concerns with this patch or better ways to
> future-proof the selection of kaudit sockets and portids?
I my solution, even there are many net namespaces belong to the same user namespace,
there will only be one audit kernel side netlink socket, so all of the user space
audit netlink sockets in these net namespaces will find out/communicate with this
kernel audit socket.
I will need to go back and have a second look to see how this works.
and the kaudit sockets, portid belong to the user namespace,they are
the one and only
in each user namespace.
Do they not currently belong to the pid namespace?
Thanks
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545
12:57 p.m.
On Tue, 2013-07-30 at 13:22 -0400, Richard Guy Briggs wrote:
On Mon, Jul 22, 2013 at 11:20:57AM +0800, Gao feng wrote:
> On 07/20/2013 05:15 AM, Richard Guy Briggs wrote:
> > On Wed, Jul 17, 2013 at 11:54:21AM +0800, Gao feng wrote:
> >> Hi, Richard
> >>
> >> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
> >>> Convert audit from only listening in init_net to use
register_pernet_subsys()
> >>> to dynamically manage the netlink socket list.
> >>>
> >>> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> >>> ---
> >>
> >> Right now audit still can't be used in uninit pid/user namespace,
> >> Consider this, when user in uninit pid/user namespace is allowed
> >> to setup/run audit subsystem, since the kernel thread always runs
> >> in init pid namespace, so we can't get right net namespace through
> >> get_net_ns_by_pid, The audit information will be sent to incorrect
> >> net namespace by kernel thread.
> >>
> >> In my opinion, This patch is limited and nonextensile.
I agree completely that this patch is limited and nonextensible. But it
gets us where we should already be today. A single global kauditd and a
single global auditd. Today if you spawn a new network namespace you
cannot send messages to the kernel audit system. You cannot run auditd
in uninit network namespace. This is wrong. The kernel should take
anything userspace wants to throw at it and it should send messages to
auditd no matter where it lives. I see this is a good patch that should
go in next window, and will likely get overwritten completely with your
future work.
Now your patch handles this and so much more.
I still detest the idea of tieing the audit namespace to the user
namespace. My NAK still stands on any such patches.
I'd think that disjoint namespaces (like networking) instead of
hierarchical namespaces (like user) would be a lot easier to do. My
thoughts have always been about completely disjoint audit namespaces and
I may have missed the nuance of some of your discussion because it
didn't really dawn on me you seem to have always been discussing
hierarchical audit namespace.
I'm wondering if we want/need both? If I decide to launch a whole
distro inside a container I may not want it to be subject to any of the
audit rules of the init namespace. disjoint namespaces are good. You
don't seem to allow this, the init namespace audit rules would also
apply.
I'm not saying hierarchical rules are bad, in fact I might be convinced
they are adequate, I just can't bring myself to that conclusion yet.
The conclusion I still feel comfortable with is that the user namespace
is a whole of bag and I don't want it tied to audit.
-Eric
8:48 p.m.
On 08/02/2013 01:57 AM, Eric Paris wrote:
On Tue, 2013-07-30 at 13:22 -0400, Richard Guy Briggs wrote:
> On Mon, Jul 22, 2013 at 11:20:57AM +0800, Gao feng wrote:
>> On 07/20/2013 05:15 AM, Richard Guy Briggs wrote:
>>> On Wed, Jul 17, 2013 at 11:54:21AM +0800, Gao feng wrote:
>>>> Hi, Richard
>>>>
>>>> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
>>>>> Convert audit from only listening in init_net to use
register_pernet_subsys()
>>>>> to dynamically manage the netlink socket list.
>>>>>
>>>>> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
>>>>> ---
>>>>
>>>> Right now audit still can't be used in uninit pid/user namespace,
>>>> Consider this, when user in uninit pid/user namespace is allowed
>>>> to setup/run audit subsystem, since the kernel thread always runs
>>>> in init pid namespace, so we can't get right net namespace through
>>>> get_net_ns_by_pid, The audit information will be sent to incorrect
>>>> net namespace by kernel thread.
>>>>
>>>> In my opinion, This patch is limited and nonextensile.
I agree completely that this patch is limited and nonextensible. But it
gets us where we should already be today. A single global kauditd and a
single global auditd. Today if you spawn a new network namespace you
cannot send messages to the kernel audit system. You cannot run auditd
in uninit network namespace.
Yes, and you cannot run auditd in uninit pid/user namespace too.
This is wrong. The kernel should take
anything userspace wants to throw at it and it should send messages to
auditd no matter where it lives. I see this is a good patch that should
go in next window, and will likely get overwritten completely with your
future work.
I'm ok if you want this patch in next window, but I have to say, The solutions
I can think out will almost revert or rewrite this patch.
Now your patch handles this and so much more.
I still detest the idea of tieing the audit namespace to the user
namespace. My NAK still stands on any such patches.
I'd think that disjoint namespaces (like networking) instead of
hierarchical namespaces (like user) would be a lot easier to do. My
thoughts have always been about completely disjoint audit namespaces and
I may have missed the nuance of some of your discussion because it
didn't really dawn on me you seem to have always been discussing
hierarchical audit namespace.
Though user namespace is hierarchical, we can make auditns nonhierarchical even
tie it to userns. this depends on our decision/implementation, doesn't depend
on if we tie aduitns to userns.
I'm wondering if we want/need both? If I decide to launch a whole
distro inside a container I may not want it to be subject to any of the
audit rules of the init namespace. disjoint namespaces are good. You
don't seem to allow this, the init namespace audit rules would also
apply.
we can isolate audit rules, auditns can have its own audit rules, and
take no effect on other auditns.
I'm not saying hierarchical rules are bad, in fact I might be
convinced
they are adequate, I just can't bring myself to that conclusion yet.
The conclusion I still feel comfortable with is that the user namespace
is a whole of bag and I don't want it tied to audit.
Get it, I feel that you really don't like this idea, There are three reasons
I choose to tie auditns to userns.
1, all other namespace(net,pid,mnt,ipc..) has a "user_ns" pointer which point
to the user namespace, this user namespace is the creator of these namespace.
So in these namespaces, we can find out the auditns through
{netns,pidns,ipcns}->user_ns->audit.
audit subsystem is a service layer, it should be available for other subsystems.
if we don't tie auditns to userns, we have to add an extra pointer for other
namespace, looks like this netns->auditns, pidns->auditns, ipcns->auditns.
2. if we don't tie auditns to userns, we have to find a way to create a new
auditns. the flags of clone is a problem.
3, there are already 6 kinds of namespaces, for me, I don't want to introduce
a complete new namespace(think about there are syslog,crypto... need to be
ioslated, there will be so much namespace...). I choose to extend the current
exist namespace and achieve the same feature.
I know it's not a very good solution,since userns totally has no relationship
with audit...
8:21 a.m.
----- Original Message -----
I still detest the idea of tieing the audit namespace to the user
namespace. My NAK still stands on any such patches.
I'd think that disjoint namespaces (like networking) instead of
hierarchical namespaces (like user) would be a lot easier to do. My
thoughts have always been about completely disjoint audit namespaces and
I may have missed the nuance of some of your discussion because it
didn't really dawn on me you seem to have always been discussing
hierarchical audit namespace.
I'm wondering if we want/need both?
Would it be possible to avoid adding more dimensions to the namespace matrix? I
appreciate that the flexibility allows a wide range of use cases, however it also makes
reasoning about the security properties extremely difficult.
(If only there were a way to put the genie back to the bottle and have the kernel
explicitly recognize something like "virt-like container" or
"webhosting-like application isolation" as kernel-space concepts and
objects...)
Mirek
8:17 p.m.
On 07/31/2013 01:22 AM, Richard Guy Briggs wrote:
On Mon, Jul 22, 2013 at 11:20:57AM +0800, Gao feng wrote:
> On 07/20/2013 05:15 AM, Richard Guy Briggs wrote:
>> On Wed, Jul 17, 2013 at 11:54:21AM +0800, Gao feng wrote:
>>> Hi, Richard
>>>
>>> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
>>>> Convert audit from only listening in init_net to use
register_pernet_subsys()
>>>> to dynamically manage the netlink socket list.
>>>>
>>>> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
>>>> ---
>>>
>>> Right now audit still can't be used in uninit pid/user namespace,
>>> Consider this, when user in uninit pid/user namespace is allowed
>>> to setup/run audit subsystem, since the kernel thread always runs
>>> in init pid namespace, so we can't get right net namespace through
>>> get_net_ns_by_pid, The audit information will be sent to incorrect
>>> net namespace by kernel thread.
>>>
>>> In my opinion, This patch is limited and nonextensile.
>>>
>>> Maybe you should check the patchset "[Part1 PATCH 00/22] Add namespace
support for audit"
>>> I sent in 06/19/2013, In my solution, audit kernel side netlink sockets
belongs
>>> to user namespace, and the user space audit netlink sockets will find the
audit
>>> kernel socket through current_net_ns()->user_ns->audit.sock.
>>
>> I already looked at your 48-patch and 22-patch sets and the threads of
>> comments. The concerns expressed in that thread haven't been fully
>> addressed yet by you.
>>
>
> Sorry, I think I had addressed all the problems in thar thread, maybe I missed
> some, please help me to point it out, fell free to keep on discussing with me
> in that thread.
There are several branches to that thread that went unresolved. I
haven't seen a followup patchset that attempts to address them:
https://www.redhat.com/archives/linux-audit/2013-June/msg00046.html
https://www.redhat.com/archives/linux-audit/2013-June/msg00056.html
https://www.redhat.com/archives/linux-audit/2013-June/msg00048.html
https://www.redhat.com/archives/linux-audit/2013-June/msg00050.html
But coming back to Eric Paris' original response and subsequent example,
neither have been addressed adequately:
https://www.redhat.com/archives/linux-audit/2013-June/msg00035.html
https://www.redhat.com/archives/linux-audit/2013-June/msg00039.html
and neither has the concern about making LSPP certification impossible.
Thanks! I will check them.
>>> The "[PATCH 04/22] netlink: Add compare function for
netlink_table" of this patchset
>>> has been merged in linux mainline. I think if you look at my patchset, you
will find
>>> the [PATCH 03/22] and [PATCH 05/22] will achieve the same aim of your patch.
>>
>> I don't have any specific issues with patch 04/22.
>>
>> For patch 05/22, I would have just stopped with comparing the two net
>> namespace pointers.
>>
>> As for patch 03/22...
>>
>> The init user namespace doesn't have a one-to-one mapping to network
>> namespace, so this won't solve the problem I was trying to solve.
>
> If your problem is auditctl is unavailable in uninit net namespace, I
> think my solution can solve this problem, since two audit netlink sockets
> can communicate with each other when the net namespaces they belong to are
> created by the same user namespace.
I don't follow how this is possible.
> Maybe I misunderstand what is your problem here.
>
>> In the initial user namespace, I can have as many network namespaces as
>> I want. I want kaudit to listen in all of them. There is already a
>> conservative check to make sure that audit won't permit changes from
>> any non-initial user namespace (or pid space):
>> kernel/audit.c:583:audit_netlink_ok():
>> if ((current_user_ns() != &init_user_ns) ||
>> (task_active_pid_ns(current) != &init_pid_ns))
>> return -EPERM;
>> This check needs to be revisited to allow some loosening of this policy,
>> but it was sound to start off too restrictive.
>> (https://bugzilla.redhat.com/show_bug.cgi?id=947530)
>
> Yes, it was too restrictive, but I can't see what the help from this patch to
> solve this problem.
It hasn't been solved yet. It is one of the next in line.
good news, fell free to send them out, I would like to know how your patch runs :)
>> The certification issues surrounding non-initial user
namespaces haven't
>> been adequately resolved yet, not having yet seen a followup patchset,
>> so we can combine these ideas once those issues have been addressed.
>>
>> I agree we will need to be careful how the specific target socket and
>> portid are selected once we end up in other pid namespaces. For now,
>> are there specific concerns with this patch or better ways to
>> future-proof the selection of kaudit sockets and portids?
>
> I my solution, even there are many net namespaces belong to the same user namespace,
> there will only be one audit kernel side netlink socket, so all of the user space
> audit netlink sockets in these net namespaces will find out/communicate with this
> kernel audit socket.
I will need to go back and have a second look to see how this works.
> and the kaudit sockets, portid belong to the user namespace,they are the one and
only
> in each user namespace.
Do they not currently belong to the pid namespace?
The reason I choose user namespace is all other namespaces(net,mnt,pid,ipc..) has a
pointer
"user_ns" which points to the user namespace, So the audit messages generated in
these namespace
can be delivered to the user namespace. and since the kauidt sockets,portid... belong to
user namespace, we can find a proper userspace audit socket, and deliver these message to
it.
Thanks
9:59 p.m.
On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
Convert audit from only listening in init_net to use
register_pernet_subsys()
to dynamically manage the netlink socket list.
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
I think it's the time for us to discuss if we should revert this
commit, since this one prevent me from continuing to achieve
audit namespace.
The major problem is in kaudit_send_skb, we have no idea which
audit sock the skb should send to.
in this patch, there only is one auditd proecess, so the
audit_sock is the only one. but when we have audit namespace.
there will be multi audit socks. we have to store audit_sock
into auditns(auditns will be passed to kauditd_send_skb),
this will cause auditns have to get a reference of netns.
and for some reason(netfilter audit target), netns will
get reference of auditns too. this is terrible...
So why not we revert this one, and use a very simple one to
replace it? the below patch will save us from the refer to
each other case, achieve the same effect.
what's your opinion?
Add a compare function which always return true for
audit netlink socket, this will cause audit netlink
sockets netns unaware, and no matter which netns the
user space audit netlink sockets belong to, they all
can find out and communicate with audit_sock.
This gets rid of the necessary to create per-netns
audit kernel side socket(audit_sock), it's pain to
depend on and get reference of netns for auditns.
Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com>
---
kernel/audit.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/audit.c b/kernel/audit.c
index 7b0e23a..468950b 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -886,12 +886,18 @@ static void audit_receive(struct sk_buff *skb)
mutex_unlock(&audit_cmd_mutex);
}
+static bool audit_compare(struct net *net, struct sock *sk)
+{
+ return true;
+}
+
/* Initialize audit support at boot time. */
static int __init audit_init(void)
{
int i;
struct netlink_kernel_cfg cfg = {
.input = audit_receive,
+ .compare = audit_compare,
};
if (audit_initialized == AUDIT_DISABLED)
12:40 p.m.
On Thu, 2013-12-19 at 11:59 +0800, Gao feng wrote:
On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
> Convert audit from only listening in init_net to use register_pernet_subsys()
> to dynamically manage the netlink socket list.
>
> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> ---
I think it's the time for us to discuss if we should revert this
commit, since this one prevent me from continuing to achieve
audit namespace.
The major problem is in kaudit_send_skb, we have no idea which
audit sock the skb should send to.
right, we have problems here no matter what...
If we stick with the current approach you will need to know socket +
portid. With your approach one only needs to know portid. Since these
are can both be part of the audit_ns structure I don't see a huge
difference...
we have to store audit_sock
into auditns(auditns will be passed to kauditd_send_skb),
this will cause auditns have to get a reference of netns.
and for some reason(netfilter audit target), netns will
get reference of auditns too. this is terrible...
I'm not sure I agree/understand this entirely...
So why not we revert this one, and use a very simple one to
replace it? the below patch will save us from the refer to
each other case, achieve the same effect.
what's your opinion?
Help me go all the way back to the beginning. What's our end goal here
again?
When thinking about this I realized we have another problem that I don't
think we've considered. Which makes me lean away from the single
socket/kauditd :(
I we have one socket and one kauditd ANY auditd can completely freeze
the audit system. Which seems problematic, especially if there isn't
equal levels of trust between the different namespaces... If one auditd
gets hung (intentionally or not) the kernel will never send another
audit message....
Makes me think we really need a kauditd thread per namespace, possibly
an skb queue per namespace. At which point an audit socket per
namespace makes a lot of sense too....
7:35 p.m.
On 12/20/2013 02:40 AM, Eric Paris wrote:
On Thu, 2013-12-19 at 11:59 +0800, Gao feng wrote:
> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
>> Convert audit from only listening in init_net to use register_pernet_subsys()
>> to dynamically manage the netlink socket list.
>>
>> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
>> ---
>
> I think it's the time for us to discuss if we should revert this
> commit, since this one prevent me from continuing to achieve
> audit namespace.
>
>
> The major problem is in kaudit_send_skb, we have no idea which
> audit sock the skb should send to.
right, we have problems here no matter what...
If we stick with the current approach you will need to know socket +
portid. With your approach one only needs to know portid. Since these
are can both be part of the audit_ns structure I don't see a huge
difference...
> we have to store audit_sock
> into auditns(auditns will be passed to kauditd_send_skb),
> this will cause auditns have to get a reference of netns.
> and for some reason(netfilter audit target), netns will
> get reference of auditns too. this is terrible...
I'm not sure I agree/understand this entirely...
My brain must be destroyed, I need to think about if auditns
should get reference of netns. it's not clear to me now. :(
but I intend to think you are right.
> So why not we revert this one, and use a very simple one to
> replace it? the below patch will save us from the refer to
> each other case, achieve the same effect.
>
> what's your opinion?
Help me go all the way back to the beginning. What's our end goal here
again?
When thinking about this I realized we have another problem that I don't
think we've considered. Which makes me lean away from the single
socket/kauditd :(
I we have one socket and one kauditd ANY auditd can completely freeze
the audit system. Which seems problematic, especially if there isn't
equal levels of trust between the different namespaces... If one auditd
gets hung (intentionally or not) the kernel will never send another
audit message....
Makes me think we really need a kauditd thread per namespace, possibly
an skb queue per namespace. At which point an audit socket per
namespace makes a lot of sense too....
You are right, and My prototype supports per kauditd/auditd/sbk queue per
audit namespace. one auditd freeze in one auditns will not affect audit
subsystem in another auditns.
Thanks!
8:46 p.m.
On 12/20/2013 02:40 AM, Eric Paris wrote:
On Thu, 2013-12-19 at 11:59 +0800, Gao feng wrote:
> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
>> Convert audit from only listening in init_net to use register_pernet_subsys()
>> to dynamically manage the netlink socket list.
>>
>> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
>> ---
>
> I think it's the time for us to discuss if we should revert this
> commit, since this one prevent me from continuing to achieve
> audit namespace.
>
>
> The major problem is in kaudit_send_skb, we have no idea which
> audit sock the skb should send to.
right, we have problems here no matter what...
If we stick with the current approach you will need to know socket +
portid. With your approach one only needs to know portid. Since these
are can both be part of the audit_ns structure I don't see a huge
difference...
> we have to store audit_sock
> into auditns(auditns will be passed to kauditd_send_skb),
> this will cause auditns have to get a reference of netns.
> and for some reason(netfilter audit target), netns will
> get reference of auditns too. this is terrible...
I'm not sure I agree/understand this entirely...
Yes, the audit_sock is created and destroyed by net namespace,
so if auditns wants to use audit_sock, it must prevent netns
from being destroyed. so auditns has to get reference of netns.
and in some case, netns will get reference of auditns too. this
is complex than making audit_sock global and getting rid of this
reference.
9:11 p.m.
On Fri, 2013-12-20 at 10:46 +0800, Gao feng wrote:
On 12/20/2013 02:40 AM, Eric Paris wrote:
> On Thu, 2013-12-19 at 11:59 +0800, Gao feng wrote:
>> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
>> we have to store audit_sock
>> into auditns(auditns will be passed to kauditd_send_skb),
>> this will cause auditns have to get a reference of netns.
>> and for some reason(netfilter audit target), netns will
>> get reference of auditns too. this is terrible...
>
> I'm not sure I agree/understand this entirely...
>
Yes, the audit_sock is created and destroyed by net namespace,
so if auditns wants to use audit_sock, it must prevent netns
from being destroyed. so auditns has to get reference of netns.
Namespace == mind blown. Ok, so:
auditd in audit_ns2 and net_ns2. <--- ONLY process in net_ns2
some process in audit_ns2 and net_ns3
Lets assume that auditd is killed improperly/dies. Because the last
process in net_ns2 is gone net_ns2 is invalid/freed.
Today in the kernel the way we detect auditd is gone is by using the
socket and getting ECONNREFUSSED. So here you think that audit_ns2
should hold a reference on net_ns2, to make sure that socket is always
valid.
I instead propose that we could run all audit_ns and reset the audit_pid
in that namespace and the audit_sock in the namespace to 0/null inside
audit_net_exit. Since obviously if the net_ns disappeared, the auditd
which was running in any audit namespace in that net_ns isn't running
any more. We didn't need to hold a reference on the net_ns. We just
have to clear the skb_queue, reset the audit_pid to 0, and reset the
socket to NULL...
Maybe the one magic socket is the right answer. I'm not arguing against
your solution. I'm really trying to understand why we are going that
way...
and in some case, netns will get reference of auditns too. this
is complex than making audit_sock global and getting rid of this
reference.
This I haven't even started to try to wrap my head around...
9:45 p.m.
On 12/20/2013 11:11 AM, Eric Paris wrote:
On Fri, 2013-12-20 at 10:46 +0800, Gao feng wrote:
> On 12/20/2013 02:40 AM, Eric Paris wrote:
>> On Thu, 2013-12-19 at 11:59 +0800, Gao feng wrote:
>>> On 07/17/2013 04:32 AM, Richard Guy Briggs wrote:
>>> we have to store audit_sock
>>> into auditns(auditns will be passed to kauditd_send_skb),
>>> this will cause auditns have to get a reference of netns.
>>> and for some reason(netfilter audit target), netns will
>>> get reference of auditns too. this is terrible...
>>
>> I'm not sure I agree/understand this entirely...
>>
>
> Yes, the audit_sock is created and destroyed by net namespace,
> so if auditns wants to use audit_sock, it must prevent netns
> from being destroyed. so auditns has to get reference of netns.
Namespace == mind blown. Ok, so:
auditd in audit_ns2 and net_ns2. <--- ONLY process in net_ns2
some process in audit_ns2 and net_ns3
Lets assume that auditd is killed improperly/dies. Because the last
process in net_ns2 is gone net_ns2 is invalid/freed.
Today in the kernel the way we detect auditd is gone is by using the
socket and getting ECONNREFUSSED. So here you think that audit_ns2
should hold a reference on net_ns2, to make sure that socket is always
valid.
I instead propose that we could run all audit_ns and reset the audit_pid
in that namespace and the audit_sock in the namespace to 0/null inside
audit_net_exit. Since obviously if the net_ns disappeared, the auditd
which was running in any audit namespace in that net_ns isn't running
any more. We didn't need to hold a reference on the net_ns. We just
have to clear the skb_queue, reset the audit_pid to 0, and reset the
socket to NULL...
multi auditns can share the same netns. it happens if you unshare
auditns. if you want to reset audit_sock to null inside audit_net_exit,
you have to maintain a list in netns, this list contains the auditnss
whose audit_sock is created in this netns. so you can foreach this
list and reset the audit socks of audit nss.
Above is unsharing auditns, consider unsharing netns. auditd is running
in auditns1 and netns1, and then who-know-why the auditd call unshare(CLONE_NEWNET)
to change it's netns from netns1 to new netns2. so the netns1 is released
and auditns->audit_sock being reset to NULL. the auditd cannot receive
the audit log. auditd will in chaos, "I'm still alive, why kernel think
I'm die?"
So maybe you will say, we can reset the audit_sock of netns2 to auditns.
ok, this is a way. but how can we decide if we should reset the auditns->audit_sock?
when we create the new netns, the old netns is still alive, so the auditns->audit_sock
is still valid in that time.
I don't know if there are some other problems we should consider.
it is too complex..
Maybe the one magic socket is the right answer. I'm not arguing against
your solution. I'm really trying to understand why we are going that
way...
That's why we should discuss :)
4019
days inactive
4176
days old
linux-audit@lists.linux-audit.osci.io
14 comments
4 participants
participants (4)
-
Eric Paris -
Gao feng -
Miloslav Trmač -
Richard Guy Briggs