Date: Thu, 21 Mar 2019 14:17:10 +0000 (UTC) From: Kristof Provost <kp@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r345377 - in stable/12: sys/netpfil/pf tests/sys/netpfil/pf Message-ID: <201903211417.x2LEHAs4085922@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kp Date: Thu Mar 21 14:17:10 2019 New Revision: 345377 URL: https://svnweb.freebsd.org/changeset/base/345377 Log: MFC r345366: pf: Ensure that IP addresses match in ICMP error packets States in pf(4) let ICMP and ICMP6 packets pass if they have a packet in their payload that matches an exiting connection. It was not checked whether the outer ICMP packet has the same destination IP as the source IP of the inner protocol packet. Enforce that these addresses match, to prevent ICMP packets that do not make sense. Reported by: Nicolas Collignon, Corentin Bayet, Eloi Vanderbeken, Luca Moro at Synacktiv Obtained from: OpenBSD Security: CVE-2019-5598 Modified: stable/12/sys/netpfil/pf/pf.c stable/12/tests/sys/netpfil/pf/pft_ping.py Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/netpfil/pf/pf.c ============================================================================== --- stable/12/sys/netpfil/pf/pf.c Thu Mar 21 13:30:48 2019 (r345376) +++ stable/12/sys/netpfil/pf/pf.c Thu Mar 21 14:17:10 2019 (r345377) @@ -4596,7 +4596,7 @@ pf_test_state_icmp(struct pf_state **state, int direct { struct pf_addr *saddr = pd->src, *daddr = pd->dst; u_int16_t icmpid = 0, *icmpsum; - u_int8_t icmptype; + u_int8_t icmptype, icmpcode; int state_icmp = 0; struct pf_state_key_cmp key; @@ -4605,6 +4605,7 @@ pf_test_state_icmp(struct pf_state **state, int direct #ifdef INET case IPPROTO_ICMP: icmptype = pd->hdr.icmp->icmp_type; + icmpcode = pd->hdr.icmp->icmp_code; icmpid = pd->hdr.icmp->icmp_id; icmpsum = &pd->hdr.icmp->icmp_cksum; @@ -4619,6 +4620,7 @@ pf_test_state_icmp(struct pf_state **state, int direct #ifdef INET6 case IPPROTO_ICMPV6: icmptype = pd->hdr.icmp6->icmp6_type; + icmpcode = pd->hdr.icmp6->icmp6_code; icmpid = pd->hdr.icmp6->icmp6_id; icmpsum = &pd->hdr.icmp6->icmp6_cksum; @@ -4817,6 +4819,23 @@ pf_test_state_icmp(struct pf_state **state, int direct #endif /* INET6 */ } + if (PF_ANEQ(pd->dst, pd2.src, pd->af)) { + if (V_pf_status.debug >= PF_DEBUG_MISC) { + printf("pf: BAD ICMP %d:%d outer dst: ", + icmptype, icmpcode); + pf_print_host(pd->src, 0, pd->af); + printf(" -> "); + pf_print_host(pd->dst, 0, pd->af); + printf(" inner src: "); + pf_print_host(pd2.src, 0, pd2.af); + printf(" -> "); + pf_print_host(pd2.dst, 0, pd2.af); + printf("\n"); + } + REASON_SET(reason, PFRES_BADSTATE); + return (PF_DROP); + } + switch (pd2.proto) { case IPPROTO_TCP: { struct tcphdr th; @@ -4873,7 +4892,7 @@ pf_test_state_icmp(struct pf_state **state, int direct !SEQ_GEQ(seq, src->seqlo - (dst->max_win << dws)))) { if (V_pf_status.debug >= PF_DEBUG_MISC) { printf("pf: BAD ICMP %d:%d ", - icmptype, pd->hdr.icmp->icmp_code); + icmptype, icmpcode); pf_print_host(pd->src, 0, pd->af); printf(" -> "); pf_print_host(pd->dst, 0, pd->af); @@ -4886,7 +4905,7 @@ pf_test_state_icmp(struct pf_state **state, int direct } else { if (V_pf_status.debug >= PF_DEBUG_MISC) { printf("pf: OK ICMP %d:%d ", - icmptype, pd->hdr.icmp->icmp_code); + icmptype, icmpcode); pf_print_host(pd->src, 0, pd->af); printf(" -> "); pf_print_host(pd->dst, 0, pd->af); Modified: stable/12/tests/sys/netpfil/pf/pft_ping.py ============================================================================== --- stable/12/tests/sys/netpfil/pf/pft_ping.py Thu Mar 21 13:30:48 2019 (r345376) +++ stable/12/tests/sys/netpfil/pf/pft_ping.py Thu Mar 21 14:17:10 2019 (r345377) @@ -8,26 +8,38 @@ import threading PAYLOAD_MAGIC = 0x42c0ffee class Sniffer(threading.Thread): - def __init__(self, recvif): + def __init__(self, args, check_function): threading.Thread.__init__(self) - self._recvif = recvif + self._args = args + self._recvif = args.recvif[0] + self._check_function = check_function + self.foundCorrectPacket = False self.start() + def _checkPacket(self, packet): + ret = self._check_function(self._args, packet) + if ret: + self.foundCorrectPacket = True + return ret + def run(self): - self.packets = sp.sniff(iface=self._recvif, timeout=3) + self.packets = sp.sniff(iface=self._recvif, + stop_filter=self._checkPacket, timeout=3) -def check_ping_request(packet, dst_ip, args): +def check_ping_request(args, packet): if args.ip6: - return check_ping6_request(packet, dst_ip, args) + return check_ping6_request(args, packet) else: - return check_ping4_request(packet, dst_ip, args) + return check_ping4_request(args, packet) -def check_ping4_request(packet, dst_ip, args): +def check_ping4_request(args, packet): """ Verify that the packet matches what we'd have sent """ + dst_ip = args.to[0] + ip = packet.getlayer(sp.IP) if not ip: return False @@ -54,13 +66,14 @@ def check_ping4_request(packet, dst_ip, args): % (ip.tos, args.expect_tos[0]) return False - return True -def check_ping6_request(packet, dst_ip, args): +def check_ping6_request(args, packet): """ Verify that the packet matches what we'd have sent """ + dst_ip = args.to[0] + ip = packet.getlayer(sp.IPv6) if not ip: return False @@ -124,7 +137,7 @@ def main(): sniffer = None if not args.recvif is None: - sniffer = Sniffer(args.recvif[0]) + sniffer = Sniffer(args, check_ping_request) if args.ip6: ping6(args.sendif[0], args.to[0], args) @@ -134,12 +147,10 @@ def main(): if sniffer: sniffer.join() - for packet in sniffer.packets: - if check_ping_request(packet, args.to[0], args): - sys.exit(0) - - # We did not get the packet we expected - sys.exit(1) + if sniffer.foundCorrectPacket: + sys.exit(0) + else: + sys.exit(1) if __name__ == '__main__': main()
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201903211417.x2LEHAs4085922>