Re: Don't send icmp redirect to the same interface a forwarded packet came in on

2018-05-05 Thread Christopher Zimmermann 
On 2018-05-05 Martin Pieuchot  wrote:
> On 03/05/18(Thu) 17:19, Christopher Zimmermann wrote:
> > On 2018-05-03 Martin Pieuchot  wrote:  
> > > On 02/05/18(Wed) 14:45, Christopher Zimmermann wrote:  
> > > > On 2018-05-02 Martin Pieuchot  wrote:
> > > > > On 02/05/18(Wed) 11:47, Christopher Zimmermann wrote:
> > > > > > I just want to bring this up again. Can some network guru give me 
> > > > > > an ok
> > > > > > or some feedback please?  
> > > > > 
> > > > > Can you explain with words why we shouldn't send a redirect?  The
> > > > > comment above your diff states clearly:
> > > > > 
> > > > >   "If forwarding packet using same interface that it came in on,
> > > > >   perhaps should send a redirect to sender to shortcut a hop."
> > > > > 
> > > > > So you're suggesting no to do that, why?
> > > > 
> > > > That's not exactly what I'm suggesting.
> > > > 
> > > > In this setting:
> > > > 
> > > > A 192.168.4.7 <--> 192.168.4.1 Gateway 192.168.1.1 <--> 192.168.1.2 B
> > > > 
> > > > I observed this senseless redirect:
> > > > 
> > > > 192.168.4.1 > 192.168.4.7: icmp: redirect 192.168.1.2 to host 
> > > > 192.168.4.1
> > > > in plain language it means:
> > > > "Hi 192.168.4.7, I'm 192.168.4.1. You sent me a packet for 192.168.1.2.
> > > > I'm not the best route, next time send it to 192.168.4.1."
> > > > So the gateway is instructing host 192.168.4.7 to use gateway
> > > > 192.168.4.1 instead of 192.168.4.1. (this is not a typo!)
> > > 
> > > How does your routing table looks like?  
> > 
> > % doas ifconfig vlan2   
> >   
> > vlan2: flags=8843 mtu 1500  
> > 
> >   
> > index 7 priority 0 llprio 3
> > encap: vnetid 2 parent em2
> > status: active
> > inet 192.168.4.1 netmask 0xfff0 broadcast 192.168.4.15
> > % doas route -n show -inet
> > Routing tables
> > 
> > Internet:
> > DestinationGatewayFlags   Refs  Use   Mtu  Prio 
> > Iface
> > default62.27.93.143   UGS563324 - 8 
> > pppoe0
> > 224/4  127.0.0.1  URS00 32768 8 lo0
> > 62.27.93.143   85.212.225.8   UHh11 - 8 
> > pppoe0
> > 85.212.225.8   85.212.225.8   UHl0 1589 - 1 
> > pppoe0
> > 127/8  127.0.0.1  UGRS   00 32768 8 lo0
> > 127.0.0.1  127.0.0.1  UHhl   9 1131 32768 1 lo0
> > 192.168.0/22   192.168.4.1UGS0  314 - 8 
> > vlan2 <- this is the culprit
> > 192.168.4.0/28 192.168.4.1UCn20 - 4 
> > vlan2
> > 
> > You'll note I manually (Flag S) added this rather senseless route to gateway
> > 192.268.4.1 (Flag G), which is the very same machine.  
> 
> See, that's why the redirect message doesn't make sense.  Because it
> uses the info from this route.  Now What I'm not getting is where did
> you configured 192.168.1.1?

It's the IP of the other end of the IPsec tunnel.


-- 
http://gmerlin.de
OpenPGP: http://gmerlin.de/christopher.pub
2779 7F73 44FD 0736 B67A  C410 69EC 7922 34B4 2566

Re: Don't send icmp redirect to the same interface a forwarded packet came in on

2018-05-05 Thread Martin Pieuchot 
On 03/05/18(Thu) 17:19, Christopher Zimmermann wrote:
> On 2018-05-03 Martin Pieuchot  wrote:
> > On 02/05/18(Wed) 14:45, Christopher Zimmermann wrote:
> > > On 2018-05-02 Martin Pieuchot  wrote:  
> > > > On 02/05/18(Wed) 11:47, Christopher Zimmermann wrote:  
> > > > > I just want to bring this up again. Can some network guru give me an 
> > > > > ok
> > > > > or some feedback please?
> > > > 
> > > > Can you explain with words why we shouldn't send a redirect?  The
> > > > comment above your diff states clearly:
> > > > 
> > > >   "If forwarding packet using same interface that it came in on,
> > > >   perhaps should send a redirect to sender to shortcut a hop."
> > > > 
> > > > So you're suggesting no to do that, why?  
> > > 
> > > That's not exactly what I'm suggesting.
> > > 
> > > In this setting:
> > > 
> > > A 192.168.4.7 <--> 192.168.4.1 Gateway 192.168.1.1 <--> 192.168.1.2 B
> > > 
> > > I observed this senseless redirect:
> > > 
> > > 192.168.4.1 > 192.168.4.7: icmp: redirect 192.168.1.2 to host 192.168.4.1
> > > in plain language it means:
> > > "Hi 192.168.4.7, I'm 192.168.4.1. You sent me a packet for 192.168.1.2.
> > > I'm not the best route, next time send it to 192.168.4.1."
> > > So the gateway is instructing host 192.168.4.7 to use gateway
> > > 192.168.4.1 instead of 192.168.4.1. (this is not a typo!)  
> > 
> > How does your routing table looks like?
> 
> % doas ifconfig vlan2 
> 
> vlan2: flags=8843 mtu 1500
>   
>   
> index 7 priority 0 llprio 3
> encap: vnetid 2 parent em2
> status: active
> inet 192.168.4.1 netmask 0xfff0 broadcast 192.168.4.15
> % doas route -n show -inet
> Routing tables
> 
> Internet:
> DestinationGatewayFlags   Refs  Use   Mtu  Prio Iface
> default62.27.93.143   UGS563324 - 8 pppoe0
> 224/4  127.0.0.1  URS00 32768 8 lo0
> 62.27.93.143   85.212.225.8   UHh11 - 8 pppoe0
> 85.212.225.8   85.212.225.8   UHl0 1589 - 1 pppoe0
> 127/8  127.0.0.1  UGRS   00 32768 8 lo0
> 127.0.0.1  127.0.0.1  UHhl   9 1131 32768 1 lo0
> 192.168.0/22   192.168.4.1UGS0  314 - 8 vlan2 
> <- this is the culprit
> 192.168.4.0/28 192.168.4.1UCn20 - 4 vlan2
> 
> You'll note I manually (Flag S) added this rather senseless route to gateway
> 192.268.4.1 (Flag G), which is the very same machine.

See, that's why the redirect message doesn't make sense.  Because it
uses the info from this route.  Now What I'm not getting is where did
you configured 192.168.1.1?

Re: Don't send icmp redirect to the same interface a forwarded packet came in on

2018-05-03 Thread Christopher Zimmermann 
On 2018-05-03 Martin Pieuchot  wrote:
> On 02/05/18(Wed) 14:45, Christopher Zimmermann wrote:
> > On 2018-05-02 Martin Pieuchot  wrote:  
> > > On 02/05/18(Wed) 11:47, Christopher Zimmermann wrote:  
> > > > I just want to bring this up again. Can some network guru give me an ok
> > > > or some feedback please?
> > > 
> > > Can you explain with words why we shouldn't send a redirect?  The
> > > comment above your diff states clearly:
> > > 
> > >   "If forwarding packet using same interface that it came in on,
> > >   perhaps should send a redirect to sender to shortcut a hop."
> > > 
> > > So you're suggesting no to do that, why?  
> > 
> > That's not exactly what I'm suggesting.
> > 
> > In this setting:
> > 
> > A 192.168.4.7 <--> 192.168.4.1 Gateway 192.168.1.1 <--> 192.168.1.2 B
> > 
> > I observed this senseless redirect:
> > 
> > 192.168.4.1 > 192.168.4.7: icmp: redirect 192.168.1.2 to host 192.168.4.1
> > in plain language it means:
> > "Hi 192.168.4.7, I'm 192.168.4.1. You sent me a packet for 192.168.1.2.
> > I'm not the best route, next time send it to 192.168.4.1."
> > So the gateway is instructing host 192.168.4.7 to use gateway
> > 192.168.4.1 instead of 192.168.4.1. (this is not a typo!)  
> 
> How does your routing table looks like?

% doas ifconfig vlan2   
  
vlan2: flags=8843 mtu 1500  
  
index 7 priority 0 llprio 3
encap: vnetid 2 parent em2
status: active
inet 192.168.4.1 netmask 0xfff0 broadcast 192.168.4.15
% doas route -n show -inet
Routing tables

Internet:
DestinationGatewayFlags   Refs  Use   Mtu  Prio Iface
default62.27.93.143   UGS563324 - 8 pppoe0
224/4  127.0.0.1  URS00 32768 8 lo0
62.27.93.143   85.212.225.8   UHh11 - 8 pppoe0
85.212.225.8   85.212.225.8   UHl0 1589 - 1 pppoe0
127/8  127.0.0.1  UGRS   00 32768 8 lo0
127.0.0.1  127.0.0.1  UHhl   9 1131 32768 1 lo0
192.168.0/22   192.168.4.1UGS0  314 - 8 vlan2   
  <- this is the culprit
192.168.4.0/28 192.168.4.1UCn20 - 4 vlan2

You'll note I manually (Flag S) added this rather senseless route to gateway
192.268.4.1 (Flag G), which is the very same machine.
Why did I do this?
To prevent leakage of packets which should be routed via IPsec. Without this
pseudo-route packets from localhost to 192.168.0/22 would be routed via default
route on pppoe0 and have the src IP masqueraded by nat-to 85.212.225.8,
which would not be caught by the IPsec flow.

If I'm doing something wrong here and should have solved this another way
please tell me.
Using a "flow esp in from 0.0.0.0/0" would work on the local side, but the
remote side would be required to accept packets from arbitrary IPs, too.


Christopher


-- 
http://gmerlin.de
OpenPGP: http://gmerlin.de/christopher.pub
2779 7F73 44FD 0736 B67A  C410 69EC 7922 34B4 2566

Re: Don't send icmp redirect to the same interface a forwarded packet came in on

2018-05-03 Thread Martin Pieuchot 
On 02/05/18(Wed) 14:45, Christopher Zimmermann wrote:
> On 2018-05-02 Martin Pieuchot  wrote:
> > On 02/05/18(Wed) 11:47, Christopher Zimmermann wrote:
> > > I just want to bring this up again. Can some network guru give me an ok
> > > or some feedback please?  
> > 
> > Can you explain with words why we shouldn't send a redirect?  The
> > comment above your diff states clearly:
> > 
> >   "If forwarding packet using same interface that it came in on,
> >   perhaps should send a redirect to sender to shortcut a hop."
> > 
> > So you're suggesting no to do that, why?
> 
> That's not exactly what I'm suggesting.
> 
> In this setting:
> 
> A 192.168.4.7 <--> 192.168.4.1 Gateway 192.168.1.1 <--> 192.168.1.2 B
> 
> I observed this senseless redirect:
> 
> 192.168.4.1 > 192.168.4.7: icmp: redirect 192.168.1.2 to host 192.168.4.1
> in plain language it means:
> "Hi 192.168.4.7, I'm 192.168.4.1. You sent me a packet for 192.168.1.2.
> I'm not the best route, next time send it to 192.168.4.1."
> So the gateway is instructing host 192.168.4.7 to use gateway
> 192.168.4.1 instead of 192.168.4.1. (this is not a typo!)

How does your routing table looks like?

Re: Don't send icmp redirect to the same interface a forwarded packet came in on

2018-05-02 Thread Christopher Zimmermann 
On 2018-05-02 Martin Pieuchot  wrote:
> On 02/05/18(Wed) 11:47, Christopher Zimmermann wrote:
> > I just want to bring this up again. Can some network guru give me an ok
> > or some feedback please?  
> 
> Can you explain with words why we shouldn't send a redirect?  The
> comment above your diff states clearly:
> 
>   "If forwarding packet using same interface that it came in on,
>   perhaps should send a redirect to sender to shortcut a hop."
> 
> So you're suggesting no to do that, why?

That's not exactly what I'm suggesting.

In this setting:

A 192.168.4.7 <--> 192.168.4.1 Gateway 192.168.1.1 <--> 192.168.1.2 B

I observed this senseless redirect:

192.168.4.1 > 192.168.4.7: icmp: redirect 192.168.1.2 to host 192.168.4.1
in plain language it means:
"Hi 192.168.4.7, I'm 192.168.4.1. You sent me a packet for 192.168.1.2.
I'm not the best route, next time send it to 192.168.4.1."
So the gateway is instructing host 192.168.4.7 to use gateway
192.168.4.1 instead of 192.168.4.1. (this is not a typo!)

I'm suggesting to skip the redirect when we would redirect the sender
to an(other) address of the very interface the original packet came in
on.

I hope this explanation is reasonable.


Christopher


-- 
http://gmerlin.de
OpenPGP: http://gmerlin.de/christopher.pub
2779 7F73 44FD 0736 B67A  C410 69EC 7922 34B4 2566

Re: Don't send icmp redirect to the same interface a forwarded packet came in on

2018-05-02 Thread Martin Pieuchot 
On 02/05/18(Wed) 11:47, Christopher Zimmermann wrote:
> I just want to bring this up again. Can some network guru give me an ok
> or some feedback please?

Can you explain with words why we shouldn't send a redirect?  The
comment above your diff states clearly:

  "If forwarding packet using same interface that it came in on,
  perhaps should send a redirect to sender to shortcut a hop."

So you're suggesting no to do that, why?

Re: Don't send icmp redirect to the same interface a forwarded packet came in on

2018-05-02 Thread Christopher Zimmermann 
Hi,

I just want to bring this up again. Can some network guru give me an ok
or some feedback please?

Christopher


On 2017-12-01 Christopher Zimmermann  wrote:
> Hi,
> 
> by accident I discovered this rather senseless redirect:
> 
> $ doas tcpdump -eptni vlan2 icmp 
> tcpdump: listening on vlan2, link-type EN10MB
> 11:11:11:11:11:11 22:22:22:22:22 0800 98: 192.168.1.2 > 192.168.4.7: icmp: 
> echo request
> 22:22:22:22:22 11:11:11:11:11:11 0800 98: 192.168.4.7 > 192.168.1.2: icmp: 
> echo reply
> 11:11:11:11:11:11 22:22:22:22:22 0800 70: 192.168.4.1 > 192.168.4.7: icmp: 
> redirect 192.168.1.2 to host 192.168.4.1
> ^C
> 110 packets received by filter
> 0 packets dropped by kernel


[...]


> In any case I'd propose the following diff which will check whether the
> redirect target is an address of the interface the forwarded packet came in 
> on.
> 
> 
> Christopher
> 
> 
> Index: ip_input.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.322
> diff -u -p -r1.322 ip_input.c
> --- ip_input.c7 Sep 2017 10:54:49 -   1.322
> +++ ip_input.c1 Dec 2017 18:00:53 -
> @@ -1514,16 +1514,27 @@ ip_forward(struct mbuf *m, struct ifnet 
>   (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0 &&
>   satosin(rt_key(rt))->sin_addr.s_addr != 0 &&
>   ipsendredirects && !srcrt &&
> - !arpproxy(satosin(rt_key(rt))->sin_addr, m->m_pkthdr.ph_rtableid)) {
> - if ((ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_netmask) ==
> - ifatoia(rt->rt_ifa)->ia_net) {
> - if (rt->rt_flags & RTF_GATEWAY)
> + !arpproxy(satosin(rt_key(rt))->sin_addr, m->m_pkthdr.ph_rtableid) &&
> + (ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_netmask) ==
> + ifatoia(rt->rt_ifa)->ia_net) {
> + struct ifaddr *ifa;
> +
> + if (rt->rt_flags & RTF_GATEWAY)
>   dest = satosin(rt->rt_gateway)->sin_addr.s_addr;
> - else
> + else
>   dest = ip->ip_dst.s_addr;
> - /* Router requirements says to only send host redirects */
> - type = ICMP_REDIRECT;
> - code = ICMP_REDIRECT_HOST;
> +
> + /* don't redirect to the interface the packet came in on. */
> + TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) {
> + if (ifa->ifa_addr->sa_family == AF_INET &&
> + satosin(ifa->ifa_addr)->sin_addr.s_addr == dest)
> + dest = 0;
> + }
> +
> + /* Router requirements says to only send host redirects */
> + if (dest != 0) {
> + type = ICMP_REDIRECT;
> + code = ICMP_REDIRECT_HOST;
>   }
>   }
> 
>  



-- 
http://gmerlin.de
OpenPGP: http://gmerlin.de/christopher.pub
2779 7F73 44FD 0736 B67A  C410 69EC 7922 34B4 2566

Don't send icmp redirect to the same interface a forwarded packet came in on

2017-12-01 Thread Christopher Zimmermann 
Hi,

by accident I discovered this rather senseless redirect:

$ doas tcpdump -eptni vlan2 icmp 
tcpdump: listening on vlan2, link-type EN10MB
11:11:11:11:11:11 22:22:22:22:22 0800 98: 192.168.1.2 > 192.168.4.7: icmp: echo 
request
22:22:22:22:22 11:11:11:11:11:11 0800 98: 192.168.4.7 > 192.168.1.2: icmp: echo 
reply
11:11:11:11:11:11 22:22:22:22:22 0800 70: 192.168.4.1 > 192.168.4.7: icmp: 
redirect 192.168.1.2 to host 192.168.4.1
^C
110 packets received by filter
0 packets dropped by kernel


The routing table on 192.168.4.1 tells you why this redirect was sent.
Have a look at the second last route.

% doas ifconfig vlan2   
  
vlan2: flags=8843 mtu 1500  
  
lladdr 11:11:11:11:11:11
index 7 priority 0 llprio 3
encap: vnetid 2 parent em2
status: active
inet 192.168.4.1 netmask 0xfff0 broadcast 192.168.4.15
% doas route -n show -inet
Routing tables

Internet:
DestinationGatewayFlags   Refs  Use   Mtu  Prio Iface
default62.27.93.143   UGS563324 - 8 pppoe0
224/4  127.0.0.1  URS00 32768 8 lo0
62.27.93.143   85.212.225.8   UHh11 - 8 pppoe0
85.212.225.8   85.212.225.8   UHl0 1589 - 1 pppoe0
127/8  127.0.0.1  UGRS   00 32768 8 lo0
127.0.0.1  127.0.0.1  UHhl   9 1131 32768 1 lo0
192.168.0/22   192.168.4.1UGS0  314 - 8 vlan2   
  <- this is the culprit
192.168.4.0/28 192.168.4.1UCn20 - 4 vlan2


Network 192.268.0/22 is remote and connected via IPsec:
$ ipsecctl -s flow
flow esp in from 192.168.0.0/22 to 192.168.4.0/24 type use  
  
flow esp out from 192.168.224.0/24 to 192.168.0.0/17 type require   


You'll note I manually (Flag S) added this rather senseless route to gateway
192.268.4.1 (Flag G), which is the very same machine.
Why did I do this?
To prevent leakage of packets which should be routed via IPsec. Without this
pseudo-route packets from localhost to 192.168.0/22 would be routed via default
route on pppoe0 and have a src IP of 85.212.225.8,
which would not be caught by the IPsec flow.

If I'm doing something wrong here and should have solved this an another way
please tell me.
Using a "flow esp in from 0.0.0.0/0" would work on the local side, but the
remote side would be required to accept packets from arbitrary IPs, too.

In any case I'd propose the following diff which will check whether the
redirect target is an address of the interface the forwarded packet came in on.


Christopher


Index: ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.322
diff -u -p -r1.322 ip_input.c
--- ip_input.c  7 Sep 2017 10:54:49 -   1.322
+++ ip_input.c  1 Dec 2017 18:00:53 -
@@ -1514,16 +1514,27 @@ ip_forward(struct mbuf *m, struct ifnet 
(rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0 &&
satosin(rt_key(rt))->sin_addr.s_addr != 0 &&
ipsendredirects && !srcrt &&
-   !arpproxy(satosin(rt_key(rt))->sin_addr, m->m_pkthdr.ph_rtableid)) {
-   if ((ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_netmask) ==
-   ifatoia(rt->rt_ifa)->ia_net) {
-   if (rt->rt_flags & RTF_GATEWAY)
+   !arpproxy(satosin(rt_key(rt))->sin_addr, m->m_pkthdr.ph_rtableid) &&
+   (ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_netmask) ==
+   ifatoia(rt->rt_ifa)->ia_net) {
+   struct ifaddr *ifa;
+
+   if (rt->rt_flags & RTF_GATEWAY)
dest = satosin(rt->rt_gateway)->sin_addr.s_addr;
-   else
+   else
dest = ip->ip_dst.s_addr;
-   /* Router requirements says to only send host redirects */
-   type = ICMP_REDIRECT;
-   code = ICMP_REDIRECT_HOST;
+
+   /* don't redirect to the interface the packet came in on. */
+   TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) {
+   if (ifa->ifa_addr->sa_family == AF_INET &&
+   satosin(ifa->ifa_addr)->sin_addr.s_addr == dest)
+   dest = 0;
+   }
+
+   /* Router requirements says to only send host redirects */
+   if (dest != 0) {
+   type = ICMP_REDIRECT;
+   code = ICMP_REDIRECT_HOST;
}
}

 
-- 
http://gmerlin.de
OpenPGP: