Opportunistically use ES cache when possible (#989)

* WIP fixing up ES cacheability in file access client

* Removed old code from before simplification

* Add more tests

Source/common/SNTCommonEnums.h

@@ -141,6 +141,7 @@ enum class FileAccessPolicyDecision {
   kDenied,
   kDeniedInvalidSignature,
   kAllowed,
+  kAllowedReadAccess,
   kAllowedAuditOnly,
 };
 #endif

Source/santad/EventProviders/SNTEndpointSecurityClientBase.h

@@ -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;

Source/santad/EventProviders/SNTEndpointSecurityFileAccessAuthorizer.mm

@@ -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);
 }
@@ -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.
@@ -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;
   }
 }
@@ -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;
@@ -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

Source/santad/EventProviders/SNTEndpointSecurityFileAccessAuthorizerTest.mm

@@ -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},
   };
 
@@ -206,6 +207,7 @@ - (void)testShouldLogDecision {
     {FileAccessPolicyDecision::kDenied, true},
     {FileAccessPolicyDecision::kDeniedInvalidSignature, true},
     {FileAccessPolicyDecision::kAllowed, false},
+    {FileAccessPolicyDecision::kAllowedReadAccess, false},
     {FileAccessPolicyDecision::kAllowedAuditOnly, true},
     {(FileAccessPolicyDecision)5, false},
   };
@@ -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

Source/santad/Logs/EndpointSecurity/Serializers/BasicString.mm

@@ -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);
   }

Source/santad/Logs/EndpointSecurity/Serializers/BasicStringTest.mm

@@ -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"},
   };

Source/santad/Logs/EndpointSecurity/Serializers/ProtobufTest.mm

@@ -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},