linux-kernel - Re: [PATCH net 5/6] net/diag: Limit TCP-MD5-diag array by max attribute length

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]

Message-ID: <20241107002555.57247-1-kuniyu@amazon.com>
Date: Wed, 6 Nov 2024 16:25:55 -0800
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <devnull+0x7f454c46.gmail.com@...nel.org>
CC: <0x7f454c46@...il.com>, <borisp@...dia.com>, <colona@...sta.com>,
	<davem@...emloft.net>, <dsahern@...nel.org>, <edumazet@...gle.com>,
	<geliang@...nel.org>, <horms@...nel.org>, <john.fastabend@...il.com>,
	<kuba@...nel.org>, <linux-kernel@...r.kernel.org>, <martineau@...nel.org>,
	<matttbe@...nel.org>, <mptcp@...ts.linux.dev>, <netdev@...r.kernel.org>,
	<pabeni@...hat.com>
Subject: Re: [PATCH net 5/6] net/diag: Limit TCP-MD5-diag array by max attribute length

From: Dmitry Safonov via B4 Relay <devnull+0x7f454c46.gmail.com@...nel.org>
Date: Wed, 06 Nov 2024 18:10:18 +0000
> From: Dmitry Safonov <0x7f454c46@...il.com>
> 
> Currently TCP-MD5 keys are dumped as an array of
> (struct tcp_diag_md5sig). All the keys from a socket go
> into the same netlink attribute. The maximum amount of TCP-MD5 keys on
> any socket is limited by /proc/sys/net/core/optmem_max, which post
> commit 4944566706b2 ("net: increase optmem_max default value") is now by
> default 128 KB. With the help of selftest I've figured out that equals
> to 963 keys, without user having to increase optmem_max:
> > test_set_md5() [963/1024]: Cannot allocate memory
> 
> The maximum length of nlattr is limited by typeof(nlattr::nla_len),
> which is (U16_MAX - 1). When there are too many keys the array written
> overflows the netlink attribute. Here is what one can see on a test,
> with no adjustments to optmem_max defaults:
> 
> > recv() = 65180
> > socket: 10.0.254.1:7013->0.0.0.0:0 (intf 3)
> >      family: 2 state: 10 timer: 0 retrans: 0
> >      expires: 0 rqueu: 0 wqueue: 1 uid: 0 inode: 456
> >              attr type: 8 (5)
> >              attr type: 15 (8)
> >              attr type: 21 (12)
> >              attr type: 22 (6)
> >              attr type: 2 (252)
> >              attr type: 18 (64804)
> > recv() = 130680
> > socket: 10.0.254.1:7013->0.0.0.0:0 (intf 3)
> >      family: 2 state: 10 timer: 0 retrans: 0
> >      expires: 0 rqueu: 0 wqueue: 1 uid: 0 inode: 456
> >              attr type: 8 (5)
> >              attr type: 15 (8)
> >              attr type: 21 (12)
> >              attr type: 22 (6)
> >              attr type: 2 (252)
> >              attr type: 18 (64768)
> >              attr type: 29555 (25966)
> > recv() = 130680
> > socket: 10.0.254.1:7013->0.0.0.0:0 (intf 3)
> >      family: 2 state: 10 timer: 0 retrans: 0
> >      expires: 0 rqueu: 0 wqueue: 1 uid: 0 inode: 456
> >              attr type: 8 (5)
> >              attr type: 15 (8)
> >              attr type: 21 (12)
> >              attr type: 22 (6)
> >              attr type: 2 (252)
> >              attr type: 18 (64768)
> >              attr type: 29555 (25966)
> >              attr type: 8265 (8236)
> 
> Here attribute type 18 is INET_DIAG_MD5SIG, the following nlattr types
> are junk made of tcp_diag_md5sig's content.
> 
> Here is the overflow of the nlattr size:
> >>> hex(64768)
> '0xfd00'
> >>> hex(130300)
> '0x1fcfc'
> 
> Limit the size of (struct tcp_diag_md5sig) array in the netlink reply by
> maximum attribute length. Not perfect as NLM_F_DUMP_INTR will be set on
> the netlink header flags, but the userspace can differ if it's due to
> inconsistency or due to maximum size of the netlink attribute.
> 
> In a following patch set, I'm planning to address this and re-introduce
> TCP-MD5-diag that actually works.

Given the issue has not been reported so far (I think), we can wait for
the series rather than backporting this.