Formatting fixes and include file hygiene.

PiperOrigin-RevId: 240346890
Change-Id: I1a9617f10a62a848b6314a6196512e016ae02643
This commit is contained in:
Christian Blichmann 2019-03-26 07:54:02 -07:00 committed by Copybara-Service
parent 33206c5d3f
commit f04be9276f
41 changed files with 99 additions and 65 deletions

View File

@ -15,7 +15,6 @@
#ifndef SANDBOXED_API_EMBED_FILE_H_
#define SANDBOXED_API_EMBED_FILE_H_
#include <string>
#include <vector>
#include "sandboxed_api/file_toc.h"

View File

@ -50,7 +50,7 @@ extern "C" int reverse_string(sapi::LenValStruct* input) {
// Free old value.
free(input->data);
// Replace pointer to our new std::string.
// Replace pointer to our new string.
input->data = new_buf;
return 1;
}

View File

@ -16,6 +16,7 @@
#define SANDBOXED_API_SANDBOX_H_
#include <memory>
#include <string>
#include <vector>
#include "sandboxed_api/file_toc.h"
@ -71,7 +72,8 @@ class Sandbox {
// Makes a call to the sandboxee.
template <typename... Args>
::sapi::Status Call(const std::string& func, v::Callable* ret, Args&&... args) {
::sapi::Status Call(const std::string& func, v::Callable* ret,
Args&&... args) {
static_assert(sizeof...(Args) <= FuncCall::kArgsMax,
"Too many arguments to sapi::Sandbox::Call()");
return Call(func, ret, {std::forward<Args>(args)...});

View File

@ -500,7 +500,7 @@ socklen_t Comms::CreateSockaddrUn(sockaddr_un* sun) {
bzero(sun->sun_path, sizeof(sun->sun_path));
// Create an 'abstract socket address' by specifying a leading null byte. The
// remainder of the path is used as a unique name, but no file is created on
// the filesystem. No need to NUL-terminate the std::string.
// the filesystem. No need to NUL-terminate the string.
// See `man 7 unix` for further explanation.
strncpy(&sun->sun_path[1], socket_name_.c_str(), sizeof(sun->sun_path) - 1);

View File

@ -23,9 +23,9 @@ This is the simplest and safest way to use sandboxing. For examples see
#include "sandboxed_api/sandbox2/executor.h"
std::string path = "path/to/binary";
std::vector<string> args = {path}; // args[0] will become the sandboxed
// process' argv[0], typically the path
// to the binary.
std::vector<std::string> args = {path}; // args[0] will become the sandboxed
// process' argv[0], typically the
// path to the binary.
auto executor = absl::make_unique<sandbox2::Executor>(path, args);
```

View File

@ -105,9 +105,11 @@ class Executor final {
/*fork_client=*/nullptr) {}
// Delegate constructor that gets called by the public ones.
Executor(int exec_fd, const std::string& path, const std::vector<std::string>& argv,
const std::vector<std::string>& envp, bool enable_sandboxing_pre_execve,
pid_t libunwind_sbox_for_pid, ForkClient* fork_client);
Executor(int exec_fd, const std::string& path,
const std::vector<std::string>& argv,
const std::vector<std::string>& envp,
bool enable_sandboxing_pre_execve, pid_t libunwind_sbox_for_pid,
ForkClient* fork_client);
// Creates a copy of the environment
static std::vector<std::string> CopyEnviron();

View File

@ -19,6 +19,7 @@
#define SANDBOXED_API_SANDBOX2_FORKSERVER_H_
#include <sys/types.h>
#include <string>
#include <vector>

View File

@ -93,7 +93,8 @@ Monitor::Monitor(Executor* executor, Policy* policy, Notify* notify)
setup_counter_(new absl::BlockingCounter(1)),
done_(false),
wait_for_execve_(executor->enable_sandboxing_pre_execve_) {
std::string path = absl::GetFlag(FLAGS_sandbox2_danger_danger_permit_all_and_log);
std::string path =
absl::GetFlag(FLAGS_sandbox2_danger_danger_permit_all_and_log);
if (!path.empty()) {
log_file_ = std::fopen(path.c_str(), "a+");
PCHECK(log_file_ != nullptr) << "Failed to open log file '" << path << "'";

View File

@ -192,7 +192,7 @@ std::string GetPlatform(absl::string_view interpreter) {
auto split = file::SplitPath(fixed_path);
absl::string_view cur = split.first;
std::string final_part = std::string(split.second);
auto final_part = std::string(split.second);
while (cur != "/") {
auto split = file::SplitPath(cur);

View File

@ -29,7 +29,6 @@
using sapi::IsOk;
using sapi::StatusIs;
using ::testing::Eq;
using ::testing::StrEq;
namespace sandbox2 {
namespace {

View File

@ -29,7 +29,6 @@
#include <cstdio>
#include <cstring>
#include <string>
#include <utility>
#include "absl/strings/str_cat.h"

View File

@ -22,6 +22,7 @@
#include <cstdint>
#include <memory>
#include <string>
#include "absl/base/macros.h"
#include "sandboxed_api/sandbox2/mounts.h"
@ -40,7 +41,8 @@ class Namespace final {
Namespace(const Namespace&) = delete;
Namespace& operator=(const Namespace&) = delete;
Namespace(bool allow_unrestricted_networking, Mounts mounts, std::string hostname);
Namespace(bool allow_unrestricted_networking, Mounts mounts,
std::string hostname);
void DisableUserNamespace();

View File

@ -186,7 +186,8 @@ TEST(MinimalTest, MinimalBinaryWorks) {
// Test that we can sandbox a minimal non-static binary returning 0.
TEST(MinimalTest, MinimalSharedBinaryWorks) {
SKIP_SANITIZERS_AND_COVERAGE;
const std::string path = GetTestSourcePath("sandbox2/testcases/minimal_dynamic");
const std::string path =
GetTestSourcePath("sandbox2/testcases/minimal_dynamic");
std::vector<std::string> args = {path};
auto executor = absl::make_unique<Executor>(path, args);
@ -212,7 +213,8 @@ TEST(MinimalTest, MinimalSharedBinaryWorks) {
// Test that the AllowSystemMalloc helper works as expected.
TEST(MallocTest, SystemMallocWorks) {
SKIP_SANITIZERS_AND_COVERAGE;
const std::string path = GetTestSourcePath("sandbox2/testcases/malloc_system");
const std::string path =
GetTestSourcePath("sandbox2/testcases/malloc_system");
std::vector<std::string> args = {path};
auto executor = absl::make_unique<Executor>(path, args);

View File

@ -624,7 +624,8 @@ PolicyBuilder& PolicyBuilder::DangerDefaultAllowAll() {
return ValidatePath(path);
}
::sapi::StatusOr<std::string> PolicyBuilder::ValidatePath(absl::string_view path) {
::sapi::StatusOr<std::string> PolicyBuilder::ValidatePath(
absl::string_view path) {
std::string fixed_path = file::CleanPath(path);
if (fixed_path != path) {
return ::sapi::InvalidArgumentError(absl::StrCat(

View File

@ -487,7 +487,8 @@ class PolicyBuilder final {
std::vector<sock_filter> ResolveBpfFunc(BpfFunc f);
static ::sapi::StatusOr<std::string> ValidateAbsolutePath(absl::string_view path);
static ::sapi::StatusOr<std::string> ValidateAbsolutePath(
absl::string_view path);
static ::sapi::StatusOr<std::string> ValidatePath(absl::string_view path);
void StoreDescription(PolicyBuilderDescription* pb_description);

View File

@ -17,6 +17,7 @@
#include <syscall.h>
#include <unistd.h>
#include <string>
#include <utility>
#include <glog/logging.h>
@ -56,7 +57,8 @@ class PolicyBuilderPeer {
int policy_size() const { return builder_->output_->user_policy_.size(); }
static ::sapi::StatusOr<std::string> ValidateAbsolutePath(absl::string_view path) {
static ::sapi::StatusOr<std::string> ValidateAbsolutePath(
absl::string_view path) {
return PolicyBuilder::ValidateAbsolutePath(path);
}
@ -154,7 +156,8 @@ TEST_F(PolicyBuilderTest, TestValidateAbsolutePath) {
}
}
std::string PolicyBuilderTest::Run(std::vector<std::string> args, bool network) {
std::string PolicyBuilderTest::Run(std::vector<std::string> args,
bool network) {
PolicyBuilder builder;
// Don't restrict the syscalls at all.
builder.DangerDefaultAllowAll();

View File

@ -107,7 +107,9 @@ class Result {
// The stacktrace must be sometimes fetched before SetExitStatusCode is
// called, because after WIFEXITED() or WIFSIGNALED() the process is just a
// zombie.
void SetStackTrace(const std::string& stack_trace) { stack_trace_ = stack_trace; }
void SetStackTrace(const std::string& stack_trace) {
stack_trace_ = stack_trace;
}
void LoadRegs(pid_t pid) {
auto regs = absl::make_unique<Regs>(pid);
@ -150,13 +152,13 @@ class Result {
// OK if the sandbox process exited normally with an exit code of 0.
::sapi::Status ToStatus() const;
// Returns a descriptive std::string for final result.
// Returns a descriptive string for final result.
std::string ToString() const;
// Converts StatusEnum to a std::string.
// Converts StatusEnum to a string.
static std::string StatusEnumToString(StatusEnum value);
// Converts ReasonCodeEnum to a std::string.
// Converts ReasonCodeEnum to a string.
static std::string ReasonCodeEnumToString(ReasonCodeEnum value);
rusage* GetRUsageMonitor() { return &rusage_monitor_; }

View File

@ -59,8 +59,8 @@ TEST(SandboxCoreDumpTest, AbortWithoutCoreDumpReturnsSignaled) {
Sandbox2 sandbox(std::move(executor), std::move(policy));
auto result = sandbox.Run();
ASSERT_EQ(result.final_status(), Result::SIGNALED);
EXPECT_EQ(result.reason_code(), SIGABRT);
ASSERT_THAT(result.final_status(), Eq(Result::SIGNALED));
EXPECT_THAT(result.reason_code(), Eq(SIGABRT));
}
// Test that with TSYNC we are able to sandbox when multithreaded and with no

View File

@ -31,6 +31,7 @@
#include <cstring>
#include <fstream>
#include <sstream>
#include <string>
#include <utility>
#include <vector>

View File

@ -19,7 +19,6 @@
#define SANDBOXED_API_SANDBOX2_SANITIZER_H_
#include <set>
#include <string>
#include "absl/base/macros.h"

View File

@ -51,7 +51,7 @@ namespace sandbox2 {
namespace {
// Runs a new process and returns 0 if the process terminated with 0.
static int RunTestcase(const std::string& path, const std::vector<std::string>& args) {
int RunTestcase(const std::string& path, const std::vector<std::string>& args) {
pid_t pid = fork();
if (pid < 0) {
PLOG(ERROR) << "fork()";
@ -79,7 +79,7 @@ static int RunTestcase(const std::string& path, const std::vector<std::string>&
}
}
static bool IsFdOpen(int fd) {
bool IsFdOpen(int fd) {
int ret = fcntl(fd, F_GETFD);
if (ret == -1) {
VLOG(1) << "FD: " << fd << " is closed";

View File

@ -62,7 +62,8 @@ class StackTracePeer {
const Mounts& mounts);
static bool LaunchLibunwindSandbox(const Regs* regs, const Mounts& mounts,
UnwindResult* result, const std::string& delim);
UnwindResult* result,
const std::string& delim);
};
std::unique_ptr<Policy> StackTracePeer::GetPolicy(pid_t target_pid,
@ -269,7 +270,7 @@ bool StackTracePeer::LaunchLibunwindSandbox(const Regs* regs,
}
std::string GetStackTrace(const Regs* regs, const Mounts& mounts,
const std::string& delim) {
const std::string& delim) {
if (absl::GetFlag(FLAGS_sandbox_disable_all_stack_traces)) {
return "";
}

View File

@ -40,7 +40,7 @@ constexpr size_t kDefaultMaxFrames = 200;
// Returns the stack-trace of the PID=pid, delimited by the delim argument.
std::string GetStackTrace(const Regs* regs, const Mounts& mounts,
const std::string& delim = " ");
const std::string& delim = " ");
// Similar to GetStackTrace() but without using the sandbox to isolate
// libunwind.

View File

@ -153,7 +153,8 @@ TEST(StackTraceTest, ForkEnterNsLibunwindDoesNotLeakFDs) {
SKIP_SANITIZERS_AND_COVERAGE;
// Get list of open FDs in the global forkserver.
pid_t forkserver_pid = GetGlobalForkServerPid();
std::string forkserver_fd_path = absl::StrCat("/proc/", forkserver_pid, "/fd");
std::string forkserver_fd_path =
absl::StrCat("/proc/", forkserver_pid, "/fd");
size_t filecount_before = FileCountInDirectory(forkserver_fd_path);
TemporaryFlagOverride<bool> temp_override(

View File

@ -11,7 +11,7 @@ namespace sandbox2 {
namespace {
std::string GetArgumentDescription(uint64_t value, SyscallTable::ArgType type,
pid_t pid) {
pid_t pid) {
std::string ret = absl::StrFormat("%#x", value);
switch (type) {
case SyscallTable::kOct:

View File

@ -15,8 +15,8 @@
// A binary that uses comms and client, to receive FDs by name, communicate
// with them, sandboxed or not.
#include <stdio.h>
#include <stdlib.h>
#include <cstdio>
#include <cstdlib>
#include <string>
#include "absl/strings/numbers.h"

View File

@ -35,7 +35,7 @@ namespace sandbox2 {
namespace {
std::string GetSymbolAt(const std::map<uint64_t, std::string>& addr_to_symbol,
uint64_t addr) {
uint64_t addr) {
auto entry_for_next_symbol = addr_to_symbol.lower_bound(addr);
if (entry_for_next_symbol != addr_to_symbol.end() &&
entry_for_next_symbol != addr_to_symbol.begin()) {

View File

@ -296,7 +296,7 @@ std::string GetSignalName(int signo) {
auto pos = path.find('\0');
if (pos == std::string::npos) {
return ::sapi::FailedPreconditionError(absl::StrCat(
"No NUL-byte inside the C std::string '", absl::CHexEscape(path), "'"));
"No NUL-byte inside the C string '", absl::CHexEscape(path), "'"));
}
path.resize(pos);
return path;

View File

@ -31,7 +31,7 @@ namespace sandbox2 {
namespace util {
// Converts an array of char* (terminated by a nullptr, like argv, or environ
// arrays), to an std::vector<string>.
// arrays), to an std::vector<std::string>.
void CharPtrArrToVecString(char* const* arr, std::vector<std::string>* vec);
// Converts a vector of strings to a newly allocated array. The array is limited

View File

@ -59,7 +59,8 @@ bool GetCWD(std::string* result) {
// Makes a path absolute with respect to base. Returns true on success. Result
// may be an alias of base or filename.
bool MakeAbsolute(const std::string& filename, const std::string& base, std::string* result) {
bool MakeAbsolute(const std::string& filename, const std::string& base,
std::string* result) {
if (filename.empty()) {
return false;
}
@ -95,10 +96,10 @@ std::string MakeAbsolute(const std::string& filename, const std::string& base) {
}
bool RemoveLastPathComponent(const std::string& file, std::string* output) {
// Point idx at the last non-slash in the std::string. This should mark the last
// Point idx at the last non-slash in the string. This should mark the last
// character of the base name.
auto idx = file.find_last_not_of('/');
// If no non-slash is found, we have all slashes or an empty std::string. Return
// If no non-slash is found, we have all slashes or an empty string. Return
// the appropriate value and false to indicate there was no path component to
// remove.
if (idx == std::string::npos) {
@ -114,7 +115,7 @@ bool RemoveLastPathComponent(const std::string& file, std::string* output) {
// Point idx at the last slash before the base name.
idx = file.find_last_of('/', idx);
// If we don't find a slash, then we have something of the form "file/*", so
// just return the empty std::string.
// just return the empty string.
if (idx == std::string::npos) {
output->clear();
} else {
@ -168,8 +169,8 @@ bool ReadLinkAbsolute(const std::string& filename, std::string* result) {
std::string Basename(absl::string_view path) {
const auto last_slash = path.find_last_of('/');
return std::string(last_slash == std::string::npos
? path
: absl::ClippedSubstr(path, last_slash + 1));
? path
: absl::ClippedSubstr(path, last_slash + 1));
}
std::string StripBasename(absl::string_view path) {
@ -189,7 +190,8 @@ bool Exists(const std::string& filename, bool fully_resolve) {
: lstat64(filename.c_str(), &st)) != -1;
}
bool ListDirectoryEntries(const std::string& directory, std::vector<std::string>* entries,
bool ListDirectoryEntries(const std::string& directory,
std::vector<std::string>* entries,
std::string* error) {
errno = 0;
std::unique_ptr<DIR, void (*)(DIR*)> dir{opendir(directory.c_str()),
@ -266,7 +268,8 @@ bool DeleteRecursively(const std::string& filename) {
return true;
}
bool CopyFile(const std::string& old_path, const std::string& new_path, int new_mode) {
bool CopyFile(const std::string& old_path, const std::string& new_path,
int new_mode) {
{
std::ifstream input(old_path, std::ios_base::binary);
std::ofstream output(new_path,

View File

@ -72,7 +72,8 @@ bool Exists(const std::string& filename, bool fully_resolve);
// On error, false is returned and error is set to a description of the
// error. The filenames in entries are just the basenames of the
// files found.
bool ListDirectoryEntries(const std::string& directory, std::vector<std::string>* entries,
bool ListDirectoryEntries(const std::string& directory,
std::vector<std::string>* entries,
std::string* error);
// Deletes the specified file or directory, including any sub-directories.
@ -81,7 +82,8 @@ bool DeleteRecursively(const std::string& filename);
// Copies a file from one location to another. The file will be overwritten if
// it already exists. If it does not exist, its mode will be new_mode. Returns
// true on success. On failure, a partial copy of the file may remain.
bool CopyFile(const std::string& old_path, const std::string& new_path, int new_mode);
bool CopyFile(const std::string& old_path, const std::string& new_path,
int new_mode);
// Makes filename absolute with respect to base. Returns an empty string on
// failure.

View File

@ -18,16 +18,18 @@
namespace sandbox2 {
::sapi::StatusOr<std::vector<MapsEntry>> ParseProcMaps(const std::string& contents) {
// Note: The format std::string
::sapi::StatusOr<std::vector<MapsEntry>> ParseProcMaps(
const std::string& contents) {
// Note: The format string
// https://github.com/torvalds/linux/blob/v4.14/fs/proc/task_mmu.c#L289
// changed to a non-format std::string implementation
// changed to a non-format string implementation
// (show_vma_header_prefix()).
static constexpr char kFormatString[] =
"%lx-%lx %c%c%c%c %lx %x:%x %lu %1023s";
static constexpr size_t kFilepathLength = 1023;
std::vector<std::string> lines = absl::StrSplit(contents, '\n', absl::SkipEmpty());
std::vector<std::string> lines =
absl::StrSplit(contents, '\n', absl::SkipEmpty());
std::vector<MapsEntry> entries;
for (const auto& line : lines) {
MapsEntry entry{};

View File

@ -16,6 +16,7 @@
#define SANDBOXED_API_SANDBOX2_UTIL_MAPS_PARSER_H_
#include <cstdint>
#include <string>
#include <vector>
#include "sandboxed_api/util/status.h"
@ -37,7 +38,8 @@ struct MapsEntry {
std::string path;
};
::sapi::StatusOr<std::vector<MapsEntry>> ParseProcMaps(const std::string& contents);
::sapi::StatusOr<std::vector<MapsEntry>> ParseProcMaps(
const std::string& contents);
} // namespace sandbox2

View File

@ -67,7 +67,7 @@ std::pair<absl::string_view, absl::string_view> SplitPath(
}
std::string CleanPath(const absl::string_view unclean_path) {
std::string path = std::string(unclean_path);
auto path = std::string(unclean_path);
const char* src = path.c_str();
std::string::iterator dst = path.begin();

View File

@ -45,7 +45,7 @@ TEST(StrErrorTest, InvalidErrorCode) {
TEST(StrErrorTest, MultipleThreads) {
// In this test, we will start up 2 threads and have each one call StrError
// 1000 times, each time with a different errnum. We expect that
// StrError(errnum) will return a std::string equal to the one returned by
// StrError(errnum) will return a string equal to the one returned by
// strerror(errnum), if the code is known. Since strerror is known to be
// thread-hostile, collect all the expected strings up front.
constexpr int kNumCodes = 1000;

View File

@ -43,7 +43,8 @@ sapi::StatusOr<std::pair<std::string, int>> CreateNamedTempFile(
return std::pair<std::string, int>{std::move(name_template), fd};
}
sapi::StatusOr<std::string> CreateNamedTempFileAndClose(absl::string_view prefix) {
sapi::StatusOr<std::string> CreateNamedTempFileAndClose(
absl::string_view prefix) {
auto result_or = CreateNamedTempFile(prefix);
if (result_or.ok()) {
std::string path;

View File

@ -15,6 +15,8 @@
#ifndef SANDBOXED_API_SANDBOX2_UTIL_TEMP_FILE_H_
#define SANDBOXED_API_SANDBOX2_UTIL_TEMP_FILE_H_
#include <string>
#include "sandboxed_api/util/statusor.h"
namespace sandbox2 {
@ -27,7 +29,8 @@ sapi::StatusOr<std::pair<std::string, int>> CreateNamedTempFile(
// Creates a temporary file under a path starting with prefix. File is not
// unlinked and its path is returned. FD of the created file is closed just
// after creation.
sapi::StatusOr<std::string> CreateNamedTempFileAndClose(absl::string_view prefix);
sapi::StatusOr<std::string> CreateNamedTempFileAndClose(
absl::string_view prefix);
// Creates a temporary directory under a path starting with prefix.
// Returns the path of the created directory.

View File

@ -16,6 +16,8 @@
#include <unistd.h>
#include <string>
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "sandboxed_api/sandbox2/testing.h"

View File

@ -44,9 +44,9 @@ struct status_type_traits {
template <typename StatusU>
static auto CheckMinimalApi(...) -> decltype(std::false_type());
using minimal_api_type = decltype(
CheckMinimalApi<StatusT>(static_cast<StatusT*>(0), static_cast<int*>(0),
static_cast<std::string*>(0), static_cast<bool*>(0)));
using minimal_api_type = decltype(CheckMinimalApi<StatusT>(
static_cast<StatusT*>(0), static_cast<int*>(0),
static_cast<std::string*>(0), static_cast<bool*>(0)));
public:
static constexpr bool is_status = minimal_api_type::value;

View File

@ -106,7 +106,9 @@ struct StringCtor {
struct StringVectorCtor {
using value_type = std::vector<std::string>;
std::vector<std::string> operator()() { return {kStringElement, kErrorMessage}; }
std::vector<std::string> operator()() {
return {kStringElement, kErrorMessage};
}
};
bool operator==(const Foo& lhs, const Foo& rhs) {

View File

@ -16,6 +16,7 @@
#define SANDBOXED_API_VAR_ABSTRACT_H_
#include <memory>
#include <string>
#include <type_traits>
#include "absl/base/macros.h"
@ -55,10 +56,10 @@ class Var {
// Returns the type of the variable.
virtual Type GetType() const = 0;
// Returns a std::string representation of the variable type.
// Returns a string representation of the variable type.
virtual std::string GetTypeString() const = 0;
// Returns a std::string representation of the variable value.
// Returns a string representation of the variable value.
virtual std::string ToString() const = 0;
virtual ~Var();