Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opportunistically use ES cache when possible #989

Merged
merged 3 commits into from
Jan 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Source/common/SNTCommonEnums.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ enum class FileAccessPolicyDecision {
kDenied,
kDeniedInvalidSignature,
kAllowed,
kAllowedReadAccess,
kAllowedAuditOnly,
};
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@
/// @param result Either ES_AUTH_RESULT_ALLOW or ES_AUTH_RESULT_DENY
/// @param cacheable true if ES should attempt to cache the result, otherwise false
/// @return true if the response was successful, otherwise false
///
/// @note If the msg event type requires a flags response, the correct ES API will automatically
/// be called. ALLOWED results will be translated to having all flags set, and DENIED results
/// will be translated to having all flags cleared.
- (bool)respondToMessage:(const santa::santad::event_providers::endpoint_security::Message &)msg
withAuthResult:(es_auth_result_t)result
cacheable:(bool)cacheable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@

NSString *kBadCertHash = @"BAD_CERT_HASH";

static constexpr uint32_t kOpenFlagsIndicatingWrite = FWRITE | O_APPEND | O_TRUNC;

static inline std::string Path(const es_file_t *esFile) {
return std::string(esFile->path.data, esFile->path.length);
}
Expand Down Expand Up @@ -78,6 +80,7 @@ es_auth_result_t FileAccessPolicyDecisionToESAuthResult(FileAccessPolicyDecision
case FileAccessPolicyDecision::kDenied: return ES_AUTH_RESULT_DENY;
case FileAccessPolicyDecision::kDeniedInvalidSignature: return ES_AUTH_RESULT_DENY;
case FileAccessPolicyDecision::kAllowed: return ES_AUTH_RESULT_ALLOW;
case FileAccessPolicyDecision::kAllowedReadAccess: return ES_AUTH_RESULT_ALLOW;
case FileAccessPolicyDecision::kAllowedAuditOnly: return ES_AUTH_RESULT_ALLOW;
default:
// This is a programming error. Bail.
Expand All @@ -91,7 +94,7 @@ bool ShouldLogDecision(FileAccessPolicyDecision decision) {
switch (decision) {
case FileAccessPolicyDecision::kDenied: return true;
case FileAccessPolicyDecision::kDeniedInvalidSignature: return true;
case FileAccessPolicyDecision::kAllowedAuditOnly: return true; ;
case FileAccessPolicyDecision::kAllowedAuditOnly: return true;
default: return false;
}
}
Expand Down Expand Up @@ -233,13 +236,11 @@ - (NSString *)getCertificateHash:(es_file_t *)esFile {

- (FileAccessPolicyDecision)specialCaseForPolicy:(std::shared_ptr<WatchItemPolicy>)policy
message:(const Message &)msg {
constexpr int openFlagsIndicatingWrite = FWRITE | O_APPEND | O_TRUNC;

switch (msg->event_type) {
case ES_EVENT_TYPE_AUTH_OPEN:
// If the policy is write-only, but the operation isn't a write action, it's allowed
if (policy->write_only && !(msg->event.open.fflag & openFlagsIndicatingWrite)) {
return FileAccessPolicyDecision::kAllowed;
if (policy->write_only && !(msg->event.open.fflag & kOpenFlagsIndicatingWrite)) {
return FileAccessPolicyDecision::kAllowedReadAccess;
}

break;
Expand Down Expand Up @@ -371,20 +372,33 @@ - (void)processMessage:(const Message &)msg {
self->_watchItems->FindPolciesForPaths(targets);

es_auth_result_t policyResult = ES_AUTH_RESULT_ALLOW;
FileAccessPolicyDecision prevDecision = FileAccessPolicyDecision::kNoPolicy;
bool allow_read_access = false;

for (size_t i = 0; i < targets.size(); i++) {
FileAccessPolicyDecision curDecision = [self handleMessage:msg
target:targets[i]
policy:versionAndPolicies.second[i]
policyVersion:versionAndPolicies.first];

policyResult = CombinePolicyResults(FileAccessPolicyDecisionToESAuthResult(prevDecision),
FileAccessPolicyDecisionToESAuthResult(curDecision));
prevDecision = curDecision;
policyResult =
CombinePolicyResults(policyResult, FileAccessPolicyDecisionToESAuthResult(curDecision));

// If the overall policy result is deny, then reset allow_read_access.
// Otherwise if the current decision would allow read access, set the flag.
if (policyResult == ES_AUTH_RESULT_DENY) {
allow_read_access = false;
} else if (curDecision == FileAccessPolicyDecision::kAllowedReadAccess) {
allow_read_access = true;
}
}

[self respondToMessage:msg withAuthResult:policyResult cacheable:false];
// IMPORTANT: A response is only cacheable if the policy result was explicitly
// allowed. An "allow read access" result must not be cached to ensure a future
// non-read accesss can be evaluated. Similarly, denied results must never be
// cached so access attempts can be logged.
[self respondToMessage:msg
withAuthResult:policyResult
cacheable:(policyResult == ES_AUTH_RESULT_ALLOW && !allow_read_access)];
}

- (void)handleMessage:(santa::santad::event_providers::endpoint_security::Message &&)esMsg
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ - (void)testFileAccessPolicyDecisionToESAuthResult {
{FileAccessPolicyDecision::kDenied, ES_AUTH_RESULT_DENY},
{FileAccessPolicyDecision::kDeniedInvalidSignature, ES_AUTH_RESULT_DENY},
{FileAccessPolicyDecision::kAllowed, ES_AUTH_RESULT_ALLOW},
{FileAccessPolicyDecision::kAllowedReadAccess, ES_AUTH_RESULT_ALLOW},
{FileAccessPolicyDecision::kAllowedAuditOnly, ES_AUTH_RESULT_ALLOW},
};

Expand All @@ -206,6 +207,7 @@ - (void)testShouldLogDecision {
{FileAccessPolicyDecision::kDenied, true},
{FileAccessPolicyDecision::kDeniedInvalidSignature, true},
{FileAccessPolicyDecision::kAllowed, false},
{FileAccessPolicyDecision::kAllowedReadAccess, false},
{FileAccessPolicyDecision::kAllowedAuditOnly, true},
{(FileAccessPolicyDecision)5, false},
};
Expand Down Expand Up @@ -271,7 +273,7 @@ - (void)testSpecialCaseForPolicyMessage {
esMsg.event.open.fflag = FREAD;
Message msg(mockESApi, &esMsg);
result = [accessClient specialCaseForPolicy:policy message:msg];
XCTAssertEqual(result, FileAccessPolicyDecision::kAllowed);
XCTAssertEqual(result, FileAccessPolicyDecision::kAllowedReadAccess);
}

// Read/Write policy, Read operation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ static inline SanitizableString FilePath(const es_file_t *file) {
case FileAccessPolicyDecision::kDenied: return "DENIED";
case FileAccessPolicyDecision::kDeniedInvalidSignature: return "DENIED_INVALID_SIGNATURE";
case FileAccessPolicyDecision::kAllowed: return "ALLOWED";
case FileAccessPolicyDecision::kAllowedReadAccess: return "ALLOWED_READ_ACCESS";
case FileAccessPolicyDecision::kAllowedAuditOnly: return "AUDIT_ONLY";
default: return "UNKNOWN_DECISION_" + std::to_string((int)decision);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ - (void)testGetFileAccessPolicyDecisionString {
{FileAccessPolicyDecision::kDenied, "DENIED"},
{FileAccessPolicyDecision::kDeniedInvalidSignature, "DENIED_INVALID_SIGNATURE"},
{FileAccessPolicyDecision::kAllowed, "ALLOWED"},
{FileAccessPolicyDecision::kAllowedReadAccess, "ALLOWED_READ_ACCESS"},
{FileAccessPolicyDecision::kAllowedAuditOnly, "AUDIT_ONLY"},
{(FileAccessPolicyDecision)1234, "UNKNOWN_DECISION_1234"},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ - (void)testGetPolicyDecision {
{FileAccessPolicyDecision::kDeniedInvalidSignature,
::pbv1::FileAccess::POLICY_DECISION_DENIED_INVALID_SIGNATURE},
{FileAccessPolicyDecision::kAllowed, ::pbv1::FileAccess::POLICY_DECISION_UNKNOWN},
{FileAccessPolicyDecision::kAllowedReadAccess, ::pbv1::FileAccess::POLICY_DECISION_UNKNOWN},
{FileAccessPolicyDecision::kAllowedAuditOnly,
::pbv1::FileAccess::POLICY_DECISION_ALLOWED_AUDIT_ONLY},
{(FileAccessPolicyDecision)1234, ::pbv1::FileAccess::POLICY_DECISION_UNKNOWN},
Expand Down