Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
495ee4e
Added io_handle to fake socket
arthuryan-k Jul 22, 2020
f781ded
Changed ListenerFilterFuzzer API and made socket addresses optional
arthuryan-k Jul 22, 2020
c554392
Updated original_src_fuzz_test and original_src corpus entries to ref…
arthuryan-k Jul 22, 2020
28b2eeb
Moved and renamed listener_filter_fakes.h
arthuryan-k Jul 22, 2020
81bb3b3
Changed namespace of listener_filter_fakes
arthuryan-k Jul 22, 2020
c53f9db
Created fake OsSysCalls and added to ListenerFilterFuzzer
arthuryan-k Jul 22, 2020
a7a4135
Added mock dispatcher to ListenerFilterFuzzer
arthuryan-k Jul 23, 2020
ffc8f11
Added fuzz target for http_inspector (WIP)
arthuryan-k Jul 24, 2020
4966faf
Added fuzz target for http_inspector 2 (WIP)
arthuryan-k Jul 27, 2020
561207d
Added HTTP/1.1 corpus entry
arthuryan-k Jul 28, 2020
be1750e
Fixed fuzzer's compatibility with original_dst and original_src
arthuryan-k Jul 28, 2020
62ab061
Increased fuzz coverage of http_inspector to 80.7% (line) by running …
arthuryan-k Jul 30, 2020
b5c7e52
Added support for multiple reads (WIP)
arthuryan-k Jul 30, 2020
855c04d
Added support for multiple reads
arthuryan-k Jul 31, 2020
4e34192
Fixed stack-use-after-scope error
arthuryan-k Jul 31, 2020
049bfc5
Added support for incomplete reads
arthuryan-k Jul 31, 2020
943d9e1
Fixed formatting issues and moved from header to implementation file
arthuryan-k Jul 31, 2020
8ba4fd5
Fixed malformed corpus entry for original_dst
arthuryan-k Aug 3, 2020
fa2e184
Defined fake socket file descriptor as a constant
arthuryan-k Aug 3, 2020
7b4b871
Added comments to explain missing address behavior
arthuryan-k Aug 3, 2020
af4e649
Made ListenerFilterFuzzer declarations static
arthuryan-k Aug 3, 2020
11ccd21
Moved listener_filter_fakes implementation into .cc file
arthuryan-k Aug 3, 2020
110dfb6
Removed unnecessary counter in ListenerFilterFuzzer
arthuryan-k Aug 3, 2020
b98406c
Fixed fuzzer crashes for original_dst and original_src
arthuryan-k Aug 3, 2020
8e0c8d0
Changed fake socket fd macro to static constexpr
arthuryan-k Aug 4, 2020
0766cca
Encapsulated reads in utility class FuzzedHeader
arthuryan-k Aug 7, 2020
656c96a
Fixed some formatting issues
arthuryan-k Aug 7, 2020
48f3598
Fixed more formatting issues
arthuryan-k Aug 7, 2020
a1c3cad
Reserve capacity for header using std::string::reserve()
arthuryan-k Aug 7, 2020
cafddca
Changed assert length to std::min()
arthuryan-k Aug 7, 2020
bccd521
Removed deprecated dependency for listener_filter_fakes
arthuryan-k Aug 10, 2020
6bba706
Added TODO comment to FakeOsSysCalls
arthuryan-k Aug 10, 2020
37662dc
Added fake methods for setting and getting transport protocol
arthuryan-k Aug 10, 2020
88f1a8d
Added fake methods for getting and setting requested server name
arthuryan-k Aug 10, 2020
734db04
Fixed formatting issues (made requestedServerName a one-liner)
arthuryan-k Aug 10, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion test/extensions/filters/listener/common/fuzz/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,20 @@ envoy_cc_test_library(
srcs = ["listener_filter_fuzzer.cc"],
hdrs = ["listener_filter_fuzzer.h"],
deps = [
":listener_filter_fakes",
":listener_filter_fuzzer_proto_cc_proto",
"//include/envoy/network:filter_interface",
"//test/mocks/network:network_fakes",
"//test/mocks/network:network_mocks",
"//test/test_common:threadsafe_singleton_injector_lib",
],
)

envoy_cc_test_library(
name = "listener_filter_fakes",
srcs = ["listener_filter_fakes.cc"],
hdrs = ["listener_filter_fakes.h"],
deps = [
"//source/common/api:os_sys_calls_lib",
"//test/mocks/network:network_mocks",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#include "test/extensions/filters/listener/common/fuzz/listener_filter_fakes.h"

namespace Envoy {
namespace Extensions {
namespace ListenerFilters {

Network::IoHandle& FakeConnectionSocket::ioHandle() { return *io_handle_; }

const Network::IoHandle& FakeConnectionSocket::ioHandle() const { return *io_handle_; }

void FakeConnectionSocket::setLocalAddress(
const Network::Address::InstanceConstSharedPtr& local_address) {
local_address_ = local_address;
if (local_address_ != nullptr) {
addr_type_ = local_address_->type();
}
}

void FakeConnectionSocket::setRemoteAddress(
const Network::Address::InstanceConstSharedPtr& remote_address) {
remote_address_ = remote_address;
}

const Network::Address::InstanceConstSharedPtr& FakeConnectionSocket::localAddress() const {
return local_address_;
}

const Network::Address::InstanceConstSharedPtr& FakeConnectionSocket::remoteAddress() const {
return remote_address_;
}

Network::Address::Type FakeConnectionSocket::addressType() const { return addr_type_; }

absl::optional<Network::Address::IpVersion> FakeConnectionSocket::ipVersion() const {
if (local_address_ == nullptr || addr_type_ != Network::Address::Type::Ip) {
return absl::nullopt;
}

return local_address_->ip()->version();
}

void FakeConnectionSocket::setDetectedTransportProtocol(absl::string_view protocol) {
transport_protocol_ = std::string(protocol);
}

absl::string_view FakeConnectionSocket::detectedTransportProtocol() const {
return transport_protocol_;
}

void FakeConnectionSocket::setRequestedApplicationProtocols(
const std::vector<absl::string_view>& protocols) {
application_protocols_.clear();
for (const auto& protocol : protocols) {
application_protocols_.emplace_back(protocol);
}
}

const std::vector<std::string>& FakeConnectionSocket::requestedApplicationProtocols() const {
return application_protocols_;
}

void FakeConnectionSocket::setRequestedServerName(absl::string_view server_name) {
server_name_ = std::string(server_name);
}

absl::string_view FakeConnectionSocket::requestedServerName() const { return server_name_; }

Api::SysCallIntResult FakeConnectionSocket::getSocketOption(int level, int, void* optval,
socklen_t*) const {
switch (level) {
case SOL_IPV6:
static_cast<sockaddr_storage*>(optval)->ss_family = AF_INET6;
break;
case SOL_IP:
static_cast<sockaddr_storage*>(optval)->ss_family = AF_INET;
break;
default:
NOT_REACHED_GCOVR_EXCL_LINE;
}

return Api::SysCallIntResult{0, 0};
}

} // namespace ListenerFilters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#include "common/api/os_sys_calls_impl.h"
#include "common/network/io_socket_handle_impl.h"

#include "test/mocks/network/mocks.h"

#include "gmock/gmock.h"

namespace Envoy {
namespace Extensions {
namespace ListenerFilters {

static constexpr int kFakeSocketFd = 42;

class FakeConnectionSocket : public Network::MockConnectionSocket {
public:
FakeConnectionSocket()
: io_handle_(std::make_unique<Network::IoSocketHandleImpl>(kFakeSocketFd)),
local_address_(nullptr), remote_address_(nullptr) {}

~FakeConnectionSocket() override { io_handle_->close(); }

Network::IoHandle& ioHandle() override;

const Network::IoHandle& ioHandle() const override;

void setLocalAddress(const Network::Address::InstanceConstSharedPtr& local_address) override;

void setRemoteAddress(const Network::Address::InstanceConstSharedPtr& remote_address) override;

const Network::Address::InstanceConstSharedPtr& localAddress() const override;

const Network::Address::InstanceConstSharedPtr& remoteAddress() const override;

Network::Address::Type addressType() const override;

absl::optional<Network::Address::IpVersion> ipVersion() const override;

void setRequestedApplicationProtocols(const std::vector<absl::string_view>& protocols) override;

const std::vector<std::string>& requestedApplicationProtocols() const override;

void setDetectedTransportProtocol(absl::string_view protocol) override;

absl::string_view detectedTransportProtocol() const override;

void setRequestedServerName(absl::string_view server_name) override;

absl::string_view requestedServerName() const override;

Api::SysCallIntResult getSocketOption(int level, int, void* optval, socklen_t*) const override;

private:
const Network::IoHandlePtr io_handle_;
Network::Address::InstanceConstSharedPtr local_address_;
Network::Address::InstanceConstSharedPtr remote_address_;
Network::Address::Type addr_type_;
std::vector<std::string> application_protocols_;
std::string transport_protocol_;
std::string server_name_;
};

// TODO: Move over to Fake (name is confusing)
class FakeOsSysCalls : public Api::OsSysCallsImpl {
public:
MOCK_METHOD(Api::SysCallSizeResult, recv, (os_fd_t, void*, size_t, int));
};

} // namespace ListenerFilters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#include "test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.h"

#include "common/network/utility.h"

namespace Envoy {
namespace Extensions {
namespace ListenerFilters {
Expand All @@ -10,21 +8,92 @@ void ListenerFilterFuzzer::fuzz(
Network::ListenerFilter& filter,
const test::extensions::filters::listener::FilterFuzzTestCase& input) {
try {
fuzzerSetup(input);
socket_.setLocalAddress(Network::Utility::resolveUrl(input.sock().local_address()));
} catch (const EnvoyException& e) {
ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what());
return;
// Socket's local address will be nullptr by default if fuzzed local address is malformed
// or missing - local address field in proto is optional
}
try {
socket_.setRemoteAddress(Network::Utility::resolveUrl(input.sock().remote_address()));
} catch (const EnvoyException& e) {
// Socket's remote address will be nullptr by default if fuzzed remote address is malformed
// or missing - remote address field in proto is optional
}

FuzzedHeader header(input);

if (!header.empty()) {
ON_CALL(os_sys_calls_, recv(kFakeSocketFd, _, _, MSG_PEEK))
.WillByDefault(testing::Return(Api::SysCallSizeResult{static_cast<ssize_t>(0), 0}));

ON_CALL(dispatcher_,
createFileEvent_(_, _, Event::FileTriggerType::Edge,
Event::FileReadyType::Read | Event::FileReadyType::Closed))
.WillByDefault(testing::DoAll(testing::SaveArg<1>(&file_event_callback_),
testing::ReturnNew<NiceMock<Event::MockFileEvent>>()));
}

filter.onAccept(cb_);

if (file_event_callback_ == nullptr) {
// If filter does not call createFileEvent (i.e. original_dst and original_src)
return;
}

if (!header.empty()) {
{
testing::InSequence s;

EXPECT_CALL(os_sys_calls_, recv(kFakeSocketFd, _, _, MSG_PEEK))
.Times(testing::AnyNumber())
.WillRepeatedly(Invoke(
[&header](os_fd_t, void* buffer, size_t length, int) -> Api::SysCallSizeResult {
return header.next(buffer, length);
}));
}

bool got_continue = false;

ON_CALL(cb_, continueFilterChain(true))
.WillByDefault(testing::InvokeWithoutArgs([&got_continue]() { got_continue = true; }));

while (!got_continue) {
if (header.done()) { // End of stream reached but not done
file_event_callback_(Event::FileReadyType::Closed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no more data to fuzz, should this be an early return? (Does "not done" mean that the input data has not been read completely, even if the filter will do a no-op?)

Copy link
Contributor Author

@arthuryan-k arthuryan-k Aug 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, "not done" should handle the case where all of the fuzzed input data has been read (end of stream reached), but the parser fails to determine http. In this case, the file event will fallback to non-http and call continueFilterChain(true), but it needs to receive Event::FileReadyType::Closed in order to trigger that behavior. The file event itself should be able to handle all other possible cases (socket read error, early return after success, waiting for the next event, etc.).

Edit: Actually, great catch! This works as-is for http_inspector due to how done is implemented (continueFilterChain handled separately), but there should be an explicit early return in general (in tls_inspector, continueFilterChain is handled inside done, which causes this while loop to run indefinitely). I'll fix this in the PR for fuzzing tls_inspector.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhhhh! Gotcha -- thanks so much for the explanation though. Sounds good for fixing later :)

} else {
file_event_callback_(Event::FileReadyType::Read);
}
}
}
}

void ListenerFilterFuzzer::socketSetup(
const test::extensions::filters::listener::FilterFuzzTestCase& input) {
socket_.setLocalAddress(Network::Utility::resolveUrl(input.sock().local_address()));
socket_.setRemoteAddress(Network::Utility::resolveUrl(input.sock().remote_address()));
FuzzedHeader::FuzzedHeader(const test::extensions::filters::listener::FilterFuzzTestCase& input)
: nreads_(input.data_size()), nread_(0) {
size_t len = 0;
for (int i = 0; i < nreads_; i++) {
len += input.data(i).size();
}

header_.reserve(len);

for (int i = 0; i < nreads_; i++) {
header_ += input.data(i);
indices_.push_back(header_.size());
}
}

Api::SysCallSizeResult FuzzedHeader::next(void* buffer, size_t length) {
if (done()) { // End of stream reached
nread_ = nreads_ - 1; // Decrement to avoid out-of-range for last recv() call
}
memcpy(buffer, header_.data(), std::min(indices_[nread_], length));
return Api::SysCallSizeResult{static_cast<ssize_t>(indices_[nread_++]), 0};
}

bool FuzzedHeader::done() { return nread_ >= nreads_; }

bool FuzzedHeader::empty() { return nreads_ == 0; }

} // namespace ListenerFilters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#include "envoy/network/filter.h"

#include "test/extensions/filters/listener/common/fuzz/listener_filter_fakes.h"
#include "test/extensions/filters/listener/common/fuzz/listener_filter_fuzzer.pb.validate.h"
#include "test/mocks/network/fakes.h"
#include "test/mocks/event/mocks.h"
#include "test/mocks/network/mocks.h"
#include "test/test_common/threadsafe_singleton_injector.h"

#include "gmock/gmock.h"

Expand All @@ -12,19 +14,40 @@ namespace ListenerFilters {

class ListenerFilterFuzzer {
public:
ListenerFilterFuzzer() {
ON_CALL(cb_, socket()).WillByDefault(testing::ReturnRef(socket_));
ON_CALL(cb_, dispatcher()).WillByDefault(testing::ReturnRef(dispatcher_));
}

void fuzz(Network::ListenerFilter& filter,
const test::extensions::filters::listener::FilterFuzzTestCase& input);

private:
void fuzzerSetup(const test::extensions::filters::listener::FilterFuzzTestCase& input) {
ON_CALL(cb_, socket()).WillByDefault(testing::ReturnRef(socket_));
socketSetup(input);
}
FakeOsSysCalls os_sys_calls_;
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls_{&os_sys_calls_};
NiceMock<Network::MockListenerFilterCallbacks> cb_;
FakeConnectionSocket socket_;
NiceMock<Event::MockDispatcher> dispatcher_;
Event::FileReadyCb file_event_callback_;
};

void socketSetup(const test::extensions::filters::listener::FilterFuzzTestCase& input);
class FuzzedHeader {
public:
FuzzedHeader(const test::extensions::filters::listener::FilterFuzzTestCase& input);

NiceMock<Network::MockListenerFilterCallbacks> cb_;
Network::FakeConnectionSocket socket_;
// Copies next read into buffer and returns the number of bytes written
Api::SysCallSizeResult next(void* buffer, size_t length);

bool done();

// Returns true if data field in proto is empty
bool empty();

private:
const int nreads_; // Number of reads
int nread_; // Counter of current read
std::string header_; // Construct header from single or multiple reads
std::vector<size_t> indices_; // Ending indices for each read
};

} // namespace ListenerFilters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ message Socket {

message FilterFuzzTestCase {
Socket sock = 1;
repeated string data = 2;
}
11 changes: 11 additions & 0 deletions test/extensions/filters/listener/http_inspector/BUILD
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_fuzz_test",
"envoy_package",
)
load(
Expand Down Expand Up @@ -42,3 +43,13 @@ envoy_extension_cc_test(
"//test/test_common:threadsafe_singleton_injector_lib",
],
)

envoy_cc_fuzz_test(
name = "http_inspector_fuzz_test",
srcs = ["http_inspector_fuzz_test.cc"],
corpus = "http_inspector_corpus",
deps = [
"//source/extensions/filters/listener/http_inspector:http_inspector_lib",
"//test/extensions/filters/listener/common/fuzz:listener_filter_fuzzer_lib",
],
)

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading