netdev - Re: [PATCH net-next] tcp: change source port selection at bind() time

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: <20240820045319.4134-1-kuniyu@amazon.com>
Date: Mon, 19 Aug 2024 21:53:19 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <kerneljasonxing@...il.com>
CC: <davem@...emloft.net>, <dsahern@...nel.org>, <edumazet@...gle.com>,
	<kernelxing@...cent.com>, <kuba@...nel.org>, <ncardwell@...gle.com>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>, <kuniyu@...zon.com>
Subject: Re: [PATCH net-next] tcp: change source port selection at bind() time

From: Jason Xing <kerneljasonxing@...il.com>
Date: Tue, 20 Aug 2024 08:53:53 +0800
> Hello Eric,
> 
> On Mon, Aug 19, 2024 at 11:45 PM Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > On Fri, Aug 16, 2024 at 5:33 PM Jason Xing <kerneljasonxing@...il.com> wrote:
> > >
> > > From: Jason Xing <kernelxing@...cent.com>
> > >
> > > This is a follow-up patch to an eariler commit 207184853dbd ("tcp/dccp:
> > > change source port selection at connect() time").
> > >
> > > This patch extends the use of IP_LOCAL_PORT_RANGE option, so that we
> > > don't need to iterate every two ports which means only favouring odd
> > > number like the old days before 2016, which can be good for some
> > > users who want to keep in consistency with IP_LOCAL_PORT_RANGE in
> > > connect().
> >
> > Except that bind() with a port reservation is not as common as a connect().
> > This is highly discouraged.
> >
> > See IP_BIND_ADDRESS_NO_PORT
> >
> > Can you provide a real use case ?
> >
> > I really feel like you are trying to push patches 'just because you can'...
> >
> > 'The old days' before 2016 were not very nice, we had P0 all the time
> > because of port exhaustion.
> > Since 2016 and IP_BIND_ADDRESS_NO_PORT I no longer have war rooms stories.
> 
> As you mentioned last night, the issues happening in connect() are
> relatively more than in bind().
> 
> To be more concise, I would like to state 3 points to see if they are valid:
> (1) Extending the option for bind() is the last puzzle of using an
> older algorithm for some users. Since we have one in connect(), how
> about adding it in bind() to provide for the people favouring the
> older algorithm.

Why do they want to use bind() to pick a random port in the first place ?

bind() behaviour is not strictly the same with connect(); the port reserved
by bind() is not reusable for connect().

Also, bind() requires SO_REUSEADDR to share a port, but by default, even
SO_REUSEADDR enabled sockets cannot share the same port if application
uses random-pick by bind((IP, 0)):

  # sysctl -w net.ipv4.ip_local_port_range="32768 32768"
  # python3
  >>> from socket import *
  >>> 
  >>> c1 = socket()
  >>> c1.setsockopt(SOL_SOCKET, SO_REUSEADDR, 1)
  >>> c1.bind(('', 0))
  >>> c1
  <socket.socket fd=4, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('0.0.0.0', 32768)>
  >>> 
  >>> c2 = socket()
  >>> c2.setsockopt(SOL_SOCKET, SO_REUSEADDR, 1)
  >>> c2.bind(('', 0))
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
  OSError: [Errno 98] Address already in use

Then, net.ipv4.ip_autobind_reuse needs to be enabled at some risk.

bind()+connect() simply decreases the number of available 4-tuple on
the netns unless all applications use bind()+connect() instead of just
connect(), and it's unlikely.


> (2) This patch will not hurt any users like in Google as an example
> which prefers odd/even port selection, which is, I admit, indeed more
> advanced.

Indeed, it won't hurt existing users but will lead new users to the
wrong way.


> (3) This patch does not come out of thin air, but from some users who I contact.
> ?

Is someone who contacted to you really aware of all of the above and
even then in favor of bind() without IP_BIND_ADDRESS_NO_PORT ?


> In my opinion, using and adjusting to the new algorithm needs some
> changes in applications. For some old applications, they still need
> more time to keep pace with a more workable solution.

They will add setsockopt(IP_LOCAL_PORT_RANGE) whether your patch is
applied or not, then, only thing they need to do is replace SO_REUSEADDR
with IP_BIND_ADDRESS_NO_PORT, simple enough ?


> After considering it a whole night, I would like to push this tiny
> feature into the upstream kernel, I wonder if you can help me review
> it? Thanks in advance, Eric.