ufw

UFW drops sane multicast traffic

Bug #720605 reported by Romain Boissat
28
This bug affects 6 people
Affects Status Importance Assigned to Milestone
ufw
Fix Released
Low
Jamie Strandboge
ufw (Ubuntu)
Fix Released
Low
Jamie Strandboge
Natty
Fix Released
Low
Jamie Strandboge

Bug Description

UFW silently drops ICMPv6 traffic from multicast echo request.

------------------------------------------------------------------------------------------

$ ufw --version
ufw 0.30.0-1ubuntu2
Copyright 2008-2010 Canonical Ltd.

------------------------------------------------------------------------------------------

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 10.10
Release: 10.10
Codename: maverick

------------------------------------------------------------------------------------------
Facts:

$ ping6 ff02::1%eth0
PING ff02::1%eth0(ff02::1) 56 data bytes
64 bytes from fe80::219:21ff:fe28:91eb: icmp_seq=1 ttl=64 time=0.112 ms

fe80::219:21ff:fe28:91eb is my eth0's link-local address and no other link-local addresses appear in ping6 output,
while wireshark and tcpdump show me a lot of them. With ufw off, ping6 reports all other link-local addresses.

Plus, Neighbor discovery protocol is working since ip -6 neigh reports several link-local addresses.

Thus ICMPv6 Echo Reply messages are dropped by ufw. I found a way to fix this behavior by adding a rule and changing
their order in /etc/ufw/before6.rules.

Here is the diff -u:

--- before6.rules 2011-02-17 10:38:09.106701972 +0100
+++ before6.rules.FIXED 2011-02-17 10:26:19.658701976 +0100
@@ -27,6 +27,14 @@
 -A ufw6-before-input -p icmpv6 --icmpv6-type router-solicitation -m hl --hl-eq 255 -j ACCEPT
 -A ufw6-before-input -p icmpv6 --icmpv6-type router-advertisement -m hl --hl-eq 255 -j ACCEPT

+# ok icmp codes
+-A ufw6-before-input -p icmpv6 --icmpv6-type destination-unreachable -j ACCEPT
+-A ufw6-before-input -p icmpv6 --icmpv6-type packet-too-big -j ACCEPT
+-A ufw6-before-input -p icmpv6 --icmpv6-type time-exceeded -j ACCEPT
+-A ufw6-before-input -p icmpv6 --icmpv6-type parameter-problem -j ACCEPT
+-A ufw6-before-input -p icmpv6 --icmpv6-type echo-request -j ACCEPT
+-A ufw6-before-input -p icmpv6 --icmpv6-type echo-reply -j ACCEPT
+
 # quickly process packets for which we already have a connection
 -A ufw6-before-input -m state --state RELATED,ESTABLISHED -j ACCEPT
 -A ufw6-before-output -m state --state RELATED,ESTABLISHED -j ACCEPT
@@ -35,13 +43,6 @@
 -A ufw6-before-input -m state --state INVALID -j ufw6-logging-deny
 -A ufw6-before-input -m state --state INVALID -j DROP

-# ok icmp codes
--A ufw6-before-input -p icmpv6 --icmpv6-type destination-unreachable -j ACCEPT
--A ufw6-before-input -p icmpv6 --icmpv6-type packet-too-big -j ACCEPT
--A ufw6-before-input -p icmpv6 --icmpv6-type time-exceeded -j ACCEPT
--A ufw6-before-input -p icmpv6 --icmpv6-type parameter-problem -j ACCEPT
--A ufw6-before-input -p icmpv6 --icmpv6-type echo-request -j ACCEPT
-
 # allow dhcp client to work
 -A ufw6-before-input -p udp --sport 67 --dport 68 -j ACCEPT

------------------------------------------------------------------------------------------
Other Remarks:

I believe that ff00::1/8 or ff00::2/8 means nothing but ff00::/8, hence duplicate rules...
Indeed:

$ ip6tables -L -n
(....)
Chain ufw6-before-input (1 references)
(...)
ACCEPT icmpv6 ff00::/8 ::/0
ACCEPT icmpv6 ::/0 ff00::/8
ACCEPT icmpv6 ff00::/8 ::/0
ACCEPT icmpv6 ::/0 ff00::/8
(...)

Thus:

 # allow MULTICAST
--A ufw6-before-input -p icmpv6 -s ff00::1/8 -j ACCEPT
--A ufw6-before-input -p icmpv6 -d ff00::1/8 -j ACCEPT
--A ufw6-before-input -p icmpv6 -s ff00::2/8 -j ACCEPT
--A ufw6-before-input -p icmpv6 -d ff00::2/8 -j ACCEPT
+-A ufw6-before-input -p icmpv6 -s ff00::/8 -j ACCEPT
+-A ufw6-before-input -p icmpv6 -d ff00::/8 -j ACCEPT

would be enough I guess.

Thanks for your attention.

Related branches

Revision history for this message
Romain Boissat (rboissat) wrote :

No news about this one? Jeez, it's important though!

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thank you for using ufw and reporting a bug. First off, you are right about ff00::1/8 and ff00::2/8. Not sure what happened there... clearly a think-o and I have committed that change to trunk.

As for the dropping sane multicast traffic part, I can confirm the behavior, but I am still thinking about it. What is happening is that the reply goes out, but the kernel is marking the reply as INVALID. Eg (this requires a patch from trunk to clearly log the packet as INVALID):
$ sudo ufw medium
$ ping6 -c 1 ff02::1%eth0
Mar 21 16:58:51 foo kernel: [4439947.485144] [UFW ALLOW] IN= OUT=eth0 SRC=fe80:0000:0000:0000:0200:1122:3344:5566 DST=ff02:0000:0000:0000:0000:0000:0000:0001 LEN=104 TC=0 HOPLIMIT=64 FLOWLBL=0 PROTO=ICMPv6 TYPE=128 CODE=0 ID=1065 SEQ=1
Mar 21 16:58:51 foo kernel: [4439947.485588] [UFW AUDIT] IN=eth0 OUT= MAC= SRC=fe80:0000:0000:0000:0200:1122:3344:5566 DST=ff02:0000:0000:0000:0000:0000:0000:0001 LEN=104 TC=0 HOPLIMIT=64 FLOWLBL=0 PROTO=ICMPv6 TYPE=128 CODE=0 ID=1065 SEQ=1
Mar 21 16:58:51 foo kernel: [4439947.487214] [UFW BLOCK INVALID] IN=eth0 OUT= MAC=... SRC=fe80:0000:0000:0000:0200:1122:3344:5577 DST=fe80:0000:0000:0000:0200:1122:3344:5566 LEN=104 TC=0 HOPLIMIT=64 FLOWLBL=0 PROTO=ICMPv6 TYPE=129 CODE=0 ID=1065 SEQ=1
Mar 21 16:58:51 foo kernel: [4439947.487609] [UFW BLOCK] IN=eth0 OUT= MAC=... SRC=fe80:0000:0000:0000:0200:1122:3344:5577 DST=fe80:0000:0000:0000:0200:1122:3344:5566 LEN=104 TC=0 HOPLIMIT=64 FLOWLBL=0 PROTO=ICMPv6 TYPE=129 CODE=0 ID=1065 SEQ=1
Mar 21 16:58:51 foo kernel: [4439947.488253] [UFW BLOCK INVALID] IN=eth0 OUT= MAC=... SRC=fe80:0000:0000:0000:0200:1122:3344:5588 DST=fe80:0000:0000:0000:0200:1122:3344:5566 LEN=104 TC=0 HOPLIMIT=64 FLOWLBL=0 PROTO=ICMPv6 TYPE=129 CODE=0 ID=1065 SEQ=1
Mar 21 16:58:51 foo kernel: [4439947.488644] [UFW BLOCK] IN=eth0 OUT= MAC=... SRC=fe80:0000:0000:0000:0200:1122:3344:5588 DST=fe80:0000:0000:0000:0200:1122:3344:5566 LEN=104 TC=0 HOPLIMIT=64 FLOWLBL=0 PROTO=ICMPv6 TYPE=129 CODE=0 ID=1065 SEQ=1

Notice the two PROTO=ICMPv6 TYPE=129 (echo reply) packets are blocked. I will probably fix this with what you suggested, but want to think about it some more.

Changed in ufw:
status: New → Confirmed
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Hmm, it wasn't a think-o. I changed the link-local based on bug #304216. I will have to think about that one too! :)

Revision history for this message
Romain Boissat (rboissat) wrote :

Jamie Strandboge: Thanks for the follow-up, if you need testers, please do not hesitate as I have several IPv6 enabled hosts available for testing.

May it pass as a security update in 11.04? Or will we wait for 11.10?

Best regards.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

This will be in 11.04 for sure. Thanks for offering to test.

My initial thoughts are that we allow MULTICAST by default with ipv4 (224.0.0.0, so why not do the same with ipv6 for consistency? I need to read through bug #304216 again as well and other multicast documentation to make sure the change I make this time is a good default.

Revision history for this message
Romain Boissat (rboissat) wrote :

A good start would be: http://tools.ietf.org/html/rfc4291#section-2.7 and http://tools.ietf.org/html/rfc2375.

Take a look at header structure, with the first eight bits shifted to 1, but the eight bits afterwards depend on the type and scope of the IPv6 Multicast address. Thus, "ff00::/16" won't suffice to cover all corner cases of IPv6 multicast.

It would be useful to check if there are any reasons for filtering out some subsets of the prefix "ff::/8" due to some security isssues or whatever, but according to RFC's, this prefix is legit and should be not dropped.

Oh, and my diff is not optimal, as "ff00::/8" should be written "ff::/8".

Thanks for your attention.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Right, and after thinking about all this, I do not want to blindly allow all multicast. I only want to allow mDNS multicast (for service discovery), which was always the intent of these lines. Therefore to fix for mDNS only, before.rules will get:
-# allow MULTICAST, be sure the MULTICAST line above is uncommented
--A ufw-before-input -s 224.0.0.0/4 -j ACCEPT
--A ufw-before-input -d 224.0.0.0/4 -j ACCEPT
+# allow MULTICAST mDNS for service discovery (be sure the MULTICAST line above
+# is uncommented)
+-A ufw-before-input -p udp -d 224.0.0.251 --dport 5353 -j ACCEPT

and before6.rules will get:
-# allow MULTICAST
--A ufw6-before-input -p icmpv6 -s ff00::/8 -j ACCEPT
--A ufw6-before-input -p icmpv6 -d ff00::/8 -j ACCEPT
+# allow MULTICAST mDNS for service discovery (be sure the MULTICAST line above
+# is uncommented)
+-A ufw6-before-input -p udp -d ff02::fb --dport 5353 -j ACCEPT

Then to fix this bug, I will add this to before6.rules, just before the INVALID rules:
+# for multicast ping replies from link-local addresses (these don't have an
+# associated connection and would otherwise be marked INVALID)
+-A ufw6-before-input -p icmpv6 --icmpv6-type echo-reply -s fe80::/10 -j ACCEPT

Revision history for this message
Romain Boissat (rboissat) wrote :

Jamie: Yes, I just tested your diff applied against default before6.rules and it works, fixing the misbehavior I initially reported.

However, I find "-p icmpv6 --icmpv6-type echo-reply" too restrictive, as multicast traffic may right now and certainly will later imply other protocols than ICMPv6 (and especially other than EchoRequest and EchoReply messages). I don't have any particular application in mind right now, but I feel confident that multicast won't be only used to play with ping6 ;)

However, we may keep it restrictive right now and open it up later, according to new needs and new bug reports.

What do you think about this?

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thank you for testing! What I am trying to achieve here is a balance between usability and protection, erring on the side of protection with an eye towards consistency. If someone enables the firewall, they expect a certain level of protection.
 * We allow pings in general, so I think allowing the multicast ping reply is ok here.
 * We allow service discoverability via mDNS, but not actually the service being protected. On the one hand, this causes a usability issue because the user can see the service, but can't connect to it, but on the other hand, the point of a default deny firewall (ie, how ufw is configured by default once enabled) is to block. Allowing mDNS provides enough information for an administrator (who has opted into the firewall protection in the first place) via the logs, to simply allow the port that mDNS is advertising (eg for DAAP music sharing)
 * We don't want to allow all multicast traffic in general, as mDNS is really the only one that is known to be useful for bastion hosts (eg file or music servers), desktops, laptops and the like. People who need the other can add them, as those are specialized environments anyway. I filed bug #740256 for restricting multicast more.

That said, I would like to err on the side of caution and be more restrictive, but am open to allowing other IPV6 multicast traffic if there is a common need for it (like you suggested).

Revision history for this message
Romain Boissat (rboissat) wrote :

Jamie: I am fine and pleased with your answer: protection and effectiveness first, but keeping your mind open about changes and different opinions. I think we could close that one and call it a victory!

And again, thanks a lot for your responsiveness once the bug was acknowledged. Clearly the most efficient bug resolution I've seen in four years.

You made my day :]

Revision history for this message
Romain Boissat (rboissat) wrote :

One question though: May the fix be backported via a security update to Maverick/Lucid or do you need extensive testing first?

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thanks for the kind words. :) I do hope to backport these to Lucid and Maverick, but only after it has been in natty for a little while.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

This was fixed in ufw 0.30.1.

Changed in ufw:
assignee: nobody → Jamie Strandboge (jdstrand)
importance: Undecided → Low
status: Confirmed → Fix Released
Changed in ufw (Ubuntu):
status: New → In Progress
importance: Undecided → Low
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in ufw (Ubuntu):
status: In Progress → Fix Committed
Changed in ufw (Ubuntu Lucid):
status: New → Triaged
importance: Undecided → Low
Changed in ufw (Ubuntu Maverick):
status: New → Triaged
importance: Undecided → Low
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ufw - 0.30.1-1ubuntu1

---------------
ufw (0.30.1-1ubuntu1) natty; urgency=low

  * Merge from Debian unstable. Remaining changes:
    - debian/rules: Don't install the upstream application profiles that are
      shipped with the Debian package.
    - debian/control: use ufw-0.30-natty for Vcs-Bzr

ufw (0.30.1-1) unstable; urgency=low

  * New upstream release which fixes the following:
    - LP: #501140
    - LP: #740249
    - LP: #740256
    - LP: #720605
  * debian/ufw.logrotate: remove upstartism thanks to Michael Biebl
    (Closes: 607696)
  * debian/sysctl.conf: merge in upstream (commented out) changes surrounding
    ipv6 forwarding and privacy addresses
  * debian/before*.rules.md5sum: updated for recent changes
 -- Jamie Strandboge <email address hidden> Tue, 22 Mar 2011 12:18:42 -0500

Changed in ufw (Ubuntu Natty):
status: Fix Committed → Fix Released
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Bib, because you are using a default deny for outgoing connections, you will need to add multicast rules to your outgoing policy. While it could be argued that this should be explicitly allowed in the default rules, I would argue that since ufw by default will runs with outgoing connections allowed, when someone actively switches that to denied they intend to do more or less full egress filtering and may intentionally want to disallow this traffic. Thanks for your report, but in the future you will want to open a new bug instead of an existing one.

Revision history for this message
Bib (bybeu) wrote :

OK Jamie
I don't say it would be a defaut rule. I just say I surely misunderstood the current default rule, thinking it is an outcoming rule because of the -d 224.... rule, when it is surely an incoming rule...strange to me, I will try to find help on forums to see how I did te mistake and help to understand the chains in before.rules (I printed a iptables -L -n -v --line-numbers to try to understand all that, which revealed I understand...nothing) . I saw you are the author of the ufw-framework man. Congratulations. Sorry for the noise.

no longer affects: ufw (Ubuntu Lucid)
no longer affects: ufw (Ubuntu Maverick)
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.