Re: pf neighbor discovery hop limit

2017-12-04 Thread Alexander Bluhm
On Mon, Dec 04, 2017 at 08:23:26PM +, Job Snijders wrote:
> On Mon, Dec 04, 2017 at 02:55:16PM +0100, Alexander Bluhm wrote:
> > RFC 4861 requires that all neighbor discovery packets have 255 in
> > their IPv6 header hop limit field.  Let pf drop neighbor solicitation,
> > neighbor advertisement, router solicitation, router advertisement,
> > and redirect ICMP6 packets that do not comply.  This enforces that
> > bogus packets cannot be routed when pf is enabled.
> > 
> > ok?
> 
> Wouldn't this be a duplicate of "if (ip6->ip6_hlim != 255)" checks done
> in sys/netinet6/{icmp6,nd6_nbr,nd6_rtr}.c ?

These checks are never done for forwarded packets.  They protect
our own stack.  pf is there to protect the network behind the
firewall.

bluhm



Re: pf neighbor discovery hop limit

2017-12-04 Thread Job Snijders
On Mon, Dec 04, 2017 at 02:55:16PM +0100, Alexander Bluhm wrote:
> RFC 4861 requires that all neighbor discovery packets have 255 in
> their IPv6 header hop limit field.  Let pf drop neighbor solicitation,
> neighbor advertisement, router solicitation, router advertisement,
> and redirect ICMP6 packets that do not comply.  This enforces that
> bogus packets cannot be routed when pf is enabled.
> 
> ok?

Wouldn't this be a duplicate of "if (ip6->ip6_hlim != 255)" checks done
in sys/netinet6/{icmp6,nd6_nbr,nd6_rtr}.c ?

Kind regards,

Job



Re: pf neighbor discovery hop limit

2017-12-04 Thread Alexandr Nedvedicky
Hello,


On Mon, Dec 04, 2017 at 02:55:16PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> RFC 4861 requires that all neighbor discovery packets have 255 in
> their IPv6 header hop limit field.  Let pf drop neighbor solicitation,
> neighbor advertisement, router solicitation, router advertisement,
> and redirect ICMP6 packets that do not comply.  This enforces that
> bogus packets cannot be routed when pf is enabled.
> 
> ok?

yes please.

OK sashan



Re: pf neighbor discovery hop limit

2017-12-04 Thread Sebastian Benoit
Alexander Bluhm(alexander.bl...@gmx.net) on 2017.12.04 14:55:16 +0100:
> Hi,
> 
> RFC 4861 requires that all neighbor discovery packets have 255 in
> their IPv6 header hop limit field.  Let pf drop neighbor solicitation,
> neighbor advertisement, router solicitation, router advertisement,
> and redirect ICMP6 packets that do not comply.  This enforces that
> bogus packets cannot be routed when pf is enabled.
> 
> ok?


ok benno@

we also check this in relevant spots in netinet6/ as far as i can see.

> bluhm
> 
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1049
> diff -u -p -r1.1049 pf.c
> --- net/pf.c  1 Dec 2017 10:33:33 -   1.1049
> +++ net/pf.c  4 Dec 2017 13:53:21 -
> @@ -6602,6 +6602,14 @@ pf_setup_pdesc(struct pf_pdesc *pd, sa_f
>   case ND_NEIGHBOR_SOLICIT:
>   case ND_NEIGHBOR_ADVERT:
>   icmp_hlen = sizeof(struct nd_neighbor_solicit);
> + /* FALLTHROUGH */
> + case ND_ROUTER_SOLICIT:
> + case ND_ROUTER_ADVERT:
> + case ND_REDIRECT:
> + if (pd->ttl != 255) {
> + REASON_SET(reason, PFRES_NORM);
> + return (PF_DROP);
> + }
>   break;
>   }
>   if (icmp_hlen > sizeof(struct icmp6_hdr) &&
> 



pf neighbor discovery hop limit

2017-12-04 Thread Alexander Bluhm
Hi,

RFC 4861 requires that all neighbor discovery packets have 255 in
their IPv6 header hop limit field.  Let pf drop neighbor solicitation,
neighbor advertisement, router solicitation, router advertisement,
and redirect ICMP6 packets that do not comply.  This enforces that
bogus packets cannot be routed when pf is enabled.

ok?

bluhm

Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1049
diff -u -p -r1.1049 pf.c
--- net/pf.c1 Dec 2017 10:33:33 -   1.1049
+++ net/pf.c4 Dec 2017 13:53:21 -
@@ -6602,6 +6602,14 @@ pf_setup_pdesc(struct pf_pdesc *pd, sa_f
case ND_NEIGHBOR_SOLICIT:
case ND_NEIGHBOR_ADVERT:
icmp_hlen = sizeof(struct nd_neighbor_solicit);
+   /* FALLTHROUGH */
+   case ND_ROUTER_SOLICIT:
+   case ND_ROUTER_ADVERT:
+   case ND_REDIRECT:
+   if (pd->ttl != 255) {
+   REASON_SET(reason, PFRES_NORM);
+   return (PF_DROP);
+   }
break;
}
if (icmp_hlen > sizeof(struct icmp6_hdr) &&