Fix: Do not flush authcache when receiving duplicate block rules from…

… the sync service (#1310)

* Change the behavior of addedRulesShouldFlushDecisionCache to flush when 1000 non-allowlist rules are added or a remove rule is encountered or any new non-allowlist rules are added

* Add tests for cache flushing behavior.

Source/santad/DataLayer/SNTRuleTable.m

@@ -463,25 +463,59 @@ - (BOOL)addRules:(NSArray *)rules
 }
 
 - (BOOL)addedRulesShouldFlushDecisionCache:(NSArray *)rules {
-  // Check for non-plain-allowlist rules first before querying the database.
+  uint64_t nonAllowRuleCount = 0;
+
   for (SNTRule *rule in rules) {
-    if (rule.state != SNTRuleStateAllow) return YES;
+    // If the rule is a remove rule, act conservatively and flush the cache.
+    // This is to make sure cached rules of different precedence rules do not
+    // impact final decision.
+    if (rule.state == SNTRuleStateRemove) {
+      return YES;
+    }
+    if (rule.state != SNTRuleStateAllow) {
+      nonAllowRuleCount++;
+
+      // Just flush if we more than 1000 block rules.
+      if (nonAllowRuleCount >= 1000) return YES;
+    }
   }
 
-  // If still here, then all rules in the array are allowlist rules.  So now we look for allowlist
-  // rules where there is a previously existing allowlist compiler rule for the same identifier.
-  // If so we find such a rule, then cache should be flushed.
+  // Check newly synced rules for any blocking rules. If any are found, check
+  // in the db to see if they already exist. If they're not found or were
+  // previously allow rules flush the cache.
+  //
+  // If all rules in the array are allowlist rules,  look for allowlist rules
+  // where there is a previously existing allowlist compiler rule for the same
+  // identifier.  If so we find such a rule, then cache should be flushed.
   __block BOOL flushDecisionCache = NO;
+
   [self inTransaction:^(FMDatabase *db, BOOL *rollback) {
     for (SNTRule *rule in rules) {
-      // Allowlist certificate rules are ignored
-      if (rule.type == SNTRuleTypeCertificate) continue;
-
-      if ([db longForQuery:
-                @"SELECT COUNT(*) FROM rules WHERE identifier=? AND type=? AND state=? LIMIT 1",
-                rule.identifier, @(SNTRuleTypeBinary), @(SNTRuleStateAllowCompiler)] > 0) {
-        flushDecisionCache = YES;
-        break;
+      // If the rule is a block rule, silent block rule, or a compiler rule check if it already
+      // exists in the database.
+      //
+      // If it does not then flush the cache. To ensure that the new rule is honored.
+      if ((rule.state != SNTRuleStateAllow)) {
+        if ([db longForQuery:
+                  @"SELECT COUNT(*) FROM rules WHERE identifier=? AND type=? AND state=? LIMIT 1",
+                  rule.identifier, @(rule.type), @(rule.state)] == 0) {
+          flushDecisionCache = YES;
+          return;
+        }
+      } else {
+        // At this point we know the rule is an allowlist rule. Check if it's
+        // overriding a compiler rule.
+
+        // Skip certificate and TeamID rules as they cannot be compiler rules.
+        if (rule.type == SNTRuleTypeCertificate || rule.type == SNTRuleTypeTeamID) continue;
+
+        if ([db longForQuery:@"SELECT COUNT(*) FROM rules WHERE identifier=? AND type IN (?, ?, ?)"
+                             @" AND state=? LIMIT 1",
+                             rule.identifier, @(SNTRuleTypeCDHash), @(SNTRuleTypeBinary),
+                             @(SNTRuleTypeSigningID), @(SNTRuleStateAllowCompiler)] > 0) {
+          flushDecisionCache = YES;
+          return;
+        }
       }
     }
   }];
@@ -549,7 +583,6 @@ - (BOOL)fillError:(NSError **)error code:(SNTRuleTableError)code message:(NSStri
   *error = [NSError errorWithDomain:@"com.google.santad.ruletable" code:code userInfo:d];
   return YES;
 }
-
 #pragma mark Querying
 
 // Retrieve all rules from the Database

Source/santad/DataLayer/SNTRuleTableTest.m

@@ -308,8 +308,11 @@ - (void)testFetchCDHashRule {
 - (void)testFetchRuleOrdering {
   NSError *err;
   [self.sut addRules:@[
-    [self _exampleCertRule], [self _exampleBinaryRule], [self _exampleTeamIDRule],
-    [self _exampleSigningIDRuleIsPlatform:NO], [self _exampleCDHashRule],
+    [self _exampleCertRule],
+    [self _exampleBinaryRule],
+    [self _exampleTeamIDRule],
+    [self _exampleSigningIDRuleIsPlatform:NO],
+    [self _exampleCDHashRule],
   ]
          ruleCleanup:SNTRuleCleanupNone
                error:&err];
@@ -437,4 +440,77 @@ - (void)testRetrieveAllRulesWithMultipleRules {
   XCTAssertEqualObjects(rules[4], [self _exampleCDHashRule]);
 }
 
+- (void)testAddedRulesShouldFlushDecisionCacheWithNewBlockRule {
+  // Ensure that a brand new block rule flushes the decision cache.
+  NSError *error;
+  SNTRule *r = [self _exampleBinaryRule];
+  [self.sut addRules:@[ r ] ruleCleanup:SNTRuleCleanupNone error:&error];
+  XCTAssertNil(error);
+  XCTAssertEqual(self.sut.ruleCount, 1);
+  XCTAssertEqual(self.sut.binaryRuleCount, 1);
+
+  // Change the identifer so that the hash of a block rule is not found in the
+  // db.
+  r.identifier = @"bfff7d3f6c389ebf7a76a666c484d42ea447834901bc29141439ae7c7b96ff09";
+  XCTAssertEqual(YES, [self.sut addedRulesShouldFlushDecisionCache:@[ r ]]);
+}
+
+// Ensure that a brand new block rule flushes the decision cache.
+- (void)testAddedRulesShouldFlushDecisionCacheWithOldBlockRule {
+  NSError *error;
+  SNTRule *r = [self _exampleBinaryRule];
+  [self.sut addRules:@[ r ] ruleCleanup:SNTRuleCleanupNone error:&error];
+  XCTAssertNil(error);
+  XCTAssertEqual(self.sut.ruleCount, 1);
+  XCTAssertEqual(self.sut.binaryRuleCount, 1);
+  XCTAssertEqual(NO, [self.sut addedRulesShouldFlushDecisionCache:@[ r ]]);
+}
+
+// Ensure that a larger number of blocks flushes the decision cache.
+- (void)testAddedRulesShouldFlushDecisionCacheWithLargeNumberOfBlocks {
+  NSError *error;
+  SNTRule *r = [self _exampleBinaryRule];
+  [self.sut addRules:@[ r ] ruleCleanup:SNTRuleCleanupNone error:&error];
+  XCTAssertNil(error);
+  XCTAssertEqual(self.sut.ruleCount, 1);
+  XCTAssertEqual(self.sut.binaryRuleCount, 1);
+  NSMutableArray<SNTRule *> *newRules = [NSMutableArray array];
+  for (int i = 0; i < 1000; i++) {
+    newRules[i] = r;
+  }
+
+  XCTAssertEqual(YES, [self.sut addedRulesShouldFlushDecisionCache:newRules]);
+}
+
+// Ensure that an allow rule that overrides a compiler rule flushes the
+// decision cache.
+- (void)testAddedRulesShouldFlushDecisionCacheWithCompilerRule {
+  NSError *error;
+  SNTRule *r = [self _exampleBinaryRule];
+  r.type = SNTRuleTypeBinary;
+  r.state = SNTRuleStateAllowCompiler;
+  [self.sut addRules:@[ r ] ruleCleanup:SNTRuleCleanupNone error:&error];
+  XCTAssertNil(error);
+  XCTAssertEqual(self.sut.ruleCount, 1);
+  XCTAssertEqual(self.sut.binaryRuleCount, 1);
+  // make the rule an allow rule
+  r.state = SNTRuleStateAllow;
+  XCTAssertEqual(YES, [self.sut addedRulesShouldFlushDecisionCache:@[ r ]]);
+}
+
+// Ensure that an Remove rule targeting an allow rule causes a flush of the cache.
+- (void)testAddedRulesShouldFlushDecisionCacheWithRemoveRule {
+  NSError *error;
+  SNTRule *r = [self _exampleBinaryRule];
+  r.type = SNTRuleTypeBinary;
+  r.state = SNTRuleStateAllow;
+  [self.sut addRules:@[ r ] ruleCleanup:SNTRuleCleanupNone error:&error];
+  XCTAssertNil(error);
+  XCTAssertEqual(self.sut.ruleCount, 1);
+  XCTAssertEqual(self.sut.binaryRuleCount, 1);
+
+  r.state = SNTRuleStateRemove;
+  XCTAssertEqual(YES, [self.sut addedRulesShouldFlushDecisionCache:@[ r ]]);
+}
+
 @end