On 16/06/2015, at 1:09 PM, Richard Procter wrote: > - I was unable to test af-to, which does a lot of packet fiddling.
I've now tested this without obvious issue. Note: a couple of one-line changes in icmp af-translation remain untested. Details attached. > - I haven't tested modification of unaligned TCP options, > for SACK and timestamp. This tested without issue, too. Also, some points raised in off-list comment suggested I was unclear in my patch post, which I'll address here. - Note this diff does not revert all of Henning's checksum work, just returns to checksum modification for pf-altered packets as in 5.3[0] but without the ugly nested pf_checksum_fixup() calls and without (I expect) loss of efficiency. I've aimed for the minimal necessary changes only. And the patch is made much easier now that pseudo-header checksums are calculated late in the output path, as pf now never sees a partial checksum. Worst-case, pf does some unnecessary arithmetic when altering locally generated packets, but no more than the original 5.3 code did, as the 5.3 modification code didn't distinguish between local and forwarded packets either. - The first patch renames pf_change_a() to pf_change_32_unaligned() as it was not being used for addresses alone, and I needed to move it aside for an address-specific pf_change_a(). - Some commented that, although the patch re-enables end-to-end verification of transport payloads traversing pf, it is unimportant as we now have far stronger end-to-end checks in ssh and TLS. These protocols, however, assume a reliable transport[1]: they terminate the connection on a failed MAC as evidence of malicious tampering[*]. If they didn't they would need to reimplement TCP's retransmission strategies, as the Internet can be expected to damage packets as a matter of course. So TCP checksums continue to play an important role for secure streams built atop TCP and this diff bears upon them, too. As usual, comment, questions, or criticism is welcome. best, Richard. [0] 5.3, not 5.4 as I stated [1] for those interested in the fine print: rfc4253 (2006) SSH p3 rfc5246 (2008) TLSv1.2 p3 [*] IIRC someone last year reported mysterious ssh disconnects due to corrupted MACs which Stuart Henderson pointed out could have been due to a flakey NAT router. Test details: - unaligned SACK, timestamp options - tested on i386, and I expect no issues for architectures with stronger alignment constraints but aiming to test this in the weekend on socppc for kicks. - pf_translate_icmp_af These two remain untested due to my lack of test nodes: nextmtu for ICMP6_PACKET_TOO_BIG pptr for ICMP_PARAMPROB but low risk as these are one line substitutions with tested primitives, and nextmtu is always changed. - translate quoted address family ("af-to") via pf_change_icmp_af pf_change_ap Testing af-to 4 -> 6 pass in on ral0 inet from any to 192.168.2.2 af-to inet6 from fec0:0:0:2::1 to fec0:0:0:2::2 af-to TCP - inet4 telnet to inet6 netcat host [TESTED] af-to ICMP - unassociated with connection - src ICMP4 ping [TESTED] - associated with connection - traceroute elicits ICMP6 port unreachable from dest; translated result inspected with wireshark [TESTED] Testing af-to 6 -> 4 pass in on vr0 inet6 from any to 64:ff9b::/96 af-to inet from 192.168.2.3 af-to TCP - inet6 telnet to inet4 netcat host [TESTED] af-to ICMP - unassociated with connection - src ICMP6 ping [TESTED] - associated with connection - traceroute6 elicits ICMP4 port unreachable from dest; translated result inspected with wireshark. [TESTED]