Re: pf icmp reflect

2021-07-26 Thread Klemens Nanni
On Mon, Jul 26, 2021 at 06:41:42PM +0200, Alexander Bluhm wrote:
> The mbuf header cleanup I added in revision 1.173 of ip_icmp.c is
> too strict.  ICMP error packets generated by pf are not passed
> immediately, but may be blocked.  Preserve PF_TAG_GENERATED flag
> in icmp_reflect() and icmp6_reflect().

OK kn



Re: pf icmp reflect

2021-07-26 Thread Patrick Wildt
On Mon, Jul 26, 2021 at 06:41:42PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> The mbuf header cleanup I added in revision 1.173 of ip_icmp.c is
> too strict.  ICMP error packets generated by pf are not passed
> immediately, but may be blocked.  Preserve PF_TAG_GENERATED flag
> in icmp_reflect() and icmp6_reflect().
> 
> ok?

While I do prefer uint8_t, the struct member is defined as u_int8_t, so
I guess for consistency we can use that.

ok patrick@

> bluhm
> 
> Index: netinet/ip_icmp.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.c,v
> retrieving revision 1.186
> diff -u -p -r1.186 ip_icmp.c
> --- netinet/ip_icmp.c 30 Mar 2021 08:37:10 -  1.186
> +++ netinet/ip_icmp.c 26 Jul 2021 14:10:37 -
> @@ -691,6 +691,7 @@ icmp_reflect(struct mbuf *m, struct mbuf
>   struct rtentry *rt = NULL;
>   int optlen = (ip->ip_hl << 2) - sizeof(struct ip);
>   u_int rtableid;
> + u_int8_t pfflags;
>  
>   if (!in_canforward(ip->ip_src) &&
>   ((ip->ip_src.s_addr & IN_CLASSA_NET) !=
> @@ -704,8 +705,10 @@ icmp_reflect(struct mbuf *m, struct mbuf
>   return (ELOOP);
>   }
>   rtableid = m->m_pkthdr.ph_rtableid;
> + pfflags = m->m_pkthdr.pf.flags;
>   m_resethdr(m);
>   m->m_pkthdr.ph_rtableid = rtableid;
> + m->m_pkthdr.pf.flags = pfflags & PF_TAG_GENERATED;
>  
>   /*
>* If the incoming packet was addressed directly to us,
> Index: netinet6/icmp6.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
> retrieving revision 1.235
> diff -u -p -r1.235 icmp6.c
> --- netinet6/icmp6.c  10 Mar 2021 10:21:49 -  1.235
> +++ netinet6/icmp6.c  26 Jul 2021 15:42:33 -
> @@ -1052,6 +1052,7 @@ icmp6_reflect(struct mbuf **mp, size_t o
>   struct in6_addr t, *src = NULL;
>   struct sockaddr_in6 sa6_src, sa6_dst;
>   u_int rtableid;
> + u_int8_t pfflags;
>  
>   CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <= MHLEN);
>  
> @@ -1069,8 +1070,10 @@ icmp6_reflect(struct mbuf **mp, size_t o
>   return (ELOOP);
>   }
>   rtableid = m->m_pkthdr.ph_rtableid;
> + pfflags = m->m_pkthdr.pf.flags;
>   m_resethdr(m);
>   m->m_pkthdr.ph_rtableid = rtableid;
> + m->m_pkthdr.pf.flags = pfflags & PF_TAG_GENERATED;
>  
>   /*
>* If there are extra headers between IPv6 and ICMPv6, strip
> 



pf icmp reflect

2021-07-26 Thread Alexander Bluhm
Hi,

The mbuf header cleanup I added in revision 1.173 of ip_icmp.c is
too strict.  ICMP error packets generated by pf are not passed
immediately, but may be blocked.  Preserve PF_TAG_GENERATED flag
in icmp_reflect() and icmp6_reflect().

ok?

bluhm

Index: netinet/ip_icmp.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.186
diff -u -p -r1.186 ip_icmp.c
--- netinet/ip_icmp.c   30 Mar 2021 08:37:10 -  1.186
+++ netinet/ip_icmp.c   26 Jul 2021 14:10:37 -
@@ -691,6 +691,7 @@ icmp_reflect(struct mbuf *m, struct mbuf
struct rtentry *rt = NULL;
int optlen = (ip->ip_hl << 2) - sizeof(struct ip);
u_int rtableid;
+   u_int8_t pfflags;
 
if (!in_canforward(ip->ip_src) &&
((ip->ip_src.s_addr & IN_CLASSA_NET) !=
@@ -704,8 +705,10 @@ icmp_reflect(struct mbuf *m, struct mbuf
return (ELOOP);
}
rtableid = m->m_pkthdr.ph_rtableid;
+   pfflags = m->m_pkthdr.pf.flags;
m_resethdr(m);
m->m_pkthdr.ph_rtableid = rtableid;
+   m->m_pkthdr.pf.flags = pfflags & PF_TAG_GENERATED;
 
/*
 * If the incoming packet was addressed directly to us,
Index: netinet6/icmp6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.235
diff -u -p -r1.235 icmp6.c
--- netinet6/icmp6.c10 Mar 2021 10:21:49 -  1.235
+++ netinet6/icmp6.c26 Jul 2021 15:42:33 -
@@ -1052,6 +1052,7 @@ icmp6_reflect(struct mbuf **mp, size_t o
struct in6_addr t, *src = NULL;
struct sockaddr_in6 sa6_src, sa6_dst;
u_int rtableid;
+   u_int8_t pfflags;
 
CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <= MHLEN);
 
@@ -1069,8 +1070,10 @@ icmp6_reflect(struct mbuf **mp, size_t o
return (ELOOP);
}
rtableid = m->m_pkthdr.ph_rtableid;
+   pfflags = m->m_pkthdr.pf.flags;
m_resethdr(m);
m->m_pkthdr.ph_rtableid = rtableid;
+   m->m_pkthdr.pf.flags = pfflags & PF_TAG_GENERATED;
 
/*
 * If there are extra headers between IPv6 and ICMPv6, strip