1
0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-11-24 04:01:07 +03:00

Make client keep track of originalDestinationConnectionId

Summary:
When I tested against picoquic, I found that an exception was thrown here: https://fburl.com/diffusion/qsnzingx.

The reason is that we modify `conn.initialDestinationConnectionId` in the event of a retry, so it's no longer the original destination connection id. By separately keeping track of the original destination connection id, we can solve this issue.

Reviewed By: mjoras

Differential Revision: D23722127

fbshipit-source-id: 94be08812e675feaf46f5af86e7304af84c1910c
This commit is contained in:
Aman Sharma
2020-11-06 15:40:31 -08:00
committed by Facebook GitHub Bot
parent 246c1102c4
commit 2560e174a9
4 changed files with 15 additions and 3 deletions

View File

@@ -55,6 +55,8 @@ QuicClientTransport::QuicClientTransport(
conn_->selfConnectionIds.emplace_back(srcConnId, kInitialSequenceNumber); conn_->selfConnectionIds.emplace_back(srcConnId, kInitialSequenceNumber);
clientConn_->initialDestinationConnectionId = clientConn_->initialDestinationConnectionId =
ConnectionId::createRandom(kMinInitialDestinationConnIdLength); ConnectionId::createRandom(kMinInitialDestinationConnIdLength);
clientConn_->originalDestinationConnectionId =
clientConn_->initialDestinationConnectionId;
conn_->clientChosenDestConnectionId = conn_->clientChosenDestConnectionId =
clientConn_->initialDestinationConnectionId; clientConn_->initialDestinationConnectionId;
VLOG(4) << "initial dcid: " VLOG(4) << "initial dcid: "
@@ -159,7 +161,7 @@ void QuicClientTransport::processPacketData(
} }
const ConnectionId* originalDstConnId = const ConnectionId* originalDstConnId =
&(*clientConn_->initialDestinationConnectionId); &(*clientConn_->originalDestinationConnectionId);
if (!clientConn_->clientHandshakeLayer->verifyRetryIntegrityTag( if (!clientConn_->clientHandshakeLayer->verifyRetryIntegrityTag(
*originalDstConnId, *retryPacket)) { *originalDstConnId, *retryPacket)) {

View File

@@ -30,6 +30,8 @@ std::unique_ptr<QuicClientConnectionState> undoAllClientStateForRetry(
newConn->clientConnectionId = conn->clientConnectionId; newConn->clientConnectionId = conn->clientConnectionId;
newConn->initialDestinationConnectionId = newConn->initialDestinationConnectionId =
conn->initialDestinationConnectionId; conn->initialDestinationConnectionId;
newConn->originalDestinationConnectionId =
conn->originalDestinationConnectionId;
// TODO: don't carry server connection id over to the new connection. // TODO: don't carry server connection id over to the new connection.
newConn->serverConnectionId = conn->serverConnectionId; newConn->serverConnectionId = conn->serverConnectionId;
newConn->ackStates.initialAckState.nextPacketNum = newConn->ackStates.initialAckState.nextPacketNum =
@@ -100,7 +102,7 @@ void processServerInitialParams(
initialSourceConnId.value() != initialSourceConnId.value() !=
conn.readCodec->getServerConnectionId() || conn.readCodec->getServerConnectionId() ||
originalDestinationConnId.value() != originalDestinationConnId.value() !=
conn.initialDestinationConnectionId) { conn.originalDestinationConnectionId) {
throw QuicTransportException( throw QuicTransportException(
"Initial CID does not match.", "Initial CID does not match.",
TransportErrorCode::TRANSPORT_PARAMETER_ERROR); TransportErrorCode::TRANSPORT_PARAMETER_ERROR);

View File

@@ -30,9 +30,16 @@ struct QuicClientConnectionState : public QuicConnectionStateBase {
// The retry token sent by the server. // The retry token sent by the server.
std::string retryToken; std::string retryToken;
// Initial destination connection id. // This is the destination connection id that will be sent in the outgoing
// client initial packet. It is modified in the event of a retry.
folly::Optional<ConnectionId> initialDestinationConnectionId; folly::Optional<ConnectionId> initialDestinationConnectionId;
// This is the original destination connection id. It is the same as the
// initialDestinationConnectionId when there is no retry involved. When
// there is retry involved, this is the value of the destination connection
// id sent in the very first initial packet.
folly::Optional<ConnectionId> originalDestinationConnectionId;
std::shared_ptr<ClientHandshakeFactory> handshakeFactory; std::shared_ptr<ClientHandshakeFactory> handshakeFactory;
ClientHandshake* clientHandshakeLayer; ClientHandshake* clientHandshakeLayer;

View File

@@ -4449,6 +4449,7 @@ TEST_F(QuicClientTransportVersionAndRetryTest, RetryPacket) {
client->getNonConstConn().qLogger = qLogger; client->getNonConstConn().qLogger = qLogger;
client->getNonConstConn().readCodec->setClientConnectionId(clientConnId); client->getNonConstConn().readCodec->setClientConnectionId(clientConnId);
client->getNonConstConn().initialDestinationConnectionId = initialDstConnId; client->getNonConstConn().initialDestinationConnectionId = initialDstConnId;
client->getNonConstConn().originalDestinationConnectionId = initialDstConnId;
StreamId streamId = *client->createBidirectionalStream(); StreamId streamId = *client->createBidirectionalStream();
auto write = IOBuf::copyBuffer("ice cream"); auto write = IOBuf::copyBuffer("ice cream");