netdev - Re: [PATCH net-next 0/3] Improve bind(addr, 0) behaviour.

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: <20200222010749.75690-1-kuniyu@amazon.co.jp>
Date:   Sat, 22 Feb 2020 10:07:49 +0900
From:   Kuniyuki Iwashima <kuniyu@...zon.co.jp>
To:     <kuniyu@...zon.co.jp>
CC:     <davem@...emloft.net>, <edumazet@...gle.com>, <kuni1840@...il.com>,
        <kuznet@....inr.ac.ru>, <netdev@...r.kernel.org>,
        <osa-contribution-log@...zon.com>, <yoshfuji@...ux-ipv6.org>
Subject: Re: [PATCH net-next 0/3] Improve bind(addr, 0) behaviour.

From:   Kuniyuki Iwashima <kuniyu@...zon.co.jp>
Date:   Sat, 22 Feb 2020 05:35:22 +0900

> I wrote a program below and run without patches and with patches.
> Without patches, we cannot reuse ports in any pattern. With patches, we can
> reuse ports if all of the socket have SO_REUSEADDR enabled and the first
> socket is not in TCP_LISTEN.
> 
> So, I am sorry that the description of my third patch is wrong.
> 
> >     In this case, we should be able to bind sockets to the same port only if
> >     the user has the first listening socket on the port
> 
> Also, I succeeded to reuse ports if both sockets are in TCP_CLOSE and have
> SO_REUSEADDR and SO_REUSEPORT enabled, and I succeeded to call listen for
> both sockets. I think only this case is not safe, so let me check the
> condition.

I changed the condition and it works safely. (I made a mistake when
converting the condition following De Morgan's law.)

```
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index a6d9ee19baeb..d27ed5fe7147 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -152,8 +152,8 @@ static int inet_csk_bind_conflict(const struct sock *sk,
                                     (!reuseport_ok &&
                                      reuseport && sk2->sk_reuseport &&
                                      !rcu_access_pointer(sk->sk_reuseport_cb) &&
-                                     (sk2->sk_state != TCP_TIME_WAIT &&
-                                      !uid_eq(uid, sock_i_uid(sk2))))) &&
+                                     (sk2->sk_state == TCP_TIME_WAIT ||
+                                      uid_eq(uid, sock_i_uid(sk2))))) &&
                                    inet_rcv_saddr_equal(sk, sk2, true))
                                        break;
                        } else if (!reuseport_ok ||
```

===result of test===
both		none		none	10.0.2.15:32768		10.0.2.15:32768		o
both		none		sk1	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		none		sk2	10.0.2.15:32768		10.0.2.15:32768		o
both		none		both	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		sk2		none	10.0.2.15:32768		10.0.2.15:32768		o
both		sk2		sk1	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		sk2		sk2	10.0.2.15:32768		10.0.2.15:32768		o
both		sk2		both	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		sk1		none	10.0.2.15:32768		10.0.2.15:32768		o
both		sk1		sk1	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		sk1		sk2	10.0.2.15:32768		10.0.2.15:32768		o
both		sk1		both	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		both		none	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		both		sk1	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		both		sk2	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
both		both		both	10.0.2.15:32768		0.0.0.0:0		x	98: Address already in use
====================


I also tested with two users. I am sorry about having tested in python,
I will rewrite it in C later.

Both of user-a and user-b can get the same port, but one of them failed to
call listen().

So, I think this patch is safe.

===user-a gets a port===
# su user-a
# whoami
user-a
# python3
...
>>> import socket
>>> sk1 = socket.socket()
>>> sk1.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
>>> sk1.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
>>> sk1.bind(('10.0.2.15', 0))
>>> sk1
<socket.socket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('10.0.2.15', 32768)>
========================

===user-b gets a port===
# su user-b
# whoami
user-b
# python3
...
>>> import socket
>>> sk2 = socket.socket()
>>> sk2.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
>>> sk2.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
>>> sk2.bind(('10.0.2.15', 0))
>>> sk2
<socket.socket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('10.0.2.15', 32768)>
========================


===user-a listen()===
>>> sk1.listen(5)
>>> sk1
<socket.socket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('10.0.2.15', 32768)>
=====================


===user-b listen() (failure)===
>>> sk2.listen(5)    
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 98] Address already in use
===============================