Bundle Events #145

tburgin · 2017-02-17T21:13:23Z

Here are the initial commits for:

detecting a blocked bundle execution
event creation for nested binaries
bundle hashing
sync server api updates for bundle events

msuozzo · 2017-02-17T21:37:56Z

👏 😃 👏

msuozzo · 2017-03-01T17:00:52Z

Source/santactl/Commands/sync/SNTCommandSyncManager.m

+
+- (void)postBundleEventsToSyncServer:(NSArray<SNTStoredEvent *> *)events {
+  SNTCommandSyncState *syncState = [self createSyncState];
+  syncState.eventBatchSize = 50;


Should we use santa_api_event_batch_size sent by Upvote for this? It's sent in preflight though and we don't do a preflight here.

Agreed, we should find a way to use the server supplied value. We could just store it in memory in santad if necessary.

Will do so in the next pull request. We can hold it in santactl since it runs as a daemon now.

russellhancox · 2017-03-06T17:51:13Z

Source/SantaGUI/SNTNotificationManager.h

@@ -18,6 +18,7 @@
 ///
 ///  Keeps track of pending notifications and ensures only one is presented to the user at a time.
 ///
-@interface SNTNotificationManager : NSObject<SNTMessageWindowControllerDelegate, SNTNotifierXPC>
+@interface SNTNotificationManager :
+    NSObject<SNTMessageWindowControllerDelegate, SNTNotifierXPC, SNTBundleNotifierXPC>


@interface SNTNotificationManager : NSObject<SNTMessageWindowControllerDelegate, SNTNotifierXPC, SNTBundleNotifierXPC>

russellhancox · 2017-03-06T17:54:19Z

Source/SantaGUI/SNTNotificationManager.m

@@ -143,4 +165,74 @@ - (void)postRuleSyncNotificationWithCustomMessage:(NSString *)message {
  [[NSUserNotificationCenter defaultUserNotificationCenter] deliverNotification:un];
 }

+#pragma mark SNTBundleNotifierXPC protocol methods
+
+- (void)updateTotalFileCountForEvent:(SNTStoredEvent *)event


This method reads weirdly because you're mentioning fileCount at the beginning but then not supplying it until the end, so it's sort of repeated. How about updateCountsForEvent:binaryCount:fileCount: or updateFileCount:binaryCount:forEvent:

updateCountsForEvent:binaryCount:fileCount: makes the most sense to me.
Done.

russellhancox · 2017-03-06T17:55:38Z

Source/SantaGUI/SNTNotificationManager.m

+
+- (void)hashBundleBinariesForEvent:(SNTStoredEvent *)event {
+  // Wait until the bundle service is connected
+  dispatch_semaphore_wait(self.bundleServiceSema, DISPATCH_TIME_FOREVER);


forever scares me, is there not a reasonable limit we can set here?

Yeah, 6 secs to allow the bundle service to reconnect is reasonable. Otherwise abandon bundle hashing and display blockable event.
Done.

russellhancox · 2017-03-06T18:01:30Z

Source/SantaGUI/SNTNotificationManager.m

+
+     // Send the results to santad. It will decide if they need to be synced.
+     SNTXPCConnection *daemonConn = [SNTXPCControlInterface configuredConnection];
+     [daemonConn resume];


Instead of making a new connection back to santad, can we have AppDelegate hand the connection it has down to notificationmanager?

The connections that AppDelegate local variables so they can be created / released during connection / re-connection attempts. It seems saner to me if we create them as needed. There is a bit more overhead, but seems worth it for cleaner code.

russellhancox · 2017-03-06T18:07:35Z

Source/common/SNTFileInfo.m

@@ -280,34 +280,29 @@ - (BOOL)isMissingPageZero {
 ///
 ///  Rationale: An NSBundle has a method executablePath for discovering the main binary within a
 ///  bundle but provides no way to get an NSBundle object when only the executablePath is known.
-///  Also a bundle can contain multiple binaries within the MacOS folder and we want any of these
+///  Also a bundle can contain multiple binaries within it's subdirectories and we want any of these


russellhancox · 2017-03-06T21:54:16Z

Source/santactl/Commands/sync/SNTCommandSyncState.h

@@ -53,7 +53,7 @@
 /// Log upload URL sent from server. If set, LogUpload phase needs to happen.
 @property NSURL *uploadLogURL;

-/// Array of bundle paths to find binaries for.
+/// Array of bundle ids to find binaries for.


russellhancox · 2017-03-06T21:54:57Z

Source/santad/DataLayer/SNTEventTable.m

+        !event.fileSHA256 ||
+        !event.filePath ||
+        !event.occurrenceDate ||
+        !event.decision) return NO;


Do you want to fail all of the events if one of them is bad?

Nope, this should be continue.

russellhancox · 2017-03-06T21:55:35Z

Source/santad/SNTDaemonControlController.m

+#pragma mark Bundle ops
+
+///
+///  This method is only used for santactl's bundleinfo command. For blocked executions, Santa GUI


s/Santa GUI/SantaGUI

russellhancox · 2017-03-06T21:55:43Z

Source/santad/SNTDaemonControlController.m

+}
+
+///
+///  Used by Santa GUI sync the offending event and potentially all the related events,


s/Santa GUI/SantaGUI

russellhancox · 2017-03-06T21:57:17Z

Source/santad/SNTDaemonControlController.m

+///
+- (void)hashBundleBinariesForEvent:(SNTStoredEvent *)event
+                             reply:(SNTBundleHashBlock)reply {
+  SNTXPCConnection *bs = [[SNTXPCConnection alloc] initClientWithServiceName:@"com.google.santabs"];


Move the service ID to a serviceId class method in SNTXPCBundleServiceInterface, following the example of SNTXPCControlInterface

tburgin · 2017-03-22T22:07:09Z

Updates coming to SNTBundleService shortly

tburgin · 2017-03-23T19:10:30Z

PTAL

googlebot · 2017-04-05T18:16:25Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

… sync

googlebot · 2017-04-05T19:47:19Z

CLAs look good, thanks!

russellhancox · 2017-04-06T21:01:58Z

Source/santad/DataLayer/SNTEventTable.m

+    @try {
+      eventData = [NSKeyedArchiver archivedDataWithRootObject:event];
+    } @catch (NSException *exception) {
+      return NO;


This should probably be continue, no?

russellhancox · 2017-04-06T21:02:32Z

Source/santad/DataLayer/SNTEventTable.m

+                                                    BOOL *stop) {
+      success = [db executeUpdate:@"INSERT INTO 'events' (idx, filesha256, eventdata)"
+                 @"VALUES (?, ?, ?)",
+                 event.idx, event.fileSHA256, eventData];


Indent both of these lines +4

russellhancox · 2017-04-06T21:05:30Z

Source/santactl/Commands/SNTCommandBundleInfo.m

+                                                               NSArray<SNTStoredEvent *> *events,
+                                                               NSNumber *time) {
+
+    printf("hashing time: %llu ms\n", time.unsignedLongLongValue);


s/hashing/Hashing

russellhancox · 2017-04-06T21:05:54Z

Source/santactl/Commands/SNTCommandBundleInfo.m

+    printf("%s\n", error.description.UTF8String);
+    exit(1);
+  } else if (!fi.bundle) {
+    printf("not a bundle\n");


russellhancox · 2017-04-06T21:06:01Z

Source/santactl/Commands/SNTCommandBundleInfo.m

@@ -0,0 +1,79 @@
+/// Copyright 2016 Google Inc. All rights reserved.


russellhancox · 2017-04-06T21:06:46Z

Source/santabs/SNTBundleService.m

+    @"%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x"
+    "%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x";
+
+  NSString *sha256 = [[NSString alloc]


[[NSString alloc] initWIthFormat:SHA256FormatString, digest[0], digest[1], digest[2], digest[3], etc. etc.

tburgin · 2017-04-07T15:54:11Z

PTAL

* santabs: Create Santa Bundle Service * common: SNTXPCConnection add initClientWithServiceName: * santad: add logic for blocked bundles * SantaGUI: add ui elements and xpc connections to / from santabs * santactl/sync: add api features for syncing bundle events * santactl/bundleinfo: add bundleinfo command for debug builds * common: prefer bundle hash over file hash for event urls * common: remove syncBackoff property - this is now handled in santactl sync * common: add properties to support the bundle event api * common: find a bundle from a nested binary * review updates * sane bundle hash time outs * post rebase updates * post review updates

tburgin requested a review from russellhancox February 17, 2017 21:13

tburgin mentioned this pull request Feb 28, 2017

disable bundle scans #146

Merged

tburgin added this to the 0.9.18 milestone Feb 28, 2017

msuozzo reviewed Mar 1, 2017

View reviewed changes

russellhancox reviewed Mar 6, 2017

View reviewed changes

tburgin added 13 commits April 5, 2017 15:34

santabs: Create Santa Bundle Service

fa23813

common: SNTXPCConnection add initClientWithServiceName:

818aa08

santad: add logic for blocked bundles

cc5c9f0

SantaGUI: add ui elements and xpc connections to / from santabs

7af7a45

santactl/sync: add api features for syncing bundle events

cd00d1b

santactl/bundleinfo: add bundleinfo command for debug builds

97be460

common: prefer bundle hash over file hash for event urls

21c8ae6

common: remove syncBackoff property - this is now handled in santactl…

fe2ec19

… sync

common: add properties to support the bundle event api

ed0cc3b

common: find a bundle from a nested binary

d6fd527

review updates

f21313a

sane bundle hash time outs

a760e3b

post rebase updates

723fe84

tburgin force-pushed the bundles branch from 61be168 to 723fe84 Compare April 5, 2017 19:47

russellhancox requested changes Apr 6, 2017

View reviewed changes

post review updates

644d6d9

russellhancox approved these changes Apr 7, 2017

View reviewed changes

tburgin merged commit 96150a9 into google:master Apr 7, 2017

		@@ -0,0 +1,79 @@
		/// Copyright 2016 Google Inc. All rights reserved.

Bundle Events #145

Bundle Events #145

Conversation

tburgin commented Feb 17, 2017

msuozzo commented Feb 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tburgin commented Mar 22, 2017

tburgin commented Mar 23, 2017

googlebot commented Apr 5, 2017

googlebot commented Apr 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tburgin commented Apr 7, 2017