This is an automated email from the ASF dual-hosted git repository.
duke8253 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 39dac9fd7b enforce SETTINGS frame rules (#9864)
39dac9fd7b is described below
commit 39dac9fd7b19d0f0d431c33690cc0c604fa1b0b5
Author: Fei Deng <[email protected]>
AuthorDate: Wed Jun 21 10:38:11 2023 -0400
enforce SETTINGS frame rules (#9864)
---
proxy/http3/Http3App.cc | 8 +-
proxy/http3/Http3App.h | 4 +-
proxy/http3/Http3FrameDispatcher.cc | 6 +-
proxy/http3/Http3FrameDispatcher.h | 2 +-
proxy/http3/Http3FrameHandler.h | 5 +-
proxy/http3/Http3HeaderVIOAdaptor.cc | 2 +-
proxy/http3/Http3HeaderVIOAdaptor.h | 3 +-
proxy/http3/Http3ProtocolEnforcer.cc | 51 ++++++++++
.../http3/{test/Mock.h => Http3ProtocolEnforcer.h} | 23 ++---
proxy/http3/Http3StreamDataVIOAdaptor.cc | 3 +-
proxy/http3/Http3StreamDataVIOAdaptor.h | 3 +-
proxy/http3/Http3Transaction.cc | 3 +-
proxy/http3/Http3Types.h | 2 +-
proxy/http3/Makefile.am | 5 +
proxy/http3/test/Mock.h | 4 +-
proxy/http3/test/test_Http3FrameDispatcher.cc | 104 ++++++++++++++++++++-
16 files changed, 192 insertions(+), 36 deletions(-)
diff --git a/proxy/http3/Http3App.cc b/proxy/http3/Http3App.cc
index 99c812b7fd..91cb43a5d9 100644
--- a/proxy/http3/Http3App.cc
+++ b/proxy/http3/Http3App.cc
@@ -37,6 +37,7 @@
#include "Http3DebugNames.h"
#include "Http3Session.h"
#include "Http3Transaction.h"
+#include "Http3ProtocolEnforcer.h"
static constexpr char debug_tag[] = "http3";
static constexpr char debug_tag_v[] = "v_http3";
@@ -51,6 +52,9 @@ Http3App::Http3App(QUICNetVConnection *client_vc,
IpAllow::ACL &&session_acl, co
this->_qc->stream_manager()->set_default_application(this);
+ this->_protocol_enforcer = new Http3ProtocolEnforcer();
+ this->_control_stream_dispatcher.add_handler(this->_protocol_enforcer);
+
this->_settings_handler = new Http3SettingsHandler(this->_ssn);
this->_control_stream_dispatcher.add_handler(this->_settings_handler);
@@ -212,7 +216,7 @@ Http3App::_handle_uni_stream_on_read_ready(int /* event */,
VIO *vio)
case Http3StreamType::CONTROL:
case Http3StreamType::PUSH: {
uint64_t nread = 0;
- this->_control_stream_dispatcher.on_read_ready(adapter->stream().id(),
*vio->get_reader(), nread);
+ this->_control_stream_dispatcher.on_read_ready(adapter->stream().id(),
type, *vio->get_reader(), nread);
// TODO: when PUSH comes from client, send stream error with
HTTP_WRONG_STREAM_DIRECTION
break;
}
@@ -365,7 +369,7 @@ Http3SettingsHandler::interests()
}
Http3ErrorUPtr
-Http3SettingsHandler::handle_frame(std::shared_ptr<const Http3Frame> frame)
+Http3SettingsHandler::handle_frame(std::shared_ptr<const Http3Frame> frame,
int32_t /* frame_seq */, Http3StreamType /* s_type */)
{
ink_assert(frame->type() == Http3FrameType::SETTINGS);
diff --git a/proxy/http3/Http3App.h b/proxy/http3/Http3App.h
index 6cc8bee49f..4ae5a7f27d 100644
--- a/proxy/http3/Http3App.h
+++ b/proxy/http3/Http3App.h
@@ -77,6 +77,7 @@ private:
void _set_qpack_stream(Http3StreamType type, QUICStreamVCAdapter *adapter);
+ Http3FrameHandler *_protocol_enforcer = nullptr;
Http3FrameHandler *_settings_handler = nullptr;
Http3FrameGenerator *_settings_framer = nullptr;
@@ -96,7 +97,8 @@ public:
// Http3FrameHandler
std::vector<Http3FrameType> interests() override;
- Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame)
override;
+ Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame, int32_t
frame_seq = -1,
+ Http3StreamType s_type =
Http3StreamType::UNKNOWN) override;
private:
// TODO: clarify Http3Session I/F for Http3SettingsHandler and Http3App
diff --git a/proxy/http3/Http3FrameDispatcher.cc
b/proxy/http3/Http3FrameDispatcher.cc
index 5a578c2eac..039bc60174 100644
--- a/proxy/http3/Http3FrameDispatcher.cc
+++ b/proxy/http3/Http3FrameDispatcher.cc
@@ -41,11 +41,12 @@ Http3FrameDispatcher::add_handler(Http3FrameHandler
*handler)
}
Http3ErrorUPtr
-Http3FrameDispatcher::on_read_ready(QUICStreamId stream_id, IOBufferReader
&reader, uint64_t &nread)
+Http3FrameDispatcher::on_read_ready(QUICStreamId stream_id, Http3StreamType
stream_type, IOBufferReader &reader, uint64_t &nread)
{
std::shared_ptr<const Http3Frame> frame(nullptr);
Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError());
nread = 0;
+ uint32_t frame_count = 0;
while (true) {
// Read a length of Type field and hopefully a length of Length field too
@@ -95,6 +96,7 @@ Http3FrameDispatcher::on_read_ready(QUICStreamId stream_id,
IOBufferReader &read
if (frame == nullptr) {
break;
}
+ ++frame_count;
// Consume buffer if frame is created
nread += frame_len;
@@ -105,7 +107,7 @@ Http3FrameDispatcher::on_read_ready(QUICStreamId stream_id,
IOBufferReader &read
Debug("http3", "[RX] [%" PRIu64 "] | %s size=%zu", stream_id,
Http3DebugNames::frame_type(type), frame_len);
std::vector<Http3FrameHandler *> handlers =
this->_handlers[static_cast<uint8_t>(type)];
for (auto h : handlers) {
- error = h->handle_frame(frame);
+ error = h->handle_frame(frame, frame_count - 1, stream_type);
if (error->cls != Http3ErrorClass::NONE) {
return error;
}
diff --git a/proxy/http3/Http3FrameDispatcher.h
b/proxy/http3/Http3FrameDispatcher.h
index 315a5b0b6e..d3f2118adb 100644
--- a/proxy/http3/Http3FrameDispatcher.h
+++ b/proxy/http3/Http3FrameDispatcher.h
@@ -33,7 +33,7 @@ class QUICStreamVCAdapter;
class Http3FrameDispatcher
{
public:
- Http3ErrorUPtr on_read_ready(QUICStreamId stream_id, IOBufferReader &reader,
uint64_t &nread);
+ Http3ErrorUPtr on_read_ready(QUICStreamId stream_id, Http3StreamType
stream_type, IOBufferReader &reader, uint64_t &nread);
void add_handler(Http3FrameHandler *handler);
diff --git a/proxy/http3/Http3FrameHandler.h b/proxy/http3/Http3FrameHandler.h
index e3c1f97bf1..79b1bd5fa9 100644
--- a/proxy/http3/Http3FrameHandler.h
+++ b/proxy/http3/Http3FrameHandler.h
@@ -31,6 +31,7 @@ class Http3FrameHandler
{
public:
virtual ~Http3FrameHandler(){};
- virtual std::vector<Http3FrameType> interests()
= 0;
- virtual Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame)
= 0;
+ virtual std::vector<Http3FrameType> interests()
= 0;
+ virtual Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame,
int32_t frame_seq = -1,
+ Http3StreamType s_type =
Http3StreamType::UNKNOWN) = 0;
};
diff --git a/proxy/http3/Http3HeaderVIOAdaptor.cc
b/proxy/http3/Http3HeaderVIOAdaptor.cc
index 47aea205a6..4d449af0b3 100644
--- a/proxy/http3/Http3HeaderVIOAdaptor.cc
+++ b/proxy/http3/Http3HeaderVIOAdaptor.cc
@@ -46,7 +46,7 @@ Http3HeaderVIOAdaptor::interests()
}
Http3ErrorUPtr
-Http3HeaderVIOAdaptor::handle_frame(std::shared_ptr<const Http3Frame> frame)
+Http3HeaderVIOAdaptor::handle_frame(std::shared_ptr<const Http3Frame> frame,
int32_t /* frame_seq */, Http3StreamType /* s_type */)
{
ink_assert(frame->type() == Http3FrameType::HEADERS);
const Http3HeadersFrame *hframe = dynamic_cast<const Http3HeadersFrame
*>(frame.get());
diff --git a/proxy/http3/Http3HeaderVIOAdaptor.h
b/proxy/http3/Http3HeaderVIOAdaptor.h
index 3419811802..f3a4de481d 100644
--- a/proxy/http3/Http3HeaderVIOAdaptor.h
+++ b/proxy/http3/Http3HeaderVIOAdaptor.h
@@ -35,7 +35,8 @@ public:
// Http3FrameHandler
std::vector<Http3FrameType> interests() override;
- Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame)
override;
+ Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame, int32_t
frame_seq = -1,
+ Http3StreamType s_type =
Http3StreamType::UNKNOWN) override;
bool is_complete();
int event_handler(int event, Event *data);
diff --git a/proxy/http3/Http3ProtocolEnforcer.cc
b/proxy/http3/Http3ProtocolEnforcer.cc
new file mode 100644
index 0000000000..cd19fbdaa9
--- /dev/null
+++ b/proxy/http3/Http3ProtocolEnforcer.cc
@@ -0,0 +1,51 @@
+/** @file
+ *
+ * A brief file description
+ *
+ * @section license License
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "Http3ProtocolEnforcer.h"
+
+std::vector<Http3FrameType>
+Http3ProtocolEnforcer::interests()
+{
+ return {Http3FrameType::DATA, Http3FrameType::HEADERS,
Http3FrameType::PRIORITY,
+ Http3FrameType::CANCEL_PUSH, Http3FrameType::SETTINGS,
Http3FrameType::PUSH_PROMISE,
+ Http3FrameType::X_RESERVED_1, Http3FrameType::GOAWAY,
Http3FrameType::X_RESERVED_2,
+ Http3FrameType::X_RESERVED_3, Http3FrameType::MAX_PUSH_ID,
Http3FrameType::DUPLICATE_PUSH_ID,
+ Http3FrameType::X_MAX_DEFINED, Http3FrameType::UNKNOWN};
+}
+
+Http3ErrorUPtr
+Http3ProtocolEnforcer::handle_frame(std::shared_ptr<const Http3Frame> frame,
int32_t frame_seq, Http3StreamType s_type)
+{
+ Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError());
+ Http3FrameType f_type = frame->type();
+ if (s_type == Http3StreamType::CONTROL) {
+ if (frame_seq == 0 && f_type != Http3FrameType::SETTINGS) {
+ error =
std::make_unique<Http3ConnectionError>(Http3ErrorCode::H3_MISSING_SETTINGS,
+ "first frame of the
control stream must be SETTINGS frame");
+ } else if (frame_seq != 0 && f_type == Http3FrameType::SETTINGS) {
+ error =
std::make_unique<Http3ConnectionError>(Http3ErrorCode::H3_FRAME_UNEXPECTED,
+ "only one SETTINGS frame
is allowed per the control stream");
+ }
+ }
+ return error;
+}
diff --git a/proxy/http3/test/Mock.h b/proxy/http3/Http3ProtocolEnforcer.h
similarity index 71%
copy from proxy/http3/test/Mock.h
copy to proxy/http3/Http3ProtocolEnforcer.h
index 2b02ed014c..a62d239c2f 100644
--- a/proxy/http3/test/Mock.h
+++ b/proxy/http3/Http3ProtocolEnforcer.h
@@ -21,27 +21,18 @@
* limitations under the License.
*/
-#include "catch.hpp"
+#pragma once
+#include "Http3Types.h"
#include "Http3FrameHandler.h"
-class Http3MockFrameHandler : public Http3FrameHandler
+class Http3ProtocolEnforcer : public Http3FrameHandler
{
public:
- int total_frame_received = 0;
+ Http3ProtocolEnforcer(){};
// Http3FrameHandler
-
- std::vector<Http3FrameType>
- interests() override
- {
- return {Http3FrameType::DATA};
- }
-
- Http3ErrorUPtr
- handle_frame(std::shared_ptr<const Http3Frame> frame) override
- {
- this->total_frame_received++;
- return Http3ErrorUPtr(new Http3NoError());
- }
+ std::vector<Http3FrameType> interests() override;
+ Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame, int32_t
frame_seq = -1,
+ Http3StreamType s_type =
Http3StreamType::UNKNOWN) override;
};
diff --git a/proxy/http3/Http3StreamDataVIOAdaptor.cc
b/proxy/http3/Http3StreamDataVIOAdaptor.cc
index cb154d8ab8..a711a629df 100644
--- a/proxy/http3/Http3StreamDataVIOAdaptor.cc
+++ b/proxy/http3/Http3StreamDataVIOAdaptor.cc
@@ -38,7 +38,8 @@ Http3StreamDataVIOAdaptor::interests()
}
Http3ErrorUPtr
-Http3StreamDataVIOAdaptor::handle_frame(std::shared_ptr<const Http3Frame>
frame)
+Http3StreamDataVIOAdaptor::handle_frame(std::shared_ptr<const Http3Frame>
frame, int32_t /* frame_seq */,
+ Http3StreamType /* s_type */)
{
ink_assert(frame->type() == Http3FrameType::DATA);
const Http3DataFrame *dframe = dynamic_cast<const Http3DataFrame
*>(frame.get());
diff --git a/proxy/http3/Http3StreamDataVIOAdaptor.h
b/proxy/http3/Http3StreamDataVIOAdaptor.h
index 5114979cd3..c14218b83c 100644
--- a/proxy/http3/Http3StreamDataVIOAdaptor.h
+++ b/proxy/http3/Http3StreamDataVIOAdaptor.h
@@ -35,7 +35,8 @@ public:
// Http3FrameHandler
std::vector<Http3FrameType> interests() override;
- Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame)
override;
+ Http3ErrorUPtr handle_frame(std::shared_ptr<const Http3Frame> frame, int32_t
frame_seq = -1,
+ Http3StreamType s_type =
Http3StreamType::UNKNOWN) override;
// Http3StreamDataVIOAdaptor
void finalize();
diff --git a/proxy/http3/Http3Transaction.cc b/proxy/http3/Http3Transaction.cc
index 841ace6600..789936c856 100644
--- a/proxy/http3/Http3Transaction.cc
+++ b/proxy/http3/Http3Transaction.cc
@@ -563,7 +563,8 @@ Http3Transaction::_process_read_vio()
SCOPED_MUTEX_LOCK(lock, this->_info.read_vio->mutex, this_ethread());
uint64_t nread = 0;
- this->_frame_dispatcher.on_read_ready(this->_info.adapter.stream().id(),
*this->_info.read_vio->get_reader(), nread);
+ this->_frame_dispatcher.on_read_ready(this->_info.adapter.stream().id(),
Http3StreamType::UNKNOWN,
+ *this->_info.read_vio->get_reader(),
nread);
this->_info.read_vio->ndone += nread;
return nread;
}
diff --git a/proxy/http3/Http3Types.h b/proxy/http3/Http3Types.h
index 6fb846b6d1..2336959ea0 100644
--- a/proxy/http3/Http3Types.h
+++ b/proxy/http3/Http3Types.h
@@ -108,7 +108,7 @@ public:
const char *msg = nullptr;
protected:
- Http3Error(){};
+ Http3Error() : app_error_code(Http3ErrorCode::H3_NO_ERROR){};
Http3Error(const Http3ErrorCode error_code, const char *error_msg = nullptr)
: cls(Http3ErrorClass::APPLICATION), app_error_code(error_code),
msg(error_msg){};
};
diff --git a/proxy/http3/Makefile.am b/proxy/http3/Makefile.am
index 41781dbcf3..5f60f70abb 100644
--- a/proxy/http3/Makefile.am
+++ b/proxy/http3/Makefile.am
@@ -52,6 +52,7 @@ libhttp3_a_SOURCES = \
Http3DataFramer.cc \
Http3HeaderVIOAdaptor.cc \
Http3StreamDataVIOAdaptor.cc \
+ Http3ProtocolEnforcer.cc \
QPACK.cc
#
@@ -86,6 +87,10 @@ test_libhttp3_LDADD = $(test_LDADD)
test_libhttp3_SOURCES = \
./test/main.cc \
./test/test_Http3Frame.cc \
+ ./test/test_Http3FrameDispatcher.cc \
+ ./Http3ProtocolEnforcer.cc \
+ ./Http3FrameDispatcher.cc \
+ ./Http3DebugNames.cc \
./Http3Config.cc \
./Http3Frame.cc
diff --git a/proxy/http3/test/Mock.h b/proxy/http3/test/Mock.h
index 2b02ed014c..f3b923d09e 100644
--- a/proxy/http3/test/Mock.h
+++ b/proxy/http3/test/Mock.h
@@ -35,11 +35,11 @@ public:
std::vector<Http3FrameType>
interests() override
{
- return {Http3FrameType::DATA};
+ return {Http3FrameType::DATA, Http3FrameType::SETTINGS};
}
Http3ErrorUPtr
- handle_frame(std::shared_ptr<const Http3Frame> frame) override
+ handle_frame(std::shared_ptr<const Http3Frame> frame, int32_t /* frame_seq
*/, Http3StreamType /* s_type */) override
{
this->total_frame_received++;
return Http3ErrorUPtr(new Http3NoError());
diff --git a/proxy/http3/test/test_Http3FrameDispatcher.cc
b/proxy/http3/test/test_Http3FrameDispatcher.cc
index 3e8a1ae496..dd796cd9ad 100644
--- a/proxy/http3/test/test_Http3FrameDispatcher.cc
+++ b/proxy/http3/test/test_Http3FrameDispatcher.cc
@@ -24,27 +24,123 @@
#include "catch.hpp"
#include "Http3FrameDispatcher.h"
+#include "Http3ProtocolEnforcer.h"
#include "Mock.h"
TEST_CASE("Http3FrameHandler dispatch", "[http3]")
{
uint8_t input[] = {// 1st frame (HEADERS)
- 0x02, 0x01, 0x00, 0x01, 0x23,
+ 0x01, 0x04, 0x11, 0x22, 0x33, 0x44,
// 2nd frame (DATA)
- 0x04, 0x00, 0x00, 0x11, 0x22, 0x33, 0x44,
+ 0x00, 0x04, 0xaa, 0xbb, 0xcc, 0xdd,
// 3rd frame (incomplete)
0xff};
Http3FrameDispatcher http3FrameDispatcher;
Http3MockFrameHandler handler;
http3FrameDispatcher.add_handler(&handler);
- uint16_t nread = 0;
+
+ MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512);
+ IOBufferReader *reader = buf->alloc_reader();
+ uint64_t nread = 0;
+ Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError());
+
+ buf->write(input, sizeof(input));
// Initial state
CHECK(handler.total_frame_received == 0);
CHECK(nread == 0);
- http3FrameDispatcher.on_read_ready(input, sizeof(input), nread);
+ error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::UNKNOWN,
*reader, nread);
+ CHECK(error->app_error_code == Http3ErrorCode::H3_NO_ERROR);
CHECK(handler.total_frame_received == 1);
CHECK(nread == 12);
}
+
+TEST_CASE("dispatch SETTINGS frame", "[http3]")
+{
+ SECTION("Only one SETTINGS frame is allowed per the control stream")
+ {
+ uint8_t input[] = {
+ 0x04, // Type
+ 0x08, // Length
+ 0x06, // Identifier
+ 0x44, 0x00, // Value
+ 0x09, // Identifier
+ 0x0f, // Value
+ 0x4a, 0x0a, // Identifier
+ 0x00, // Value
+ 0x04, // Type
+ 0x08, // Length
+ 0x06, // Identifier
+ 0x44, 0x00, // Value
+ 0x09, // Identifier
+ 0x0f, // Value
+ 0x4a, 0x0a, // Identifier
+ 0x00, // Value
+ };
+
+ Http3FrameDispatcher http3FrameDispatcher;
+ Http3ProtocolEnforcer enforcer;
+ Http3MockFrameHandler handler;
+
+ http3FrameDispatcher.add_handler(&enforcer);
+ http3FrameDispatcher.add_handler(&handler);
+
+ MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512);
+ IOBufferReader *reader = buf->alloc_reader();
+ uint64_t nread = 0;
+ Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError());
+
+ buf->write(input, sizeof(input));
+
+ // Initial state
+ CHECK(handler.total_frame_received == 0);
+ CHECK(nread == 0);
+
+ error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::CONTROL,
*reader, nread);
+ CHECK(error->app_error_code == Http3ErrorCode::H3_FRAME_UNEXPECTED);
+ CHECK(handler.total_frame_received == 1);
+ CHECK(nread == sizeof(input));
+ }
+
+ SECTION("first frame of the control stream must be SETTINGS frame")
+ {
+ uint8_t input[] = {
+ 0x0d, // Type
+ 0x01, // Length
+ 0x01, // Push ID
+ 0x04, // Type
+ 0x08, // Length
+ 0x06, // Identifier
+ 0x44, 0x00, // Value
+ 0x09, // Identifier
+ 0x0f, // Value
+ 0x4a, 0x0a, // Identifier
+ 0x00, // Value
+ };
+
+ Http3FrameDispatcher http3FrameDispatcher;
+ Http3ProtocolEnforcer enforcer;
+ Http3MockFrameHandler handler;
+
+ http3FrameDispatcher.add_handler(&enforcer);
+ http3FrameDispatcher.add_handler(&handler);
+
+ MIOBuffer *buf = new_MIOBuffer(BUFFER_SIZE_INDEX_512);
+ IOBufferReader *reader = buf->alloc_reader();
+ uint64_t nread = 0;
+ Http3ErrorUPtr error = Http3ErrorUPtr(new Http3NoError());
+
+ buf->write(input, sizeof(input));
+
+ // Initial state
+ CHECK(handler.total_frame_received == 0);
+ CHECK(nread == 0);
+
+ error = http3FrameDispatcher.on_read_ready(0, Http3StreamType::CONTROL,
*reader, nread);
+ CHECK(error->app_error_code == Http3ErrorCode::H3_MISSING_SETTINGS);
+ CHECK(handler.total_frame_received == 0);
+ CHECK(nread == 3);
+ }
+}
