| 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: <90946257-e667-dff1-b941-8d596636d1bf@tycho.nsa.gov>
Date: Thu, 14 Feb 2019 13:59:27 -0500
From: Stephen Smalley <sds@...ho.nsa.gov>
To: Nazarov Sergey <s-nazarov@...dex.ru>,
Paul Moore <paul@...l-moore.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-security-module@...r.kernel.org"
<linux-security-module@...r.kernel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuznet@....inr.ac.ru" <kuznet@....inr.ac.ru>,
"yoshfuji@...ux-ipv6.org" <yoshfuji@...ux-ipv6.org>
Subject: Re: [PATCH] NETWORKING: avoid use IPCB in cipso_v4_error
On 2/14/19 1:00 PM, Nazarov Sergey wrote:
> Hi, Paul!
> I've found the problem and testing it with some very specific custom lsm module. The test case was simple:
> standard TCP/IP client-server application, where server opens CIPSO labeled TCP socket, and client connecting
> to this socket with forbidden labels. After several connections kernel crashing with general memory protection or
> kernel cache inconsistent error.
> I think, the similar behaviour should be with selinux or smack in the same conditions. But I don't know them
> so good to reproduce situation.
For SELinux, you can use
https://github.com/SELinuxProject/selinux-testsuite
That includes testing of CIPSO, both connecting from a client with an
authorized level and from a client with an unauthorized level.
Not sure about Smack; there were some tests in LTP but I don't know if
they would exercise it.
> After applying patch, I haven't kernel crashes.
> But now I've made additional checks and found no response icmp packets. The ip_options_compile requires
> CAP_NET_RAW capability when CIPSO option compiling, if skb is NULL. I have no other ideas than returning to
> the early patch version with ip_options_compile modified. What do you think about that?
>
> 14.02.2019, 00:42, "Paul Moore" <paul@...l-moore.com>:
>> On Tue, Feb 12, 2019 at 10:10 AM Nazarov Sergey <s-nazarov@...dex.ru> wrote:
>>> Since cipso_v4_error might be called from different network stack layers, we can't safely use icmp_send there.
>>> icmp_send copies IP options with ip_option_echo, which uses IPCB to take access to IP header compiled data.
>>> But after commit 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses"), IPCB can't be used
>>> above IP layer.
>>> This patch fixes the problem by creating in cipso_v4_error a local copy of compiled IP options and using it with
>>> introduced __icmp_send function. This looks some overloaded, but in quite rare error conditions only.
>>>
>>> The original discussion is here:
>>> https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/
>>>
>>> Signed-off-by: Sergey Nazarov <s-nazarov@...dex.ru>
>>> ---
>>> include/net/icmp.h | 9 ++++++++-
>>> net/ipv4/cipso_ipv4.c | 18 ++++++++++++++++--
>>> net/ipv4/icmp.c | 7 ++++---
>>> 3 files changed, 28 insertions(+), 6 deletions(-)
>>
>> Hi Sergey,
>>
>> Thanks for your work on finding this and putting a fix together. As
>> we discussed previously, I think this looks good, but can you describe
>> the testing you did to verify that this works correctly?
>>
>>> diff --git a/include/net/icmp.h b/include/net/icmp.h
>>> index 6ac3a5b..e0f709d 100644
>>> --- a/include/net/icmp.h
>>> +++ b/include/net/icmp.h
>>> @@ -22,6 +22,7 @@
>>>
>>> #include <net/inet_sock.h>
>>> #include <net/snmp.h>
>>> +#include <net/ip.h>
>>>
>>> struct icmp_err {
>>> int errno;
>>> @@ -39,7 +40,13 @@ struct icmp_err {
>>> struct sk_buff;
>>> struct net;
>>>
>>> -void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info);
>>> +void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
>>> + const struct ip_options *opt);
>>> +static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
>>> +{
>>> + __icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt);
>>> +}
>>> +
>>> int icmp_rcv(struct sk_buff *skb);
>>> int icmp_err(struct sk_buff *skb, u32 info);
>>> int icmp_init(void);
>>> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
>>> index 777fa3b..234d12e 100644
>>> --- a/net/ipv4/cipso_ipv4.c
>>> +++ b/net/ipv4/cipso_ipv4.c
>>> @@ -1735,13 +1735,27 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
>>> */
>>> void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
>>> {
>>> + unsigned char optbuf[sizeof(struct ip_options) + 40];
>>> + struct ip_options *opt = (struct ip_options *)optbuf;
>>> +
>>> if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
>>> return;
>>>
>>> + /*
>>> + * We might be called above the IP layer,
>>> + * so we can not use icmp_send and IPCB here.
>>> + */
>>> +
>>> + memset(opt, 0, sizeof(struct ip_options));
>>> + opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
>>> + memcpy(opt->__data, (unsigned char *)&(ip_hdr(skb)[1]), opt->optlen);
>>> + if (ip_options_compile(dev_net(skb->dev), opt, NULL))
>>> + return;
>>> +
>>> if (gateway)
>>> - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0);
>>> + __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
>>> else
>>> - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0);
>>> + __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
>>> }
>>>
>>> /**
>>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>>> index 065997f..3f24414 100644
>>> --- a/net/ipv4/icmp.c
>>> +++ b/net/ipv4/icmp.c
>>> @@ -570,7 +570,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
>>> * MUST reply to only the first fragment.
>>> */
>>>
>>> -void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
>>> +void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
>>> + const struct ip_options *opt)
>>> {
>>> struct iphdr *iph;
>>> int room;
>>> @@ -691,7 +692,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
>>> iph->tos;
>>> mark = IP4_REPLY_MARK(net, skb_in->mark);
>>>
>>> - if (ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in))
>>> + if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in, opt))
>>> goto out_unlock;
>>>
>>> @@ -742,7 +743,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
>>> local_bh_enable();
>>> out:;
>>> }
>>> -EXPORT_SYMBOL(icmp_send);
>>> +EXPORT_SYMBOL(__icmp_send);
>>>
>>> static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
>>> --
>>
>> --
>> paul moore
>> www.paul-moore.com
Powered by blists - more mailing lists