netdev - Re: [PATCH net-next 2/2] l2tp: add peer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 28 Dec 2017 15:53:40 +0100
From:   Guillaume Nault <g.nault@...halink.fr>
To:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, jchapman@...alix.com,
        liuhangbin@...il.com
Subject: Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter

On Fri, Dec 22, 2017 at 03:10:18PM +0100, Lorenzo Bianconi wrote:
> Introduce peer_offset parameter in order to add the capability
> to specify two different values for payload offset on tx/rx side.
> If just offset is provided by userspace use it for rx side as well
> in order to maintain compatibility with older l2tp versions
> 
Sorry for being late on this, I originally missed this patchset and
only noticed it yesterday.

Lorenzo, can you give some context around this new feature?
Quite frankly I can't see the point of it. I've never heard of offsets
in L2TPv3, and for L2TPv2, the offset value is already encoded in the
header.

After a quick review of L2TPv3 and pseudowires RFCs, I still don't see
how adding some padding between the L2TPv3 header and the payload could
constitute a valid frame. Of course, the base feature is already there,
but after a quick test, it looks like the padding bits aren't
initialised and leak memory.

So, unless I missed something, setting offsets in L2TPv3 is
non-compliant, the current implementation is buggy and most likely
unused. I'd really prefer getting the implementation fixed, or even
removed entirely. Extending it to allow asymmetrical offset values
looks wrong to me, unless you have a use case in mind.

Regards,

Guillaume

PS: I also noticed that iproute2 has a "peer_offset" option, but it's a
noop.