On Fri, 2015-08-14 at 18:59 +0200, Ralph Schmieder wrote:
> Here we go again. Thanks for the comments, hope that I got everything
> right. For getting the TCLASS I could have used the word instead of
> the longword, too. But I guess there's no penalty for doing it this
> way, or is there? And it could use some testing beyond the simple
> IPv4 in IPv4 use case of mine :)
Thanks again for working on this, and apologies again for the delay.
I'm still slightly nervous about the whole concept ? we are
deliberately leaking information from the inner packet into the outer
packet. So people will be able to *see* that we're doing VoIP
traffic.... which in practice they could have inferred quite trivially
from the packet size and regularity anyway.
But now I look harder, I see that OpenVPN does already have this
facility, at least for Legacy IP, with the --passtos option. It's
disabled by default though, and I wonder if we should do the same. And
make the option have the same name too?
I might ask on the OpenVPN list about passing the values through
between Legacy IP and IPv6, and propose a patch.
As for the code... it looks good, in general, with a few minor problems
remaining:
The IPV6_TCLASS sockopt requires an 'int', not a 'uint8_t'. I think
that IP_TOS can also take an 'int' on all platforms (that's what
OpenVPN uses), so let's just change that in dtls_mainloop().
(Actually, I also wonder if we should just be setting it per-packet by
using sendmsg(), rather than separately calling setsockopt() each time
it changes?)
I think your initial value of vpninfo->dtls_tos_current wants to be
something that's *completely* outside the range of normal values, to
ensure that it does correctly get set the first time.
I'm also not sure you're extracting the tclass from the IPv6 header
correctly:
tos = (ntohl(0x0FF00000) & load_be32(this->data)) >> 20;
I don't
think the 0x0FF00000 needs to be byte-swapped, does it? You're *always*
going to get zero with the above version on a little-endian machine?
The bits you're after are the low 4 bits of the first byte (as the high nybble of tos), and the high 4 bits of the second byte (as low nybble). So I think this would give you the correct result:
tos = (load_be16(this->data) >> 4) & 0xff;
Finally, I think we need to expose this to the library API with an
openconnect_set_pass_tos() function.
--
David Woodhouse Open Source Technology Centre
David.Woodhouse at intel.com Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5691 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20151007/fb9fbc41/attachment-0001.bin>
