[openssl-dev] [openssl/openssl] Dtls listen refactor (#5024)

[Matt Caswell]

On 17/01/18 16:34, Michael Richardson wrote:
> 
> Matt Caswell <matt at openssl.org> wrote:
>     >> Matt Caswell <matt at openssl.org> wrote: >> a) when the existing FD is
>     >> connect(2) any future traffic to the bound >> port will get rejected
>     >> with no port.  So the application really has to >> open a new socket
>     >> first.  The application can do this two ways: it can >> open a new
>     >> socket on which to receive new connections, or it can open >> a new
>     >> socket on which to communicate with the new client.  The second >>
>     >> method is better for reason (b) below.  Either way, it socket to >>
>     >> communicate with the client needs to be bind(2) to the address that >>
>     >> the client used to communicate with the server, and DTLSv1_listen() >>
>     >> didn't collect or return that information.
>     >>
>     >> > The second way is what is intended.
>     >>
>     >> Unfortunately, there remains a race condition because we have to call
>     >> bind() before connect() on the new socket.  Under load, if a packet is
>     >> received between the bind() and the connect(), it might go onto the
>     >> wrong socket queue. So some packets that could have been processed
>     >> will get dropped and have to be retransmitted by the client.
> 
>     > This seems like a non-issue to me. At this point in the handshake the
>     > client will have sent its ClientHello and won't progress until it gets
>     > the server's flight back (ServerHello etc), i.e. in the vast majority
>     > of cases it won't be sending anything.
> 
> *That* client will be waiting, but other clients may be sending new ClientHello
> messages (with or without cookies).

So how does your refactor solve this issue? AFAICT this also just does a
bind then connect:

+
if(bind(rfd,BIO_ADDR_sockaddr(ouraddr),BIO_ADDR_sockaddr_size(ouraddr))
!= 0){
 +      goto end;
 +    }
 +
if(connect(rfd,BIO_ADDR_sockaddr(client),BIO_ADDR_sockaddr_size(client))
!= 0) {
 +      goto end;
 +    }

Doesn't this suffer from the same problem? i.e. packets could arrive
from other clients between the bind and connect.

Matt