On Sun, Dec 20, 2020 at 01:01:58AM +0100, Alexander Bluhm wrote:
> Hi,
>
> In revision 1.87 of ip_icmp.c claudio@ added ignoring reject routes
> to icmp_mtudisc_clone(). Otherwise TCP would clone these routes
> for PMTU discovery. They will not work, even after dynamic routing
> has found a better route than the reject route.
>
> With IPsec the use case is different. First you need a route, but
> then the flow handles the packet without routing. Usually this
> route should be a reject route to avoid sending unencrypted traffic
> if the flow is missing. But IPsec needs this route for PMTU
> discovery, which currently does not work.
>
> So accept reject and blackhole routes for IPsec PMTU discovery.
>
> ok?
This makes sense and is fine. OK claudio@
> bluhm
>
> Index: netinet/ip_icmp.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.c,v
> retrieving revision 1.183
> diff -u -p -r1.183 ip_icmp.c
> --- netinet/ip_icmp.c 22 Aug 2020 17:55:54 -0000 1.183
> +++ netinet/ip_icmp.c 18 Dec 2020 16:59:25 -0000
> @@ -928,7 +928,7 @@ icmp_sysctl_icmpstat(void *oldp, size_t
> }
>
> struct rtentry *
> -icmp_mtudisc_clone(struct in_addr dst, u_int rtableid)
> +icmp_mtudisc_clone(struct in_addr dst, u_int rtableid, int ipsec)
> {
> struct sockaddr_in sin;
> struct rtentry *rt;
> @@ -942,7 +942,10 @@ icmp_mtudisc_clone(struct in_addr dst, u
> rt = rtalloc(sintosa(&sin), RT_RESOLVE, rtableid);
>
> /* Check if the route is actually usable */
> - if (!rtisvalid(rt) || (rt->rt_flags & (RTF_REJECT|RTF_BLACKHOLE)))
> + if (!rtisvalid(rt))
> + goto bad;
> + /* IPsec needs the route only for PMTU, it can use reject for that */
> + if (!ipsec && (rt->rt_flags & (RTF_REJECT|RTF_BLACKHOLE)))
> goto bad;
>
> /*
> @@ -1000,7 +1003,7 @@ icmp_mtudisc(struct icmp *icp, u_int rta
> struct ifnet *ifp;
> u_long mtu = ntohs(icp->icmp_nextmtu); /* Why a long? IPv6 */
>
> - rt = icmp_mtudisc_clone(icp->icmp_ip.ip_dst, rtableid);
> + rt = icmp_mtudisc_clone(icp->icmp_ip.ip_dst, rtableid, 0);
> if (rt == NULL)
> return;
>
> Index: netinet/ip_icmp.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.h,v
> retrieving revision 1.31
> diff -u -p -r1.31 ip_icmp.h
> --- netinet/ip_icmp.h 5 Nov 2018 21:50:39 -0000 1.31
> +++ netinet/ip_icmp.h 18 Dec 2020 16:59:25 -0000
> @@ -239,7 +239,7 @@ int icmp_reflect(struct mbuf *, struct m
> void icmp_send(struct mbuf *, struct mbuf *);
> int icmp_sysctl(int *, u_int, void *, size_t *, void *, size_t);
> struct rtentry *
> - icmp_mtudisc_clone(struct in_addr, u_int);
> + icmp_mtudisc_clone(struct in_addr, u_int, int);
> void icmp_mtudisc(struct icmp *, u_int);
> int icmp_do_exthdr(struct mbuf *, u_int16_t, u_int8_t, void *, size_t);
> #endif /* _KERNEL */
> Index: netinet/ip_output.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.357
> diff -u -p -r1.357 ip_output.c
> --- netinet/ip_output.c 24 Jun 2020 22:03:43 -0000 1.357
> +++ netinet/ip_output.c 18 Dec 2020 16:59:25 -0000
> @@ -605,7 +605,7 @@ ip_output_ipsec_send(struct tdb *tdb, st
> rt = NULL;
> else if (rt == NULL || (rt->rt_flags & RTF_HOST) == 0) {
> rt = icmp_mtudisc_clone(ip->ip_dst,
> - m->m_pkthdr.ph_rtableid);
> + m->m_pkthdr.ph_rtableid, 1);
> rt_mtucloned = 1;
> }
> DPRINTF(("%s: spi %08x mtu %d rt %p cloned %d\n", __func__,
> Index: netinet/tcp_timer.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_timer.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 tcp_timer.c
> --- netinet/tcp_timer.c 11 Jun 2018 07:40:26 -0000 1.67
> +++ netinet/tcp_timer.c 18 Dec 2020 16:59:25 -0000
> @@ -292,7 +292,7 @@ tcp_timer_rexmt(void *arg)
> #endif
> case PF_INET:
> rt = icmp_mtudisc_clone(inp->inp_faddr,
> - inp->inp_rtableid);
> + inp->inp_rtableid, 0);
> break;
> }
> if (rt != NULL) {
>
--
:wq Claudio
