masque_server_backend fails to find client state when connection id changes

[veryniceguy12]

I am running the masque_server and masque_client (following https://www.chromium.org/quic/playing-with-quic/), and changing the IP address on the masque_client to trigger a migration, which in turn results in failed subsequent connect-udp requests.

The issue I see is that backend_client_states_ uses a map that keys on the initial connection_id whenever a masque_server session is initiated. When the masque_client migrates to another address, this change causes a failure to find the client_state because of a changed connection id, resulting in a request failure. Shouldn't the key, i.e., the connection_id, for the backend_clientstates be updated after a migration event?

I am running the masque_server with this cmd:

./out/Default/masque_server --certificate_file=net/tools/quic/certs/out/leaf_cert.pem  \
--key_file=net/tools/quic/certs/out/leaf_cert.pkcs8 --cache_dir=/tmp/quic-data/

and the client with this cmd:

./out/Default/masque_client [IPv6_address]:9661 https://cloudflare-quic.com https://youtube.com \
 https://www.facebook.com --disable_certificate_verification=true

This is the error I am running into:

[0720/043019.607921:ERROR:masque_server_backend.cc(88)] Could not find backend client for
{
  :method CONNECT
  :protocol connect-udp
  :scheme https
  :authority [IPv6_address]:9661
  :path /.well-known/masque/udp/www.facebook.com/443/
}

[DavidSchinazi]

You're right, that's a bug. Back when this code was written, QUIC connection IDs were immutable for the lifetime of a QUIC connection. That no longer holds, in particular when migration happens. I'll have to figure out a new key. No promises when I'll find the time to fix this though.

[veryniceguy12]

Thank you for the response! So I was trying to get this to work and how I ended up implementing is that inside the QuicConnection::OnEffectivePeerMigrationValidated function I call a function on the session visitor carrying the previous and current server connection ids and update the backend_client_states_ with the new connection id as the key. Do you think that’s a good idea or is there a better way to keep a constant key for a connection?

[DavidSchinazi]

That should also work! I'd happily review and integrate a PR if you got this to work. Please read CONTRIBUTING.md before writing it though, we need contributors to sign our Contributor License Agreement before we're allowed to accept code.

[veryniceguy12]

Sure! I can do that and make a PR.