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

[Matt Caswell]

On 16/01/18 19:44, Michael Richardson 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.

It is possible that the client may retransmit the ClientHello if the
server hasn't responded within a timely manner. Retransmit times are
usually quite a while (relatively speaking) after the original
transmission, so the chances of this happening immediately after we've
processed the original ClientHello and before we've called connect()
seem quite small - although possible. If a retransmitted ClientHello
arrives *after* the connect() it will be dropped anyway by OpenSSL so we
really don't care about these messages going missing.

Another possibility is that the client transmits an alert of some form.
This also seems quite unlikely (i.e. send a ClientHello and then
immediately send an alert before the server has had a chance to respond)
- but again, its possible. It would be bad luck indeed for this unlikely
scenario to happen and then for the alert to not make it onto the right
fd due to the race: but if it did its not a big deal. This connection is
doomed in any case (an alert was sent) and we have to be prepared to
deal with packets getting dropped anyway (this is DTLS!). It is very
common for clients to just abort without sending an alert anyway - so
this will just appear like that.

> 
> It could be solved if there was a way to punt packets received on the wrong
> socket to the correct BIO on the input side.  I looked into this, but decided
> it was too difficult...
> 
> That would also let one operate a multitude of DTLS connections using single
> socket which might be a boon to some users.  Mis-designed it would scale
> badly on multi-threaded machines and involve lots of ugly locks.
> I don't want to consider the impacts if one had to pass packets between processes...
> I don't advocate a solution like this (I'll live with the dropped packets),
> but I think it's worth making people aware that they might see mis-directed
> packets get dropped.
> 
>     > Maybe I misunderstand your point -
>     > but the client address *is* returned? Admittedly its wrapped in a
>     > BIO_ADDR, but its easy to get the underlying "raw" address using
>     > BIO_ADDR_rawaddress():
> 
>     > Why isn't recvfrom() suitable (which is what the code currently uses to
>     > get the address)?
> 
> The address of the remote client is returned ("getpeername()") by DTLSv1_listen().
> That's all that recvfrom() gives you.
> 
> recvfrom() was a reasonable API for SunOS 3.x machines with a single 10Mb/s
> interface with a single IPv4 address.  I loved all that at the time...
> But it doesn't work that well when we might have a dozen different kind of
> IPv6 addresses on each virtual interface.
> 
> The address that I'm talking about needing is the one the remote client used
> to reach us.  That's the destination IP of the incoming packet ("getsockname()" in TCP speak).

Ahhh....its the *server's* address you are after. This requirement seems
more reasonable. I think the API is designed to expect you to only bind
to a single IP. I'd be interested in Richard Levitte's opinion on this.

It seems like a fairly simple solution could solve this. Currently we
have BIO_dgram_get_peer() which returns the peer's address for the last
message read from a BIO. You could imagine a new call being introduced
to get our own address. You could then call that immediately after a
successful DTLSv1_listen() call. Obviously we'd have to change the dgram
BIO to use recvmsg for this to work.

Matt