Created attachment 188720 [details] Drop malicious packets as required by RFC 6980 RFC6980 says: ---8<--- Nodes MUST silently ignore the following Neighbor Discovery and SEcure Neighbor Discovery messages if the packets carrying them include an IPv6 Fragmentation Header: o Neighbor Solicitation o Neighbor Advertisement o Router Solicitation o Router Advertisement o Redirect o Certification Path Solicitation Nodes SHOULD normally process the following messages when the packets carrying them include an IPv6 Fragmentation Header: o Certification Path Advertisement SEND nodes SHOULD NOT employ keys that would result in fragmented CPA messages. ---8<--- A recent talk about this RFC showed, that FreeBSD fails in all respects to fulfill the minimal protection: http://www.denog.de/de/meetings/denog9/agenda.html#rfc6980 I hope, that the attached patch does fix the issue: - Fragment handling becomes a protocol specific mbuf-flag. - At detailed protocol level silent drop is implemented. - Certification Path messages are unsupported, so no sysctl to deal with the SHOULD is needed.
More details from the original work: https://insinuator.net/2017/06/testing-rfc-6980-implementations-of-freebsd/
pf also does reassemble of IPv6 packets (see pf_reassemble6()), so that would also need a 'm->m_flags |= M_REASSEMBLED;'.
Created attachment 188749 [details] Drop malicious packets as required by RFC 6980
Thank your for the pointer to functionality duplication in other modules. I moved the flag-setting to the fragment header removal code (which is more appropriate anyway). This code is more likely to be shared by other modules (in the case of pf, it is used). Furthermore the flag is renamed to be more self explanatory and the code was brushed up to match the existing coding style. Did I miss anything else?
(In reply to lutz from comment #4) > Thank your for the pointer to functionality duplication in other modules. > > I moved the flag-setting to the fragment header removal code (which is more > appropriate anyway). This code is more likely to be shared by other modules > (in the case of pf, it is used). > > Furthermore the flag is renamed to be more self explanatory and the code was > brushed up to match the existing coding style. > > Did I miss anything else? This looks good to me.
A commit references this bug: Author: ae Date: Fri Dec 15 12:37:32 UTC 2017 New revision: 326876 URL: https://svnweb.freebsd.org/changeset/base/326876 Log: Follow the RFC6980 and silently ignore following IPv6 NDP messages that had the IPv6 fragmentation header: o Neighbor Solicitation o Neighbor Advertisement o Router Solicitation o Router Advertisement o Redirect Introduce M_FRAGMENTED mbuf flag, and set it after IPv6 fragment reassembly is completed. Then check the presence of this flag in correspondig ND6 handling routines. PR: 224247 MFC after: 2 weeks Changes: head/sys/netinet6/frag6.c head/sys/netinet6/icmp6.c head/sys/netinet6/in6.h head/sys/netinet6/nd6_nbr.c head/sys/netinet6/nd6_rtr.c
A commit references this bug: Author: ae Date: Fri Dec 29 10:47:25 UTC 2017 New revision: 327337 URL: https://svnweb.freebsd.org/changeset/base/327337 Log: MFC r326876: Follow the RFC6980 and silently ignore following IPv6 NDP messages that had the IPv6 fragmentation header: o Neighbor Solicitation o Neighbor Advertisement o Router Solicitation o Router Advertisement o Redirect Introduce M_FRAGMENTED mbuf flag, and set it after IPv6 fragment reassembly is completed. Then check the presence of this flag in correspondig ND6 handling routines. PR: 224247 Changes: _U stable/11/ stable/11/sys/netinet6/frag6.c stable/11/sys/netinet6/icmp6.c stable/11/sys/netinet6/in6.h stable/11/sys/netinet6/nd6_nbr.c stable/11/sys/netinet6/nd6_rtr.c
Should the state of this ticket modified to "assigned", "patched", or even "fixed/closed"?
oops, looks like there are still issues. see pages 31-33 of jacky hammer's preso: https://ripe76.ripe.net/wp-content/uploads/presentations/67-RIPE76_JHammer_RFC6980.pdf
I just relayed a question to Jacky in order to obtain solid data. From the referenced presentation itself it's hard to tell, if they tested against 11.1-RELEASE (with/out patch) or 11-STABLE.
Jacky provided additional information (at lot of ...) They tested again using "11.1-RELEASE #0 r321309". First question is, if this version did contain the patch or not.
r321309 was 11.1-RELEASE, dated 2017-07-20. The fix was commit r327337, dated 2017-12-29. Looks like she may have tested the wrong version :-(
Waiting for rechecks from Jacky. Changed state according to best knowledge so far.