Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

----- On Jun 30, 2020, at 7:50 PM, Eric Dumazet edumazet@google.com wrote:

> On Tue, Jun 30, 2020 at 4:47 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> ----- On Jun 30, 2020, at 7:41 PM, Eric Dumazet edumazet@google.com wrote:
>>
>> > MD5 keys are read with RCU protection, and tcp_md5_do_add()
>> > might update in-place a prior key.
>> >
>> > Normally, typical RCU updates would allocate a new piece
>> > of memory. In this case only key->key and key->keylen might
>> > be updated, and we do not care if an incoming packet could
>> > see the old key, the new one, or some intermediate value,
>> > since changing the key on a live flow is known to be problematic
>> > anyway.
>>
>> What makes it acceptable to observe an intermediate bogus key during the
>> transition ?
> 
> If you change a key while packets are in flight, the result is that :
> 
> 1) Either your packet has the correct key and is handled
> 
> 2) Or the key do not match, packet is dropped.
> 
> Sender will retransmit eventually.

This train of thoughts seem to apply to incoming traffic, what about outgoing ?

> 
> If this was not the case, then we could not revert the patch you are
> complaining about :)

Please let me know where I'm incorrect with the following scenario:

- Local peer is a Linux host, which supports only a single MD5 key
  per socket at any given time.
- Remote peer is a Ericsson/Redback device allowing 2 passwords (old/new)
  to co-exist until both sides are OK with the new password.

The local peer updates the MD5 password for a socket in parallel with
packets flow. If we guarantee that no intermediate bogus key state is
observable, the flow going out of the Linux peer should always be seen as
valid, with either the old or the new key.

Allowing an intermediate bogus key to be observable means this introduces
a race condition during which the remote peer can observe a corrupted key.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com