On Fri, Sep 30, 2016 at 11:55:48PM +0200, Alexandr Nedvedicky wrote:
> The patch makes PF to send 'challenge ACK' for SYN packet, which matches
> session in established state.

The pf diff is OK bluhm@

> The patch also comes with test case. I've just learned it's bit tricky
> to use scapy for sending TCP packets. There are basically two pitfalls:

I have some remarks to the test.  First of all, I like it!

>       - the session established by scapy interferes with host's TCP stack,
>       where scapy is running. Host's TCP answers with RST to SYN-ACK,
>       which is sent as a response to scapy's SYN. Adding PF rule, which
>       blocks outbound RST solves the problem.

I have a better trick for that.  I use a non existing address on a
local net of the test machine.  As the packet will get lost after
unanswered arp requests, there is no RST.

In some tests I call it FAKE_ADDR in others FAKE_NET_ADDR depending
where I want the packet to be routed.  I will think about a solution
how to put you use case into my framework consistently.

>       - the scapy's sr() function (send and receive) is very smart not
>       to receive a challenge_ack sent by remote PF. Test case uses
>       sniffer to verify remote PF answers with challenge ACK. If you
>       are not interested in further details stop reading here.

Been there, seen that.

> I gave up and opted for poor man's solution to use a sniffer instead.

I also did this before.  Your solution with the thread is nicer
than the fork I do.  I will try wether it is useful for my tests.

>  run-regress-tcp: stamp-pfctl
>       @echo '\n======== $@ ========'
> +     #
> +     # The TCP handshake in challenge_ack.py is initiated by scapy,
> +     # SRC's local TCP stack is very surprised to see SYN-ACK coming
> +     # from ECO, so it answers with RST. The RST must not leave SRC,
> +     # otherwise testing would be spoiled.
> +     #
> +     echo "block out on ${SRC_IF} proto tcp from ${SRC_OUT} to ${ECO_IN}\
> +             port=echo flags R/R" | ${SUDO} pfctl -e -f -
> +     ${SUDO} ${PYTHON}challenge_ack.py ${SRC_OUT} ${ECO_IN}
> +     ${SUDO} pfctl -d -Fa
>  .for ip in ECO_IN ECO_OUT RDR_IN RDR_OUT RTT_IN
>       @echo Check tcp ${ip}:
>       ${SUDO} route -n delete -host -inet ${${ip}} || true

Do not reuse this target for your test.  I want many tests and
targets, so I can see what is failing.  The pf_forward test has
already reached a lot of complexity, I think it is better to create
a new subdir sys/net/pf_tcp where we can test properies of TCP
handling.  Copy the pf_forward Makefile and remove all the nat,
redirect and af-to stuff.  Everything in one test is not maintainable.

I will play with that idea together with the FAKE_ADDR.

> +srcaddr=sys.argv[1]
> +dstaddr=sys.argv[2]
> +port=os.getpid() & 0xffff
> +
> +ip=IP(src=srcaddr, dst=dstaddr)

Perhaps we can use FAKE_ADDR and ECO_IN directly.  I have to check
how other tests do it.

> +if sniffer.captured == None:
> +     print "ERROR: no packet received"
> +        exit(1)

> +if challenge_ack == None:
> +     print "No ACK has been seen"
> +        exit(1)

Here are tab vs. space errors.

bluhm

Reply via email to