Skip to content

Commit

Permalink
Check Chosen Version matches the version in use by the connection
Browse files Browse the repository at this point in the history
When processing the remote Version Information transport parameter,
check the Chosen Version matches the version in use by the connection or
abort the connection with a VERSION_NEGOTIATION_ERROR.

We also fix a bug in our server implementation, the transport parameters
need to be updated to reflect the Chosen Version.
  • Loading branch information
jlaine committed Jun 30, 2024
1 parent aadd4be commit 79a8caf
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 22 deletions.
66 changes: 45 additions & 21 deletions src/aioquic/quic/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -1169,7 +1169,7 @@ def _alpn_handler(self, alpn_protocol: str) -> None:
)

# For servers, determine the Negotiated Version.
if not self._is_client:
if not self._is_client and not self._version_negotiated_compatible:
if self._remote_version_information is not None:
# Pick the first version we support in the client's available versions,
# which is compatible with the current version.
Expand All @@ -1186,6 +1186,14 @@ def _alpn_handler(self, alpn_protocol: str) -> None:
self._cryptos[tls.Epoch.INITIAL] = self._cryptos_initial[
version
]

# Update our transport parameters to reflect the chosen version.
self.tls.handshake_extensions = [
(
tls.ExtensionType.QUIC_TRANSPORT_PARAMETERS,
self._serialize_transport_parameters(),
)
]
break
self._version_negotiated_compatible = True
self._logger.info(
Expand Down Expand Up @@ -2697,25 +2705,6 @@ def _parse_transport_parameters(
reason_phrase="%s is not allowed for clients" % attr,
)

# If a server receives Version Information where the Chosen Version
# is not included in Available Versions, it MUST treat is as a
# parsing failure.
#
# https://datatracker.ietf.org/doc/html/rfc9368#section-4
if (
quic_transport_parameters.version_information is not None
and quic_transport_parameters.version_information.chosen_version
not in quic_transport_parameters.version_information.available_versions
):
raise QuicConnectionError(
error_code=QuicErrorCode.TRANSPORT_PARAMETER_ERROR,
frame_type=QuicFrameType.CRYPTO,
reason_phrase=(
"version_information's chosen_version is not included "
"in available_versions"
),
)

if not from_session_ticket:
if (
quic_transport_parameters.initial_source_connection_id
Expand Down Expand Up @@ -2783,7 +2772,42 @@ def _parse_transport_parameters(
),
)

# store remote parameters
# Validate Version Information extension.
#
# https://datatracker.ietf.org/doc/html/rfc9368#section-4
if quic_transport_parameters.version_information is not None:
version_information = quic_transport_parameters.version_information

# If a server receives Version Information where the Chosen Version
# is not included in Available Versions, it MUST treat is as a
# parsing failure.
if (
not self._is_client
and version_information.chosen_version
not in version_information.available_versions
):
raise QuicConnectionError(
error_code=QuicErrorCode.TRANSPORT_PARAMETER_ERROR,
frame_type=QuicFrameType.CRYPTO,
reason_phrase=(
"version_information's chosen_version is not included "
"in available_versions"
),
)

# Validate that the Chosen Version matches the version in use for the
# connection.
if version_information.chosen_version != self._crypto_packet_version:
raise QuicConnectionError(
error_code=QuicErrorCode.VERSION_NEGOTIATION_ERROR,
frame_type=QuicFrameType.CRYPTO,
reason_phrase=(
"version_information's chosen_version does not match "
"the version in use"
),
)

# Store remote parameters.
if not from_session_ticket:
if quic_transport_parameters.ack_delay_exponent is not None:
self._remote_ack_delay_exponent = self._remote_ack_delay_exponent
Expand Down
1 change: 1 addition & 0 deletions src/aioquic/quic/packet.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class QuicErrorCode(IntEnum):
CRYPTO_BUFFER_EXCEEDED = 0xD
KEY_UPDATE_ERROR = 0xE
AEAD_LIMIT_REACHED = 0xF
VERSION_NEGOTIATION_ERROR = 0x11
CRYPTO_ERROR = 0x100


Expand Down
27 changes: 26 additions & 1 deletion tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -2652,7 +2652,7 @@ def test_parse_transport_parameters_with_bad_initial_source_connection_id(self):
cm.exception.reason_phrase, "initial_source_connection_id does not match"
)

def test_parse_transport_parameters_with_bad_version_information(self):
def test_parse_transport_parameters_with_bad_version_information_1(self):
server = create_standalone_server(self)
data = encode_transport_parameters(
QuicTransportParameters(
Expand All @@ -2674,6 +2674,31 @@ def test_parse_transport_parameters_with_bad_version_information(self):
"available_versions",
)

def test_parse_transport_parameters_with_bad_version_information_2(self):
server = create_standalone_server(self)
data = encode_transport_parameters(
QuicTransportParameters(
version_information=QuicVersionInformation(
chosen_version=QuicProtocolVersion.VERSION_1,
available_versions=[
QuicProtocolVersion.VERSION_1,
QuicProtocolVersion.VERSION_2,
],
)
)
)
server._crypto_packet_version = QuicProtocolVersion.VERSION_2
with self.assertRaises(QuicConnectionError) as cm:
server._parse_transport_parameters(data)
self.assertEqual(
cm.exception.error_code, QuicErrorCode.VERSION_NEGOTIATION_ERROR
)
self.assertEqual(cm.exception.frame_type, QuicFrameType.CRYPTO)
self.assertEqual(
cm.exception.reason_phrase,
"version_information's chosen_version does not match the version in use",
)

def test_parse_transport_parameters_with_server_only_parameter(self):
server = create_standalone_server(self)
for active_connection_id_limit in [0, 1]:
Expand Down

0 comments on commit 79a8caf

Please sign in to comment.