This is an automated email from the ASF dual-hosted git repository. masaori pushed a commit to branch quic-latest in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit fc1bc3600758cd0c712788896271db946dec669a Author: Masaori Koshiba <[email protected]> AuthorDate: Wed Apr 4 14:28:43 2018 +0900 Fix QUICPacketFactory for Version Negotiation Packet --- iocore/net/quic/QUICHandshake.cc | 3 +- iocore/net/quic/QUICPacket.cc | 38 +++++++++++++++++--------- iocore/net/quic/QUICPacket.h | 2 +- iocore/net/quic/test/test_QUICPacketFactory.cc | 6 ++-- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/iocore/net/quic/QUICHandshake.cc b/iocore/net/quic/QUICHandshake.cc index aec0907..f6a6cec 100644 --- a/iocore/net/quic/QUICHandshake.cc +++ b/iocore/net/quic/QUICHandshake.cc @@ -137,8 +137,7 @@ QUICHandshake::start(const QUICPacket *initial_packet, QUICPacketFactory *packet this->_load_local_server_transport_parameters(initial_packet->version()); packet_factory->set_version(this->_version_negotiator->negotiated_version()); } else { - this->_client_qc->transmit_packet( - packet_factory->create_version_negotiation_packet(initial_packet, _client_qc->largest_acked_packet_number())); + this->_client_qc->transmit_packet(packet_factory->create_version_negotiation_packet(initial_packet)); QUICHSDebug("Version negotiation failed: %x", initial_packet->version()); } } else { diff --git a/iocore/net/quic/QUICPacket.cc b/iocore/net/quic/QUICPacket.cc index 66ed12c..a195f20 100644 --- a/iocore/net/quic/QUICPacket.cc +++ b/iocore/net/quic/QUICPacket.cc @@ -196,7 +196,11 @@ const uint8_t * QUICPacketLongHeader::payload() const { if (this->_buf) { - return this->_buf.get() + LONG_HDR_OFFSET_PAYLOAD; + if (this->type() == QUICPacketType::VERSION_NEGOTIATION) { + return this->_buf.get() + VERSION_NEGOTIATION_PKT_HEADER_LENGTH; + } else { + return this->_buf.get() + LONG_HDR_OFFSET_PAYLOAD; + } } else { return this->_payload.get(); } @@ -253,11 +257,13 @@ QUICPacketLongHeader::store(uint8_t *buf, size_t *len) const QUICTypeUtil::write_QUICVersion(this->_version, buf + *len, &n); *len += n; - QUICPacketNumber dst = 0; - size_t dst_len = 4; - QUICPacket::encode_packet_number(dst, this->_packet_number, dst_len); - QUICTypeUtil::write_QUICPacketNumber(dst, dst_len, buf + *len, &n); - *len += n; + if (this->_type != QUICPacketType::VERSION_NEGOTIATION) { + QUICPacketNumber dst = 0; + size_t dst_len = 4; + QUICPacket::encode_packet_number(dst, this->_packet_number, dst_len); + QUICTypeUtil::write_QUICPacketNumber(dst, dst_len, buf + *len, &n); + *len += n; + } } // @@ -667,14 +673,20 @@ QUICPacketFactory::create(ats_unique_buf buf, size_t len, QUICPacketNumber base_ QUICPacketHeaderUPtr header = QUICPacketHeader::load(std::move(buf), len, base_packet_number); if (header->has_version() && !QUICTypeUtil::is_supported_version(header->version())) { - // We can't decrypt packets that have unknown versions - result = QUICPacketCreationResult::UNSUPPORTED; + if (header->type() == QUICPacketType::VERSION_NEGOTIATION) { + // version of VN packet is 0x00000000 + // This packet is unprotected. Just copy the payload + result = QUICPacketCreationResult::SUCCESS; + } else { + // We can't decrypt packets that have unknown versions + result = QUICPacketCreationResult::UNSUPPORTED; + } + memcpy(plain_txt.get(), header->payload(), header->payload_size()); plain_txt_len = header->payload_size(); } else { Debug("quic_packet", "Decrypting %s packet #%" PRIu64, QUICDebugNames::packet_type(header->type()), header->packet_number()); switch (header->type()) { - case QUICPacketType::VERSION_NEGOTIATION: case QUICPacketType::STATELESS_RESET: // These packets are unprotected. Just copy the payload memcpy(plain_txt.get(), header->payload(), header->payload_size()); @@ -768,7 +780,7 @@ QUICPacketFactory::create(ats_unique_buf buf, size_t len, QUICPacketNumber base_ } QUICPacketUPtr -QUICPacketFactory::create_version_negotiation_packet(const QUICPacket *packet_sent_by_client, QUICPacketNumber base_packet_number) +QUICPacketFactory::create_version_negotiation_packet(const QUICPacket *packet_sent_by_client) { size_t len = sizeof(QUICVersion) * countof(QUIC_SUPPORTED_VERSIONS); ats_unique_buf versions(reinterpret_cast<uint8_t *>(ats_malloc(len)), [](void *p) { ats_free(p); }); @@ -780,9 +792,9 @@ QUICPacketFactory::create_version_negotiation_packet(const QUICPacket *packet_se p += n; } - QUICPacketHeaderUPtr header = - QUICPacketHeader::build(QUICPacketType::VERSION_NEGOTIATION, packet_sent_by_client->connection_id(), - packet_sent_by_client->packet_number(), base_packet_number, 0x00, std::move(versions), len); + // VN packet dosen't have packet number field and version field is always 0x00000000 + QUICPacketHeaderUPtr header = QUICPacketHeader::build(QUICPacketType::VERSION_NEGOTIATION, packet_sent_by_client->connection_id(), + 0x00, 0x00, 0x00, std::move(versions), len); return QUICPacketFactory::_create_unprotected_packet(std::move(header)); } diff --git a/iocore/net/quic/QUICPacket.h b/iocore/net/quic/QUICPacket.h index cca3571..1bc3229 100644 --- a/iocore/net/quic/QUICPacket.h +++ b/iocore/net/quic/QUICPacket.h @@ -339,7 +339,7 @@ public: static QUICPacketUPtr create_null_packet(); QUICPacketUPtr create(ats_unique_buf buf, size_t len, QUICPacketNumber base_packet_number, QUICPacketCreationResult &result); - QUICPacketUPtr create_version_negotiation_packet(const QUICPacket *packet_sent_by_client, QUICPacketNumber base_packet_number); + QUICPacketUPtr create_version_negotiation_packet(const QUICPacket *packet_sent_by_client); QUICPacketUPtr create_initial_packet(QUICConnectionId connection_id, QUICPacketNumber base_packet_number, QUICVersion version, ats_unique_buf payload, size_t len); QUICPacketUPtr create_retry_packet(QUICConnectionId connection_id, QUICPacketNumber packet_number, ats_unique_buf payload, diff --git a/iocore/net/quic/test/test_QUICPacketFactory.cc b/iocore/net/quic/test/test_QUICPacketFactory.cc index 78e3820..e0be9f1 100644 --- a/iocore/net/quic/test/test_QUICPacketFactory.cc +++ b/iocore/net/quic/test/test_QUICPacketFactory.cc @@ -46,12 +46,14 @@ TEST_CASE("QUICPacketFactory_Create_VersionNegotiationPacket", "[quic]") QUICPacket initial_packet(std::move(header), ats_unique_buf(initial_packet_payload, [](void *) {}), sizeof(initial_packet_payload), 0); - QUICPacketUPtr packet = factory.create_version_negotiation_packet(&initial_packet, 0); + QUICPacketUPtr packet = factory.create_version_negotiation_packet(&initial_packet); CHECK(packet->type() == QUICPacketType::VERSION_NEGOTIATION); CHECK(packet->connection_id() == initial_packet.connection_id()); CHECK(packet->packet_number() == initial_packet.packet_number()); CHECK(packet->version() == 0x00); - CHECK(memcmp(packet->payload(), "\xff\x00\x00\x09", 4) == 0); + + QUICVersion supported_version = QUICTypeUtil::read_QUICVersion(packet->payload()); + CHECK(supported_version == QUIC_SUPPORTED_VERSIONS[0]); } TEST_CASE("QUICPacketFactory_Create_Retry", "[quic]") -- To stop receiving notification emails like this one, please contact [email protected].
