10:58 a.m.
Most audit control messages are sent over netlink. In order to properly
log the identity of the sender of audit control messages, we would like
to add the loginuid to the netlink_creds structure, as per the attached
patch.
thanks,
-serge
Signed-off-by: Serge Hallyn <serue(a)us.ibm.com>
Index: linux-2.6.10/include/linux/audit.h
===================================================================
--- linux-2.6.10.orig/include/linux/audit.h 2005-01-27 10:46:57.887036520 -0600
+++ linux-2.6.10/include/linux/audit.h 2005-01-27 10:51:37.408542792 -0600
@@ -145,7 +145,7 @@ extern void audit_inode(const char *name
/* Private API (for audit.c only) */
extern int audit_receive_filter(int type, int pid, int uid, int seq,
- void *data);
+ void *data, uid_t loginuid);
extern void audit_get_stamp(struct audit_context *ctx,
struct timespec *t, int *serial);
extern int audit_set_loginuid(struct audit_context *ctx, uid_t loginuid);
@@ -179,10 +179,10 @@ extern void audit_log_d_path(struct
const char *prefix,
struct dentry *dentry,
struct vfsmount *vfsmnt);
-extern int audit_set_rate_limit(int limit);
-extern int audit_set_backlog_limit(int limit);
-extern int audit_set_enabled(int state);
-extern int audit_set_failure(int state);
+extern int audit_set_rate_limit(int limit, uid_t loginuid);
+extern int audit_set_backlog_limit(int limit, uid_t loginuid);
+extern int audit_set_enabled(int state, uid_t loginuid);
+extern int audit_set_failure(int state, uid_t loginuid);
/* Private API (for auditsc.c only) */
extern void audit_send_reply(int pid, int seq, int type,
Index: linux-2.6.10/include/linux/netlink.h
===================================================================
--- linux-2.6.10.orig/include/linux/netlink.h 2005-01-27 10:46:57.888036368 -0600
+++ linux-2.6.10/include/linux/netlink.h 2005-01-27 10:51:37.409542640 -0600
@@ -110,6 +110,7 @@ struct netlink_skb_parms
__u32 dst_pid;
__u32 dst_groups;
kernel_cap_t eff_cap;
+ __u32 loginuid; /* Login (audit) uid */
};
#define NETLINK_CB(skb) (*(struct netlink_skb_parms*)&((skb)->cb))
Index: linux-2.6.10/kernel/audit.c
===================================================================
--- linux-2.6.10.orig/kernel/audit.c 2005-01-27 10:46:57.888036368 -0600
+++ linux-2.6.10/kernel/audit.c 2005-01-27 10:52:28.753737136 -0600
@@ -236,36 +236,36 @@ void audit_log_lost(const char *message)
}
-int audit_set_rate_limit(int limit)
+int audit_set_rate_limit(int limit, uid_t loginuid)
{
int old = audit_rate_limit;
audit_rate_limit = limit;
- audit_log(current->audit_context, "audit_rate_limit=%d old=%d",
- audit_rate_limit, old);
+ audit_log(NULL, "audit_rate_limit=%d old=%d by loginuid %u",
+ audit_rate_limit, old, loginuid);
return old;
}
-int audit_set_backlog_limit(int limit)
+int audit_set_backlog_limit(int limit, uid_t loginuid)
{
int old = audit_backlog_limit;
audit_backlog_limit = limit;
- audit_log(current->audit_context, "audit_backlog_limit=%d old=%d",
- audit_backlog_limit, old);
+ audit_log(NULL, "audit_backlog_limit=%d old=%d by loginuid %u",
+ audit_backlog_limit, old, loginuid);
return old;
}
-int audit_set_enabled(int state)
+int audit_set_enabled(int state, uid_t loginuid)
{
int old = audit_enabled;
if (state != 0 && state != 1)
return -EINVAL;
audit_enabled = state;
- audit_log(current->audit_context, "audit_enabled=%d old=%d",
- audit_enabled, old);
+ audit_log(NULL, "audit_enabled=%d old=%d by loginuid %u",
+ audit_enabled, old, loginuid);
return old;
}
-int audit_set_failure(int state)
+int audit_set_failure(int state, uid_t loginuid)
{
int old = audit_failure;
if (state != AUDIT_FAIL_SILENT
@@ -273,8 +273,8 @@ int audit_set_failure(int state)
&& state != AUDIT_FAIL_PANIC)
return -EINVAL;
audit_failure = state;
- audit_log(current->audit_context, "audit_failure=%d old=%d",
- audit_failure, old);
+ audit_log(NULL, "audit_failure=%d old=%d by loginuid %u",
+ audit_failure, old, loginuid);
return old;
}
@@ -341,6 +341,7 @@ static int audit_receive_msg(struct sk_b
int err;
struct audit_buffer *ab;
u16 msg_type = nlh->nlmsg_type;
+ uid_t loginuid; /* loginuid of sender */
err = audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
if (err)
@@ -348,6 +349,7 @@ static int audit_receive_msg(struct sk_b
pid = NETLINK_CREDS(skb)->pid;
uid = NETLINK_CREDS(skb)->uid;
+ loginuid = NETLINK_CB(skb).loginuid;
seq = nlh->nlmsg_seq;
data = NLMSG_DATA(nlh);
@@ -368,31 +370,33 @@ static int audit_receive_msg(struct sk_b
return -EINVAL;
status_get = (struct audit_status *)data;
if (status_get->mask & AUDIT_STATUS_ENABLED) {
- err = audit_set_enabled(status_get->enabled);
+ err = audit_set_enabled(status_get->enabled, loginuid);
if (err < 0) return err;
}
if (status_get->mask & AUDIT_STATUS_FAILURE) {
- err = audit_set_failure(status_get->failure);
+ err = audit_set_failure(status_get->failure, loginuid);
if (err < 0) return err;
}
if (status_get->mask & AUDIT_STATUS_PID) {
int old = audit_pid;
audit_pid = status_get->pid;
- audit_log(current->audit_context,
- "audit_pid=%d old=%d", audit_pid, old);
+ audit_log(NULL, "audit_pid=%d old=%d by loginuid %u",
+ audit_pid, old, loginuid);
}
if (status_get->mask & AUDIT_STATUS_RATE_LIMIT)
- audit_set_rate_limit(status_get->rate_limit);
+ audit_set_rate_limit(status_get->rate_limit, loginuid);
if (status_get->mask & AUDIT_STATUS_BACKLOG_LIMIT)
- audit_set_backlog_limit(status_get->backlog_limit);
+ audit_set_backlog_limit(status_get->backlog_limit,
+ loginuid);
break;
case AUDIT_USER:
ab = audit_log_start(NULL);
if (!ab)
break; /* audit_panic has been called */
audit_log_format(ab,
- "user pid=%d uid=%d length=%d msg='%.1024s'",
- pid, uid,
+ "user pid=%d uid=%d loginuid=%u length=%d"
+ " msg='%.1024s'",
+ pid, uid, loginuid,
(int)(nlh->nlmsg_len
- ((char *)data - (char *)nlh)),
(char *)data);
@@ -408,7 +412,7 @@ static int audit_receive_msg(struct sk_b
case AUDIT_LIST:
#ifdef CONFIG_AUDITSYSCALL
err = audit_receive_filter(nlh->nlmsg_type, pid, uid, seq,
- data);
+ data, loginuid);
#else
err = -EOPNOTSUPP;
#endif
Index: linux-2.6.10/kernel/auditsc.c
===================================================================
--- linux-2.6.10.orig/kernel/auditsc.c 2005-01-27 10:46:57.890036064 -0600
+++ linux-2.6.10/kernel/auditsc.c 2005-01-27 10:52:53.776933032 -0600
@@ -228,7 +228,8 @@ static int audit_copy_rule(struct audit_
return 0;
}
-int audit_receive_filter(int type, int pid, int uid, int seq, void *data)
+int audit_receive_filter(int type, int pid, int uid, int seq, void *data,
+ uid_t loginuid)
{
u32 flags;
struct audit_entry *entry;
@@ -263,6 +264,7 @@ int audit_receive_filter(int type, int p
err = audit_add_rule(entry, &audit_entlist);
if (!err && (flags & AUDIT_AT_EXIT))
err = audit_add_rule(entry, &audit_extlist);
+ audit_log(NULL, "loginuid %u added an audit rule\n", loginuid);
break;
case AUDIT_DEL:
flags =((struct audit_rule *)data)->flags;
@@ -272,6 +274,8 @@ int audit_receive_filter(int type, int p
err = audit_del_rule(data, &audit_entlist);
if (!err && (flags & AUDIT_AT_EXIT))
err = audit_del_rule(data, &audit_extlist);
+ audit_log(NULL, "loginuid %u removed an audit rule\n",
+ loginuid);
break;
default:
return -EINVAL;
Index: linux-2.6.10/net/netlink/af_netlink.c
===================================================================
--- linux-2.6.10.orig/net/netlink/af_netlink.c 2005-01-27 10:46:57.891035912 -0600
+++ linux-2.6.10/net/netlink/af_netlink.c 2005-01-27 10:51:37.411542336 -0600
@@ -928,6 +928,7 @@ static int netlink_sendmsg(struct kiocb
NETLINK_CB(skb).groups = nlk->groups;
NETLINK_CB(skb).dst_pid = dst_pid;
NETLINK_CB(skb).dst_groups = dst_groups;
+ NETLINK_CB(skb).loginuid = audit_get_loginuid(current->audit_context);
memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
/* What can I do? Netlink is asynchronous, so that
12:04 a.m.
On Fri, 4 Feb 2005, Serge E. Hallyn wrote:
Most audit control messages are sent over netlink. In order to
properly
log the identity of the sender of audit control messages, we would like
to add the loginuid to the netlink_creds structure, as per the attached
patch.
Reception of netlink messages in the kernel happens in the context
of the sending process, so you can simply call
audit_get_loginuid(current->audit_context) in audit_receive_msg().
Regards
Patrick
7:34 a.m.
On Tue, 2005-02-08 at 01:04, Patrick McHardy wrote:
Reception of netlink messages in the kernel happens in the context
of the sending process, so you can simply call
audit_get_loginuid(current->audit_context) in audit_receive_msg().
Then why does netlink_sendmsg() need to save the effective capability
set of the sender in the control buffer (via security_netlink_send) for
later checking by other receive functions in the kernel (via
security_netlink_recv)? What prevents audit_receive() or other similar
receive functions in the kernel from processing messages sent by
multiple senders?
--
Stephen Smalley <sds(a)epoch.ncsc.mil>
National Security Agency
8:19 a.m.
Hello!
> Reception of netlink messages in the kernel happens in the
context
> of the sending process, so you can simply call
> audit_get_loginuid(current->audit_context) in audit_receive_msg().
Then why does netlink_sendmsg() need to save the effective capability
Yes, when kernel receives a message, it can be processed in context
of another process. This happens with rtnetlink, which queues messages
when someone holds netadmin semaphore and processing of backlog happens
in context of process which holds the semaphore.
Unfortunately, audit uses the same twisted way. Actually, if people
expected synchronous processing, it is better to replace
if (down_trylock(&audit_netlink_sem))
return;
with plain down(&audit_netlink_sem);
Alexey
8:17 a.m.
On Fri, 2005-02-04 at 10:58 -0600, Serge E. Hallyn wrote:
Most audit control messages are sent over netlink. In order to
properly
log the identity of the sender of audit control messages, we would like
to add the loginuid to the netlink_creds structure, as per the attached
patch.
I think it would be better to leave the loginuid in the payload of the
audit packets, not put it into generic netlink structures.
In the common case where audit messages are being generated by the
kernel, the loginuid can be trusted anyway, and doesn't need to be
handled by netlink.
The only time it's possibly worth verifying it is for the case where
userspace is sending AUDIT_USER messages -- for which the process needs
CAP_AUDIT_WRITE anyway. And if you're then going to trust the rest of
what that process sends, what's wrong with trusting the loginuid which
it provides too?
Why should it be impossible for a trusted logging dæmon to log actions
of another process, running with a loginuid other than the loginuid of
the dæmon?
--
dwmw2
8:50 a.m.
On Wed, 2005-02-09 at 14:17 +0000, David Woodhouse wrote:
The only time it's possibly worth verifying it is for the case
where
userspace is sending AUDIT_USER messages -- for which the process needs
CAP_AUDIT_WRITE anyway.
CAP_AUDIT_WRITE is needed, but not CAP_AUDIT_CONTROL, which is needed to
set the loginuid. Of course, an LSM could check at
security_netlink_send whether the login_uid in the payload is the same
as the real loginuid. Otherwise, we're wasting a (very precious)
capability bit.
In either case, have we decided we don't want it in the netlink
credentials after all?
thanks,
-serge
--
Serge Hallyn <serue(a)us.ibm.com>
12:23 p.m.
On Wed, 2005-02-09 at 09:50, Serge Hallyn wrote:
CAP_AUDIT_WRITE is needed, but not CAP_AUDIT_CONTROL, which is needed
to
set the loginuid. Of course, an LSM could check at
security_netlink_send whether the login_uid in the payload is the same
as the real loginuid. Otherwise, we're wasting a (very precious)
capability bit.
In either case, have we decided we don't want it in the netlink
credentials after all?
If the audit subsystem truly needs to include the loginuid in audit
messages generated upon processing netlink messages, then I think it
belongs in the control buffer as per your patch. Alexey has confirmed
that we cannot use the current task's audit context regardless.
As a side bar, a similar security field in the control buffer would
likewise be very useful so that SELinux could set the SID for use in
permission checks by receive functions.
--
Stephen Smalley <sds(a)epoch.ncsc.mil>
National Security Agency
12:37 p.m.
* Stephen Smalley (sds(a)epoch.ncsc.mil) wrote:
On Wed, 2005-02-09 at 09:50, Serge Hallyn wrote:
> CAP_AUDIT_WRITE is needed, but not CAP_AUDIT_CONTROL, which is needed to
> set the loginuid. Of course, an LSM could check at
> security_netlink_send whether the login_uid in the payload is the same
> as the real loginuid. Otherwise, we're wasting a (very precious)
> capability bit.
>
> In either case, have we decided we don't want it in the netlink
> credentials after all?
If the audit subsystem truly needs to include the loginuid in audit
messages generated upon processing netlink messages, then I think it
belongs in the control buffer as per your patch. Alexey has confirmed
that we cannot use the current task's audit context regardless.
As a side bar, a similar security field in the control buffer would
likewise be very useful so that SELinux could set the SID for use in
permission checks by receive functions.
This means sendmsg hook would set the SID? And in that case, you'd
stomp on loginuid for audit messages unless they are special cased.
The loginuid is special case to audit, it doesn't make sense to me that
it is in generic netlink_skb_parms structure unless it's used by more
netlink users.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
12:40 p.m.
On Wed, 2005-02-09 at 13:37, Chris Wright wrote:
This means sendmsg hook would set the SID? And in that case,
you'd
stomp on loginuid for audit messages unless they are special cased.
I was referring to a separate field for use by security modules, not
re-use of the same field being proposed for the loginuid. Yes, it would
be set by the security_netlink_send hook. The principal problem with
such a security field is that unless we mandate it to be a simple
integer value (like a SELinux SID), we have to deal with lifecycle
management for it, i.e. a set of hooks that starts to look like the
sk_buff security hooks from the old LSM patch. But if we can limit it
to a simple value, then it would be useful for such security
identifiers, and allow receiver-side permission checks based on the
sender SID.
The loginuid is special case to audit, it doesn't make sense to
me that
it is in generic netlink_skb_parms structure unless it's used by more
netlink users.
So you also think it should be in the payload? That would require
security_netlink_send to dig into the payload if we wanted to control
who can specify other loginuids, as Serge noted.
--
Stephen Smalley <sds(a)epoch.ncsc.mil>
National Security Agency
5:38 p.m.
* Stephen Smalley (sds(a)epoch.ncsc.mil) wrote:
On Wed, 2005-02-09 at 13:37, Chris Wright wrote:
> This means sendmsg hook would set the SID? And in that case, you'd
> stomp on loginuid for audit messages unless they are special cased.
I was referring to a separate field for use by security modules, not
re-use of the same field being proposed for the loginuid. Yes, it would
be set by the security_netlink_send hook. The principal problem with
such a security field is that unless we mandate it to be a simple
integer value (like a SELinux SID), we have to deal with lifecycle
management for it, i.e. a set of hooks that starts to look like the
sk_buff security hooks from the old LSM patch. But if we can limit it
to a simple value, then it would be useful for such security
identifiers, and allow receiver-side permission checks based on the
sender SID.
This makes sense to me. Just an extension of existing eff_cap and would
be used by security modules for each netlink packet.
> The loginuid is special case to audit, it doesn't make sense
to me that
> it is in generic netlink_skb_parms structure unless it's used by more
> netlink users.
So you also think it should be in the payload? That would require
security_netlink_send to dig into the payload if we wanted to control
who can specify other loginuids, as Serge noted.
I just don't see it making sense to add another credential for a special
case. The signal code already peaks into the siginfo struct when queueing
a signal to make sure some user isn't trying to send si_code == SI_KERNEL
or similar. Perhaps audit could do that with it's own payload during send.
No matter how we slice it, it's a special case.
Hmm, perhaps we could eliminate the whole asynchronous issue by allowing
registration of a netlink link specific security handler. Something like:
netlink_kernel_create_sec(unit, rx, sec_handler)
Then the check would be done before the packet was ever queued. This
would eliminate the if (NETLINK_CREDS(skb)->$cred == bad) on receipt
side, and push it to sender side. It would also be link specific so
audit could do it's audit payload loginuid check here. I think it would
also eliminate SELinux's need to tag the packet for later checking on
receipt. Thoughts?
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
5:56 p.m.
On Wed, 2005-02-09 at 15:38 -0800, Chris Wright wrote:
> So you also think it should be in the payload? That would
require
> security_netlink_send to dig into the payload if we wanted to control
> who can specify other loginuids, as Serge noted.
I just don't see it making sense to add another credential for a special
case. The signal code already peaks into the siginfo struct when queueing
a signal to make sure some user isn't trying to send si_code == SI_KERNEL
or similar. Perhaps audit could do that with it's own payload during send.
No matter how we slice it, it's a special case.
I'm not entirely sure the check is needed anyway. This is a trusted
application sending audit messages. Why shouldn't it be permitted to log
auditable events which were triggered by someone _else_?
If we want to audit the actions of the userspace logging dæmon itself
and see what it sends, then we can quite happily do so within the audit
framework. That's a _different_ issue, surely?
--
dwmw2
6:19 p.m.
* David Woodhouse (dwmw2(a)infradead.org) wrote:
On Wed, 2005-02-09 at 15:38 -0800, Chris Wright wrote:
>I just don't see it making sense to add another credential for a special
>case. The signal code already peaks into the siginfo struct when queueing
>a signal to make sure some user isn't trying to send si_code == SI_KERNEL
>or similar. Perhaps audit could do that with it's own payload during send.
>No matter how we slice it, it's a special case.
I'm not entirely sure the check is needed anyway. This is a trusted
application sending audit messages. Why shouldn't it be permitted to log
auditable events which were triggered by someone _else_?
Then it comes back to the question of how to protect loginuid. If it
can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
write protected by CAP_AUDIT_CONTROL.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
3:20 a.m.
On Wed, 2005-02-09 at 16:19 -0800, Chris Wright wrote:
Then it comes back to the question of how to protect loginuid. If
it
can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
write protected by CAP_AUDIT_CONTROL.
I'm not sure I agree with that. With CAP_AUDIT_WRITE you _can't_ modify
the loginuid of the audit logs of your own actions. You can only modify
the loginuid on the messages you pull out of thin air and send. You can
already make up the rest of the payload -- why shouldn't you be allowed
to make up the loginuid too? You could be reporting something that
someone _else_ has done, after all.
Or am I misunderstanding the intended use of CAP_AUDIT_WRITE?
--
dwmw2
6:40 a.m.
On Wed, 2005-02-09 at 19:19, Chris Wright wrote:
Then it comes back to the question of how to protect loginuid. If
it
can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
write protected by CAP_AUDIT_CONTROL.
To be precise, isn't it true that someone with only CAP_AUDIT_WRITE
would only be able to spoof loginuids in the AUDIT_USER messages they
generate? The loginuid on any syscall audit messages for the task would
still be the one associated with the task's audit context, so that would
not be spoofable.
--
Stephen Smalley <sds(a)epoch.ncsc.mil>
National Security Agency
6:49 a.m.
On Thu, 2005-02-10 at 07:40 -0500, Stephen Smalley wrote:
To be precise, isn't it true that someone with only
CAP_AUDIT_WRITE
would only be able to spoof loginuids in the AUDIT_USER messages they
generate? The loginuid on any syscall audit messages for the task would
still be the one associated with the task's audit context, so that would
not be spoofable.
Correct.
--
dwmw2
11:14 a.m.
* Stephen Smalley (sds(a)epoch.ncsc.mil) wrote:
On Wed, 2005-02-09 at 19:19, Chris Wright wrote:
> Then it comes back to the question of how to protect loginuid. If it
> can be spoofed by someone with CAP_AUDIT_WRITE, then it shouldn't be
> write protected by CAP_AUDIT_CONTROL.
To be precise, isn't it true that someone with only CAP_AUDIT_WRITE
would only be able to spoof loginuids in the AUDIT_USER messages they
generate? The loginuid on any syscall audit messages for the task would
still be the one associated with the task's audit context, so that would
not be spoofable.
Yes, that's true.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
7:11 p.m.
* Chris Wright (chrisw(a)osdl.org) wrote:
Hmm, perhaps we could eliminate the whole asynchronous issue by
allowing
registration of a netlink link specific security handler. Something like:
netlink_kernel_create_sec(unit, rx, sec_handler)
Then the check would be done before the packet was ever queued. This
would eliminate the if (NETLINK_CREDS(skb)->$cred == bad) on receipt
side, and push it to sender side. It would also be link specific so
audit could do it's audit payload loginuid check here. I think it would
also eliminate SELinux's need to tag the packet for later checking on
receipt. Thoughts?
Here's an example of what I mean. It's quite rough, doesn't yet eliminate
any of the code that it eventually could, and doesn't deal with broadcast.
I gave it a quick test with audit netlink, and it does what I expect.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
===== kernel/audit.c 1.9 vs edited =====
--- 1.9/kernel/audit.c 2005-01-30 22:33:47 -08:00
+++ edited/kernel/audit.c 2005-02-09 17:07:22 -08:00
@@ -333,6 +333,15 @@ static int audit_netlink_ok(kernel_cap_t
return err;
}
+static int audit_check_sender(struct sk_buff *skb)
+{
+ struct nlmsghdr *nlh = (struct nlmsghdr *)skb->data;
+ u16 msg_type = nlh->nlmsg_type;
+
+ return audit_netlink_ok(NETLINK_CB(skb).eff_cap, msg_type);
+
+}
+
static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
{
u32 uid, pid, seq;
@@ -551,7 +560,7 @@ int __init audit_init(void)
{
printk(KERN_INFO "audit: initializing netlink socket (%s)\n",
audit_default ? "enabled" : "disabled");
- audit_sock = netlink_kernel_create(NETLINK_AUDIT, audit_receive);
+ audit_sock = netlink_kernel_create_check(NETLINK_AUDIT, audit_receive,
audit_check_sender);
if (!audit_sock)
audit_panic("cannot initialize netlink socket");
===== net/netlink/af_netlink.c 1.69 vs edited =====
--- 1.69/net/netlink/af_netlink.c 2005-01-21 12:25:32 -08:00
+++ edited/net/netlink/af_netlink.c 2005-02-09 16:39:15 -08:00
@@ -71,6 +71,7 @@ struct netlink_opt
struct netlink_callback *cb;
spinlock_t cb_lock;
void (*data_ready)(struct sock *sk, int bytes);
+ int (*send_check)(struct sk_buff *skb);
};
#define nlk_sk(__sk) ((struct netlink_opt *)(__sk)->sk_protinfo)
@@ -639,6 +640,14 @@ int netlink_sendskb(struct sock *sk, str
int len = skb->len;
nlk = nlk_sk(sk);
+
+ if (nlk->send_check)
+ if (nlk->send_check(skb)) {
+ kfree_skb(skb);
+ sock_put(sk);
+ return -EPERM;
+ }
+
#ifdef NL_EMULATE_DEV
if (nlk->handler) {
skb_orphan(skb);
@@ -1033,7 +1042,8 @@ static void netlink_data_ready(struct so
*/
struct sock *
-netlink_kernel_create(int unit, void (*input)(struct sock *sk, int len))
+netlink_kernel_create_check(int unit, void (*input)(struct sock *sk, int len),
+ int (*check)(struct sk_buff *skb))
{
struct socket *sock;
struct sock *sk;
@@ -1053,8 +1063,10 @@ netlink_kernel_create(int unit, void (*i
}
sk = sock->sk;
sk->sk_data_ready = netlink_data_ready;
- if (input)
+ if (input) {
nlk_sk(sk)->data_ready = input;
+ nlk_sk(sk)->send_check = check;
+ }
if (netlink_insert(sk, 0)) {
sock_release(sock);
@@ -1459,7 +1471,7 @@ MODULE_ALIAS_NETPROTO(PF_NETLINK);
EXPORT_SYMBOL(netlink_ack);
EXPORT_SYMBOL(netlink_broadcast);
EXPORT_SYMBOL(netlink_dump_start);
-EXPORT_SYMBOL(netlink_kernel_create);
+EXPORT_SYMBOL(netlink_kernel_create_check);
EXPORT_SYMBOL(netlink_register_notifier);
EXPORT_SYMBOL(netlink_set_err);
EXPORT_SYMBOL(netlink_set_nonroot);
===== include/linux/netlink.h 1.23 vs edited =====
--- 1.23/include/linux/netlink.h 2005-02-06 21:59:39 -08:00
+++ edited/include/linux/netlink.h 2005-02-09 16:36:45 -08:00
@@ -116,7 +116,11 @@ struct netlink_skb_parms
#define NETLINK_CREDS(skb) (&NETLINK_CB((skb)).creds)
-extern struct sock *netlink_kernel_create(int unit, void (*input)(struct sock *sk, int
len));
+extern struct sock *netlink_kernel_create_check(int unit, void (*input)(struct sock *sk,
int len), int (*check)(struct sk_buff *skb));
+static inline struct sock *netlink_kernel_create(int unit, void (*input)(struct sock *sk,
int len))
+{
+ return netlink_kernel_create_check(unit, input, NULL);
+}
extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
extern int netlink_unicast(struct sock *ssk, struct sk_buff *skb, __u32 pid, int
nonblock);
extern int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, __u32 pid,
6:36 a.m.
On Wed, 2005-02-09 at 20:11, Chris Wright wrote:
* Chris Wright (chrisw(a)osdl.org) wrote:
> Hmm, perhaps we could eliminate the whole asynchronous issue by allowing
> registration of a netlink link specific security handler. Something like:
>
> netlink_kernel_create_sec(unit, rx, sec_handler)
>
> Then the check would be done before the packet was ever queued. This
> would eliminate the if (NETLINK_CREDS(skb)->$cred == bad) on receipt
> side, and push it to sender side. It would also be link specific so
> audit could do it's audit payload loginuid check here. I think it would
> also eliminate SELinux's need to tag the packet for later checking on
> receipt. Thoughts?
Here's an example of what I mean. It's quite rough, doesn't yet eliminate
any of the code that it eventually could, and doesn't deal with broadcast.
I gave it a quick test with audit netlink, and it does what I expect.
Why not just call the security handler from the security_netlink_send()
function, which is already called by netlink_sendmsg()? Note that
SELinux (thanks to work by James Morris a while back) does apply
fine-grained netlink controls using its selinux_netlink_send() hook
function, but the problem with this approach is that it requires
maintaining a netlink message type table within SELinux itself that maps
netlink operations to generic flow operations (read, write), effectively
duplicating state between SELinux and the netlink protocol
implementations. It also isn't necessarily sufficiently granular for
all needs. It would be preferable to have the netlink protocol
implementations to maintain such mappings and just invoke security
modules to check the generic permissions based on its own internal
mapping (same idea applies for ioctls, where the device driver or
filesystem code should handle the mapping to generic permissions and
only invoke the security module to check the generic permission).
--
Stephen Smalley <sds(a)epoch.ncsc.mil>
National Security Agency
6:51 a.m.
On Thu, 2005-02-10 at 07:36, Stephen Smalley wrote:
Why not just call the security handler from the
security_netlink_send()
function, which is already called by netlink_sendmsg()?
Ok, I can see why you wouldn't want to do that, but why not just call
the handler immediately after security_netlink_send() then in
netlink_sendmsg()? What is the advantage of deferring to
netlink_sendskb (and some other location for broadcasts)?
--
Stephen Smalley <sds(a)tycho.nsa.gov>
National Security Agency
9:07 a.m.
Chris Wright wrote:
Here's an example of what I mean. It's quite rough, doesn't yet eliminate
any of the code that it eventually could, and doesn't deal with broadcast.
I gave it a quick test with audit netlink, and it does what I expect.
thanks,
-chris
It seems that this approach would work for keeping the loginuid in the payload.
The audit handler can then just set the loginuid in the payload - basically
ignoring whatever the application put into this field. This is assuming that we
are dealing with the structures in kernel memory here - I'm not that familiar
with this code...
Regarding an application setting the loginuid: Setting the loginuid is an
operation that is crucial to the reliability of the audit records, and is
appropriately protected with CAP_AUDIT_CONTROL. It is my understanding that the
loginuid associated with an audit record should be the loginuid of the process
that caused the record to be generated. If a userspace application wants to log
messages for another user, it can do so by simply adding that user's
identification information into the message it is writing (it can put anything
it wants to in there), the loginuid associated with the record still needs to be
that of the process which caused the record to be generated - the process
writing the record from userspace.
Are the changes of this method really better than simply adding the loginuid to
the loginuid to the netlink_skb_parms structure? I guess they would be if there
are other netlinkers that could make use of the check function.
--
Darrel
7255
days inactive
7261
days old
linux-audit@lists.linux-audit.osci.io
20 comments
9 participants
participants (9)
-
Alexey Kuznetsov -
Chris Wright -
Darrel Goeddel -
David Woodhouse -
Patrick McHardy -
Serge E. Hallyn -
Serge Hallyn
-
Stephen Smalley -
Stephen Smalley