CVS commit: [netbsd-8] src/sys/net/npf

["Martin Husemann" <martin%netbsd.org@localhost>]

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/sys/net/npf/npf_alg_icmp.c
cvs rdiff -u -r1.37 -r1.37.6.1 src/sys/net/npf/npf_handler.c \
    src/sys/net/npf/npf_inet.c
cvs rdiff -u -r1.16 -r1.16.8.1 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.