Re: [PATCH net] net: l2tp: reduce log level when passing up invalid packets

From: Matthias Schiffer <mschiffer@universe-factory.net>
To: Tom Parkin <tparkin@katalix.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH net] net: l2tp: reduce log level when passing up invalid packets
Date: Tue, 2 Mar 2021 00:23:27 +0100	[thread overview]
Message-ID: <1b888cc7-cbc8-c241-6546-7f2e4c85a7e3@universe-factory.net> (raw)
In-Reply-To: <20210223094722.GB12377@katalix.com>

[-- Attachment #1.1: Type: text/plain, Size: 1893 bytes --]

On 2/23/21 10:47 AM, Tom Parkin wrote:
> On  Mon, Feb 22, 2021 at 14:31:38 -0800, Jakub Kicinski wrote:
>> On Mon, 22 Feb 2021 17:40:16 +0100 Matthias Schiffer wrote:
>>>>> This will not be sufficient for my usecase: To stay compatible with older
>>>>> versions of fastd, I can't set the T flag in the first packet of the
>>>>> handshake, as it won't be known whether the peer has a new enough fastd
>>>>> version to understand packets that have this bit set. Luckily, the second
>>>>> handshake byte is always 0 in fastd's protocol, so these packets fail the
>>>>> tunnel version check and are passed to userspace regardless.
>>>>>
>>>>> I'm aware that this usecase is far outside of the original intentions of the
>>>>> code and can only be described as a hack, but I still consider this a
>>>>> regression in the kernel, as it was working fine in the past, without
>>>>> visible warnings.
>>>>>   
>>>>
>>>> I'm sorry, but for the reasons stated above I disagree about it being
>>>> a regression.
>>>
>>> Hmm, is it common for protocol implementations in the kernel to warn about
>>> invalid packets they receive? While L2TP uses connected sockets and thus
>>> usually no unrelated packets end up in the socket, a simple UDP port scan
>>> originating from the configured remote address/port will trigger the "short
>>> packet" warning now (nmap uses a zero-length payload for UDP scans by
>>> default). Log spam caused by a malicous party might also be a concern.
>>
>> Indeed, seems like appropriate counters would be a good fit here?
>> The prints are both potentially problematic for security and lossy.
> 
> Yes, I agree with this argument.
> 

Sounds good, I'll send an updated patch adding a counter for invalid packets.

By now I've found another project affected by the kernel warnings:
https://github.com/wlanslovenija/tunneldigger/issues/160

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]