FailClosed in Configurator now wraps checking client mode

google · Feb 8, 2024 · 3b155e9 · 3b155e9
1 parent 76d33bb
commit 3b155e9
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 27 deletions.
diff --git a/Source/common/SNTConfigurator.m b/Source/common/SNTConfigurator.m
@@ -604,8 +604,11 @@ - (void)setSyncServerClientMode:(SNTClientMode)newMode {
 
 - (BOOL)failClosed {
   NSNumber *n = self.configState[kFailClosedKey];
-  if (n) return [n boolValue];
-  return NO;
+  if ([n boolValue] && self.clientMode == SNTClientModeLockdown) {
+    return YES;
+  } else {
+    return NO;
+  }
 }
 
 - (BOOL)enableTransitiveRules {

diff --git a/Source/santad/EventProviders/SNTEndpointSecurityClient.mm b/Source/santad/EventProviders/SNTEndpointSecurityClient.mm
@@ -303,10 +303,6 @@ - (void)processMessage:(Message &&)msg handler:(void (^)(const Message &))messag
   int64_t processingBudget = [self computeBudgetForDeadline:msg->deadline
                                                 currentTime:mach_absolute_time()];
 
-  // TODO(mlw): How should we handle `deadlineNano <= timeout`. Will currently
-  // result in the deadline block being dispatched immediately (and therefore
-  // the event will be denied).
-
   // Workaround for compiler bug that doesn't properly close over variables
   __block Message processMsg = msg;
   __block Message deadlineMsg = msg;
@@ -318,7 +314,7 @@ - (void)processMessage:(Message &&)msg handler:(void (^)(const Message &))messag
     }
 
     es_auth_result_t authResult;
-    if (self.configurator.failClosed && self.configurator.clientMode == SNTClientModeLockdown) {
+    if (self.configurator.failClosed) {
       authResult = ES_AUTH_RESULT_DENY;
     } else {
       authResult = ES_AUTH_RESULT_ALLOW;

diff --git a/Source/santad/EventProviders/SNTEndpointSecurityClientTest.mm b/Source/santad/EventProviders/SNTEndpointSecurityClientTest.mm
@@ -548,7 +548,7 @@ - (void)testComputeBudgetForDeadlineCurrentTime {
   XCTBubbleMockVerifyAndClearExpectations(mockESApi.get());
 }
 
-- (void)checkDeadlineExpiredFailClosed:(BOOL)shouldFailClosed clientMode:(SNTClientMode)clientMode {
+- (void)checkDeadlineExpiredFailClosed:(BOOL)shouldFailClosed {
   // Set a es_message_t deadline of 750ms
   // Set a deadline leeway in the `SNTEndpointSecurityClient` of 500ms
   // Mock `RespondFlagsResult` which is called from the deadline handler
@@ -571,9 +571,7 @@ - (void)checkDeadlineExpiredFailClosed:(BOOL)shouldFailClosed clientMode:(SNTCli
   dispatch_semaphore_t deadlineSema = dispatch_semaphore_create(0);
   dispatch_semaphore_t controlSema = dispatch_semaphore_create(0);
 
-  es_auth_result_t wantAuthResult = (shouldFailClosed && clientMode == SNTClientModeLockdown)
-                                      ? ES_AUTH_RESULT_DENY
-                                      : ES_AUTH_RESULT_ALLOW;
+  es_auth_result_t wantAuthResult = shouldFailClosed ? ES_AUTH_RESULT_DENY : ES_AUTH_RESULT_ALLOW;
   EXPECT_CALL(*mockESApi, RespondAuthResult(testing::_, testing::_, wantAuthResult, false))
     .WillOnce(testing::InvokeWithoutArgs(^() {
       // Signal deadlineSema to let the handler block continue execution
@@ -583,10 +581,8 @@ - (void)checkDeadlineExpiredFailClosed:(BOOL)shouldFailClosed clientMode:(SNTCli
 
   id mockConfigurator = OCMClassMock([SNTConfigurator class]);
   OCMStub([mockConfigurator configurator]).andReturn(mockConfigurator);
+
   OCMExpect([mockConfigurator failClosed]).andReturn(shouldFailClosed);
-  if (shouldFailClosed) {
-    OCMExpect([mockConfigurator clientMode]).andReturn(clientMode);
-  }
 
   SNTEndpointSecurityClient *client =
     [[SNTEndpointSecurityClient alloc] initWithESAPI:mockESApi
@@ -630,20 +626,12 @@ - (void)checkDeadlineExpiredFailClosed:(BOOL)shouldFailClosed clientMode:(SNTCli
   [mockConfigurator stopMocking];
 }
 
-- (void)testDeadlineExpiredFailClosedLockdown {
-  [self checkDeadlineExpiredFailClosed:YES clientMode:SNTClientModeLockdown];
-}
-
-- (void)testDeadlineExpiredFailOpenLockdown {
-  [self checkDeadlineExpiredFailClosed:NO clientMode:SNTClientModeLockdown];
-}
-
-- (void)testDeadlineExpiredFailClosedMonitor {
-  [self checkDeadlineExpiredFailClosed:YES clientMode:SNTClientModeMonitor];
+- (void)testDeadlineExpiredFailClosed {
+  [self checkDeadlineExpiredFailClosed:YES];
 }
 
-- (void)testDeadlineExpiredFailOpenMonitor {
-  [self checkDeadlineExpiredFailClosed:NO clientMode:SNTClientModeMonitor];
+- (void)testDeadlineExpiredFailOpen {
+  [self checkDeadlineExpiredFailClosed:NO];
 }
 
 @end
diff --git a/Source/santad/SNTExecutionController.mm b/Source/santad/SNTExecutionController.mm
@@ -230,7 +230,7 @@ - (void)validateExecEvent:(const Message &)esMsg postAction:(bool (^)(SNTAction)
   SNTFileInfo *binInfo = [[SNTFileInfo alloc] initWithEndpointSecurityFile:targetProc->executable
                                                                      error:&fileInfoError];
   if (unlikely(!binInfo)) {
-    if (config.failClosed && config.clientMode == SNTClientModeLockdown) {
+    if (config.failClosed) {
       LOGE(@"Failed to read file %@: %@ and denying action", @(targetProc->executable->path.data),
            fileInfoError.localizedDescription);
       postAction(SNTActionRespondDeny);