Re: pf(4) drops valid IGMP/MLD messages
I've just seen that this has been fixed in 7.3, now I see that prune, graft, graft-ack messages are not blocked by PF Thanks a lot Il giorno gio 16 mar 2023 alle ore 16:45 Luca Di Gregorio ha scritto: > Ok, thanks a lot for the info > > Il giorno gio 16 mar 2023 alle 16:40 Theo de Raadt > ha scritto: > >> Luca Di Gregorio wrote: >> >> > do you think that the correction proposed by Alexandr could be done >> with a >> > syspatch, or in the next release? >> >> It does not meet the treshold for becoming a syspatch. >> >
Re: pf(4) drops valid IGMP/MLD messages
Ok, thanks a lot for the info Il giorno gio 16 mar 2023 alle 16:40 Theo de Raadt ha scritto: > Luca Di Gregorio wrote: > > > do you think that the correction proposed by Alexandr could be done with > a > > syspatch, or in the next release? > > It does not meet the treshold for becoming a syspatch. >
Re: pf(4) drops valid IGMP/MLD messages
Hi,
do you think that the correction proposed by Alexandr could be done with a
syspatch, or in the next release?
Thanks, regards
Il giorno sab 4 mar 2023 alle ore 06:34 Luca Di Gregorio
ha scritto:
> Hi, my modest opinions:
>
> > Instead of implementing more and more details of RFC, we should
> > discuss what the goal is.
>
> I'd like to have multicast routing working properly.
> Multicast routing with good network architecture allows people
> to save tons of resources on hosts, as well as saving a lot of
> network bandwidth.
>
> > What are the IGMP illegal packets that an attacker might use? We
> > should drop them. This IGMP logic is deep down in pf that a user
> > cannot override.
>
> Good point, and I'm happy that OpenBSD people always have security
> in mind. That is the main reason - not the only one - why OpenBSD
> is always my first choice for my projects.
>
> A couple of ideas here:
>
> 1 - discard IP and IGMP packets with illegal lengths:
> Ref:
> https://support.huawei.com/enterprise/en/doc/EDOC1100112357/11182319/defense-against-malformed-packet-attacks
>
> 2 - discard IGMP packets if multicast=NO (rcctl get multicast)
> Maybe the kernel already does this, but I'm not sure about it.
>
> Thanks again, regards
>
> Il giorno sab 4 mar 2023 alle ore 00:38 Alexander Bluhm <
> alexander.bl...@gmx.net> ha scritto:
>
>> On Sat, Mar 04, 2023 at 12:09:41AM +0100, Alexandr Nedvedicky wrote:
>> > 6847 /* IGMP packets have router alert options, allow them */
>> > 6848 if (pd->proto == IPPROTO_IGMP) {
>> > 6849 /*
>> > 6850 * According to RFC 1112 ttl must be set to 1 in
>> all IGMP
>> > 6851 * packets sent do 224.0.0.1
>> > 6852 */
>> > 6853 if ((h->ip_ttl != 1) &&
>> > 6854 (h->ip_dst.s_addr == INADDR_ALLHOSTS_GROUP)) {
>> > 6855 DPFPRINTF(LOG_NOTICE, "Invalid IGMP");
>> > 6856 REASON_SET(reason, PFRES_IPOPTIONS);
>> > 6857 return (PF_DROP);
>> > 6858 }
>> > 6859 CLR(pd->badopts, PF_OPT_ROUTER_ALERT);
>> >
>> > This change should make pf(4) reasonably paranoid while keeping IGMP
>> working.
>>
>> OK bluhm@
>>
>
Re: pf(4) drops valid IGMP/MLD messages
Hi, my modest opinions:
> Instead of implementing more and more details of RFC, we should
> discuss what the goal is.
I'd like to have multicast routing working properly.
Multicast routing with good network architecture allows people
to save tons of resources on hosts, as well as saving a lot of
network bandwidth.
> What are the IGMP illegal packets that an attacker might use? We
> should drop them. This IGMP logic is deep down in pf that a user
> cannot override.
Good point, and I'm happy that OpenBSD people always have security
in mind. That is the main reason - not the only one - why OpenBSD
is always my first choice for my projects.
A couple of ideas here:
1 - discard IP and IGMP packets with illegal lengths:
Ref:
https://support.huawei.com/enterprise/en/doc/EDOC1100112357/11182319/defense-against-malformed-packet-attacks
2 - discard IGMP packets if multicast=NO (rcctl get multicast)
Maybe the kernel already does this, but I'm not sure about it.
Thanks again, regards
Il giorno sab 4 mar 2023 alle ore 00:38 Alexander Bluhm <
alexander.bl...@gmx.net> ha scritto:
> On Sat, Mar 04, 2023 at 12:09:41AM +0100, Alexandr Nedvedicky wrote:
> > 6847 /* IGMP packets have router alert options, allow them */
> > 6848 if (pd->proto == IPPROTO_IGMP) {
> > 6849 /*
> > 6850 * According to RFC 1112 ttl must be set to 1 in
> all IGMP
> > 6851 * packets sent do 224.0.0.1
> > 6852 */
> > 6853 if ((h->ip_ttl != 1) &&
> > 6854 (h->ip_dst.s_addr == INADDR_ALLHOSTS_GROUP)) {
> > 6855 DPFPRINTF(LOG_NOTICE, "Invalid IGMP");
> > 6856 REASON_SET(reason, PFRES_IPOPTIONS);
> > 6857 return (PF_DROP);
> > 6858 }
> > 6859 CLR(pd->badopts, PF_OPT_ROUTER_ALERT);
> >
> > This change should make pf(4) reasonably paranoid while keeping IGMP
> working.
>
> OK bluhm@
>
Re: pf(4) drops valid IGMP/MLD messages
Hi, just another bit of info about this issue.
I've installed from github the newest version of mrouted on a Linux machine.
Just like the built-in OpenBSD's version of mrouted, this github version
sends DVMRP Prune messages
with IP Destination Address = Unicast Address of the adjacent router, and
TTL=255.
Here you can find a tcpdump:
08:49:19.363829 IP (tos 0xc0, ttl 255, id 52275, offset 0, flags [none],
proto IGMP (2), length 44, options (RA))
10.0.12.101 > 10.0.12.1: igmp dvmrp Prune src 10.254.2.0 grp
239.255.255.42 timer 1h47m2s
Best regards,
Luca
Il giorno mar 28 feb 2023 alle ore 15:39 Alexandr Nedvedicky <
sas...@fastmail.net> ha scritto:
> Hello Matthieu,
>
>
>
> On Tue, Feb 28, 2023 at 02:05:50PM +0100, Matthieu Herrb wrote:
> > > --- a/sys/net/pf.c
> > > +++ b/sys/net/pf.c
> > > @@ -6846,8 +6846,12 @@ pf_walk_header(struct pf_pdesc *pd, struct ip
> *h, u_short *reason)
> > > pd->proto = h->ip_p;
> > > /* IGMP packets have router alert options, allow them */
> > > if (pd->proto == IPPROTO_IGMP) {
> > > - /* According to RFC 1112 ttl must be set to 1. */
> > > - if ((h->ip_ttl != 1) || !IN_MULTICAST(h->ip_dst.s_addr)) {
> > > + /*
> > > +* According to RFC 1112 ttl must be set to 1 in all IGMP
> > > +* packets sent do 224.0.0.1
> > > +*/
> > > + if ((h->ip_ttl != 1) &&
> > > + (h->ip_dst.s_addr == INADDR_ALLHOSTS_GROUP)) {
> > > DPFPRINTF(LOG_NOTICE, "Invalid IGMP");
> > > REASON_SET(reason, PFRES_IPOPTIONS);
> > > return (PF_DROP);
> > >
> >
> >
> > Hi,
> >
> > The expression look wrong to me again.
> > I read in RFC1112 that correct packets should have:
> >
> > (h->ip_ttl == 1 && h->ip.ip_dst.s_addr == INADDR_ALLHOST_GROUP)
>
> the expression above is true for for Query/Report messages
> which are a sub-set of IGMP protocol. See Luca's emails with
> references to RFCs which further extend IGMP protocol.
>
> here in this particular place (pf_walk_header()) I think we really
> want to simply discard all IGMP datagrams sent to 224.0.0.1 with TTL
> different to 1. anything else should be passed further up and let
> pf(4)
> rules to make a decision on those cases.
>
> thanks and
> regards
> sashan
>
Re: Disabling MULTICAST flag on an interface. Force outgoing multicast traffic to a specific interface
Hi Philip, thanks 1) ok, I’ll take it into account for my configurations 2) adding the route for 224.0.0.0/4 to the specific interface works Thanks again Luca Il giorno mer 1 mar 2023 alle 22:47 Philip Guenther ha scritto: > On Wed, Mar 1, 2023 at 9:58 AM Luca Di Gregorio wrote: > >> 1) does anyone know if there is a way to disable MULTICAST on a single >> interface? >> I don't see any option in ifconfig to do this. >> > > There is not. > > >> 2) Can outgoing multicast traffic be routed to a specific interface? I >> don't see any option in route. It seems that the first interface, by id, >> is >> chosen. >> > > Unless you enable a multicast routing daemon, it just follows normal IP > routing rules and will follow the default route if there's no better > match. If you want to force it to a specific interface, just add a route > for 224.0.0.0/4 pointing to that interface. (Of course, if you haven't > set multicast=YES in /etc/rc.conf.local then /etc/netstart will create one > of those routes itself with the 'reject' flag set to block all multicast, > but presumably you've already set that correctly.) > > > Philip Guenther > > >
Disabling MULTICAST flag on an interface. Force outgoing multicast traffic to a specific interface
Hi, 1) does anyone know if there is a way to disable MULTICAST on a single interface? I don't see any option in ifconfig to do this. 2) Can outgoing multicast traffic be routed to a specific interface? I don't see any option in route. It seems that the first interface, by id, is chosen. Thanks, regards
Re: pf(4) drops valid IGMP/MLD messages
Hi Matthieu,
DVMRP messages are sent with IGMP protocol. Some Multicast Control messages
(Query, Report) have an IP Destination Address belonging to 224.0.0.0/4,
with TTL=1.
Other DVMRP multicast control messages (Prune, Graft, Graft-Ack) have an IP
Destination Address = Unicast Address of the adjacent multicast router,
with TTL >1.
After revision 1.1128, PF drops IGMP messages with TTL != 1 or IP
Destination Address ! IN_MULTICAST.
I think that your proposed test to drop invalid IGMP is too restrictive.
In fact, it would block not only Prune, Graft, Graft-Ack messages (the IP
Destination Address is Unicast and TTL > 1), but also:
- DVMRP Probe and Report (TTL = 1 but IP Destination Address = 224.0.0.4 -
DVMRP Routers)
- Multicast Control messages sent to 224.0.0.2 (All Routers on this Subnet)
- Multicast Control messages sent to 224.0.0.22 (IGMP)
For reference:
https://www.iana.org/assignments/multicast-addresses/multicast-addresses.xhtml
Regards
Luca
Il giorno mar 28 feb 2023 alle ore 14:05 Matthieu Herrb
ha scritto:
> On Mon, Feb 27, 2023 at 12:04:54AM +0100, Alexandr Nedvedicky wrote:
> > Hello,
> >
> >
> > > >
> 8<---8<---8<--8<
> > > > diff --git a/sys/net/pf.c b/sys/net/pf.c
> > > > index 8cb1326a160..c328109026c 100644
> > > > --- a/sys/net/pf.c
> > > > +++ b/sys/net/pf.c
> > > > @@ -6847,7 +6847,7 @@ pf_walk_header(struct pf_pdesc *pd, struct ip
> *h, u_short *reason)
> > > > /* IGMP packets have router alert options, allow them */
> > > > if (pd->proto == IPPROTO_IGMP) {
> > > > /* According to RFC 1112 ttl must be set to 1. */
> > > > - if ((h->ip_ttl != 1) || !IN_MULTICAST(h->ip_dst.s_addr)) {
> > > > + if ((h->ip_ttl != 1) && IN_MULTICAST(h->ip_dst.s_addr)) {
> > > > DPFPRINTF(LOG_NOTICE, "Invalid IGMP");
> > > > REASON_SET(reason, PFRES_IPOPTIONS);
> > > > return (PF_DROP);
> > > > @@ -7101,8 +7101,8 @@ pf_walk_header6(struct pf_pdesc *pd, struct
> ip6_hdr *h, u_short *reason)
> > > >* missing then MLD message is invalid and
> > > >* should be discarded.
> > > >*/
> > > > - if ((h->ip6_hlim != 1) ||
> > > > - !IN6_IS_ADDR_LINKLOCAL(&h->ip6_src)) {
> > > > + if ((h->ip6_hlim != 1) &&
> > > > + IN6_IS_ADDR_LINKLOCAL(&h->ip6_src)) {
> > > > DPFPRINTF(LOG_NOTICE, "Invalid
> MLD");
> > > > REASON_SET(reason,
> PFRES_IPOPTIONS);
> > > > return (PF_DROP);
> > > >
> > >
> > > Unless I'm missing more context, this hunk looks wrong to me. Valid
> > > MLD packets must have a ttl of 1 *and* come from a LL address. The
> > > initial logic seems ok to me.
> > >
> >
> > yes you are right. Your comment made me to take better look
> > at RFC 1112 [1]. Section 'Informal Protocol Description'
> > reads as follows:
> >
> >Multicast routers send Host Membership Query messages (hereinafter
> >called Queries) to discover which host groups have members on
> their
> >attached local networks. Queries are addressed to the all-hosts
> >group (address 224.0.0.1), and carry an IP time-to-live of 1.
> >
> > I think I've confused all-hosts group (224.0.0.1) with any multicast
> > address (any class-D 224.0.0.0). I think the diff below is what we
> > actually need to get IPv4 IGMP working again:
> >
> > [1] https://www.ietf.org/rfc/rfc1112.txt
> >
> > 8<---8<---8<--8<
> > diff --git a/sys/net/pf.c b/sys/net/pf.c
> > index 8cb1326a160..c50173186da 100644
> > --- a/sys/net/pf.c
> > +++ b/sys/net/pf.c
> > @@ -6846,8 +6846,12 @@ pf_walk_header(struct pf_pdesc *pd, struct ip *h,
> u_short *reason)
> > pd->proto = h->ip_p;
> > /* IGMP packets have router alert options, allow them */
> > if (pd->proto == IPPROTO_IGMP) {
> > - /* According to RFC 1112 ttl must be set to 1. */
> > - if ((h->ip_ttl != 1) || !IN_MULTICAST(h->ip_dst.s_addr)) {
> > + /*
> > + * According to RFC 1112 ttl must be set to 1 in all IGMP
> > + * packets sent do 224.0.0.1
> > + */
> > + if ((h->ip_ttl != 1) &&
> > + (h->ip_dst.s_addr == INADDR_ALLHOSTS_GROUP)) {
> > DPFPRINTF(LOG_NOTICE, "Invalid IGMP");
> > REASON_SET(reason, PFRES_IPOPTIONS);
> > return (PF_DROP);
> >
>
>
> Hi,
>
> The expression look wrong to me again.
> I read in RFC1112 that correct packets should have:
>
> (h->ip_ttl == 1 && h->ip.ip_dst.s_addr == INADDR_ALLHOST_GROUP)
>
> so the test to drop invalid IGMP should be:
>
>if (h->ip_ttl != 1 || h->ip.ip
