| 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
| ||
|
Date: Fri, 15 Aug 2014 17:24:12 +0800
From: wangyufen <wangyufen@...wei.com>
To: Jonathan Toppins <jtoppins@...ulusnetworks.com>
CC: <davem@...emloft.net>, <netdev@...r.kernel.org>,
"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [PATCH v2 7/8] net: Use netlink_ns_capable to verify the permisions
of netlink messages
On 2014/8/15 15:38, wangyufen wrote:
> On 2014/8/1 6:06, Jonathan Toppins wrote:
>> On 7/25/14, 4:22 AM, Wangyufen wrote:
>>> From: "Eric W. Biederman" <ebiederm@...ssion.com>
>>>
>>> It is possible by passing a netlink socket to a more privileged
>>> executable and then to fool that executable into writing to the socket
>>> data that happens to be valid netlink message to do something that
>>> privileged executable did not intend to do.
>>>
>>> To keep this from happening replace bare capable and ns_capable calls
>>> with netlink_capable, netlink_net_calls and netlink_ns_capable calls.
>>> Which act the same as the previous calls except they verify that the
>>> opener of the socket had the desired permissions as well.
>>>
>>> Reported-by: Andy Lutomirski <luto@...capital.net>
>>> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
>>> Signed-off-by: David S. Miller <davem@...emloft.net>
>>> Signed-off-by: Wang Yufen <wangyufen@...wei.com>
>>> ---
>>> crypto/crypto_user.c | 2 +-
>>> drivers/connector/cn_proc.c | 2 +-
>>> drivers/scsi/scsi_netlink.c | 2 +-
>>> kernel/audit.c | 4 ++--
>>> net/core/rtnetlink.c | 2 +-
>>> net/decnet/dn_dev.c | 2 +-
>>> net/decnet/netfilter/dn_rtmsg.c | 2 +-
>>> net/netfilter/nfnetlink.c | 2 +-
>>> net/netlink/genetlink.c | 2 +-
>>> net/phonet/pn_netlink.c | 4 ++--
>>> net/tipc/netlink.c | 2 +-
>>> net/xfrm/xfrm_user.c | 2 +-
>>> 12 files changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
>>> index 910497b..c86969e 100644
>>> --- a/crypto/crypto_user.c
>>> +++ b/crypto/crypto_user.c
>>> @@ -452,7 +452,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>>> type -= CRYPTO_MSG_BASE;
>>> link = &crypto_dispatch[type];
>>>
>>> - if (!capable(CAP_NET_ADMIN))
>>> + if (!netlink_capable(skb, CAP_NET_ADMIN))
>>> return -EPERM;
>>>
>>> if ((type == (CRYPTO_MSG_GETALG - CRYPTO_MSG_BASE) &&
>>> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
>>> index 094a710..46856ae 100644
>>> --- a/drivers/connector/cn_proc.c
>>> +++ b/drivers/connector/cn_proc.c
>>> @@ -332,7 +332,7 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
>>> return;
>>>
>>> /* Can only change if privileged. */
>>> - if (!capable(CAP_NET_ADMIN)) {
>>> + if (!__netlink_ns_capable(nsp, &init_user_ns, CAP_NET_ADMIN)) {
>>> err = EPERM;
>>> goto out;
>>> }
>>> diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
>>> index c77628a..a930b66 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 (!capable(CAP_SYS_ADMIN)) {
>>> + if (!netlink_capable(skb, CAP_SYS_ADMIN)) {
>>> err = -EPERM;
>>> goto next_msg;
>>> }
>>> diff --git a/kernel/audit.c b/kernel/audit.c
>>> index b4efae8..3c3a31c 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 (!capable(CAP_AUDIT_CONTROL))
>>> + if (!netlink_capable(skb, 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 (!capable(CAP_AUDIT_WRITE))
>>> + if (!netlink_capable(skb, CAP_AUDIT_WRITE))
>>> err = -EPERM;
>>> break;
>>> default: /* bad msg */
>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>>> index a133427..d3ac150 100644
>>> --- a/net/core/rtnetlink.c
>>> +++ b/net/core/rtnetlink.c
>>> @@ -2010,7 +2010,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>>> sz_idx = type>>2;
>>> kind = type&3;
>>>
>>> - if (kind != 2 && !capable(CAP_NET_ADMIN))
>>> + if (kind != 2 && !netlink_net_capable(skb, CAP_NET_ADMIN))
>>> return -EPERM;
>>>
>>> if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) {
>>> diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
>>> index c00e307..b79ce1e 100644
>>> --- a/net/decnet/dn_dev.c
>>> +++ b/net/decnet/dn_dev.c
>>> @@ -440,7 +440,7 @@ int dn_dev_ioctl(unsigned int cmd, void __user *arg)
>>> case SIOCGIFADDR:
>>> break;
>>> case SIOCSIFADDR:
>>> - if (!capable(CAP_NET_ADMIN))
>>> + if (!netlink_capable(skb, CAP_NET_ADMIN))
>>
>> Hello,
>>
>> I am working on the 3.2 backport based off of these patches and am
>> getting a compile error here. It appears even the 3.4 series doesn't
>> compile, snippet of compile log for 3.4 series below:
>>
>> net/decnet/dn_dev.c: In function ‘dn_dev_ioctl’:
>> net/decnet/dn_dev.c:443:24: error: ‘skb’ undeclared (first use in this
>> function)
>> net/decnet/dn_dev.c:443:24: note: each undeclared identifier is reported
>> only once for each function it appears in
>> make[2]: *** [net/decnet/dn_dev.o] Error 1
>> make[1]: *** [net/decnet] Error 2
>>
>> jtoppins@...ian-devel:~/linux/linux-stable$ git log --oneline -10
>> a926d22 netlink: Only check file credentials for implicit destinations
>> 070d4a0 net: Use netlink_ns_capable to verify the permisions of netlink
>> messages
>> 31393c4 net: Add variants of capable for use on netlink messages
>> 45a1d1f net: Add variants of capable for use on on sockets
>> 9f2effc netlink: Rename netlink_capable netlink_allowed
>> f4d5163 Add file_ns_capable() helper function for open-time capability
>> checking
>> 3e8d4ac userns: make each net (net_ns) belong to a user_ns
>> 9087c45 netlink: Make the sending netlink socket availabe in NETLINK_CB
>> 82f9c4a Linux 3.4.100
>> 21870a3 iommu/vt-d: Disable translation if already enabled
>>
>> Am I missing something?
>>
>> Thanks,
>> -Jon
>>
>>
>
> sorry for that, I made a big mistake.
> I used the wrong .config file and net/decnet/dn_dev.c did't compiled in.
> I checked the patch, there is no need to change net/decnet/dn_dev.c, it's ioctl not netlink.
>
>>> return -EACCES;
>>> if (sdn->sdn_family != AF_DECnet)
>>> return -EINVAL;
>>> diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
>>> index 1531135..dc750e2 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 (!capable(CAP_NET_ADMIN))
>>> + if (!netlink_capable(skb, CAP_NET_ADMIN))
>>> RCV_SKB_FAIL(-EPERM);
>>>
>>> /* Eventually we might send routing messages too */
>>> diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
>>> index e6ddde1..5cfc865 100644
>>> --- a/net/netfilter/nfnetlink.c
>>> +++ b/net/netfilter/nfnetlink.c
>>> @@ -129,7 +129,7 @@ static int nfnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>>> const struct nfnetlink_subsystem *ss;
>>> int type, err;
>>>
>>> - if (!capable(CAP_NET_ADMIN))
>>> + if (!netlink_net_capable(skb, 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 73d3f0c..dff8562 100644
>>> --- a/net/netlink/genetlink.c
>>> +++ b/net/netlink/genetlink.c
>>> @@ -556,7 +556,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>>> return -EOPNOTSUPP;
>>>
>>> if ((ops->flags & GENL_ADMIN_PERM) &&
>>> - !capable(CAP_NET_ADMIN))
>>> + !netlink_capable(skb, CAP_NET_ADMIN))
>>> return -EPERM;
>>>
>>> if (nlh->nlmsg_flags & NLM_F_DUMP) {
>>> diff --git a/net/phonet/pn_netlink.c b/net/phonet/pn_netlink.c
>>> index d61f676..18485cd 100644
>>> --- a/net/phonet/pn_netlink.c
>>> +++ b/net/phonet/pn_netlink.c
>>> @@ -70,7 +70,7 @@ static int addr_doit(struct sk_buff *skb, struct nlmsghdr *nlh, void *attr)
>>> int err;
>>> u8 pnaddr;
>>>
>>> - if (!capable(CAP_SYS_ADMIN))
>>> + if (!netlink_capable(skb, CAP_SYS_ADMIN))
>>> return -EPERM;
>>>
>>> ASSERT_RTNL();
>>> @@ -228,7 +228,7 @@ static int route_doit(struct sk_buff *skb, struct nlmsghdr *nlh, void *attr)
>>> int err;
>>> u8 dst;
>>>
>>> - if (!capable(CAP_SYS_ADMIN))
>>> + if (!netlink_capable(skb, CAP_SYS_ADMIN))
>>> return -EPERM;
>>>
>>> ASSERT_RTNL();
>>> diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c
>>> index 7bda8e3..0b4cf4f 100644
>>> --- a/net/tipc/netlink.c
>>> +++ b/net/tipc/netlink.c
>>> @@ -47,7 +47,7 @@ static int handle_cmd(struct sk_buff *skb, struct genl_info *info)
>>> int hdr_space = NLMSG_SPACE(GENL_HDRLEN + TIPC_GENL_HDRLEN);
>>> u16 cmd;
>>>
>>> - if ((req_userhdr->cmd & 0xC000) && (!capable(CAP_NET_ADMIN)))
>>> + if ((req_userhdr->cmd & 0xC000) && (!netlink_capable(skb, CAP_NET_ADMIN)))
>>> cmd = TIPC_CMD_NOT_NET_ADMIN;
>>> else
>>> cmd = req_userhdr->cmd;
>>> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
>>> index c8b903d..ce16eba 100644
>>> --- a/net/xfrm/xfrm_user.c
>>> +++ b/net/xfrm/xfrm_user.c
>>> @@ -2317,7 +2317,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 (!capable(CAP_NET_ADMIN))
>>> + if (!netlink_net_capable(skb, CAP_NET_ADMIN))
>>> return -EPERM;
>>>
>>> if ((type == (XFRM_MSG_GETSA - XFRM_MSG_BASE) ||
>>>
>>
>>
>
>
> --
> 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
>
> .
>
This patch would be OK, I checked Kconfig, opened menuconfig options
to ensure thoes files be compiled in.
From: "Eric W. Biederman" <ebiederm@...ssion.com>
It is possible by passing a netlink socket to a more privileged
executable and then to fool that executable into writing to the socket
data that happens to be valid netlink message to do something that
privileged executable did not intend to do.
To keep this from happening replace bare capable and ns_capable calls
with netlink_capable, netlink_net_calls and netlink_ns_capable calls.
Which act the same as the previous calls except they verify that the
opener of the socket had the desired permissions as well.
Reported-by: Andy Lutomirski <luto@...capital.net>
Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
Signed-off-by: David S. Miller <davem@...emloft.net>
Signed-off-by: Wang Yufen <wangyufen@...wei.com>
---
crypto/crypto_user.c | 2 +-
drivers/connector/cn_proc.c | 2 +-
drivers/scsi/scsi_netlink.c | 2 +-
kernel/audit.c | 4 ++--
net/core/rtnetlink.c | 2 +-
net/decnet/netfilter/dn_rtmsg.c | 2 +-
net/netfilter/nfnetlink.c | 2 +-
net/netlink/genetlink.c | 2 +-
net/phonet/pn_netlink.c | 4 ++--
net/tipc/netlink.c | 2 +-
net/xfrm/xfrm_user.c | 2 +-
11 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index 910497b..c86969e 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -452,7 +452,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
type -= CRYPTO_MSG_BASE;
link = &crypto_dispatch[type];
- if (!capable(CAP_NET_ADMIN))
+ if (!netlink_capable(skb, CAP_NET_ADMIN))
return -EPERM;
if ((type == (CRYPTO_MSG_GETALG - CRYPTO_MSG_BASE) &&
diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 094a710..46856ae 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -332,7 +332,7 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
return;
/* Can only change if privileged. */
- if (!capable(CAP_NET_ADMIN)) {
+ if (!__netlink_ns_capable(nsp, &init_user_ns, CAP_NET_ADMIN)) {
err = EPERM;
goto out;
}
diff --git a/drivers/scsi/scsi_netlink.c b/drivers/scsi/scsi_netlink.c
index c77628a..a930b66 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 (!capable(CAP_SYS_ADMIN)) {
+ if (!netlink_capable(skb, CAP_SYS_ADMIN)) {
err = -EPERM;
goto next_msg;
}
diff --git a/kernel/audit.c b/kernel/audit.c
index b4efae8..3c3a31c 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 (!capable(CAP_AUDIT_CONTROL))
+ if (!netlink_capable(skb, 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 (!capable(CAP_AUDIT_WRITE))
+ if (!netlink_capable(skb, CAP_AUDIT_WRITE))
err = -EPERM;
break;
default: /* bad msg */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a133427..d3ac150 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2010,7 +2010,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
sz_idx = type>>2;
kind = type&3;
- if (kind != 2 && !capable(CAP_NET_ADMIN))
+ if (kind != 2 && !netlink_net_capable(skb, 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 1531135..dc750e2 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 (!capable(CAP_NET_ADMIN))
+ if (!netlink_capable(skb, CAP_NET_ADMIN))
RCV_SKB_FAIL(-EPERM);
/* Eventually we might send routing messages too */
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index e6ddde1..5cfc865 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -129,7 +129,7 @@ static int nfnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
const struct nfnetlink_subsystem *ss;
int type, err;
- if (!capable(CAP_NET_ADMIN))
+ if (!netlink_net_capable(skb, 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 73d3f0c..dff8562 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -556,7 +556,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
return -EOPNOTSUPP;
if ((ops->flags & GENL_ADMIN_PERM) &&
- !capable(CAP_NET_ADMIN))
+ !netlink_capable(skb, CAP_NET_ADMIN))
return -EPERM;
if (nlh->nlmsg_flags & NLM_F_DUMP) {
diff --git a/net/phonet/pn_netlink.c b/net/phonet/pn_netlink.c
index d61f676..18485cd 100644
--- a/net/phonet/pn_netlink.c
+++ b/net/phonet/pn_netlink.c
@@ -70,7 +70,7 @@ static int addr_doit(struct sk_buff *skb, struct nlmsghdr *nlh, void *attr)
int err;
u8 pnaddr;
- if (!capable(CAP_SYS_ADMIN))
+ if (!netlink_capable(skb, CAP_SYS_ADMIN))
return -EPERM;
ASSERT_RTNL();
@@ -228,7 +228,7 @@ static int route_doit(struct sk_buff *skb, struct nlmsghdr *nlh, void *attr)
int err;
u8 dst;
- if (!capable(CAP_SYS_ADMIN))
+ if (!netlink_capable(skb, CAP_SYS_ADMIN))
return -EPERM;
ASSERT_RTNL();
diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c
index 7bda8e3..0b4cf4f 100644
--- a/net/tipc/netlink.c
+++ b/net/tipc/netlink.c
@@ -47,7 +47,7 @@ static int handle_cmd(struct sk_buff *skb, struct genl_info *info)
int hdr_space = NLMSG_SPACE(GENL_HDRLEN + TIPC_GENL_HDRLEN);
u16 cmd;
- if ((req_userhdr->cmd & 0xC000) && (!capable(CAP_NET_ADMIN)))
+ if ((req_userhdr->cmd & 0xC000) && (!netlink_capable(skb, CAP_NET_ADMIN)))
cmd = TIPC_CMD_NOT_NET_ADMIN;
else
cmd = req_userhdr->cmd;
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index c8b903d..ce16eba 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2317,7 +2317,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 (!capable(CAP_NET_ADMIN))
+ if (!netlink_net_capable(skb, CAP_NET_ADMIN))
return -EPERM;
if ((type == (XFRM_MSG_GETSA - XFRM_MSG_BASE) ||
--
1.8.0
--
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
Powered by blists - more mailing lists