Change the QuicTransport code to use client indication mechanism instead of allowed origin list. See https://github.com/vasilvv/webtransport/pull/1 for the description, and https://github.com/WICG/web-transport/issues/60 for rationale. gfe-relnote: n/a (code not in production) PiperOrigin-RevId: 272967561 Change-Id: I45770d67614270f3388ffb8b23576182b703cae9
diff --git a/quic/quic_transport/quic_transport_client_session.cc b/quic/quic_transport/quic_transport_client_session.cc index 35d7a82..51e20b9 100644 --- a/quic/quic_transport/quic_transport_client_session.cc +++ b/quic/quic_transport/quic_transport_client_session.cc
@@ -4,10 +4,14 @@ #include "net/third_party/quiche/src/quic/quic_transport/quic_transport_client_session.h" +#include <cstdint> +#include <limits> #include <memory> #include "url/gurl.h" #include "net/third_party/quiche/src/quic/core/quic_crypto_client_stream.h" +#include "net/third_party/quiche/src/quic/core/quic_data_writer.h" +#include "net/third_party/quiche/src/quic/core/quic_error_codes.h" #include "net/third_party/quiche/src/quic/core/quic_session.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_versions.h" @@ -17,7 +21,8 @@ namespace quic { -const char* kQuicTransportAlpn = "wq-draft01"; +const char* kQuicTransportAlpn = "wq-vvv-01"; +const QuicStreamId kClientIndicationStream = 2; namespace { // ProofHandler is primarily used by QUIC crypto to persist QUIC server configs @@ -64,48 +69,60 @@ return; } - auto it = config()->received_custom_transport_parameters().find( - WebAcceptedOriginsParameter()); - if (it == config()->received_custom_transport_parameters().end()) { + SendClientIndication(); +} + +std::string QuicTransportClientSession::SerializeClientIndication() { + std::string serialized_origin = origin_.Serialize(); + if (serialized_origin.size() > std::numeric_limits<uint16_t>::max()) { + QUIC_BUG << "Client origin too long"; connection()->CloseConnection( - QUIC_HANDSHAKE_FAILED, - "QuicTransport requires web_accepted_origins transport parameter", + QUIC_INTERNAL_ERROR, "Client origin too long", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return ""; + } + QUIC_DLOG(INFO) << "Sending client indication with origin " + << serialized_origin; + + std::string buffer; + buffer.resize(/* key */ sizeof(QuicTransportClientIndicationKeys) + + /* length */ sizeof(uint16_t) + serialized_origin.size()); + QuicDataWriter writer(buffer.size(), &buffer[0]); + writer.WriteUInt16( + static_cast<uint16_t>(QuicTransportClientIndicationKeys::kOrigin)); + writer.WriteUInt16(serialized_origin.size()); + writer.WriteStringPiece(serialized_origin); + + buffer.resize(writer.length()); + return buffer; +} + +void QuicTransportClientSession::SendClientIndication() { + if (!crypto_stream_->encryption_established()) { + QUIC_BUG << "Client indication may only be sent once the encryption is " + "established."; + connection()->CloseConnection( + QUIC_INTERNAL_ERROR, "Attempted to send client indication unencrypted", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return; + } + if (client_indication_sent_) { + QUIC_BUG << "Client indication may only be sent once."; + connection()->CloseConnection( + QUIC_INTERNAL_ERROR, "Attempted to send client indication twice", ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); return; } - QUIC_DLOG(INFO) << "QuicTransport using origin: " << origin_.Serialize(); - QUIC_DLOG(INFO) << "QuicTransport origins offered: " << it->second; + auto client_indication_owned = std::make_unique<ClientIndication>( + /*stream_id=*/kClientIndicationStream, this, /*is_static=*/false, + WRITE_UNIDIRECTIONAL); + ClientIndication* client_indication = client_indication_owned.get(); + ActivateStream(std::move(client_indication_owned)); - if (CheckOrigin(it->second)) { - is_origin_valid_ = true; - } else { - QUIC_DLOG(ERROR) << "Origin check failed for " << origin_ - << ", allowed origin list: " << it->second; - connection()->CloseConnection( - QUIC_HANDSHAKE_FAILED, "QuicTransport origin check failed", - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); - } -} - -bool QuicTransportClientSession::CheckOrigin( - QuicStringPiece raw_accepted_origins) { - if (raw_accepted_origins == "*") { - return true; - } - - std::vector<QuicStringPiece> accepted_origins = - QuicTextUtils::Split(raw_accepted_origins, ','); - for (QuicStringPiece raw_origin : accepted_origins) { - url::Origin accepted_origin = - url::Origin::Create(GURL(std::string(raw_origin))); - QUIC_DVLOG(1) << "QuicTransport offered origin normalized: " - << accepted_origin.Serialize(); - if (accepted_origin.IsSameOriginWith(origin_)) { - return true; - } - } - return false; + client_indication->WriteOrBufferData(SerializeClientIndication(), + /*fin=*/true, nullptr); + client_indication_sent_ = true; } } // namespace quic
diff --git a/quic/quic_transport/quic_transport_client_session.h b/quic/quic_transport/quic_transport_client_session.h index 7e1cf73..9ae8500 100644 --- a/quic/quic_transport/quic_transport_client_session.h +++ b/quic/quic_transport/quic_transport_client_session.h
@@ -16,20 +16,22 @@ #include "net/third_party/quiche/src/quic/core/quic_crypto_stream.h" #include "net/third_party/quiche/src/quic/core/quic_server_id.h" #include "net/third_party/quiche/src/quic/core/quic_session.h" +#include "net/third_party/quiche/src/quic/core/quic_stream.h" #include "net/third_party/quiche/src/quic/core/quic_versions.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" #include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h" namespace quic { -// The web_accepted_origins transport parameter ID. -constexpr TransportParameters::TransportParameterId -WebAcceptedOriginsParameter() { - return static_cast<TransportParameters::TransportParameterId>(0xffc8); -} - // The ALPN used by QuicTransport. QUIC_EXPORT extern const char* kQuicTransportAlpn; +QUIC_EXPORT extern const QuicStreamId kClientIndicationStream; + +enum class QuicTransportClientIndicationKeys : uint16_t { + kOrigin = 0x0000, +}; + // A client session for the QuicTransport protocol. class QUIC_EXPORT QuicTransportClientSession : public QuicSession { public: @@ -57,20 +59,33 @@ } bool IsSessionReady() const { - return IsCryptoHandshakeConfirmed() && is_origin_valid_; + return IsCryptoHandshakeConfirmed() && client_indication_sent_ && + connection()->connected(); } void OnCryptoHandshakeEvent(CryptoHandshakeEvent event) override; protected: - // Accepts the list of accepted origins in a format specified in - // <https://tools.ietf.org/html/draft-vvv-webtransport-quic-00#section-3.2>, - // and verifies that at least one of them matches |origin_|. - bool CheckOrigin(QuicStringPiece raw_accepted_origins); + class ClientIndication : public QuicStream { + public: + using QuicStream::QuicStream; + + // This method should never be called, since the stream is client-initiated + // unidirectional. + void OnDataAvailable() override { + QUIC_BUG << "Received data on a write-only stream"; + } + }; + + // Serializes the client indication as described in + // https://vasilvv.github.io/webtransport/draft-vvv-webtransport-quic.html#rfc.section.3.2 + std::string SerializeClientIndication(); + // Creates the client indication stream and sends the client indication on it. + void SendClientIndication(); std::unique_ptr<QuicCryptoClientStream> crypto_stream_; url::Origin origin_; - bool is_origin_valid_ = false; + bool client_indication_sent_ = false; }; } // namespace quic
diff --git a/quic/quic_transport/quic_transport_client_session_test.cc b/quic/quic_transport/quic_transport_client_session_test.cc index c500e2d..a6af854 100644 --- a/quic/quic_transport/quic_transport_client_session_test.cc +++ b/quic/quic_transport/quic_transport_client_session_test.cc
@@ -7,11 +7,17 @@ #include <memory> #include "url/gurl.h" +#include "net/third_party/quiche/src/quic/core/quic_data_writer.h" #include "net/third_party/quiche/src/quic/core/quic_server_id.h" +#include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" #include "net/third_party/quiche/src/quic/platform/api/quic_str_cat.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" #include "net/third_party/quiche/src/quic/test_tools/crypto_test_utils.h" +#include "net/third_party/quiche/src/quic/test_tools/quic_session_peer.h" +#include "net/third_party/quiche/src/quic/test_tools/quic_stream_peer.h" #include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h" namespace quic { @@ -22,7 +28,8 @@ using testing::ElementsAre; const char* kTestOrigin = "https://test-origin.test"; -const char* kTestOriginInsecure = "http://test-origin.test"; +constexpr char kTestOriginClientIndication[] = + "\0\0\0\x18https://test-origin.test"; url::Origin GetTestOrigin() { GURL origin_url(kTestOrigin); return url::Origin::Create(origin_url); @@ -32,6 +39,16 @@ return {ParsedQuicVersion{PROTOCOL_TLS1_3, QUIC_VERSION_99}}; } +std::string DataInStream(QuicStream* stream) { + QuicStreamSendBuffer& send_buffer = QuicStreamPeer::SendBuffer(stream); + std::string result; + result.resize(send_buffer.stream_offset()); + QuicDataWriter writer(result.size(), &result[0]); + EXPECT_TRUE( + send_buffer.WriteStreamData(0, send_buffer.stream_offset(), &writer)); + return result; +} + class TestClientSession : public QuicTransportClientSession { public: using QuicTransportClientSession::QuicTransportClientSession; @@ -68,20 +85,21 @@ server_id_("test.example.com", 443), crypto_config_(crypto_test_utils::ProofVerifierForTesting()) { SetQuicReloadableFlag(quic_supports_tls_handshake, true); + CreateSession(GetTestOrigin()); + } + + void CreateSession(url::Origin origin) { session_ = std::make_unique<TestClientSession>( &connection_, nullptr, DefaultQuicConfig(), GetVersions(), server_id_, - &crypto_config_, GetTestOrigin()); + &crypto_config_, origin); session_->Initialize(); crypto_stream_ = static_cast<QuicCryptoClientStream*>( session_->GetMutableCryptoStream()); } - void ConnectWithOriginList(std::string accepted_origins) { + void Connect() { session_->CryptoConnect(); QuicConfig server_config = DefaultQuicConfig(); - server_config - .custom_transport_parameters_to_send()[WebAcceptedOriginsParameter()] = - accepted_origins; crypto_test_utils::HandshakeWithFakeServer( &server_config, &helper_, &alarm_factory_, &connection_, crypto_stream_, kQuicTransportAlpn); @@ -102,54 +120,27 @@ } TEST_F(QuicTransportClientSessionTest, SuccessfulConnection) { - ConnectWithOriginList(GetTestOrigin().Serialize()); + Connect(); EXPECT_TRUE(session_->IsSessionReady()); + + QuicStream* client_indication_stream = + QuicSessionPeer::zombie_streams(session_.get())[kClientIndicationStream] + .get(); + ASSERT_TRUE(client_indication_stream != nullptr); + const std::string client_indication = DataInStream(client_indication_stream); + const std::string expected_client_indication{ + kTestOriginClientIndication, + QUIC_ARRAYSIZE(kTestOriginClientIndication) - 1}; + EXPECT_EQ(client_indication, expected_client_indication); } -TEST_F(QuicTransportClientSessionTest, SuccessfulConnectionManyOrigins) { - ConnectWithOriginList( - QuicStrCat("http://example.org,", kTestOrigin, ",https://example.com")); - EXPECT_TRUE(session_->IsSessionReady()); -} +TEST_F(QuicTransportClientSessionTest, OriginTooLong) { + std::string long_string(68000, 'a'); + GURL bad_origin_url{"https://" + long_string + ".example/"}; + EXPECT_TRUE(bad_origin_url.is_valid()); + CreateSession(url::Origin::Create(bad_origin_url)); -TEST_F(QuicTransportClientSessionTest, SuccessfulConnectionWildcardOrigin) { - ConnectWithOriginList("*"); - EXPECT_TRUE(session_->IsSessionReady()); -} - -TEST_F(QuicTransportClientSessionTest, OriginMismatch) { - EXPECT_CALL(connection_, - CloseConnection(_, "QuicTransport origin check failed", _)); - ConnectWithOriginList("https://obviously-wrong-website.test"); - EXPECT_FALSE(session_->IsSessionReady()); -} - -TEST_F(QuicTransportClientSessionTest, OriginSchemaMismatch) { - EXPECT_CALL(connection_, - CloseConnection(_, "QuicTransport origin check failed", _)); - ConnectWithOriginList(kTestOriginInsecure); - EXPECT_FALSE(session_->IsSessionReady()); -} - -TEST_F(QuicTransportClientSessionTest, OriginListMissing) { - EXPECT_CALL( - connection_, - CloseConnection( - _, "QuicTransport requires web_accepted_origins transport parameter", - _)); - session_->CryptoConnect(); - QuicConfig server_config = DefaultQuicConfig(); - crypto_test_utils::HandshakeWithFakeServer( - &server_config, &helper_, &alarm_factory_, &connection_, crypto_stream_, - kQuicTransportAlpn); - EXPECT_FALSE(session_->IsSessionReady()); -} - -TEST_F(QuicTransportClientSessionTest, OriginListEmpty) { - EXPECT_CALL(connection_, - CloseConnection(_, "QuicTransport origin check failed", _)); - ConnectWithOriginList(""); - EXPECT_FALSE(session_->IsSessionReady()); + EXPECT_QUIC_BUG(Connect(), "Client origin too long"); } } // namespace