[Swan-dev] sanitizer and ephemeral ports .. Re: [Swan-commit]
antony at phenome.org
Tue Jan 28 08:14:26 UTC 2020
On Sat, Jan 25, 2020 at 09:41:39PM -0500, Andrew Cagney wrote:
> On Sat, 25 Jan 2020 at 15:29, Antony Antony <antony at phenome.org> wrote:
> > First, I noticed sanitizers have improved a lot. Thanks.
> > I know iptable change was discused a while ago.
> > Now we are sanitizing sport and dport when it is not default, however, for
> > some tests like mobike it is not a good idea.
> The old sanitizers did that - they sanitized any port with 4 or more
> digits. Mostly. Maybe. I suspect this was because any port >1024
> was once considered ephemeral.
> Now, like the comment says:
> # ephemeral ports
> # - according to IANA: 49152-65535
> # - according to Linux: 32768-61000
> # the below matches 30000-.. which is good enough
> we're sanitizing anything >=32k (ok, technically, 30k :-) because it
> _is_ ephemeral.
> > I am still thinking how to change the tests to preserve the ports when we
> > want them to, and sanitize when we should not care. I guess using different
> > NAT port ranges would help. I red the comments in ephemeral-ports.sed.
> > Andrew,www
> > I have a feeling the following commit along with other ephemeral-ports.sed
> > changes have gone a bit too far some tests.
> > We should keep ports of ip xfrm state in mobike and few other tests, crypto
> > values are not important in mobike. That is why it is not an "ipsec look".
> > Also ip xfrm state is called several times in those tests, "ipsec look"
> > output would be too long and we are likely to overlook changes/regression.
> > https://testing.libreswan.org/v3.28-1508-gca5c702fb3-master/ikev2-mobike-06/OUTPUT/east.console.diff
> > eg these are not important in mobike
> > + replay-window 32 flag af-unspec
> > + auth-trunc hmac(sha256) 0xHASHKEY 128
> otoh they do no harm and bring things into line with other tests.
> like the comment points out:
When those crypto lines repeat many times it is harder to read output and
notice regression. Especially with 0.0.0.0 from ip xfrm policy. mobike
tests may need "ip xfrm policy" and ip xfrm state 3-5 times. So it is better to keep just relevant lines. Sometime we just grep one line out of "ip xfrm "
output. These tests only need one or two lines. These tests are not about
crypto, auth-trunc hmac(sha256), reqid. Ports and IP addresses are more
important, we should look at what is relevant to the test. On the other hand
most tests IP address and ports won't change and crypto is important. I
find one screen output much easier to read! running ipsec look will totally
defeat the purpose. And when there is regression ipsec look output mangled.
We probably mis regression!
Also something like ip-xfrm.sed is needed for strongswan output too.
I will work on this a bit today.
> # Paul: this is _crazy_, nothing is ephemeral here so it completely breaks
> # everything that tries to use this. It seems like it tried to fixup
> # older kernel vs newer kernel ip xfrm output or something ????
> and a grep shows we're down to two tests (and they don't appear in
> TESTLIST anyway):
> > this line should be there.
> > + encap type espinudp sport 4500 dport EPHEMERAL addr 0.0.0.0
> > I have a feeling that dport EPHEMERAL is important in this test and
> > shouldn't be sanitized. I will take a closer look when working on the
> > sanitizer.
> The port comes from:
> iptables -t nat -L -n | grep 184.108.40.206 || iptables -t nat -A
> POSTROUTING -s 220.127.116.11/32 -p udp -j SNAT --to-source
> 18.104.22.168:33000-34000 && iptables -t nat -A POSTROUTING -s
> 22.214.171.124/32 -j SNAT --to-source 126.96.36.199
> which is ephemeral - i.e., above 32k and random.
yes. I am wondering for mobike with NAT tests we should use range bellow 30K
then it won't be sanitized, because < 30 are not ephemeral.
> > I will try to fix them, however, do not want to fight with your changes.
> > I think, some how the ephemeral ports should kept in mobike tests. Which
> > possibly means on nic specify NAT sports to be bellow 30K?
> > if nic has narrow range, with 2 or 3 ports then mobike tests should get
> > predicable port. Atleast that is the theory we will see.
> > May be revert this commit for mobikes tests?
> Now I'm confused. If ip-xfrm.sed is re-instated, the above line will
> be deleted. So is that above line important or not?
yes. I know it is confusing. I am not clear about the fixes either. I
noticed your fixes seems to go in the opposite direction. So I started this
thread. I will get better idea when I fix mobike tests, soon.
So far my feeling is that mobike may need its own sanitizer, rename
ip-xfrm.sed ip-xfrm-mobike.sed and add to mobike tests.
More information about the Swan-dev