From 67883c5200ee22410086cbf0581be7c1c96d50d8 Mon Sep 17 00:00:00 2001 From: Russell Hancox Date: Wed, 15 May 2024 16:27:28 -0400 Subject: [PATCH 1/4] GUI: Fix unicode rendering of attributed messages (#1351) Also added a test to stop this from happening again --- Source/common/BUILD | 5 ++++- Source/common/SNTBlockMessage.m | 6 +++++- Source/common/SNTBlockMessageTest.m | 6 ++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Source/common/BUILD b/Source/common/BUILD index 8b4911a86..8c2c3e739 100644 --- a/Source/common/BUILD +++ b/Source/common/BUILD @@ -478,13 +478,16 @@ santa_unit_test( name = "SNTBlockMessageTest", srcs = ["SNTBlockMessageTest.m"], deps = [ - ":SNTBlockMessage", + ":SNTBlockMessage_SantaGUI", ":SNTConfigurator", ":SNTFileAccessEvent", ":SNTStoredEvent", ":SNTSystemInfo", "@OCMock", ], + sdk_frameworks = [ + "AppKit", + ], ) santa_unit_test( diff --git a/Source/common/SNTBlockMessage.m b/Source/common/SNTBlockMessage.m index c5471729f..c122ab34b 100644 --- a/Source/common/SNTBlockMessage.m +++ b/Source/common/SNTBlockMessage.m @@ -56,7 +56,11 @@ + (NSAttributedString *)formatMessage:(NSString *)message { #ifdef SANTAGUI NSData *htmlData = [fullHTML dataUsingEncoding:NSUTF8StringEncoding]; - return [[NSAttributedString alloc] initWithHTML:htmlData documentAttributes:NULL]; + NSDictionary *options = @{ + NSDocumentTypeDocumentAttribute : NSHTMLTextDocumentType, + NSCharacterEncodingDocumentAttribute : @(NSUTF8StringEncoding), + }; + return [[NSAttributedString alloc] initWithHTML:htmlData options:options documentAttributes:NULL]; #else NSString *strippedHTML = [self stringFromHTML:fullHTML]; if (!strippedHTML) { diff --git a/Source/common/SNTBlockMessageTest.m b/Source/common/SNTBlockMessageTest.m index 3bd0cd3d7..617cf0c95 100644 --- a/Source/common/SNTBlockMessageTest.m +++ b/Source/common/SNTBlockMessageTest.m @@ -39,6 +39,12 @@ - (void)setUp { OCMStub([self.mockSystemInfo serialNumber]).andReturn(@"my_s"); } +- (void)testFormatMessage { + NSString *input = @"Testing with somé Ünicode çharacters"; + NSAttributedString *got = [SNTBlockMessage formatMessage:input]; + XCTAssertEqualObjects([got string], input); +} + - (void)testEventDetailURLForEvent { SNTStoredEvent *se = [[SNTStoredEvent alloc] init]; From 9b184ed4fb9e4e720699b905f7b04f5c7b5cf018 Mon Sep 17 00:00:00 2001 From: Matt W <436037+mlw@users.noreply.github.com> Date: Thu, 16 May 2024 16:13:29 -0400 Subject: [PATCH 2/4] Add metric for when the file on disk is not the file being evaluated (#1348) * Add metrics for stat change detection * Fix test related issues due to partially constructed messages * lint * Convert errno to enum class StatResult * Cleanup from PR feedback --- Source/santad/BUILD | 1 + .../EventProviders/EndpointSecurity/Message.h | 13 ++++ .../EndpointSecurity/Message.mm | 26 +++++++ .../SNTEndpointSecurityClient.mm | 9 ++- Source/santad/Metrics.h | 22 +++++- Source/santad/Metrics.mm | 58 +++++++++++++- Source/santad/MetricsTest.mm | 75 +++++++++++++++---- Source/santad/SNTExecutionController.mm | 8 +- Source/santad/SNTPolicyProcessor.h | 7 ++ Source/santad/SNTPolicyProcessor.mm | 44 ++++++++--- 10 files changed, 225 insertions(+), 38 deletions(-) diff --git a/Source/santad/BUILD b/Source/santad/BUILD index d0d73b7dc..814e15c3a 100644 --- a/Source/santad/BUILD +++ b/Source/santad/BUILD @@ -657,6 +657,7 @@ objc_library( hdrs = ["EventProviders/EndpointSecurity/Message.h"], deps = [ ":EndpointSecurityClient", + ":Metrics", ":WatchItemPolicy", "//Source/santad/ProcessTree:process_tree", ], diff --git a/Source/santad/EventProviders/EndpointSecurity/Message.h b/Source/santad/EventProviders/EndpointSecurity/Message.h index 58f860c6c..4b1add8c6 100644 --- a/Source/santad/EventProviders/EndpointSecurity/Message.h +++ b/Source/santad/EventProviders/EndpointSecurity/Message.h @@ -20,6 +20,7 @@ #include #include +#include "Source/santad/Metrics.h" #include "Source/santad/ProcessTree/process_tree.h" namespace santa::santad::event_providers::endpoint_security { @@ -53,12 +54,24 @@ class Message { std::string ParentProcessName() const; + void UpdateStatState(santa::santad::StatChangeStep step) const; + + inline santa::santad::StatChangeStep StatChangeStep() const { + return stat_change_step_; + } + inline StatResult StatError() const { return stat_result_; } + private: std::shared_ptr esapi_; const es_message_t* es_msg_; std::optional process_token_; std::string GetProcessName(pid_t pid) const; + + mutable santa::santad::StatChangeStep stat_change_step_ = + santa::santad::StatChangeStep::kNoChange; + mutable santa::santad::StatResult stat_result_ = + santa::santad::StatResult::kOK; }; } // namespace santa::santad::event_providers::endpoint_security diff --git a/Source/santad/EventProviders/EndpointSecurity/Message.mm b/Source/santad/EventProviders/EndpointSecurity/Message.mm index 5b44275bc..caa6f9647 100644 --- a/Source/santad/EventProviders/EndpointSecurity/Message.mm +++ b/Source/santad/EventProviders/EndpointSecurity/Message.mm @@ -16,6 +16,7 @@ #include #include +#include #include "Source/santad/EventProviders/EndpointSecurity/EndpointSecurityAPI.h" @@ -24,6 +25,7 @@ Message::Message(std::shared_ptr esapi, const es_message_t *es_msg) : esapi_(std::move(esapi)), es_msg_(es_msg), process_token_(std::nullopt) { esapi_->RetainMessage(es_msg); + UpdateStatState(santa::santad::StatChangeStep::kMessageCreate); } Message::~Message() { @@ -38,6 +40,8 @@ other.es_msg_ = nullptr; process_token_ = std::move(other.process_token_); other.process_token_ = std::nullopt; + stat_change_step_ = other.stat_change_step_; + stat_result_ = other.stat_result_; } Message::Message(const Message &other) { @@ -45,6 +49,28 @@ es_msg_ = other.es_msg_; esapi_->RetainMessage(es_msg_); process_token_ = other.process_token_; + stat_change_step_ = other.stat_change_step_; + stat_result_ = other.stat_result_; +} + +void Message::UpdateStatState(santa::santad::StatChangeStep step) const { + // Only update state for AUTH EXEC events and if no previous change was detected + if (es_msg_->event_type == ES_EVENT_TYPE_AUTH_EXEC && + stat_change_step_ == santa::santad::StatChangeStep::kNoChange && + // Note: The following checks are required due to tests that only + // partially construct an es_message_t. + es_msg_->event.exec.target && es_msg_->event.exec.target->executable) { + struct stat &es_sb = es_msg_->event.exec.target->executable->stat; + struct stat sb; + int ret = stat(es_msg_->event.exec.target->executable->path.data, &sb); + // If stat failed, or if devno/inode changed, update state. + if (ret != 0 || es_sb.st_ino != sb.st_ino || es_sb.st_dev != sb.st_dev) { + stat_change_step_ = step; + // Determine the specific condition that failed for tracking purposes + stat_result_ = (ret != 0) ? santa::santad::StatResult::kStatError + : santa::santad::StatResult::kDevnoInodeMismatch; + } + } } void Message::SetProcessToken(process_tree::ProcessToken tok) { diff --git a/Source/santad/EventProviders/SNTEndpointSecurityClient.mm b/Source/santad/EventProviders/SNTEndpointSecurityClient.mm index 04b84362a..ae4bb4114 100644 --- a/Source/santad/EventProviders/SNTEndpointSecurityClient.mm +++ b/Source/santad/EventProviders/SNTEndpointSecurityClient.mm @@ -151,7 +151,8 @@ - (void)establishClientOrDie { if ([self handleContextMessage:esMsg]) { int64_t processingEnd = clock_gettime_nsec_np(CLOCK_MONOTONIC); self->_metrics->SetEventMetrics(self->_processor, eventType, EventDisposition::kProcessed, - processingEnd - processingStart); + processingEnd - processingStart, esMsg.StatChangeStep(), + esMsg.StatError()); return; } @@ -160,12 +161,14 @@ - (void)establishClientOrDie { recordEventMetrics:^(EventDisposition disposition) { int64_t processingEnd = clock_gettime_nsec_np(CLOCK_MONOTONIC); self->_metrics->SetEventMetrics(self->_processor, eventType, disposition, - processingEnd - processingStart); + processingEnd - processingStart, esMsg.StatChangeStep(), + esMsg.StatError()); }]; } else { int64_t processingEnd = clock_gettime_nsec_np(CLOCK_MONOTONIC); self->_metrics->SetEventMetrics(self->_processor, eventType, EventDisposition::kDropped, - processingEnd - processingStart); + processingEnd - processingStart, esMsg.StatChangeStep(), + esMsg.StatError()); } }); diff --git a/Source/santad/Metrics.h b/Source/santad/Metrics.h index 5881a4868..876f362de 100644 --- a/Source/santad/Metrics.h +++ b/Source/santad/Metrics.h @@ -51,9 +51,22 @@ enum class FileAccessMetricStatus { kBlockedUser, }; +enum class StatChangeStep { + kNoChange = 0, + kMessageCreate, + kCodesignValidation, +}; + +enum class StatResult { + kOK = 0, + kStatError, + kDevnoInodeMismatch, +}; + using EventCountTuple = std::tuple; using EventTimesTuple = std::tuple; using EventStatsTuple = std::tuple; +using EventStatChangeTuple = std::tuple; using FileAccessMetricsPolicyVersion = std::string; using FileAccessMetricsPolicyName = std::string; using FileAccessEventCountTuple = @@ -67,8 +80,8 @@ class Metrics : public std::enable_shared_from_this { Metrics(dispatch_queue_t q, dispatch_source_t timer_source, uint64_t interval, SNTMetricInt64Gauge *event_processing_times, SNTMetricCounter *event_counts, SNTMetricCounter *rate_limit_counts, SNTMetricCounter *drop_counts, - SNTMetricCounter *faa_event_counts, SNTMetricSet *metric_set, - void (^run_on_first_start)(Metrics *)); + SNTMetricCounter *faa_event_counts, SNTMetricCounter *stat_change_counts, + SNTMetricSet *metric_set, void (^run_on_first_start)(Metrics *)); ~Metrics(); @@ -84,7 +97,8 @@ class Metrics : public std::enable_shared_from_this { void UpdateEventStats(Processor processor, const es_message_t *msg); void SetEventMetrics(Processor processor, es_event_type_t event_type, - EventDisposition disposition, int64_t nanos); + EventDisposition disposition, int64_t nanos, StatChangeStep step, + StatResult stat_result); void SetRateLimitingMetrics(Processor processor, int64_t events_rate_limited_count); @@ -112,6 +126,7 @@ class Metrics : public std::enable_shared_from_this { SNTMetricCounter *rate_limit_counts_; SNTMetricCounter *faa_event_counts_; SNTMetricCounter *drop_counts_; + SNTMetricCounter *stat_change_counts_; SNTMetricSet *metric_set_; // Tracks whether or not the timer_source should be running. // This helps manage dispatch source state to ensure the source is not @@ -129,6 +144,7 @@ class Metrics : public std::enable_shared_from_this { std::map rate_limit_counts_cache_; std::map faa_event_counts_cache_; std::map drop_cache_; + std::map stat_change_cache_; }; } // namespace santa::santad diff --git a/Source/santad/Metrics.mm b/Source/santad/Metrics.mm index b3ec7ef1c..2ee65bc8a 100644 --- a/Source/santad/Metrics.mm +++ b/Source/santad/Metrics.mm @@ -54,6 +54,14 @@ static NSString *const kEventDispositionDropped = @"Dropped"; static NSString *const kEventDispositionProcessed = @"Processed"; +static NSString *const kStatChangeStepNoChange = @"NoChange"; +static NSString *const kStatChangeStepMessageCreate = @"MessageCreate"; +static NSString *const kStatChangeStepCodesignValidation = @"CodesignValidation"; + +static NSString *const kStatResultOK = @"OK"; +static NSString *const kStatResultStatError = @"StatError"; +static NSString *const kStatResultDevnoInodeMismatch = @"DevnoInodeMismatch"; + // Compat values static NSString *const kFileAccessMetricStatusOK = @"OK"; static NSString *const kFileAccessMetricStatusBlockedUser = @"BLOCKED_USER"; @@ -148,6 +156,30 @@ } } +NSString *const StatChangeStepToString(StatChangeStep step) { + switch (step) { + case StatChangeStep::kNoChange: return kStatChangeStepNoChange; + case StatChangeStep::kMessageCreate: return kStatChangeStepMessageCreate; + case StatChangeStep::kCodesignValidation: return kStatChangeStepCodesignValidation; + default: + [NSException raise:@"Invalid stat change step" + format:@"Unknown stat change step value: %d", static_cast(step)]; + return nil; + } +} + +NSString *const StatResultToString(StatResult result) { + switch (result) { + case StatResult::kOK: return kStatResultOK; + case StatResult::kStatError: return kStatResultStatError; + case StatResult::kDevnoInodeMismatch: return kStatResultDevnoInodeMismatch; + default: + [NSException raise:@"Invalid stat result" + format:@"Unknown stat result value: %d", static_cast(result)]; + return nil; + } +} + std::shared_ptr Metrics::Create(SNTMetricSet *metric_set, uint64_t interval) { dispatch_queue_t q = dispatch_queue_create("com.google.santa.santametricsservice.q", DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL); @@ -181,9 +213,14 @@ fieldNames:@[ @"Processor", @"Event" ] helpText:@"Count of the number of drops for each event"]; + SNTMetricCounter *stat_change_counts = + [metric_set counterWithName:@"/santa/event_stat_change_count" + fieldNames:@[ @"step", @"error" ] + helpText:@"Count of times a stat info changed for a binary being evalauted"]; + std::shared_ptr metrics = std::make_shared( q, timer_source, interval, event_processing_times, event_counts, rate_limit_counts, - faa_event_counts, drop_counts, metric_set, ^(Metrics *metrics) { + faa_event_counts, drop_counts, stat_change_counts, metric_set, ^(Metrics *metrics) { SNTRegisterCoreMetrics(); metrics->EstablishConnection(); }); @@ -204,8 +241,8 @@ Metrics::Metrics(dispatch_queue_t q, dispatch_source_t timer_source, uint64_t interval, SNTMetricInt64Gauge *event_processing_times, SNTMetricCounter *event_counts, SNTMetricCounter *rate_limit_counts, SNTMetricCounter *faa_event_counts, - SNTMetricCounter *drop_counts, SNTMetricSet *metric_set, - void (^run_on_first_start)(Metrics *)) + SNTMetricCounter *drop_counts, SNTMetricCounter *stat_change_counts, + SNTMetricSet *metric_set, void (^run_on_first_start)(Metrics *)) : q_(q), timer_source_(timer_source), interval_(interval), @@ -214,6 +251,7 @@ rate_limit_counts_(rate_limit_counts), faa_event_counts_(faa_event_counts), drop_counts_(drop_counts), + stat_change_counts_(stat_change_counts), metric_set_(metric_set), run_on_first_start_(run_on_first_start) { SetInterval(interval_); @@ -307,6 +345,15 @@ } } + for (const auto &[key, count] : stat_change_cache_) { + if (count > 0) { + NSString *stepName = StatChangeStepToString(std::get(key)); + NSString *error = StatResultToString(std::get(key)); + + [stat_change_counts_ incrementBy:count forFieldValues:@[ stepName, error ]]; + } + } + // Reset the maps so the next cycle begins with a clean state // IMPORTANT: Do not reset drop_cache_, the sequence numbers must persist // for accurate accounting @@ -314,6 +361,7 @@ event_times_cache_ = {}; rate_limit_counts_cache_ = {}; faa_event_counts_cache_ = {}; + stat_change_cache_ = {}; }); } @@ -356,10 +404,12 @@ } void Metrics::SetEventMetrics(Processor processor, es_event_type_t event_type, - EventDisposition event_disposition, int64_t nanos) { + EventDisposition event_disposition, int64_t nanos, + StatChangeStep step, StatResult stat_result) { dispatch_sync(events_q_, ^{ event_counts_cache_[EventCountTuple{processor, event_type, event_disposition}]++; event_times_cache_[EventTimesTuple{processor, event_type}] = nanos; + stat_change_cache_[EventStatChangeTuple{step, stat_result}]++; }); } diff --git a/Source/santad/MetricsTest.mm b/Source/santad/MetricsTest.mm index efd0138ad..f3ce3c525 100644 --- a/Source/santad/MetricsTest.mm +++ b/Source/santad/MetricsTest.mm @@ -26,10 +26,13 @@ using santa::santad::EventCountTuple; using santa::santad::EventDisposition; +using santa::santad::EventStatChangeTuple; using santa::santad::EventStatsTuple; using santa::santad::EventTimesTuple; using santa::santad::FileAccessEventCountTuple; using santa::santad::Processor; +using santa::santad::StatChangeStep; +using santa::santad::StatResult; namespace santa::santad { @@ -38,6 +41,8 @@ extern NSString *const EventDispositionToString(EventDisposition d); extern NSString *const FileAccessMetricStatusToString(FileAccessMetricStatus status); extern NSString *const FileAccessPolicyDecisionToString(FileAccessPolicyDecision decision); +extern NSString *const StatChangeStepToString(StatChangeStep decision); +extern NSString *const StatResultToString(StatResult decision); class MetricsPeer : public Metrics { public: @@ -55,6 +60,7 @@ using Metrics::interval_; using Metrics::rate_limit_counts_cache_; using Metrics::running_; + using Metrics::stat_change_cache_; using Metrics::SequenceStats; }; @@ -69,10 +75,12 @@ using santa::santad::Metrics; using santa::santad::MetricsPeer; using santa::santad::ProcessorToString; +using santa::santad::StatChangeStepToString; +using santa::santad::StatResultToString; std::shared_ptr CreateBasicMetricsPeer(dispatch_queue_t q, void (^block)(Metrics *)) { dispatch_source_t timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, q); - return std::make_shared(q, timer, 100, nil, nil, nil, nil, nil, nil, block); + return std::make_shared(q, timer, 100, nil, nil, nil, nil, nil, nil, nil, block); } @interface MetricsTest : XCTestCase @@ -228,6 +236,34 @@ - (void)testFileAccessPolicyDecisionToString { } } +- (void)testStatChangeStepToString { + std::map stepToString = { + {StatChangeStep::kNoChange, @"NoChange"}, + {StatChangeStep::kMessageCreate, @"MessageCreate"}, + {StatChangeStep::kCodesignValidation, @"CodesignValidation"}, + }; + + for (const auto &kv : stepToString) { + XCTAssertEqualObjects(StatChangeStepToString(kv.first), kv.second); + } + + XCTAssertThrows(StatChangeStepToString((StatChangeStep)12345)); +} + +- (void)testStatResultToString { + std::map resultToString = { + {StatResult::kOK, @"OK"}, + {StatResult::kStatError, @"StatError"}, + {StatResult::kDevnoInodeMismatch, @"DevnoInodeMismatch"}, + }; + + for (const auto &kv : resultToString) { + XCTAssertEqualObjects(StatResultToString(kv.first), kv.second); + } + + XCTAssertThrows(StatResultToString((StatResult)12345)); +} + - (void)testSetEventMetrics { int64_t nanos = 1234; @@ -237,22 +273,28 @@ - (void)testSetEventMetrics { // Initial maps are empty XCTAssertEqual(metrics->event_counts_cache_.size(), 0); XCTAssertEqual(metrics->event_times_cache_.size(), 0); + XCTAssertEqual(metrics->stat_change_cache_.size(), 0); metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC, - EventDisposition::kProcessed, nanos); + EventDisposition::kProcessed, nanos, StatChangeStep::kNoChange, + StatResult::kOK); // Check sizes after setting metrics once XCTAssertEqual(metrics->event_counts_cache_.size(), 1); XCTAssertEqual(metrics->event_times_cache_.size(), 1); + XCTAssertEqual(metrics->stat_change_cache_.size(), 1); metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC, - EventDisposition::kProcessed, nanos); + EventDisposition::kProcessed, nanos, StatChangeStep::kMessageCreate, + StatResult::kDevnoInodeMismatch); metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_OPEN, - EventDisposition::kProcessed, nanos * 2); + EventDisposition::kProcessed, nanos * 2, StatChangeStep::kNoChange, + StatResult::kOK); // Re-check expected counts. One was an update, so should only be 2 items XCTAssertEqual(metrics->event_counts_cache_.size(), 2); XCTAssertEqual(metrics->event_times_cache_.size(), 2); + XCTAssertEqual(metrics->stat_change_cache_.size(), 2); // Check map values EventCountTuple ecExec{Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC, @@ -261,11 +303,13 @@ - (void)testSetEventMetrics { EventDisposition::kProcessed}; EventTimesTuple etExec{Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC}; EventTimesTuple etOpen{Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_OPEN}; + EventStatChangeTuple noChange{StatChangeStep::kNoChange, StatResult::kOK}; XCTAssertEqual(metrics->event_counts_cache_[ecExec], 2); XCTAssertEqual(metrics->event_counts_cache_[ecOpen], 1); XCTAssertEqual(metrics->event_times_cache_[etExec], nanos); XCTAssertEqual(metrics->event_times_cache_[etOpen], nanos * 2); + XCTAssertEqual(metrics->stat_change_cache_[noChange], 2); } - (void)testSetRateLimitingMetrics { @@ -395,17 +439,19 @@ - (void)testFlushMetrics { }); dispatch_source_t timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, self.q); - auto metrics = - std::make_shared(self.q, timer, 100, mockEventProcessingTimes, mockEventCounts, - mockEventCounts, mockEventCounts, mockEventCounts, nil, - ^(santa::santad::Metrics *m){ - // This block intentionally left blank - }); + auto metrics = std::make_shared(self.q, timer, 100, mockEventProcessingTimes, + mockEventCounts, mockEventCounts, mockEventCounts, + mockEventCounts, mockEventCounts, nil, + ^(santa::santad::Metrics *m){ + // This block intentionally left blank + }); metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC, - EventDisposition::kProcessed, nanos); + EventDisposition::kProcessed, nanos, StatChangeStep::kNoChange, + StatResult::kOK); metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_OPEN, - EventDisposition::kProcessed, nanos * 2); + EventDisposition::kProcessed, nanos * 2, + StatChangeStep::kCodesignValidation, StatResult::kStatError); metrics->UpdateEventStats(Processor::kRecorder, &esMsgWithDrops); metrics->SetRateLimitingMetrics(Processor::kFileAccessAuthorizer, 123); metrics->SetFileAccessEventMetrics("v1.0", "rule_abc", FileAccessMetricStatus::kOK, @@ -416,6 +462,7 @@ - (void)testFlushMetrics { XCTAssertEqual(metrics->event_times_cache_.size(), 2); XCTAssertEqual(metrics->rate_limit_counts_cache_.size(), 1); XCTAssertEqual(metrics->faa_event_counts_cache_.size(), 1); + XCTAssertEqual(metrics->stat_change_cache_.size(), 2); XCTAssertEqual(metrics->drop_cache_.size(), 2); EventStatsTuple eventStats{Processor::kRecorder, esMsgWithDrops.event_type}; @@ -430,10 +477,11 @@ - (void)testFlushMetrics { // Expected call count is 8: // 2: event counts // 2: event times + // 2: stat change step // 1: rate limit // 1: FAA // 2: drops (1 event, 1 global) - int expectedCalls = 8; + int expectedCalls = 10; for (int i = 0; i < expectedCalls; i++) { XCTAssertSemaTrue(self.sema, 5, "Failed waiting for metrics to flush"); } @@ -443,6 +491,7 @@ - (void)testFlushMetrics { XCTAssertEqual(metrics->event_times_cache_.size(), 0); XCTAssertEqual(metrics->rate_limit_counts_cache_.size(), 0); XCTAssertEqual(metrics->faa_event_counts_cache_.size(), 0); + XCTAssertEqual(metrics->stat_change_cache_.size(), 0); // Note: The drop_cache_ should not be reset back to size 0. Instead, each // entry has the sequence number left intact, but drop counts reset to 0. XCTAssertEqual(metrics->drop_cache_.size(), 2); diff --git a/Source/santad/SNTExecutionController.mm b/Source/santad/SNTExecutionController.mm index 6bec271c7..b1ac945f7 100644 --- a/Source/santad/SNTExecutionController.mm +++ b/Source/santad/SNTExecutionController.mm @@ -256,9 +256,11 @@ - (void)validateExecEvent:(const Message &)esMsg postAction:(bool (^)(SNTAction) // TODO(markowsky): Maybe add a metric here for how many large executables we're seeing. // if (binInfo.fileSize > SomeUpperLimit) ... - SNTCachedDecision *cd = [self.policyProcessor - decisionForFileInfo:binInfo - targetProcess:targetProc + SNTCachedDecision *cd = [self.policyProcessor decisionForFileInfo:binInfo + targetProcess:targetProc + preCodesignCheckCallback:^(void) { + esMsg.UpdateStatState(santa::santad::StatChangeStep::kCodesignValidation); + } entitlementsFilterCallback:^NSDictionary *(const char *teamID, NSDictionary *entitlements) { if (!entitlements) { return nil; diff --git a/Source/santad/SNTPolicyProcessor.h b/Source/santad/SNTPolicyProcessor.h index f8f17d5d5..56d221952 100644 --- a/Source/santad/SNTPolicyProcessor.h +++ b/Source/santad/SNTPolicyProcessor.h @@ -51,6 +51,13 @@ (NSDictionary *_Nullable (^_Nonnull)( const char *_Nullable teamID, NSDictionary *_Nullable entitlements))entitlementsFilterCallback; +- (nonnull SNTCachedDecision *)decisionForFileInfo:(nonnull SNTFileInfo *)fileInfo + targetProcess:(nonnull const es_process_t *)targetProc + preCodesignCheckCallback:(void (^_Nullable)(void))preCodesignCheckCallback + entitlementsFilterCallback: + (NSDictionary *_Nullable (^_Nonnull)( + const char *_Nullable teamID, + NSDictionary *_Nullable entitlements))entitlementsFilterCallback; /// /// A wrapper for decisionForFileInfo:fileSHA256:certificateSHA256:. This method is slower as it diff --git a/Source/santad/SNTPolicyProcessor.mm b/Source/santad/SNTPolicyProcessor.mm index 70a261e92..dd0d66fd4 100644 --- a/Source/santad/SNTPolicyProcessor.mm +++ b/Source/santad/SNTPolicyProcessor.mm @@ -157,16 +157,17 @@ static void UpdateCachedDecisionSigningInfo( } } -- (nonnull SNTCachedDecision *)decisionForFileInfo:(nonnull SNTFileInfo *)fileInfo - cdhash:(nullable NSString *)cdhash - fileSHA256:(nullable NSString *)fileSHA256 - certificateSHA256:(nullable NSString *)certificateSHA256 - teamID:(nullable NSString *)teamID - signingID:(nullable NSString *)signingID - isProdSignedCallback:(BOOL (^_Nonnull)())isProdSignedCallback - entitlementsFilterCallback: - (NSDictionary *_Nullable (^_Nullable)( - NSDictionary *_Nullable entitlements))entitlementsFilterCallback { +- (nonnull SNTCachedDecision *) + decisionForFileInfo:(nonnull SNTFileInfo *)fileInfo + cdhash:(nullable NSString *)cdhash + fileSHA256:(nullable NSString *)fileSHA256 + certificateSHA256:(nullable NSString *)certificateSHA256 + teamID:(nullable NSString *)teamID + signingID:(nullable NSString *)signingID + isProdSignedCallback:(BOOL (^_Nonnull)())isProdSignedCallback + entitlementsFilterCallback:(NSDictionary *_Nullable (^_Nullable)( + NSDictionary *_Nullable entitlements))entitlementsFilterCallback + preCodesignCheckCallback:(void (^_Nullable)(void))preCodesignCheckCallback { // Check the hash before allocating a SNTCachedDecision. NSString *fileHash = fileSHA256 ?: fileInfo.SHA256; SNTClientMode mode = [self.configurator clientMode]; @@ -192,6 +193,10 @@ - (nonnull SNTCachedDecision *)decisionForFileInfo:(nonnull SNTFileInfo *)fileIn if (certificateSHA256.length) { cd.certSHA256 = certificateSHA256; } else { + if (preCodesignCheckCallback) { + preCodesignCheckCallback(); + } + // Grab the code signature, if there's an error don't try to capture // any of the signature details. Also clear out any rule lookup parameters // that would require being validly signed. @@ -271,6 +276,19 @@ - (nonnull SNTCachedDecision *)decisionForFileInfo:(nonnull SNTFileInfo *)fileIn (NSDictionary *_Nullable (^_Nonnull)( const char *_Nullable teamID, NSDictionary *_Nullable entitlements))entitlementsFilterCallback { + return [self decisionForFileInfo:fileInfo + targetProcess:targetProc + preCodesignCheckCallback:nil + entitlementsFilterCallback:entitlementsFilterCallback]; +} + +- (nonnull SNTCachedDecision *)decisionForFileInfo:(nonnull SNTFileInfo *)fileInfo + targetProcess:(nonnull const es_process_t *)targetProc + preCodesignCheckCallback:(void (^_Nullable)(void))preCodesignCheckCallback + entitlementsFilterCallback: + (NSDictionary *_Nullable (^_Nonnull)( + const char *_Nullable teamID, + NSDictionary *_Nullable entitlements))entitlementsFilterCallback { NSString *signingID; NSString *teamID; NSString *cdhash; @@ -319,7 +337,8 @@ - (nonnull SNTCachedDecision *)decisionForFileInfo:(nonnull SNTFileInfo *)fileIn } entitlementsFilterCallback:^NSDictionary *(NSDictionary *entitlements) { return entitlementsFilterCallback(entitlementsFilterTeamID, entitlements); - }]; + } + preCodesignCheckCallback:preCodesignCheckCallback]; } // Used by `$ santactl fileinfo`. @@ -356,7 +375,8 @@ - (nonnull SNTCachedDecision *)decisionForFilePath:(nonnull NSString *)filePath return NO; } } - entitlementsFilterCallback:nil]; + entitlementsFilterCallback:nil + preCodesignCheckCallback:nil]; } /// From ac1c9d8b05332fb9c561bb697909b5155de31ab5 Mon Sep 17 00:00:00 2001 From: Matt W <436037+mlw@users.noreply.github.com> Date: Fri, 17 May 2024 12:15:48 -0400 Subject: [PATCH 3/4] Fix stat metrics accounting. Refactor setting metrics to be more general. (#1354) --- Source/common/SNTCommonEnums.h | 14 ++++ Source/santad/BUILD | 6 +- .../EventProviders/EndpointSecurity/Message.h | 19 +++-- .../EndpointSecurity/Message.mm | 9 +-- .../SNTEndpointSecurityClient.mm | 17 ++-- Source/santad/Metrics.h | 18 +---- Source/santad/Metrics.mm | 17 ++-- Source/santad/MetricsTest.mm | 77 +++++++++++++------ Source/santad/SNTExecutionController.mm | 2 +- 9 files changed, 108 insertions(+), 71 deletions(-) diff --git a/Source/common/SNTCommonEnums.h b/Source/common/SNTCommonEnums.h index 36fc9e5b1..9668334fd 100644 --- a/Source/common/SNTCommonEnums.h +++ b/Source/common/SNTCommonEnums.h @@ -182,6 +182,7 @@ typedef NS_ENUM(NSInteger, SNTRuleCleanup) { }; #ifdef __cplusplus + enum class FileAccessPolicyDecision { kNoPolicy, kDenied, @@ -190,6 +191,19 @@ enum class FileAccessPolicyDecision { kAllowedReadAccess, kAllowedAuditOnly, }; + +enum class StatChangeStep { + kNoChange = 0, + kMessageCreate, + kCodesignValidation, +}; + +enum class StatResult { + kOK = 0, + kStatError, + kDevnoInodeMismatch, +}; + #endif static const char *kSantaDPath = diff --git a/Source/santad/BUILD b/Source/santad/BUILD index 814e15c3a..61ee30c5e 100644 --- a/Source/santad/BUILD +++ b/Source/santad/BUILD @@ -657,8 +657,8 @@ objc_library( hdrs = ["EventProviders/EndpointSecurity/Message.h"], deps = [ ":EndpointSecurityClient", - ":Metrics", ":WatchItemPolicy", + "//Source/common:SNTCommonEnums", "//Source/santad/ProcessTree:process_tree", ], ) @@ -719,6 +719,7 @@ objc_library( srcs = ["Metrics.mm"], hdrs = ["Metrics.h"], deps = [ + ":EndpointSecurityMessage", ":SNTApplicationCoreMetrics", "//Source/common:SNTCommonEnums", "//Source/common:SNTLogging", @@ -1184,7 +1185,10 @@ santa_unit_test( name = "MetricsTest", srcs = ["MetricsTest.mm"], deps = [ + ":EndpointSecurityMessage", ":Metrics", + ":MockEndpointSecurityAPI", + "//Source/common:SNTCommonEnums", "//Source/common:SNTMetricSet", "//Source/common:TestUtils", "@OCMock", diff --git a/Source/santad/EventProviders/EndpointSecurity/Message.h b/Source/santad/EventProviders/EndpointSecurity/Message.h index 4b1add8c6..f4663df49 100644 --- a/Source/santad/EventProviders/EndpointSecurity/Message.h +++ b/Source/santad/EventProviders/EndpointSecurity/Message.h @@ -20,12 +20,13 @@ #include #include -#include "Source/santad/Metrics.h" +#import "Source/common/SNTCommonEnums.h" #include "Source/santad/ProcessTree/process_tree.h" namespace santa::santad::event_providers::endpoint_security { class EndpointSecurityAPI; +class MessagePeer; class Message { public: @@ -54,12 +55,12 @@ class Message { std::string ParentProcessName() const; - void UpdateStatState(santa::santad::StatChangeStep step) const; + void UpdateStatState(enum StatChangeStep step) const; - inline santa::santad::StatChangeStep StatChangeStep() const { - return stat_change_step_; - } - inline StatResult StatError() const { return stat_result_; } + inline StatChangeStep StatChangeStep() const { return stat_change_step_; } + inline StatResult StatResult() const { return stat_result_; } + + friend class santa::santad::event_providers::endpoint_security::MessagePeer; private: std::shared_ptr esapi_; @@ -68,10 +69,8 @@ class Message { std::string GetProcessName(pid_t pid) const; - mutable santa::santad::StatChangeStep stat_change_step_ = - santa::santad::StatChangeStep::kNoChange; - mutable santa::santad::StatResult stat_result_ = - santa::santad::StatResult::kOK; + mutable enum StatChangeStep stat_change_step_ = StatChangeStep::kNoChange; + mutable enum StatResult stat_result_ = StatResult::kOK; }; } // namespace santa::santad::event_providers::endpoint_security diff --git a/Source/santad/EventProviders/EndpointSecurity/Message.mm b/Source/santad/EventProviders/EndpointSecurity/Message.mm index caa6f9647..5514ab33e 100644 --- a/Source/santad/EventProviders/EndpointSecurity/Message.mm +++ b/Source/santad/EventProviders/EndpointSecurity/Message.mm @@ -25,7 +25,7 @@ Message::Message(std::shared_ptr esapi, const es_message_t *es_msg) : esapi_(std::move(esapi)), es_msg_(es_msg), process_token_(std::nullopt) { esapi_->RetainMessage(es_msg); - UpdateStatState(santa::santad::StatChangeStep::kMessageCreate); + UpdateStatState(StatChangeStep::kMessageCreate); } Message::~Message() { @@ -53,10 +53,10 @@ stat_result_ = other.stat_result_; } -void Message::UpdateStatState(santa::santad::StatChangeStep step) const { +void Message::UpdateStatState(enum StatChangeStep step) const { // Only update state for AUTH EXEC events and if no previous change was detected if (es_msg_->event_type == ES_EVENT_TYPE_AUTH_EXEC && - stat_change_step_ == santa::santad::StatChangeStep::kNoChange && + stat_change_step_ == StatChangeStep::kNoChange && // Note: The following checks are required due to tests that only // partially construct an es_message_t. es_msg_->event.exec.target && es_msg_->event.exec.target->executable) { @@ -67,8 +67,7 @@ if (ret != 0 || es_sb.st_ino != sb.st_ino || es_sb.st_dev != sb.st_dev) { stat_change_step_ = step; // Determine the specific condition that failed for tracking purposes - stat_result_ = (ret != 0) ? santa::santad::StatResult::kStatError - : santa::santad::StatResult::kDevnoInodeMismatch; + stat_result_ = (ret != 0) ? StatResult::kStatError : StatResult::kDevnoInodeMismatch; } } } diff --git a/Source/santad/EventProviders/SNTEndpointSecurityClient.mm b/Source/santad/EventProviders/SNTEndpointSecurityClient.mm index ae4bb4114..ee2f4cc48 100644 --- a/Source/santad/EventProviders/SNTEndpointSecurityClient.mm +++ b/Source/santad/EventProviders/SNTEndpointSecurityClient.mm @@ -146,13 +146,10 @@ - (void)establishClientOrDie { // sequence numbers are processed in order. self->_metrics->UpdateEventStats(self->_processor, esMsg.operator->()); - es_event_type_t eventType = esMsg->event_type; - if ([self handleContextMessage:esMsg]) { int64_t processingEnd = clock_gettime_nsec_np(CLOCK_MONOTONIC); - self->_metrics->SetEventMetrics(self->_processor, eventType, EventDisposition::kProcessed, - processingEnd - processingStart, esMsg.StatChangeStep(), - esMsg.StatError()); + self->_metrics->SetEventMetrics(self->_processor, EventDisposition::kProcessed, + processingEnd - processingStart, esMsg); return; } @@ -160,15 +157,13 @@ - (void)establishClientOrDie { [self handleMessage:std::move(esMsg) recordEventMetrics:^(EventDisposition disposition) { int64_t processingEnd = clock_gettime_nsec_np(CLOCK_MONOTONIC); - self->_metrics->SetEventMetrics(self->_processor, eventType, disposition, - processingEnd - processingStart, esMsg.StatChangeStep(), - esMsg.StatError()); + self->_metrics->SetEventMetrics(self->_processor, disposition, + processingEnd - processingStart, esMsg); }]; } else { int64_t processingEnd = clock_gettime_nsec_np(CLOCK_MONOTONIC); - self->_metrics->SetEventMetrics(self->_processor, eventType, EventDisposition::kDropped, - processingEnd - processingStart, esMsg.StatChangeStep(), - esMsg.StatError()); + self->_metrics->SetEventMetrics(self->_processor, EventDisposition::kDropped, + processingEnd - processingStart, esMsg); } }); diff --git a/Source/santad/Metrics.h b/Source/santad/Metrics.h index 876f362de..3f8e2b0a0 100644 --- a/Source/santad/Metrics.h +++ b/Source/santad/Metrics.h @@ -26,6 +26,7 @@ #import "Source/common/SNTCommonEnums.h" #import "Source/common/SNTMetricSet.h" +#include "Source/santad/EventProviders/EndpointSecurity/Message.h" namespace santa::santad { @@ -51,18 +52,6 @@ enum class FileAccessMetricStatus { kBlockedUser, }; -enum class StatChangeStep { - kNoChange = 0, - kMessageCreate, - kCodesignValidation, -}; - -enum class StatResult { - kOK = 0, - kStatError, - kDevnoInodeMismatch, -}; - using EventCountTuple = std::tuple; using EventTimesTuple = std::tuple; using EventStatsTuple = std::tuple; @@ -96,9 +85,8 @@ class Metrics : public std::enable_shared_from_this { // Used for tracking event sequence numbers to determine if drops occured void UpdateEventStats(Processor processor, const es_message_t *msg); - void SetEventMetrics(Processor processor, es_event_type_t event_type, - EventDisposition disposition, int64_t nanos, StatChangeStep step, - StatResult stat_result); + void SetEventMetrics(Processor processor, EventDisposition event_disposition, int64_t nanos, + const santa::santad::event_providers::endpoint_security::Message &msg); void SetRateLimitingMetrics(Processor processor, int64_t events_rate_limited_count); diff --git a/Source/santad/Metrics.mm b/Source/santad/Metrics.mm index 2ee65bc8a..c2392cb97 100644 --- a/Source/santad/Metrics.mm +++ b/Source/santad/Metrics.mm @@ -13,6 +13,7 @@ /// limitations under the License. #include "Source/santad/Metrics.h" + #include #include @@ -403,13 +404,17 @@ }); } -void Metrics::SetEventMetrics(Processor processor, es_event_type_t event_type, - EventDisposition event_disposition, int64_t nanos, - StatChangeStep step, StatResult stat_result) { +void Metrics::SetEventMetrics( + Processor processor, EventDisposition event_disposition, int64_t nanos, + const santa::santad::event_providers::endpoint_security::Message &msg) { dispatch_sync(events_q_, ^{ - event_counts_cache_[EventCountTuple{processor, event_type, event_disposition}]++; - event_times_cache_[EventTimesTuple{processor, event_type}] = nanos; - stat_change_cache_[EventStatChangeTuple{step, stat_result}]++; + event_counts_cache_[EventCountTuple{processor, msg->event_type, event_disposition}]++; + event_times_cache_[EventTimesTuple{processor, msg->event_type}] = nanos; + + // Stat changes are only tracked for AUTH EXEC events + if (msg->event_type == ES_EVENT_TYPE_AUTH_EXEC) { + stat_change_cache_[EventStatChangeTuple{msg.StatChangeStep(), msg.StatResult()}]++; + } }); } diff --git a/Source/santad/MetricsTest.mm b/Source/santad/MetricsTest.mm index f3ce3c525..e8ce82d3e 100644 --- a/Source/santad/MetricsTest.mm +++ b/Source/santad/MetricsTest.mm @@ -12,6 +12,8 @@ /// See the License for the specific language governing permissions and /// limitations under the License. +#include "Source/santad/Metrics.h" + #include #import #import @@ -19,10 +21,13 @@ #include #include +#include +#import "Source/common/SNTCommonEnums.h" #include "Source/common/SNTMetricSet.h" #include "Source/common/TestUtils.h" -#include "Source/santad/Metrics.h" +#include "Source/santad/EventProviders/EndpointSecurity/Message.h" +#include "Source/santad/EventProviders/EndpointSecurity/MockEndpointSecurityAPI.h" using santa::santad::EventCountTuple; using santa::santad::EventDisposition; @@ -31,8 +36,6 @@ using santa::santad::EventTimesTuple; using santa::santad::FileAccessEventCountTuple; using santa::santad::Processor; -using santa::santad::StatChangeStep; -using santa::santad::StatResult; namespace santa::santad { @@ -67,6 +70,20 @@ } // namespace santa::santad +namespace santa::santad::event_providers::endpoint_security { + +class MessagePeer : public Message { + public: + // Make base class constructors visible + using Message::Message; + + // Private member variables + using Message::stat_change_step_; + using Message::stat_result_; +}; + +} // namespace santa::santad::event_providers::endpoint_security + using santa::santad::EventDispositionToString; using santa::santad::EventTypeToString; using santa::santad::FileAccessMetricStatus; @@ -77,6 +94,7 @@ using santa::santad::ProcessorToString; using santa::santad::StatChangeStepToString; using santa::santad::StatResultToString; +using santa::santad::event_providers::endpoint_security::MessagePeer; std::shared_ptr CreateBasicMetricsPeer(dispatch_queue_t q, void (^block)(Metrics *)) { dispatch_source_t timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, q); @@ -266,6 +284,14 @@ - (void)testStatResultToString { - (void)testSetEventMetrics { int64_t nanos = 1234; + es_message_t esExecMsg = MakeESMessage(ES_EVENT_TYPE_AUTH_EXEC, NULL); + es_message_t esOpenMsg = MakeESMessage(ES_EVENT_TYPE_AUTH_OPEN, NULL); + + auto mockESApi = std::make_shared(); + mockESApi->SetExpectationsRetainReleaseMessage(); + + MessagePeer execMsg(mockESApi, &esExecMsg); + MessagePeer openMsg(mockESApi, &esOpenMsg); std::shared_ptr metrics = CreateBasicMetricsPeer(self.q, ^(Metrics *){ }); @@ -275,25 +301,24 @@ - (void)testSetEventMetrics { XCTAssertEqual(metrics->event_times_cache_.size(), 0); XCTAssertEqual(metrics->stat_change_cache_.size(), 0); - metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC, - EventDisposition::kProcessed, nanos, StatChangeStep::kNoChange, - StatResult::kOK); + metrics->SetEventMetrics(Processor::kAuthorizer, EventDisposition::kProcessed, nanos, execMsg); // Check sizes after setting metrics once XCTAssertEqual(metrics->event_counts_cache_.size(), 1); XCTAssertEqual(metrics->event_times_cache_.size(), 1); XCTAssertEqual(metrics->stat_change_cache_.size(), 1); - metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC, - EventDisposition::kProcessed, nanos, StatChangeStep::kMessageCreate, - StatResult::kDevnoInodeMismatch); - metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_OPEN, - EventDisposition::kProcessed, nanos * 2, StatChangeStep::kNoChange, - StatResult::kOK); + execMsg.stat_change_step_ = StatChangeStep::kMessageCreate; + execMsg.stat_result_ = StatResult::kDevnoInodeMismatch; + metrics->SetEventMetrics(Processor::kAuthorizer, EventDisposition::kProcessed, nanos, execMsg); + + metrics->SetEventMetrics(Processor::kAuthorizer, EventDisposition::kProcessed, nanos * 2, + openMsg); // Re-check expected counts. One was an update, so should only be 2 items XCTAssertEqual(metrics->event_counts_cache_.size(), 2); XCTAssertEqual(metrics->event_times_cache_.size(), 2); + // Stat change counts should be 2 because one call wasn't an AUTH EXEC event XCTAssertEqual(metrics->stat_change_cache_.size(), 2); // Check map values @@ -304,12 +329,15 @@ - (void)testSetEventMetrics { EventTimesTuple etExec{Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC}; EventTimesTuple etOpen{Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_OPEN}; EventStatChangeTuple noChange{StatChangeStep::kNoChange, StatResult::kOK}; + EventStatChangeTuple msgCreateChange{StatChangeStep::kMessageCreate, + StatResult::kDevnoInodeMismatch}; XCTAssertEqual(metrics->event_counts_cache_[ecExec], 2); XCTAssertEqual(metrics->event_counts_cache_[ecOpen], 1); XCTAssertEqual(metrics->event_times_cache_[etExec], nanos); XCTAssertEqual(metrics->event_times_cache_[etOpen], nanos * 2); - XCTAssertEqual(metrics->stat_change_cache_[noChange], 2); + XCTAssertEqual(metrics->stat_change_cache_[noChange], 1); + XCTAssertEqual(metrics->stat_change_cache_[msgCreateChange], 1); } - (void)testSetRateLimitingMetrics { @@ -420,6 +448,14 @@ - (void)testFlushMetrics { id mockEventProcessingTimes = OCMClassMock([SNTMetricInt64Gauge class]); id mockEventCounts = OCMClassMock([SNTMetricCounter class]); int64_t nanos = 1234; + es_message_t esExecMsg = MakeESMessage(ES_EVENT_TYPE_AUTH_EXEC, NULL); + es_message_t esOpenMsg = MakeESMessage(ES_EVENT_TYPE_AUTH_OPEN, NULL); + + auto mockESApi = std::make_shared(); + mockESApi->SetExpectationsRetainReleaseMessage(); + + MessagePeer execMsg(mockESApi, &esExecMsg); + MessagePeer openMsg(mockESApi, &esOpenMsg); // Initial update will have non-zero sequence numbers, triggering drop detection es_message_t esMsgWithDrops = MakeESMessage(ES_EVENT_TYPE_NOTIFY_EXEC, NULL); @@ -446,12 +482,9 @@ - (void)testFlushMetrics { // This block intentionally left blank }); - metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_EXEC, - EventDisposition::kProcessed, nanos, StatChangeStep::kNoChange, - StatResult::kOK); - metrics->SetEventMetrics(Processor::kAuthorizer, ES_EVENT_TYPE_AUTH_OPEN, - EventDisposition::kProcessed, nanos * 2, - StatChangeStep::kCodesignValidation, StatResult::kStatError); + metrics->SetEventMetrics(Processor::kAuthorizer, EventDisposition::kProcessed, nanos, execMsg); + metrics->SetEventMetrics(Processor::kAuthorizer, EventDisposition::kProcessed, nanos * 2, + openMsg); metrics->UpdateEventStats(Processor::kRecorder, &esMsgWithDrops); metrics->SetRateLimitingMetrics(Processor::kFileAccessAuthorizer, 123); metrics->SetFileAccessEventMetrics("v1.0", "rule_abc", FileAccessMetricStatus::kOK, @@ -462,7 +495,7 @@ - (void)testFlushMetrics { XCTAssertEqual(metrics->event_times_cache_.size(), 2); XCTAssertEqual(metrics->rate_limit_counts_cache_.size(), 1); XCTAssertEqual(metrics->faa_event_counts_cache_.size(), 1); - XCTAssertEqual(metrics->stat_change_cache_.size(), 2); + XCTAssertEqual(metrics->stat_change_cache_.size(), 1); XCTAssertEqual(metrics->drop_cache_.size(), 2); EventStatsTuple eventStats{Processor::kRecorder, esMsgWithDrops.event_type}; @@ -477,11 +510,11 @@ - (void)testFlushMetrics { // Expected call count is 8: // 2: event counts // 2: event times - // 2: stat change step + // 1: stat change step // 1: rate limit // 1: FAA // 2: drops (1 event, 1 global) - int expectedCalls = 10; + int expectedCalls = 9; for (int i = 0; i < expectedCalls; i++) { XCTAssertSemaTrue(self.sema, 5, "Failed waiting for metrics to flush"); } diff --git a/Source/santad/SNTExecutionController.mm b/Source/santad/SNTExecutionController.mm index b1ac945f7..2694a411d 100644 --- a/Source/santad/SNTExecutionController.mm +++ b/Source/santad/SNTExecutionController.mm @@ -259,7 +259,7 @@ - (void)validateExecEvent:(const Message &)esMsg postAction:(bool (^)(SNTAction) SNTCachedDecision *cd = [self.policyProcessor decisionForFileInfo:binInfo targetProcess:targetProc preCodesignCheckCallback:^(void) { - esMsg.UpdateStatState(santa::santad::StatChangeStep::kCodesignValidation); + esMsg.UpdateStatState(StatChangeStep::kCodesignValidation); } entitlementsFilterCallback:^NSDictionary *(const char *teamID, NSDictionary *entitlements) { if (!entitlements) { From d8928ac3206a19913e7e66b37e067c160d6b8554 Mon Sep 17 00:00:00 2001 From: np5 Date: Mon, 20 May 2024 16:45:36 +0200 Subject: [PATCH 4/4] Add cdhash, teamID, and signingID to bundle events (#1353) Fix #1352 --- Source/santabundleservice/SNTBundleService.m | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Source/santabundleservice/SNTBundleService.m b/Source/santabundleservice/SNTBundleService.m index b800959a1..84cdfa83b 100644 --- a/Source/santabundleservice/SNTBundleService.m +++ b/Source/santabundleservice/SNTBundleService.m @@ -226,6 +226,15 @@ - (NSDictionary *)generateEventsFromBinaries:(NSArray *)fis MOLCodesignChecker *cs = [fi codesignCheckerWithError:NULL]; se.signingChain = cs.certificates; + se.cdhash = cs.cdhash; + se.teamID = cs.teamID; + if (cs.signingID) { + if (cs.teamID) { + se.signingID = [NSString stringWithFormat:@"%@:%@", cs.teamID, cs.signingID]; + } else if (cs.platformBinary) { + se.signingID = [NSString stringWithFormat:@"platform:%@", cs.signingID]; + } + } dispatch_sync(dispatch_get_main_queue(), ^{ relatedEvents[se.fileSHA256] = se;