* [PATCH net 0/2] ipv4: less uses of shared IP generator
@ 2022-01-26 20:05 Eric Dumazet
2022-01-26 20:05 ` [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages Eric Dumazet
2022-01-26 20:05 ` [PATCH net 2/2] ipv4: avoid using shared IP generator for connected sockets Eric Dumazet
0 siblings, 2 replies; 8+ messages in thread
From: Eric Dumazet @ 2022-01-26 20:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: David Ahern, netdev, Eric Dumazet, Eric Dumazet
From: Eric Dumazet <edumazet@google.com>
We keep receiving research reports based on linux IPID generation.
Before breaking part of the Internet by switching to pure
random generator, this series reduces the need for the
shared IP generator for TCP sockets.
Eric Dumazet (2):
ipv4: tcp: send zero IPID in SYNACK messages
ipv4: avoid using shared IP generator for connected sockets
include/net/ip.h | 21 ++++++++++-----------
net/ipv4/ip_output.c | 11 +++++++++--
2 files changed, 19 insertions(+), 13 deletions(-)
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages
2022-01-26 20:05 [PATCH net 0/2] ipv4: less uses of shared IP generator Eric Dumazet
@ 2022-01-26 20:05 ` Eric Dumazet
2022-01-26 20:35 ` David Ahern
` (2 more replies)
2022-01-26 20:05 ` [PATCH net 2/2] ipv4: avoid using shared IP generator for connected sockets Eric Dumazet
1 sibling, 3 replies; 8+ messages in thread
From: Eric Dumazet @ 2022-01-26 20:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: David Ahern, netdev, Eric Dumazet, Eric Dumazet, Ray Che,
Geoff Alexander, Willy Tarreau
From: Eric Dumazet <edumazet@google.com>
In commit 431280eebed9 ("ipv4: tcp: send zero IPID for RST and
ACK sent in SYN-RECV and TIME-WAIT state") we took care of some
ctl packets sent by TCP.
It turns out we need to use a similar strategy for SYNACK packets.
By default, they carry IP_DF and IPID==0, but there are ways
to ask them to use the hashed IP ident generator and thus
be used to build off-path attacks.
(Ref: Off-Path TCP Exploits of the Mixed IPID Assignment)
One of this way is to force (before listener is started)
echo 1 >/proc/sys/net/ipv4/ip_no_pmtu_disc
Another way is using forged ICMP ICMP_FRAG_NEEDED
with a very small MTU (like 68) to force a false return from
ip_dont_fragment()
In this patch, ip_build_and_send_pkt() uses the following
heuristics.
1) Most SYNACK packets are smaller than IPV4_MIN_MTU and therefore
can use IP_DF regardless of the listener or route pmtu setting.
2) In case the SYNACK packet is bigger than IPV4_MIN_MTU,
we use prandom_u32() generator instead of the IPv4 hashed ident one.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Ray Che <xijiache@gmail.com>
Cc: Geoff Alexander <alexandg@cs.unm.edu>
Cc: Willy Tarreau <w@1wt.eu>
---
net/ipv4/ip_output.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index e331c8d4e6cfc4f2199a7877d8257b3b3b519561..6529484e8a36e1d9aef942879a5d2d18cecf2dc9 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -162,12 +162,19 @@ int ip_build_and_send_pkt(struct sk_buff *skb, const struct sock *sk,
iph->daddr = (opt && opt->opt.srr ? opt->opt.faddr : daddr);
iph->saddr = saddr;
iph->protocol = sk->sk_protocol;
- if (ip_dont_fragment(sk, &rt->dst)) {
+ /* Do not bother generating IPID for small packets (eg SYNACK) */
+ if (skb->len <= IPV4_MIN_MTU || ip_dont_fragment(sk, &rt->dst)) {
iph->frag_off = htons(IP_DF);
iph->id = 0;
} else {
iph->frag_off = 0;
- __ip_select_ident(net, iph, 1);
+ /* TCP packets here are SYNACK with fat IPv4/TCP options.
+ * Avoid using the hashed IP ident generator.
+ */
+ if (sk->sk_protocol == IPPROTO_TCP)
+ iph->id = prandom_u32();
+ else
+ __ip_select_ident(net, iph, 1);
}
if (opt && opt->opt.optlen) {
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net 2/2] ipv4: avoid using shared IP generator for connected sockets
2022-01-26 20:05 [PATCH net 0/2] ipv4: less uses of shared IP generator Eric Dumazet
2022-01-26 20:05 ` [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages Eric Dumazet
@ 2022-01-26 20:05 ` Eric Dumazet
2022-01-26 20:36 ` David Ahern
1 sibling, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2022-01-26 20:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: David Ahern, netdev, Eric Dumazet, Eric Dumazet, Ray Che,
Willy Tarreau
From: Eric Dumazet <edumazet@google.com>
ip_select_ident_segs() has been very conservative about using
the connected socket private generator only for packets with IP_DF
set, claiming it was needed for some VJ compression implementations.
As mentioned in this referenced document, this can be abused.
(Ref: Off-Path TCP Exploits of the Mixed IPID Assignment)
Before switching to pure random IPID generation and possibly hurt
some workloads, lets use the private inet socket generator.
Not only this will remove one vulnerability, this will also
improve performance of TCP flows using pmtudisc==IP_PMTUDISC_DONT
Fixes: 73f156a6e8c1 ("inetpeer: get rid of ip_id_count")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Ray Che <xijiache@gmail.com>
Cc: Willy Tarreau <w@1wt.eu>
---
include/net/ip.h | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 81e23a102a0d5edec859b78239b81e6dcd82c54d..b51bae43b0ddb00735a09718530aa3fff4a04872 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -525,19 +525,18 @@ static inline void ip_select_ident_segs(struct net *net, struct sk_buff *skb,
{
struct iphdr *iph = ip_hdr(skb);
+ /* We had many attacks based on IPID, use the private
+ * generator as much as we can.
+ */
+ if (sk && inet_sk(sk)->inet_daddr) {
+ iph->id = htons(inet_sk(sk)->inet_id);
+ inet_sk(sk)->inet_id += segs;
+ return;
+ }
if ((iph->frag_off & htons(IP_DF)) && !skb->ignore_df) {
- /* This is only to work around buggy Windows95/2000
- * VJ compression implementations. If the ID field
- * does not change, they drop every other packet in
- * a TCP stream using header compression.
- */
- if (sk && inet_sk(sk)->inet_daddr) {
- iph->id = htons(inet_sk(sk)->inet_id);
- inet_sk(sk)->inet_id += segs;
- } else {
- iph->id = 0;
- }
+ iph->id = 0;
} else {
+ /* Unfortunately we need the big hammer to get a suitable IPID */
__ip_select_ident(net, iph, segs);
}
}
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages
2022-01-26 20:05 ` [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages Eric Dumazet
@ 2022-01-26 20:35 ` David Ahern
2022-01-27 0:56 ` Jakub Kicinski
2022-01-27 1:00 ` kernel test robot
2 siblings, 0 replies; 8+ messages in thread
From: David Ahern @ 2022-01-26 20:35 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski
Cc: David Ahern, netdev, Eric Dumazet, Ray Che, Geoff Alexander,
Willy Tarreau
On 1/26/22 1:05 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> In commit 431280eebed9 ("ipv4: tcp: send zero IPID for RST and
> ACK sent in SYN-RECV and TIME-WAIT state") we took care of some
> ctl packets sent by TCP.
>
> It turns out we need to use a similar strategy for SYNACK packets.
>
> By default, they carry IP_DF and IPID==0, but there are ways
> to ask them to use the hashed IP ident generator and thus
> be used to build off-path attacks.
> (Ref: Off-Path TCP Exploits of the Mixed IPID Assignment)
>
> One of this way is to force (before listener is started)
> echo 1 >/proc/sys/net/ipv4/ip_no_pmtu_disc
>
> Another way is using forged ICMP ICMP_FRAG_NEEDED
> with a very small MTU (like 68) to force a false return from
> ip_dont_fragment()
>
> In this patch, ip_build_and_send_pkt() uses the following
> heuristics.
>
> 1) Most SYNACK packets are smaller than IPV4_MIN_MTU and therefore
> can use IP_DF regardless of the listener or route pmtu setting.
>
> 2) In case the SYNACK packet is bigger than IPV4_MIN_MTU,
> we use prandom_u32() generator instead of the IPv4 hashed ident one.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Ray Che <xijiache@gmail.com>
> Cc: Geoff Alexander <alexandg@cs.unm.edu>
> Cc: Willy Tarreau <w@1wt.eu>
> ---
> net/ipv4/ip_output.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
Reviewed-by: David Ahern <dsahern@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 2/2] ipv4: avoid using shared IP generator for connected sockets
2022-01-26 20:05 ` [PATCH net 2/2] ipv4: avoid using shared IP generator for connected sockets Eric Dumazet
@ 2022-01-26 20:36 ` David Ahern
0 siblings, 0 replies; 8+ messages in thread
From: David Ahern @ 2022-01-26 20:36 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski
Cc: David Ahern, netdev, Eric Dumazet, Ray Che, Willy Tarreau
On 1/26/22 1:05 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> ip_select_ident_segs() has been very conservative about using
> the connected socket private generator only for packets with IP_DF
> set, claiming it was needed for some VJ compression implementations.
>
> As mentioned in this referenced document, this can be abused.
> (Ref: Off-Path TCP Exploits of the Mixed IPID Assignment)
>
> Before switching to pure random IPID generation and possibly hurt
> some workloads, lets use the private inet socket generator.
>
> Not only this will remove one vulnerability, this will also
> improve performance of TCP flows using pmtudisc==IP_PMTUDISC_DONT
>
> Fixes: 73f156a6e8c1 ("inetpeer: get rid of ip_id_count")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Ray Che <xijiache@gmail.com>
> Cc: Willy Tarreau <w@1wt.eu>
> ---
> include/net/ip.h | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
Reviewed-by: David Ahern <dsahern@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages
2022-01-26 20:05 ` [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages Eric Dumazet
2022-01-26 20:35 ` David Ahern
@ 2022-01-27 0:56 ` Jakub Kicinski
2022-01-27 1:02 ` Eric Dumazet
2022-01-27 1:00 ` kernel test robot
2 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2022-01-27 0:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, David Ahern, netdev, Eric Dumazet, Ray Che,
Geoff Alexander, Willy Tarreau
On Wed, 26 Jan 2022 12:05:17 -0800 Eric Dumazet wrote:
> + /* TCP packets here are SYNACK with fat IPv4/TCP options.
> + * Avoid using the hashed IP ident generator.
> + */
> + if (sk->sk_protocol == IPPROTO_TCP)
> + iph->id = prandom_u32();
Is it worth marking this as (__force __be32) to avoid the false
positive sparse warning?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages
2022-01-26 20:05 ` [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages Eric Dumazet
2022-01-26 20:35 ` David Ahern
2022-01-27 0:56 ` Jakub Kicinski
@ 2022-01-27 1:00 ` kernel test robot
2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-01-27 1:00 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski
Cc: kbuild-all, David Ahern, netdev, Eric Dumazet, Ray Che,
Geoff Alexander, Willy Tarreau
Hi Eric,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net/master]
url: https://github.com/0day-ci/linux/commits/Eric-Dumazet/ipv4-less-uses-of-shared-IP-generator/20220127-040810
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 429c3be8a5e2695b5b92a6a12361eb89eb185495
config: i386-randconfig-s002 (https://download.01.org/0day-ci/archive/20220127/202201270807.HsUjGLC8-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/37d3b618591c7c736c2ad3b3febe12779e01369c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eric-Dumazet/ipv4-less-uses-of-shared-IP-generator/20220127-040810
git checkout 37d3b618591c7c736c2ad3b3febe12779e01369c
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash net/ipv4/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> net/ipv4/ip_output.c:175:33: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be16 [usertype] id @@ got unsigned int @@
net/ipv4/ip_output.c:175:33: sparse: expected restricted __be16 [usertype] id
net/ipv4/ip_output.c:175:33: sparse: got unsigned int
net/ipv4/ip_output.c: note: in included file (through include/net/ip.h):
include/net/route.h:373:48: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned int [usertype] key @@ got restricted __be32 [usertype] daddr @@
include/net/route.h:373:48: sparse: expected unsigned int [usertype] key
include/net/route.h:373:48: sparse: got restricted __be32 [usertype] daddr
include/net/route.h:373:48: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned int [usertype] key @@ got restricted __be32 [usertype] daddr @@
include/net/route.h:373:48: sparse: expected unsigned int [usertype] key
include/net/route.h:373:48: sparse: got restricted __be32 [usertype] daddr
vim +175 net/ipv4/ip_output.c
140
141 /*
142 * Add an ip header to a skbuff and send it out.
143 *
144 */
145 int ip_build_and_send_pkt(struct sk_buff *skb, const struct sock *sk,
146 __be32 saddr, __be32 daddr, struct ip_options_rcu *opt,
147 u8 tos)
148 {
149 struct inet_sock *inet = inet_sk(sk);
150 struct rtable *rt = skb_rtable(skb);
151 struct net *net = sock_net(sk);
152 struct iphdr *iph;
153
154 /* Build the IP header. */
155 skb_push(skb, sizeof(struct iphdr) + (opt ? opt->opt.optlen : 0));
156 skb_reset_network_header(skb);
157 iph = ip_hdr(skb);
158 iph->version = 4;
159 iph->ihl = 5;
160 iph->tos = tos;
161 iph->ttl = ip_select_ttl(inet, &rt->dst);
162 iph->daddr = (opt && opt->opt.srr ? opt->opt.faddr : daddr);
163 iph->saddr = saddr;
164 iph->protocol = sk->sk_protocol;
165 /* Do not bother generating IPID for small packets (eg SYNACK) */
166 if (skb->len <= IPV4_MIN_MTU || ip_dont_fragment(sk, &rt->dst)) {
167 iph->frag_off = htons(IP_DF);
168 iph->id = 0;
169 } else {
170 iph->frag_off = 0;
171 /* TCP packets here are SYNACK with fat IPv4/TCP options.
172 * Avoid using the hashed IP ident generator.
173 */
174 if (sk->sk_protocol == IPPROTO_TCP)
> 175 iph->id = prandom_u32();
176 else
177 __ip_select_ident(net, iph, 1);
178 }
179
180 if (opt && opt->opt.optlen) {
181 iph->ihl += opt->opt.optlen>>2;
182 ip_options_build(skb, &opt->opt, daddr, rt, 0);
183 }
184
185 skb->priority = sk->sk_priority;
186 if (!skb->mark)
187 skb->mark = sk->sk_mark;
188
189 /* Send it out. */
190 return ip_local_out(net, skb->sk, skb);
191 }
192 EXPORT_SYMBOL_GPL(ip_build_and_send_pkt);
193
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages
2022-01-27 0:56 ` Jakub Kicinski
@ 2022-01-27 1:02 ` Eric Dumazet
0 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2022-01-27 1:02 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Eric Dumazet, David S . Miller, David Ahern, netdev, Ray Che,
Geoff Alexander, Willy Tarreau
On Wed, Jan 26, 2022 at 4:56 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 26 Jan 2022 12:05:17 -0800 Eric Dumazet wrote:
> > + /* TCP packets here are SYNACK with fat IPv4/TCP options.
> > + * Avoid using the hashed IP ident generator.
> > + */
> > + if (sk->sk_protocol == IPPROTO_TCP)
> > + iph->id = prandom_u32();
>
> Is it worth marking this as (__force __be32) to avoid the false
> positive sparse warning?
Sure thing, we can add this (I guess this needs to be __be16 ?)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-27 1:02 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-26 20:05 [PATCH net 0/2] ipv4: less uses of shared IP generator Eric Dumazet
2022-01-26 20:05 ` [PATCH net 1/2] ipv4: tcp: send zero IPID in SYNACK messages Eric Dumazet
2022-01-26 20:35 ` David Ahern
2022-01-27 0:56 ` Jakub Kicinski
2022-01-27 1:02 ` Eric Dumazet
2022-01-27 1:00 ` kernel test robot
2022-01-26 20:05 ` [PATCH net 2/2] ipv4: avoid using shared IP generator for connected sockets Eric Dumazet
2022-01-26 20:36 ` David Ahern
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).