From owner-svn-src-stable-12@freebsd.org Thu Mar 21 14:17:11 2019 Return-Path: Delivered-To: svn-src-stable-12@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8C5631541CE2; Thu, 21 Mar 2019 14:17:11 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 2972F8D3D5; Thu, 21 Mar 2019 14:17:11 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 145209EB7; Thu, 21 Mar 2019 14:17:11 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x2LEHA2G085924; Thu, 21 Mar 2019 14:17:10 GMT (envelope-from kp@FreeBSD.org) Received: (from kp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x2LEHAs4085922; Thu, 21 Mar 2019 14:17:10 GMT (envelope-from kp@FreeBSD.org) Message-Id: <201903211417.x2LEHAs4085922@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kp set sender to kp@FreeBSD.org using -f From: Kristof Provost Date: Thu, 21 Mar 2019 14:17:10 +0000 (UTC) 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 X-SVN-Group: stable-12 X-SVN-Commit-Author: kp X-SVN-Commit-Paths: in stable/12: sys/netpfil/pf tests/sys/netpfil/pf X-SVN-Commit-Revision: 345377 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 2972F8D3D5 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.97 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.998,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.98)[-0.977,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-stable-12@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for only the 12-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Mar 2019 14:17:11 -0000 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()