quic: gate connections before handshake#3283
Conversation
There's only one listener per Transport. |
sukunrt
force-pushed
the
push-qlvuzllukwpw
branch
5 times, most recently
from
8e5ae86 to
8db6131
Compare
sukunrt
force-pushed
the
push-vqqpzlmlwvry
branch
from
b437e0b to
ce994ca
Compare
We can only set a single `ConnContext` per quic-go Transport. So we cannot set a different `ConnContext` for listeners on the same address. To keep the API simple, the`ConnContext` option on quicreuse.ConnManager is not configurable per listener.
sukunrt
force-pushed
the
push-qlvuzllukwpw
branch
from
8db6131 to
ff6533b
Compare
p2p/test/transport/transport_test.go
Outdated
| return nil, fmt.Errorf("connections blocked") | ||
| }) | ||
| if strings.HasPrefix(tc.Name, "QUIC") || strings.HasPrefix(tc.Name, "WebTransport") { | ||
| // QUIC and WebTransport may can OpenConnection multiple times depending on when the |
There was a problem hiding this comment.
Why isn't this a leak? Do all the times have a matching .Done?
There was a problem hiding this comment.
We would have a matching Done.
We setup context.AfterFunc to close the scope when context completes. Having said that, I should debug this again to confirm that this is what's exactly happening. I've removed this change.
There was a problem hiding this comment.
This is correct.
We need to do this multiple times because quic-go will call OpenConnection multiple times if the TLS ClientHello is split into two and we reject the new connection in the first OpenConnection call.
Specifically in tests, it was happening because of this experimental keyshare extension that was increasing the size of the ClientHello:
golang/go@4b7f7cd
X25519Kyber768Draft00
This uses source address verification to ensure that an attacker cannot completely block connections from specific IPs by sending tls ClientHellos with spoofed IP addresses.
sukunrt
force-pushed
the
push-qlvuzllukwpw
branch
from
91bd5e7 to
dd4c1d7
Compare
sukunrt
force-pushed
the
push-qlvuzllukwpw
branch
2 times, most recently
from
e25f8d9 to
b9b1ed8
Compare
sukunrt
force-pushed
the
push-qlvuzllukwpw
branch
4 times, most recently
from
d087327 to
7c3d452
Compare
MarcoPolo
left a comment
MarcoPolo
left a comment
There was a problem hiding this comment.
We've talked about setting up address verification in QUIC forever. Thanks for tackling this!
sukunrt
force-pushed
the
push-qlvuzllukwpw
branch
from
7c3d452 to
6f93fec
Compare
sukunrt
force-pushed
the
push-qlvuzllukwpw
branch
from
6f93fec to
12b15ce
Compare
sukunrt
force-pushed
the
push-qlvuzllukwpw
branch
from
fe27edf to
162e665
Compare
3 participants
We can only set a single
ConnContextper quic-go Transport. So we cannot set a differentConnContextfor listeners on the same address. To keep the API simple, theConnContextoption on quicreuse.ConnManager is not configurable per listener.Depends on quic-go/quic-go#5122 & #3279