| 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
| ||
|
Message-ID: <1320848342.10093.57.camel@localhost>
Date: Wed, 09 Nov 2011 09:19:02 -0500
From: Eric Paris <eparis@...hat.com>
To: "Serge E. Hallyn" <serge@...lyn.com>
Cc: linux-kernel@...r.kernel.org,
containers@...ts.linux-foundation.org, oleg@...hat.com,
richard@....at, akpm@...ux-foundation.org, ebiederm@...ssion.com,
dhowells@...hat.com, "Serge E. Hallyn" <serge.hallyn@...onical.com>
Subject: Re: [PATCH 6/6] protect cap_netlink_recv from user namespaces
On Tue, 2011-11-08 at 03:29 +0000, Serge E. Hallyn wrote:
> Quoting Eric Paris (eparis@...hat.com):
> But, regardless, your point is valid in that I'm not tightening down as
> much as I could. So how about I don't change the security_netlink_recv()
> and callers yet, and instead I change cap_netlink_recv() to do:
>
> if (!IN_ROOT_USER_NS && !cap_raised(current_cap(), cap))
Actually better thought. Remove the LSM hook altogether and just use
capable() in the callers. This hook, being used this way, was
introduced in c7bdb545 back when we took the effective perms from the
skb. We don't use the skb any more since netlink is synchronous. This
is functionally equivalent except the capabilities code checks against
the init_user_ns (something we want) and it will now set PF_PRIV (which
also seems like a good thing) Something like:
security: remove the security_netlink_recv hook as it is equivalent to capable()
Once upon a time netlink was not sync and we had to get the effective
capabilities from the skb that was being received. Today we instead get
the capabilities from the current task. This has rendered the entire
purpose of the hook moot as it is now functionally equivalent to the
capable() call.
Signed-off-by: Eric Paris <eparis@...hat.com>
---
drivers/scsi/scsi_netlink.c | 2 +-
include/linux/security.h | 14 --------------
kernel/audit.c | 4 ++--
net/core/rtnetlink.c | 2 +-
net/decnet/netfilter/dn_rtmsg.c | 2 +-
net/ipv4/netfilter/ip_queue.c | 2 +-
net/ipv6/netfilter/ip6_queue.c | 2 +-
net/netfilter/nfnetlink.c | 2 +-
net/netlink/genetlink.c | 2 +-
net/xfrm/xfrm_user.c | 2 +-
security/capability.c | 1 -
security/commoncap.c | 8 --------
security/security.c | 6 ------
security/selinux/hooks.c | 19 -------------------
14 files changed, 10 insertions(+), 58 deletions(-)
diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
index 44f76e8..c77628a 100644
--- a/drivers/scsi/scsi_netlink.c
+++ b/drivers/scsi/scsi_netlink.c
@@ -112,7 +112,7 @@ scsi_nl_rcv_msg(struct sk_buff *skb)
goto next_msg;
}
- if (security_netlink_recv(skb, CAP_SYS_ADMIN)) {
+ if (!capable(CAP_SYS_ADMIN)) {
err = -EPERM;
goto next_msg;
}
diff --git a/include/linux/security.h b/include/linux/security.h
index 1bb742b..bb7e8a0 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -96,7 +96,6 @@ struct xfrm_user_sec_ctx;
struct seq_file;
extern int cap_netlink_send(struct sock *sk, struct sk_buff *skb);
-extern int cap_netlink_recv(struct sk_buff *skb, int cap);
void reset_security_ops(void);
@@ -798,12 +797,6 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @skb contains the sk_buff structure for the netlink message.
* Return 0 if the information was successfully saved and message
* is allowed to be transmitted.
- * @netlink_recv:
- * Check permission before processing the received netlink message in
- * @skb.
- * @skb contains the sk_buff structure for the netlink message.
- * @cap indicates the capability required
- * Return 0 if permission is granted.
*
* Security hooks for Unix domain networking.
*
@@ -1564,7 +1557,6 @@ struct security_operations {
struct sembuf *sops, unsigned nsops, int alter);
int (*netlink_send) (struct sock *sk, struct sk_buff *skb);
- int (*netlink_recv) (struct sk_buff *skb, int cap);
void (*d_instantiate) (struct dentry *dentry, struct inode *inode);
@@ -1817,7 +1809,6 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode);
int security_getprocattr(struct task_struct *p, char *name, char **value);
int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size);
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
-int security_netlink_recv(struct sk_buff *skb, int cap);
int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
void security_release_secctx(char *secdata, u32 seclen);
@@ -2505,11 +2496,6 @@ static inline int security_netlink_send(struct sock *sk, struct sk_buff *skb)
return cap_netlink_send(sk, skb);
}
-static inline int security_netlink_recv(struct sk_buff *skb, int cap)
-{
- return cap_netlink_recv(skb, cap);
-}
-
static inline int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
{
return -EOPNOTSUPP;
diff --git a/kernel/audit.c b/kernel/audit.c
index 2c1d6ab..57e3f51 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -601,13 +601,13 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
case AUDIT_TTY_SET:
case AUDIT_TRIM:
case AUDIT_MAKE_EQUIV:
- if (security_netlink_recv(skb, CAP_AUDIT_CONTROL))
+ if (!capable(CAP_AUDIT_CONTROL))
err = -EPERM;
break;
case AUDIT_USER:
case AUDIT_FIRST_USER_MSG ... AUDIT_LAST_USER_MSG:
case AUDIT_FIRST_USER_MSG2 ... AUDIT_LAST_USER_MSG2:
- if (security_netlink_recv(skb, CAP_AUDIT_WRITE))
+ if (!capable(CAP_AUDIT_WRITE))
err = -EPERM;
break;
default: /* bad msg */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 39f8dd6..98ee1b6 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1930,7 +1930,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
sz_idx = type>>2;
kind = type&3;
- if (kind != 2 && security_netlink_recv(skb, CAP_NET_ADMIN))
+ if (kind != 2 && !capable(CAP_NET_ADMIN))
return -EPERM;
if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) {
diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
index 69975e0..1531135 100644
--- a/net/decnet/netfilter/dn_rtmsg.c
+++ b/net/decnet/netfilter/dn_rtmsg.c
@@ -108,7 +108,7 @@ static inline void dnrmg_receive_user_skb(struct sk_buff *skb)
if (nlh->nlmsg_len < sizeof(*nlh) || skb->len < nlh->nlmsg_len)
return;
- if (security_netlink_recv(skb, CAP_NET_ADMIN))
+ if (!capable(CAP_NET_ADMIN))
RCV_SKB_FAIL(-EPERM);
/* Eventually we might send routing messages too */
diff --git a/net/ipv4/netfilter/ip_queue.c b/net/ipv4/netfilter/ip_queue.c
index e59aabd..ffabb26 100644
--- a/net/ipv4/netfilter/ip_queue.c
+++ b/net/ipv4/netfilter/ip_queue.c
@@ -430,7 +430,7 @@ __ipq_rcv_skb(struct sk_buff *skb)
if (type <= IPQM_BASE)
return;
- if (security_netlink_recv(skb, CAP_NET_ADMIN))
+ if (!capable(CAP_NET_ADMIN))
RCV_SKB_FAIL(-EPERM);
spin_lock_bh(&queue_lock);
diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c
index e63c397..5e5ce77 100644
--- a/net/ipv6/netfilter/ip6_queue.c
+++ b/net/ipv6/netfilter/ip6_queue.c
@@ -431,7 +431,7 @@ __ipq_rcv_skb(struct sk_buff *skb)
if (type <= IPQM_BASE)
return;
- if (security_netlink_recv(skb, CAP_NET_ADMIN))
+ if (!capable(CAP_NET_ADMIN))
RCV_SKB_FAIL(-EPERM);
spin_lock_bh(&queue_lock);
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index c879c1a..9da4fc5 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -130,7 +130,7 @@ static int nfnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
const struct nfnetlink_subsystem *ss;
int type, err;
- if (security_netlink_recv(skb, CAP_NET_ADMIN))
+ if (!capable(CAP_NET_ADMIN))
return -EPERM;
/* All the messages must at least contain nfgenmsg */
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 482fa57..05fedbf 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -516,7 +516,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
return -EOPNOTSUPP;
if ((ops->flags & GENL_ADMIN_PERM) &&
- security_netlink_recv(skb, CAP_NET_ADMIN))
+ !capable(CAP_NET_ADMIN))
return -EPERM;
if (nlh->nlmsg_flags & NLM_F_DUMP) {
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index d0a42df..7ea716d 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2290,7 +2290,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
link = &xfrm_dispatch[type];
/* All operations require privileges, even GET */
- if (security_netlink_recv(skb, CAP_NET_ADMIN))
+ if (!capable(CAP_NET_ADMIN))
return -EPERM;
if ((type == (XFRM_MSG_GETSA - XFRM_MSG_BASE) ||
diff --git a/security/capability.c b/security/capability.c
index 3cf5ae3..5c1aea0 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -1004,7 +1004,6 @@ void __init security_fixup_ops(struct security_operations *ops)
set_to_cap_if_null(ops, sem_semctl);
set_to_cap_if_null(ops, sem_semop);
set_to_cap_if_null(ops, netlink_send);
- set_to_cap_if_null(ops, netlink_recv);
set_to_cap_if_null(ops, d_instantiate);
set_to_cap_if_null(ops, getprocattr);
set_to_cap_if_null(ops, setprocattr);
diff --git a/security/commoncap.c b/security/commoncap.c
index 90fdf97..454e974 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -56,14 +56,6 @@ int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
return 0;
}
-int cap_netlink_recv(struct sk_buff *skb, int cap)
-{
- if (!cap_raised(current_cap(), cap))
- return -EPERM;
- return 0;
-}
-EXPORT_SYMBOL(cap_netlink_recv);
-
/**
* cap_capable - Determine whether a task has a particular effective capability
* @cred: The credentials to use
diff --git a/security/security.c b/security/security.c
index 5560472..17ee1c0 100644
--- a/security/security.c
+++ b/security/security.c
@@ -983,12 +983,6 @@ int security_netlink_send(struct sock *sk, struct sk_buff *skb)
return security_ops->netlink_send(sk, skb);
}
-int security_netlink_recv(struct sk_buff *skb, int cap)
-{
- return security_ops->netlink_recv(skb, cap);
-}
-EXPORT_SYMBOL(security_netlink_recv);
-
int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
{
return security_ops->secid_to_secctx(secid, secdata, seclen);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3deee07..668fe48 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4731,24 +4731,6 @@ static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
return selinux_nlmsg_perm(sk, skb);
}
-static int selinux_netlink_recv(struct sk_buff *skb, int capability)
-{
- int err;
- struct common_audit_data ad;
- u32 sid;
-
- err = cap_netlink_recv(skb, capability);
- if (err)
- return err;
-
- COMMON_AUDIT_DATA_INIT(&ad, CAP);
- ad.u.cap = capability;
-
- security_task_getsecid(current, &sid);
- return avc_has_perm(sid, sid, SECCLASS_CAPABILITY,
- CAP_TO_MASK(capability), &ad);
-}
-
static int ipc_alloc_security(struct task_struct *task,
struct kern_ipc_perm *perm,
u16 sclass)
@@ -5477,7 +5459,6 @@ static struct security_operations selinux_ops = {
.vm_enough_memory = selinux_vm_enough_memory,
.netlink_send = selinux_netlink_send,
- .netlink_recv = selinux_netlink_recv,
.bprm_set_creds = selinux_bprm_set_creds,
.bprm_committing_creds = selinux_bprm_committing_creds,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists