Skip to content

Commit

Permalink
Merge 9e0252e into c118949
Browse files Browse the repository at this point in the history
  • Loading branch information
@mlw
mlw committed Aug 7, 2023
2 parents c118949 + 9e0252e commit a4b9e1a
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 10 deletions.
2 changes: 1 addition & 1 deletion Source/common/TestUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ audit_token_t MakeAuditToken(pid_t pid, pid_t pidver);
struct stat MakeStat(int offset = 0);

es_string_token_t MakeESStringToken(const char *s);
es_file_t MakeESFile(const char *path, struct stat sb = {});
es_file_t MakeESFile(const char *path, struct stat sb = MakeStat());
es_process_t MakeESProcess(es_file_t *file, audit_token_t tok = {}, audit_token_t parent_tok = {});
es_message_t MakeESMessage(es_event_type_t et, es_process_t *proc,
ActionType action_type = ActionType::Notify,
Expand Down
144 changes: 139 additions & 5 deletions Source/santad/EventProviders/SNTEndpointSecurityFileAccessAuthorizer.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,17 @@
#import <MOLCodesignChecker/MOLCodesignChecker.h>
#include <bsm/libbsm.h>
#include <sys/fcntl.h>
#include <sys/types.h>

#include <algorithm>
#include <array>
#include <cstdlib>
#include <memory>
#include <optional>
#include <set>
#include <type_traits>
#include <unordered_map>
#include <unordered_set>
#include <utility>
#include <variant>

#include "Source/common/Platform.h"
Expand Down Expand Up @@ -59,6 +62,13 @@
using santa::santad::event_providers::endpoint_security::Message;
using santa::santad::logs::endpoint_security::Logger;

struct PairHash {
template <typename T1, typename T2>
std::size_t operator()(const std::pair<T1, T2> &pair) const {
return std::hash<T1>()(pair.first) << 1 ^ std::hash<T2>()(pair.second);
}
};

NSString *kBadCertHash = @"BAD_CERT_HASH";

static constexpr uint32_t kOpenFlagsIndicatingWrite = FWRITE | O_APPEND | O_TRUNC;
Expand All @@ -70,6 +80,106 @@
struct PathTarget {
std::string path;
bool isReadable;
std::optional<std::pair<dev_t, ino_t>> devnoIno;
};

// This is a bespoke cache for mapping processes to a set of files the process
// has previously been allowed to read as defined by policy. It has simialr
// semantics to SantaCache in terms of clearing the cache keys and values when
// max sizs are reached.
// TODO: We need a proper LRU cache
//
// NB: SantaCache should not be used here.
// 1.) It doesn't efficiently support non-primitive value types. Since the
// value of each key needs to be a set, we want to refrain from having to
// unnecessarily copy the value.
// 2.) It doesn't support size limits on value types
class ProcessFiles {
using FileSet = std::unordered_set<std::pair<dev_t, ino_t>, PairHash>;

public:
ProcessFiles() {
q_ = dispatch_queue_create(
"com.google.santa.daemon.faa",
dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL,
QOS_CLASS_USER_INTERACTIVE, 0));
};

// Add the given target to the set of files a process can read
void Set(const es_process_t *proc, const PathTarget &target) {
if (!target.devnoIno.has_value()) {
return;
}

std::pair<pid_t, pid_t> pidPidver = {audit_token_to_pid(proc->audit_token),
audit_token_to_pidversion(proc->audit_token)};

dispatch_sync(q_, ^{
// If we hit the size limit, clear the cache to prevent unbounded growth
if (cache_.size() >= kMaxCacheSize) {
ClearLocked();
}

FileSet &fs = cache_[std::move(pidPidver)];

// If we hit the per-entry size limit, clear the entry to prevent unbounded growth
if (fs.size() >= kMaxCacheEntrySize) {
fs.clear();
}

fs.insert(*target.devnoIno);
});
}

// Remove the given process from the cache
void Remove(const es_process_t *proc) {
std::pair<pid_t, pid_t> pidPidver = {audit_token_to_pid(proc->audit_token),
audit_token_to_pidversion(proc->audit_token)};
dispatch_sync(q_, ^{
cache_.erase(pidPidver);
});
}

// Check if the set of files for a given process contains the given file
bool Exists(const es_process_t *proc, const es_file_t *file) {
std::pair<pid_t, pid_t> pidPidver = {audit_token_to_pid(proc->audit_token),
audit_token_to_pidversion(proc->audit_token)};
std::pair<dev_t, ino_t> devnoIno = {file->stat.st_dev, file->stat.st_ino};

__block bool exists = false;

dispatch_sync(q_, ^{
const auto &iter = cache_.find(pidPidver);

if (iter != cache_.end() && iter->second.count(devnoIno) > 0) {
exists = true;
}
});

return exists;
}

// Clear all cache entries
void Clear() {
dispatch_sync(q_, ^{
ClearLocked();
});
}

private:
// Remove everything in the cache.
void ClearLocked() { cache_.clear(); }

dispatch_queue_t q_;
std::unordered_map<std::pair<pid_t, pid_t>, FileSet, PairHash> cache_;

// Cache limits are merely meant to protect against unbounded growth. In practice,
// the observed cache size is typically small for normal WatchItems rules (those
// that do not target high-volume paths). The per entry size was observed to vary
// quite dramatically based on the type of process (e.g. large, complex applications
// were observed to frequently have several thousands of entries).
static constexpr size_t kMaxCacheSize = 512;
static constexpr size_t kMaxCacheEntrySize = 8192;
};

static inline std::string Path(const es_file_t *esFile) {
Expand All @@ -83,14 +193,18 @@
static inline void PushBackIfNotTruncated(std::vector<PathTarget> &vec, const es_file_t *esFile,
bool isReadable = false) {
if (!esFile->path_truncated) {
vec.push_back({Path(esFile), isReadable});
vec.push_back({Path(esFile), isReadable,
isReadable ? std::make_optional<std::pair<dev_t, ino_t>>(
{esFile->stat.st_dev, esFile->stat.st_ino})
: std::nullopt});
}
}

// Note: This variant of PushBackIfNotTruncated can never be marked "isReadable"
static inline void PushBackIfNotTruncated(std::vector<PathTarget> &vec, const es_file_t *dir,
const es_string_token_t &name, bool isReadable = false) {
const es_string_token_t &name) {
if (!dir->path_truncated) {
vec.push_back({Path(dir) + "/" + Path(name), isReadable});
vec.push_back({Path(dir) + "/" + Path(name), false, std::nullopt});
}
}

Expand Down Expand Up @@ -213,6 +327,7 @@ @implementation SNTEndpointSecurityFileAccessAuthorizer {
std::shared_ptr<RateLimiter> _rateLimiter;
SantaCache<SantaVnode, NSString *> _certHashCache;
std::shared_ptr<TTYWriter> _ttyWriter;
ProcessFiles _readsCache;
}

- (instancetype)
Expand Down Expand Up @@ -427,6 +542,11 @@ - (FileAccessPolicyDecision)applyPolicy:

std::shared_ptr<WatchItemPolicy> policy = optionalPolicy.value();

// If policy allows reading, add target to the cache
if (policy->allow_read_access && target.isReadable) {
self->_readsCache.Set(msg->process, target);
}

// Check if this action contains any special case that would produce
// an immediate result.
FileAccessPolicyDecision specialCase = [self specialCaseForPolicy:policy
Expand Down Expand Up @@ -566,6 +686,18 @@ - (void)processMessage:(const Message &)msg {

- (void)handleMessage:(santa::santad::event_providers::endpoint_security::Message &&)esMsg
recordEventMetrics:(void (^)(EventDisposition))recordEventMetrics {
if (esMsg->event_type == ES_EVENT_TYPE_AUTH_OPEN &&
!(esMsg->event.open.fflag & kOpenFlagsIndicatingWrite)) {
if (self->_readsCache.Exists(esMsg->process, esMsg->event.open.file)) {
[self respondToMessage:esMsg withAuthResult:ES_AUTH_RESULT_ALLOW cacheable:false];
return;
}
} else if (esMsg->event_type == ES_EVENT_TYPE_NOTIFY_EXIT) {
// On process exit, remove the cache entry
self->_readsCache.Remove(esMsg->process);
return;
}

[self processMessage:std::move(esMsg)
handler:^(const Message &msg) {
[self processMessage:msg];
Expand All @@ -577,7 +709,7 @@ - (void)enable {
std::set<es_event_type_t> events = {
ES_EVENT_TYPE_AUTH_CLONE, ES_EVENT_TYPE_AUTH_CREATE, ES_EVENT_TYPE_AUTH_EXCHANGEDATA,
ES_EVENT_TYPE_AUTH_LINK, ES_EVENT_TYPE_AUTH_OPEN, ES_EVENT_TYPE_AUTH_RENAME,
ES_EVENT_TYPE_AUTH_TRUNCATE, ES_EVENT_TYPE_AUTH_UNLINK,
ES_EVENT_TYPE_AUTH_TRUNCATE, ES_EVENT_TYPE_AUTH_UNLINK, ES_EVENT_TYPE_NOTIFY_EXIT,
};

#if HAVE_MACOS_12
Expand Down Expand Up @@ -621,6 +753,8 @@ - (void)watchItemsCount:(size_t)count
// begin receiving events (if not already)
[self enable];
}

self->_readsCache.Clear();
}

@end
32 changes: 28 additions & 4 deletions Source/santad/EventProviders/SNTEndpointSecurityFileAccessAuthorizerTest.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <sys/fcntl.h>
#include <sys/types.h>
#include <cstring>

#include <array>
Expand Down Expand Up @@ -50,6 +51,7 @@
struct PathTarget {
std::string path;
bool isReadable;
std::optional<std::pair<dev_t, ino_t>> devnoIno;
};

using PathTargetsPair = std::pair<std::optional<std::string>, std::optional<std::string>>;
Expand All @@ -59,6 +61,10 @@
extern bool ShouldNotifyUserDecision(FileAccessPolicyDecision decision);
extern es_auth_result_t CombinePolicyResults(es_auth_result_t result1, es_auth_result_t result2);

static inline std::pair<dev_t, ino_t> FileID(const es_file_t &file) {
return std::make_pair(file.stat.st_dev, file.stat.st_ino);
}

void SetExpectationsForFileAccessAuthorizerInit(
std::shared_ptr<MockEndpointSecurityAPI> mockESApi) {
EXPECT_CALL(*mockESApi, InvertTargetPathMuting).WillOnce(testing::Return(true));
Expand Down Expand Up @@ -663,7 +669,7 @@ - (void)testEnable {
std::set<es_event_type_t> expectedEventSubs = {
ES_EVENT_TYPE_AUTH_CLONE, ES_EVENT_TYPE_AUTH_CREATE, ES_EVENT_TYPE_AUTH_EXCHANGEDATA,
ES_EVENT_TYPE_AUTH_LINK, ES_EVENT_TYPE_AUTH_OPEN, ES_EVENT_TYPE_AUTH_RENAME,
ES_EVENT_TYPE_AUTH_TRUNCATE, ES_EVENT_TYPE_AUTH_UNLINK,
ES_EVENT_TYPE_AUTH_TRUNCATE, ES_EVENT_TYPE_AUTH_UNLINK, ES_EVENT_TYPE_NOTIFY_EXIT,
};

#if HAVE_MACOS_12
Expand Down Expand Up @@ -714,9 +720,9 @@ - (void)testDisable {
- (void)testGetPathTargets {
// This test ensures that the `GetPathTargets` functions returns the
// expected combination of targets for each handled event variant
es_file_t testFile1 = MakeESFile("test_file_1");
es_file_t testFile2 = MakeESFile("test_file_2");
es_file_t testDir = MakeESFile("test_dir");
es_file_t testFile1 = MakeESFile("test_file_1", MakeStat(100));
es_file_t testFile2 = MakeESFile("test_file_2", MakeStat(200));
es_file_t testDir = MakeESFile("test_dir", MakeStat(300));
es_string_token_t testTok = MakeESStringToken("test_tok");
std::string dirTok = std::string(testDir.path.data) + "/" + std::string(testTok.data);

Expand All @@ -737,6 +743,7 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 1);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertTrue(targets[0].isReadable);
XCTAssertEqual(targets[0].devnoIno.value(), FileID(testFile1));
}

{
Expand All @@ -751,8 +758,10 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 2);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertFalse(targets[0].isReadable);
XCTAssertFalse(targets[0].devnoIno.has_value());
XCTAssertCppStringEqual(targets[1].path, dirTok);
XCTAssertFalse(targets[1].isReadable);
XCTAssertFalse(targets[1].devnoIno.has_value());
}

{
Expand All @@ -769,8 +778,10 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 2);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertFalse(targets[0].isReadable);
XCTAssertFalse(targets[0].devnoIno.has_value());
XCTAssertCStringEqual(targets[1].path.c_str(), testFile2.path.data);
XCTAssertFalse(targets[1].isReadable);
XCTAssertFalse(targets[1].devnoIno.has_value());
}

{
Expand All @@ -784,8 +795,10 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 2);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertFalse(targets[0].isReadable);
XCTAssertFalse(targets[0].devnoIno.has_value());
XCTAssertCppStringEqual(targets[1].path, dirTok);
XCTAssertFalse(targets[1].isReadable);
XCTAssertFalse(targets[1].devnoIno.has_value());
}
}

Expand All @@ -799,6 +812,7 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 1);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertFalse(targets[0].isReadable);
XCTAssertFalse(targets[0].devnoIno.has_value());
}

{
Expand All @@ -813,8 +827,10 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 2);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertTrue(targets[0].isReadable);
XCTAssertEqual(targets[0].devnoIno.value(), FileID(testFile1));
XCTAssertCppStringEqual(targets[1].path, dirTok);
XCTAssertFalse(targets[1].isReadable);
XCTAssertFalse(targets[1].devnoIno.has_value());
}

{
Expand All @@ -828,8 +844,10 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 2);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertFalse(targets[0].isReadable);
XCTAssertFalse(targets[0].devnoIno.has_value());
XCTAssertCStringEqual(targets[1].path.c_str(), testFile2.path.data);
XCTAssertFalse(targets[1].isReadable);
XCTAssertFalse(targets[1].devnoIno.has_value());
}

{
Expand All @@ -844,6 +862,7 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 1);
XCTAssertCppStringEqual(targets[0].path, dirTok);
XCTAssertFalse(targets[0].isReadable);
XCTAssertFalse(targets[0].devnoIno.has_value());
}

{
Expand All @@ -856,6 +875,7 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 1);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertFalse(targets[0].isReadable);
XCTAssertFalse(targets[0].devnoIno.has_value());
}

if (@available(macOS 12.0, *)) {
Expand All @@ -874,8 +894,10 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 2);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertTrue(targets[0].isReadable);
XCTAssertEqual(targets[0].devnoIno.value(), FileID(testFile1));
XCTAssertCppStringEqual(targets[1].path, dirTok);
XCTAssertFalse(targets[1].isReadable);
XCTAssertFalse(targets[1].devnoIno.has_value());
}

{
Expand All @@ -887,8 +909,10 @@ - (void)testGetPathTargets {
XCTAssertEqual(targets.size(), 2);
XCTAssertCStringEqual(targets[0].path.c_str(), testFile1.path.data);
XCTAssertTrue(targets[0].isReadable);
XCTAssertEqual(targets[0].devnoIno.value(), FileID(testFile1));
XCTAssertCStringEqual(targets[1].path.c_str(), testFile2.path.data);
XCTAssertFalse(targets[1].isReadable);
XCTAssertFalse(targets[1].devnoIno.has_value());
}
}
}
Expand Down

0 comments on commit a4b9e1a

Please sign in to comment.