aws / s2n-quic

An implementation of the IETF QUIC protocol
https://crates.io/crates/s2n-quic
Apache License 2.0
1.15k stars 120 forks source link

Avoid race condition with peer in server path secret map read #2314

Open Mark-Simulacrum opened 1 month ago

Mark-Simulacrum commented 1 month ago

Security issue notifications

If you discover a potential security issue in s2n-quic we ask that you notify AWS Security via our vulnerability reporting page. Please do not create a public github issue.

Problem:

Currently a sequence like:

Solution:

We need to delay the insertion into the peer set (i.e. by-ip index) on the server until the handshake is fully complete (and so the client is able to decrypt that IP). That probably involves changes to the dc::Endpoint trait to communicate that new state.

Requirements / Acceptance Criteria:

n/a

Out of scope:

dougch commented 1 month ago

Thanks for the issue, we'll have a look.

WesleyRosenblum commented 1 month ago

If the server delays insertion, couldn't the same thing happen in the opposite direction? As in, the client sends a datagram packet encrypted with B that makes it to the server before the ACK that fully completes the handshake on the server side?

Mark-Simulacrum commented 1 month ago

The server would need to insert into the id set (so that it can decrypt packets) as soon as it knows it trusts the client -- I'd guess that's the same time as today. But it should only insert into the peer set (the one used to lookup a path secret by IP) once the client has ack'd the handshake.

I think that closes the gap you're calling out.