Skip to content

Commit

Permalink
face: improve error handling in UnixStreamChannel
Browse files Browse the repository at this point in the history 
Change-Id: I5d37b56e74264490089a4c18f527bbcc202a480d
  • Loading branch information
@Pesa
Pesa committed Jan 13, 2024
1 parent 4c95771 commit 4b1921f
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 52 deletions.
63 changes: 38 additions & 25 deletions daemon/face/unix-stream-channel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include "common/global.hpp"

#include <boost/filesystem.hpp>
#include <sys/stat.h> // for chmod()

namespace nfd::face {

Expand All @@ -50,10 +49,10 @@ UnixStreamChannel::~UnixStreamChannel()
{
if (isListening()) {
// use the non-throwing variants during destruction and ignore any errors
boost::system::error_code error;
m_acceptor.close(error);
boost::system::error_code ec;
m_acceptor.close(ec);
NFD_LOG_CHAN_TRACE("Removing socket file");
boost::filesystem::remove(m_endpoint.path(), error);
boost::filesystem::remove(m_endpoint.path(), ec);
}
}

Expand All @@ -70,42 +69,56 @@ UnixStreamChannel::listen(const FaceCreatedCallback& onFaceCreated,
namespace fs = boost::filesystem;

fs::path socketPath = m_endpoint.path();
fs::file_type type = fs::symlink_status(socketPath).type();
// ensure parent directory exists
fs::path parent = socketPath.parent_path();
if (!parent.empty() && fs::create_directories(parent)) {
NFD_LOG_CHAN_TRACE("Created directory " << parent);
}

boost::system::error_code ec;
fs::file_type type = fs::symlink_status(socketPath).type();
if (type == fs::socket_file) {
boost::system::error_code error;
// if the socket file already exists, there may be another instance
// of NFD running on the system: make sure we don't steal its socket
boost::asio::local::stream_protocol::socket socket(getGlobalIoService());
socket.connect(m_endpoint, error);
NFD_LOG_CHAN_TRACE("connect() on existing socket file returned: " << error.message());
if (!error) {
socket.connect(m_endpoint, ec);
NFD_LOG_CHAN_TRACE("connect() on existing socket file returned: " << ec.message());
if (!ec) {
// someone answered, leave the socket alone
NDN_THROW(Error("Socket file at " + m_endpoint.path() + " belongs to another process"));
ec = boost::system::errc::make_error_code(boost::system::errc::address_in_use);
NDN_THROW_NO_STACK(fs::filesystem_error("UnixStreamChannel::listen", socketPath, ec));
}
else if (error == boost::asio::error::connection_refused ||
error == boost::asio::error::timed_out) {
// no one is listening on the remote side,
// we can safely remove the stale socket
else if (ec == boost::asio::error::connection_refused ||
ec == boost::asio::error::timed_out) {
// no one is listening on the remote side, we can safely remove the stale socket
NFD_LOG_CHAN_DEBUG("Removing stale socket file");
fs::remove(socketPath);
}
}
else if (type != fs::file_not_found) {
NDN_THROW(Error(m_endpoint.path() + " already exists and is not a socket file"));
// the file exists but is not a socket: this is a fatal error as we cannot
// safely overwrite the file without potentially risking data loss
ec = boost::system::errc::make_error_code(boost::system::errc::not_a_socket);
NDN_THROW_NO_STACK(fs::filesystem_error("UnixStreamChannel::listen", socketPath, ec));
}

// ensure parent directory exists before creating socket
fs::path parent = socketPath.parent_path();
if (!parent.empty() && fs::create_directories(parent)) {
NFD_LOG_CHAN_TRACE("Created directory " << parent);
try {
m_acceptor.open();
m_acceptor.bind(m_endpoint);
m_acceptor.listen(backlog);
}
catch (const boost::system::system_error& e) {
// exceptions thrown by Boost.Asio are very terse, add more context
NDN_THROW_NO_STACK(fs::filesystem_error("UnixStreamChannel::listen: "s + e.std::runtime_error::what(),
socketPath, e.code()));
}

m_acceptor.open();
m_acceptor.bind(m_endpoint);
m_acceptor.listen(backlog);
// do this here so that, even if the calls below fail,
// the destructor will still remove the socket file
m_isListening = true;

if (::chmod(m_endpoint.path().data(), 0666) < 0) {
NDN_THROW_ERRNO(Error("Failed to chmod " + m_endpoint.path()));
}
fs::permissions(socketPath, fs::owner_read | fs::group_read | fs::others_read |
fs::owner_write | fs::group_write | fs::others_write);

accept(onFaceCreated, onAcceptFailed);
NFD_LOG_CHAN_DEBUG("Started listening");
Expand Down
16 changes: 4 additions & 12 deletions daemon/face/unix-stream-channel.hpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
* Copyright (c) 2014-2023, Regents of the University of California,
* Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
Expand Down Expand Up @@ -45,15 +45,6 @@ namespace nfd::face {
class UnixStreamChannel final : public Channel
{
public:
/**
* \brief UnixStreamChannel-related error.
*/
class Error : public std::runtime_error
{
public:
using std::runtime_error::runtime_error;
};

/**
* \brief Create a UnixStream channel for the specified \p endpoint.
*
Expand All @@ -67,7 +58,7 @@ class UnixStreamChannel final : public Channel
bool
isListening() const final
{
return m_acceptor.is_open();
return m_isListening;
}

size_t
Expand All @@ -89,7 +80,7 @@ class UnixStreamChannel final : public Channel
* returns an error)
* \param backlog The maximum length of the queue of pending incoming
* connections
* \throw Error
* \throw boost::system::system_error
*/
void
listen(const FaceCreatedCallback& onFaceCreated,
Expand All @@ -104,6 +95,7 @@ class UnixStreamChannel final : public Channel
private:
const unix_stream::Endpoint m_endpoint;
const bool m_wantCongestionMarking;
bool m_isListening = false;
boost::asio::local::stream_protocol::acceptor m_acceptor;
size_t m_size = 0;
};
Expand Down
14 changes: 10 additions & 4 deletions daemon/main.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
* Copyright (c) 2014-2023, Regents of the University of California,
* Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
Expand Down Expand Up @@ -36,10 +36,10 @@
#include <boost/asio/signal_set.hpp>
#include <boost/config.hpp>
#include <boost/exception/diagnostic_information.hpp>
#include <boost/filesystem.hpp>
#include <boost/program_options/options_description.hpp>
#include <boost/program_options/parsers.hpp>
#include <boost/program_options/variables_map.hpp>
#include <boost/system/system_error.hpp>
#include <boost/version.hpp>

#include <atomic>
Expand Down Expand Up @@ -317,13 +317,19 @@ main(int argc, char** argv)
", with ndn-cxx version " NDN_CXX_VERSION_BUILD_STRING
<< std::endl;

auto flush = ndn::make_scope_exit([] { ndn::util::Logging::flush(); });

NfdRunner runner(configFile);
try {
runner.initialize();
}
catch (const boost::filesystem::filesystem_error& e) {
catch (const boost::system::system_error& e) {
NFD_LOG_FATAL(boost::diagnostic_information(e));
return e.code() == boost::system::errc::permission_denied ? 4 : 1;
if (e.code() == boost::system::errc::operation_not_permitted ||
e.code() == boost::system::errc::permission_denied) {
return 4;
}
return 1;
}
catch (const std::exception& e) {
NFD_LOG_FATAL(boost::diagnostic_information(e));
Expand Down
128 changes: 117 additions & 11 deletions tests/daemon/face/unix-stream-channel.t.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,17 @@ class UnixStreamChannelFixture : public ChannelFixture<UnixStreamChannel, unix_s
UnixStreamChannelFixture()
{
listenerEp = unix_stream::Endpoint(socketPath.string());
fs::remove_all(socketPath.parent_path());

// in case an earlier test run crashed without a chance to run the destructor
boost::system::error_code ec;
fs::remove_all(testDir, ec);
}

~UnixStreamChannelFixture() override
{
// cleanup
boost::system::error_code ec;
fs::remove_all(testDir, ec);
}

shared_ptr<UnixStreamChannel>
Expand Down Expand Up @@ -76,7 +86,8 @@ class UnixStreamChannelFixture : public ChannelFixture<UnixStreamChannel, unix_s
}

protected:
fs::path socketPath = fs::path(UNIT_TESTS_TMPDIR) / "unix-stream-channel" / "test" / "foo.sock";
fs::path testDir = fs::path(UNIT_TESTS_TMPDIR) / "unix-stream-channel";
fs::path socketPath = testDir / "test" / "foo.sock";
};

BOOST_AUTO_TEST_SUITE(Face)
Expand All @@ -97,7 +108,7 @@ BOOST_AUTO_TEST_CASE(Listen)
BOOST_CHECK_EQUAL(channel->isListening(), true);

// listen() is idempotent
BOOST_CHECK_NO_THROW(channel->listen(nullptr, nullptr));
channel->listen(nullptr, nullptr);
BOOST_CHECK_EQUAL(channel->isListening(), true);
}

Expand Down Expand Up @@ -130,7 +141,9 @@ BOOST_AUTO_TEST_CASE(MultipleAccepts)
}
}

BOOST_AUTO_TEST_CASE(SocketFile)
BOOST_AUTO_TEST_SUITE(SocketFile)

BOOST_AUTO_TEST_CASE(CreateAndRemove)
{
auto channel = makeChannel();
BOOST_CHECK_EQUAL(fs::symlink_status(socketPath).type(), fs::file_not_found);
Expand All @@ -145,35 +158,128 @@ BOOST_AUTO_TEST_CASE(SocketFile)
BOOST_CHECK_EQUAL(fs::symlink_status(socketPath).type(), fs::file_not_found);
}

BOOST_AUTO_TEST_CASE(ExistingSocketFile)
BOOST_AUTO_TEST_CASE(InUse)
{
auto channel = makeChannel();
fs::create_directories(socketPath.parent_path());

local::stream_protocol::acceptor acceptor(g_io, listenerEp);
BOOST_CHECK_EQUAL(fs::symlink_status(socketPath).type(), fs::socket_file);

BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::address_in_use &&
e.path1() == socketPath &&
std::string_view(e.what()).find("UnixStreamChannel::listen") != std::string_view::npos;
});
}

BOOST_AUTO_TEST_CASE(Stale)
{
auto channel = makeChannel();
BOOST_CHECK_THROW(channel->listen(nullptr, nullptr), UnixStreamChannel::Error);
fs::create_directories(socketPath.parent_path());

local::stream_protocol::acceptor acceptor(g_io, listenerEp);
acceptor.close();
// the socket file is not removed when the acceptor is closed
BOOST_CHECK_EQUAL(fs::symlink_status(socketPath).type(), fs::socket_file);

// drop write permission from the parent directory
fs::permissions(socketPath.parent_path(), fs::owner_all & ~fs::owner_write);
// removal of the "stale" socket fails due to insufficient permissions
BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::permission_denied &&
e.path1() == socketPath &&
std::string_view(e.what()).find("remove") != std::string_view::npos;
});
BOOST_CHECK_EQUAL(fs::symlink_status(socketPath).type(), fs::socket_file);

BOOST_CHECK_NO_THROW(channel->listen(nullptr, nullptr));
// restore all permissions
fs::permissions(socketPath.parent_path(), fs::owner_all);
// the socket file should be considered "stale" and overwritten
channel->listen(nullptr, nullptr);
BOOST_CHECK_EQUAL(fs::symlink_status(socketPath).type(), fs::socket_file);
}

BOOST_AUTO_TEST_CASE(ExistingRegularFile)
BOOST_AUTO_TEST_CASE(NotASocket)
{
auto guard = ndn::make_scope_exit([=] { fs::remove(socketPath); });
auto channel = makeChannel();

fs::create_directories(socketPath.parent_path());
fs::create_directories(socketPath);
BOOST_CHECK_EQUAL(fs::symlink_status(socketPath).type(), fs::directory_file);
BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::not_a_socket &&
e.path1() == socketPath &&
std::string_view(e.what()).find("UnixStreamChannel::listen") != std::string_view::npos;
});

fs::remove(socketPath);
std::ofstream f(socketPath.string());
f.close();
BOOST_CHECK_EQUAL(fs::symlink_status(socketPath).type(), fs::regular_file);
BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::not_a_socket &&
e.path1() == socketPath &&
std::string_view(e.what()).find("UnixStreamChannel::listen") != std::string_view::npos;
});
}

BOOST_AUTO_TEST_CASE(ParentConflict)
{
auto channel = makeChannel();
BOOST_CHECK_THROW(channel->listen(nullptr, nullptr), UnixStreamChannel::Error);
fs::create_directories(testDir);

auto parent = socketPath.parent_path();
std::ofstream f(parent.string());
f.close();
BOOST_CHECK_EQUAL(fs::symlink_status(parent).type(), fs::regular_file);
BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::file_exists &&
e.path1() == parent &&
std::string_view(e.what()).find("create_dir") != std::string_view::npos;
});
}

BOOST_AUTO_TEST_CASE(PermissionDenied)
{
auto channel = makeChannel();
fs::create_directories(testDir);

fs::permissions(testDir, fs::no_perms);
BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::permission_denied &&
e.path1() == socketPath.parent_path() &&
std::string_view(e.what()).find("create_dir") != std::string_view::npos;
});

fs::permissions(testDir, fs::owner_read | fs::owner_exe);
BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::permission_denied &&
e.path1() == socketPath.parent_path() &&
std::string_view(e.what()).find("create_dir") != std::string_view::npos;
});

fs::permissions(testDir, fs::owner_all);
fs::create_directories(socketPath.parent_path());

fs::permissions(socketPath.parent_path(), fs::no_perms);
BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::permission_denied &&
e.path1() == socketPath &&
std::string_view(e.what()).find("status") != std::string_view::npos;
});

fs::permissions(socketPath.parent_path(), fs::owner_read | fs::owner_exe);
BOOST_CHECK_EXCEPTION(channel->listen(nullptr, nullptr), fs::filesystem_error, [&] (const auto& e) {
return e.code() == boost::system::errc::permission_denied &&
e.path1() == socketPath &&
std::string_view(e.what()).find("bind") != std::string_view::npos;
});

fs::permissions(socketPath.parent_path(), fs::owner_all);
}

BOOST_AUTO_TEST_SUITE_END() // SocketFile

BOOST_AUTO_TEST_SUITE_END() // TestUnixStreamChannel
BOOST_AUTO_TEST_SUITE_END() // Face

Expand Down

0 comments on commit 4b1921f

Please sign in to comment.