CVS commit: [netbsd-8] src/sys/net/npf
Module Name:src Committed By: martin Date: Tue Jul 10 14:44:05 UTC 2018 Modified Files: src/sys/net/npf [netbsd-8]: npf_handler.c Log Message: Pull up following revision(s) (requested by maxv in ticket #919): sys/net/npf/npf_handler.c: revision 1.41 Update the pointer when fast-kicking, because it may have been freed. Before my changes the nonsensical pointer ininitialization held, but when I started introducing sanity checks the whole thing collapsed. Need pullup-8. To generate a diff of this commit: cvs rdiff -u -r1.37.6.1 -r1.37.6.2 src/sys/net/npf/npf_handler.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: [netbsd-8] src/sys/net/npf
Module Name:src Committed By: martin Date: Mon May 14 19:22:30 UTC 2018 Modified Files: src/sys/net/npf [netbsd-8]: npf_alg_icmp.c npf_inet.c npf_sendpkt.c Log Message: Pull up following revision(s) (requested by maxv in ticket #823): sys/net/npf/npf_inet.c: revision 1.45-1.47 sys/net/npf/npf_alg_icmp.c: revision 1.27-1.30 sys/net/npf/npf_sendpkt.c: revision 1.19 Fix use-after-free. The nbuf can be reallocated as a result of caching 'enpc', so it is necessary to recache 'npc', otherwise it contains pointers to the freed mbuf - pointers which are then used in the ruleset machinery. We recache 'npc' when we are sure we won't use 'enpc' anymore, because 'enpc' can be clobbered as a result of caching 'npc' (in other words, only one of the two can be cached at the same time). Also, we recache 'npc' unconditionally, because there is no way to know whether the nbuf got clobbered relatively to it. We can't use the NBUF_DATAREF_RESET flag, because it is stored in the nbuf and not in the cache. Discussed with rmind@. Change npf_cache_all so that it ensures the potential ICMP Query Id is in the nbuf. In such a way that we don't need to ensure that later. Change npfa_icmp4_inspect and npfa_icmp6_inspect so that they touch neither the nbuf nor npc. Adapt their callers accordingly. In the end, if a packet has a Query Id, we set NPC_ICMP_ID in npc and leave right away, without recaching npc (not needed since we didn't touch the nbuf). This fixes the handling of Query Id packets (that I broke in my previous commit), and also fixes another possible use-after-free. Retrieve the complete IPv4 header right away, and make sure we did retrieve the IPv6 option header we were iterating on. Ah, fix compilation. I tested my previous change by loading the kernel module from the filesystem, but the Makefile didn't have DIAGNOSTIC enabled, and the two KASSERTs I added did not compile properly. If we fail to advance inside TCP/UDP/ICMPv4/ICMPv6, stop pretending L4 is unknown, and error out right away. This prevents bugs in machinery, if a place looks for L4 in 'npc_proto' without checking the cache too. I've seen a ~similar problem already. In addition to checking L4 in the cache, here we also need to check the protocol. The NPF entry point does not ensure that ICMPv6 can be set only in IPv6 ICMPv4 can be set only in IPv4 So we could have ICMPv6 in IPv4. apply some INET6 so this compiles in INET6-less kernels again. To generate a diff of this commit: cvs rdiff -u -r1.24.8.1 -r1.24.8.2 src/sys/net/npf/npf_alg_icmp.c cvs rdiff -u -r1.37.6.1 -r1.37.6.2 src/sys/net/npf/npf_inet.c cvs rdiff -u -r1.16.8.1 -r1.16.8.2 src/sys/net/npf/npf_sendpkt.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: [netbsd-8] src/sys/net/npf
Module Name:src Committed By: martin Date: Wed May 9 15:35:37 UTC 2018 Modified Files: src/sys/net/npf [netbsd-8]: npf.h npf_alg_icmp.c npf_handler.c npf_inet.c npf_sendpkt.c Log Message: Pull up following revision(s) (requested by maxv in ticket #817): sys/net/npf/npf_inet.c: revision 1.38-1.44 sys/net/npf/npf_handler.c: revision 1.38-1.39 sys/net/npf/npf_alg_icmp.c: revision 1.26 sys/net/npf/npf.h: revision 1.56 sys/net/npf/npf_sendpkt.c: revision 1.17-1.18 Declare NPC_FMTERR, and use it to kick malformed packets. Several sanity checks are added in IPv6; after we see the first IPPROTO_FRAGMENT header, we are allowed to fail to advance, otherwise we kick the packet. Sent on tech-net@ a few days ago, no response, but I'm committing it now anyway. Switch nptr to uint8_t, and use nbuf_ensure_contig. Makes us use fewer magic values. Remove dead branches, 'npc' can't be NULL (and it is dereferenced earlier). Fix two consecutive mistakes. The first mistake was npf_inet.c rev1.37: "Don't reassemble ipv6 fragments, instead treat the first fragment as a regular packet (subject to filtering rules), and pass subsequent fragments in the same group unconditionally." Doing this was entirely wrong, because then a packet just had to push the L4 payload in a secondary fragment, and NPF wouldn't apply rules on it - meaning any IPv6 packet could bypass >=L4 filtering. This mistake was supposed to be a fix for the second mistake. The second mistake was that ip6_reass_packet (in npf_reassembly) was getting called with npc->npc_hlen. But npc_hlen pointed to the last encountered header in the IPv6 chain, which was not necessarily the fragment header. So ip6_reass_packet was given garbage, and would fail, resulting in the packet getting kicked. So basically IPv6 was broken by NPF. The first mistake is reverted, and the second one is fixed by doing: - hlen = sizeof(struct ip6_frag); + hlen = 0; Now the iteration stops on the fragment header, and the call to ip6_reass_packet is valid. My npf_inet.c rev1.38 is partially reverted: we don't need to worry about failing properly to advance; once the packet is reassembled npf_cache_ip gets called again, and this time the whole chain should be there. Tested with a simple UDPv6 server - send a 3000-byte-sized buffer, the packet gets correctly reassembled by NPF now. Mmh, put back the RFC6946 check (about dummy fragments), otherwise NPF is not happy in npf_reassembly, because NPC_IPFRAG is again returned after the packet was reassembled. I'm wondering whether it would not be better to just remove the fragment header in frag6_input directly. Fix the "return-rst" rule on IPv6 packets. The scopes needed to be set on the addresses before invoking ip6_output, because ip6_output needs them. The reason they are not here already is because pfil_run_hooks (in ip6_input) is called _before_ the kernel initializes the scopes. Until now ip6_output was always failing, and the IPv6-TCP-RST packet was never actually sent. Perhaps it would be better to have the kernel initialize the scopes before invoking pfil_run_hooks, but several things will need to be fixed in several places. Tested with a simple TCPv6 server. Until now the client would block waiting for an answer that never came; now it receives an RST right away and closes the connection, as expected. I believe that the same problem exists in the "return-icmp" rules, but I can't investigate this right now (some problems with wireshark). Fix the IPv6 payload computation in npf_tcpsaw. It was incorrect, and this caused the "return-rst" rules to send back an RST with the wrong ACK when the received SYN had an IPv6 option. Set the scopes before calling icmp6_error(). This fixes a bug similar to the one I fixed in rev1.17: since the scopes were not set the packet was never actually sent. Tested with wireshark, now the ICMPv6 reply is correctly sent, as expected. Don't read the L4 payload after IPPROTO_AH when handling IPv6 packets. AH must be considered as the payload, otherwise a block all pass in proto ah from any pass out proto ah from any configuration will actually block everything, because NPF checks the protocol against the one found after AH, and not AH itself. In addition it may have been a problem for stateful connections; an AH packet sent by an attacker with an incorrect authentication and a correct TCP/UDP/whatever payload from an active connection could manage to change NPF's FSM state, which would perhaps have altered the legitimate connection with the authenticated remote IPsec host. Note that IPv4 already doesn't go beyond AH, which is the correct behavior. Add XXX (we don't handle IPv6 Jumbograms), and whitespace. To generate a diff of this commit: cvs rdiff -u -r1.54.6.1 -r1.54.6.2 src/sys/net/npf/npf.h cvs rdiff -u -r1.24 -r1.24.8.1 src/sy
CVS commit: [netbsd-8] src/sys/net/npf
Module Name:src Committed By: martin Date: Sat May 5 19:15:55 UTC 2018 Modified Files: src/sys/net/npf [netbsd-8]: npf_nat.c Log Message: Pull up following revision(s) (requested by prlw1 in ticket #795): sys/net/npf/npf_nat.c: revision 1.42 PR/53207: David Binderman: Use logical and To generate a diff of this commit: cvs rdiff -u -r1.41 -r1.41.8.1 src/sys/net/npf/npf_nat.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: [netbsd-8] src/sys/net/npf
Module Name:src Committed By: martin Date: Wed Apr 4 16:40:42 UTC 2018 Modified Files: src/sys/net/npf [netbsd-8]: npf.h Log Message: Pull up following revision(s) (requested by maxv in ticket #693): sys/net/npf/npf.h: revision 1.55 Fix a vulnerability in NPF, that allows whatever incoming IPv6 packet to bypass a certain number of filtering rules. Basically there is an integer overflow in npf_cache_ip: npc_hlen is a 8bit unsigned int, and can wrap to zero if the IPv6 packet being processed has large extensions. As a result of an overflow, (mbuf + npc_hlen) won't point at the real protocol header, but instead at some garbage within the packet. That garbage, is what NPF applies its rules on. If these filtering rules allow the packet to enter, that packet is given to the main IPv6 entry point. This entry point, however, is not subject to an integer overflow, so it will actually parse the correct protocol header. The result is: NPF read a wrong header, allowed the packet to enter, the kernel read the correct header, and delivered the packet depending on this correct header. So the offending packet was supposed to be kicked, but still went through the firewall. Simple example, a packet with: packet + 0 = IP6 Header packet + 40 = IP6 Routing header (ip6r_len = 31) packet + 48 = Crafted UDP header (uh_dport = ) packet + 296 = IP6 Dest header (ip6e_len = 0) packet + 304 = Real UDP header (uh_dport = ) Will bypass a rule of the kind "block port ". Here NPF reads the crafted UDP header, sees , lets the packet in; later the kernel reads the real UDP header, and delivers it on port . Fix this by using uint32_t. While here, it seems to me there is also a memory overflow: still in npf_cache_ip, npc_hlen may be incremented with a value that goes beyond the mbuf. To generate a diff of this commit: cvs rdiff -u -r1.54 -r1.54.6.1 src/sys/net/npf/npf.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: [netbsd-8] src/sys/net/npf
Module Name:src Committed By: snj Date: Tue Jul 25 02:17:16 UTC 2017 Modified Files: src/sys/net/npf [netbsd-8]: npf_os.c Log Message: Pull up following revision(s) (requested by pgoyette in ticket #155): sys/net/npf/npf_os.c: revision 1.7 The npf module depends on some stuff from the bpf module, so set the required modules list accordingly. To generate a diff of this commit: cvs rdiff -u -r1.6 -r1.6.8.1 src/sys/net/npf/npf_os.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
