* [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
* 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 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-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
* 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
* [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 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
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