netdev - Re: [PATCH net-next v5 05/25] ovpn: add basic interface creation/destruction/management routines

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: Windows password security audit tool. GUI, reports in PDF.

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

Date: Mon, 1 Jul 2024 10:48:42 +0200
From: Antonio Quartulli <antonio@...nvpn.net>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: netdev@...r.kernel.org, kuba@...nel.org, ryazanov.s.a@...il.com,
 pabeni@...hat.com, edumazet@...gle.com, andrew@...n.ch
Subject: Re: [PATCH net-next v5 05/25] ovpn: add basic interface
 creation/destruction/management routines

Hi,

On 29/06/2024 00:11, Sabrina Dubroca wrote:
> 2024-06-27, 15:08:23 +0200, Antonio Quartulli wrote:
>> +static void ovpn_setup(struct net_device *dev)
>> +{
>> +	/* compute the overhead considering AEAD encryption */
>> +	const int overhead = sizeof(u32) + NONCE_WIRE_SIZE + 16 +
> 
> Is that 16 equal to OVPN_MAX_PADDING? Or some other constant that
> would help figure out the details of that overhead?

That's the size of the auth tag.

Sadly the explanation of the OpenVPN header (with AEAD encryption) is 
only described in a comment inside ovpn_aead_encrypt().

Here the comment is much "slimmer"..

In crypto_aead.c it is defined as AUTH_TAG_SIZE.
Maybe I could export that define and reuse it here.

Same goes for the "4" in ovpn_aead_encap_overhead(), which is actually 
NONCE_WIRE_SIZE.

> 
>> +			     sizeof(struct udphdr) +
>> +			     max(sizeof(struct ipv6hdr), sizeof(struct iphdr));
>> +
>> +	netdev_features_t feat = NETIF_F_SG | NETIF_F_LLTX |
> [...]
>> +	dev->features |= feat;
>> +	dev->hw_features |= feat;
> 
> I'm not sure we want NETIF_F_LLTX to be part of hw_features, I think
> it's more of a property of the device than something we want to let
> users toggle. AFAICT no other virtual driver does that.

I agree. It should *not* be part of hw_features.
I'll remove it from 'feat' and assign it only to '->features'.


Regards,


-- 
Antonio Quartulli
OpenVPN Inc.