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]




Reply via email to