Add CMake support, implement fuzzers #284

alessandrogario · 2018-06-20T14:06:30Z

The new CMake project allows the user to select which SDK version to use. The Xcode path is also configurable to support non-standard installation paths and/or systems with multiple versions installed.

Code signing can now be configured via command line, using the CODESIGN_IDENTITY environment variable.

New fuzzing targets (libFuzzer)

SantaCache
santactl
santad

New make targets:

tests: Runs the tests
fuzz: Runs the fuzzer
redist: Regenerates the redistributable folder
install: Installs Santa

A small issue has been found in santactl when the the sync-server returns a valid JSON that does not get deserialized to a dictionary object (not yet fixed inside this PR).

The new CMake project allows the user to select which SDK version to use. The Xcode path is also configurable to support non-standard installation paths and/or systems with multiple versions installed. Code signing can now be configured via command line, using the CODESIGN_IDENTITY environment variable. New fuzzing targets (libFuzzer) - SantaCache - santactl - santad New make targets: - tests: Runs the tests - fuzz: Runs the fuzzer - redist: Regenerates the redistributable folder - install: Installs Santa

googlebot · 2018-06-20T14:06:33Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

alessandrogario · 2018-06-20T14:15:45Z

I have signed the CLA! 👍

googlebot · 2018-06-20T14:15:48Z

CLAs look good, thanks!

russellhancox

Thank you for the PR, sorry it took so long to review. I had a few comments but they're mostly minor things.

russellhancox · 2018-06-26T16:52:34Z

CMakeLists.txt

+  endif()
+
+  if(NOT MACOSX_VERSION_MIN)
+    set(MACOSX_VERSION_MIN 10.13)


Thus far we've been targeting 10.9 as the minimum version, that seems like a reasonable default here.

russellhancox · 2018-06-26T16:52:34Z

CMakeLists.txt

+  endif()
+
+  if(NOT XCODE_ROOT_FOLDER)
+    set(XCODE_ROOT_FOLDER "/Applications/Xcode.app")


Could this instead be determined using xcode-select -p?

russellhancox · 2018-06-26T16:52:34Z

Conf/install.sh

@@ -47,6 +53,9 @@ mkdir -p /usr/local/bin
 # Reload syslogd to pick up ASL configuration change.
 /usr/bin/killall -HUP syslogd

+# Install the configuration file
+/usr/bin/profiles install -path=${SOURCE}/conf/com.google.santa.example.mobileconfig


I think I'd prefer to leave this line out, the example mobileconfig file isn't all that useful on its own and I generally wouldn't expect a sysadmin to package the mobileconfig up with the Santa package - it should either be distributed as its own package or installed using a config management tool (an MDM, Puppet, etc.)

russellhancox · 2018-06-26T16:52:34Z

Conf/uninstall.sh

@@ -21,6 +30,10 @@ user=$(/usr/bin/stat -f '%u' /dev/console)
 /bin/rm -f /private/etc/asl/com.google.santa.asl.conf
 /bin/rm -f /private/etc/newsyslog.d/com.google.santa.newsyslog.conf
 /bin/rm -f /usr/local/bin/santactl # just a symlink
+
+# Uninstall the configuration file
+/usr/bin/profiles remove -path=${SOURCE}/conf/com.google.santa.example.mobileconfig


russellhancox · 2018-06-26T16:52:34Z

Fuzzing/santactl/src/main.mm

+    return 0;
+  }
+
+	SNTCommandSyncRuleDownload *obj = [[SNTCommandSyncRuleDownload alloc] initWithState:state];


nit: indented too far

russellhancox · 2018-06-26T16:52:34Z

Fuzzing/santactl/CMakeLists.txt

+  )
+
+  target_compile_definitions("${PROJECT_NAME}" PRIVATE
+    SNT_OS_LOG_CATEGORY=\"${PROJECT_NAME}\"


What's this define for?

russellhancox · 2018-06-26T16:52:34Z

Fuzzing/santactl/src/main.mm

+#include <SNTRule.h>
+
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data, std::size_t size) {
+  NSData* buffer = [NSData dataWithBytes:static_cast<const void *>(data) length:size];


To be consistent with the rest of the source, the * should be with the variable name:

NSData *buffer

russellhancox · 2018-06-26T16:52:34Z

Fuzzing/santactl/src/main.mm

+
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data, std::size_t size) {
+  NSData* buffer = [NSData dataWithBytes:static_cast<const void *>(data) length:size];
+  if (buffer == nil) {


Also consistency: we generally use the form if (buffer) { here.

russellhancox · 2018-06-26T16:52:34Z

Fuzzing/santad/src/checkCacheForVnodeID.mm

+
+#pragma pack(push, 1)
+
+#pragma pack(pop)


Are these 2 pragma's leftovers from something?

russellhancox · 2018-06-26T16:52:34Z

Tests/santa-driver/main.mm

@@ -0,0 +1,51 @@
+#import "SNTDriverManager.h"


Can you add a comment at the top of this file describing what it's testing. I'm guessing it's a test that attempts to connect to the driver from multiple threads simultaneously doesn't cause crashes.

You are right! I have added a comment at the top of the file

alessandrogario · 2018-06-28T15:11:35Z

@russellhancox Thanks so much for the review! I've updated the branch with the changes you have requested

russellhancox · 2018-06-29T18:15:43Z

Thanks again for the submission!

The new CMake project allows the user to select which SDK version to use. The Xcode path is also configurable to support non-standard installation paths and/or systems with multiple versions installed. Code signing can now be configured via command line, using the CODESIGN_IDENTITY environment variable. New fuzzing targets (libFuzzer) - SantaCache - santactl - santad New make targets: - tests: Runs the tests - fuzz: Runs the fuzzer - redist: Regenerates the redistributable folder - install: Installs Santa

russellhancox self-assigned this Jun 26, 2018

russellhancox added enhancement cla:yes labels Jun 26, 2018

russellhancox requested changes Jun 26, 2018

View changes

Code review changes

Loading status checks…

b2fce4b

russellhancox approved these changes Jun 29, 2018

View changes

russellhancox merged commit 629e702 into google:master Jun 29, 2018
2 checks passed

2 checks passed

cla/google All necessary CLAs are signed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

russellhancox mentioned this pull request Sep 26, 2018

Standardize build system #303

Open

google / santa

Add CMake support, implement fuzzers #284

Add CMake support, implement fuzzers #284

alessandrogario commented Jun 20, 2018

googlebot commented Jun 20, 2018

alessandrogario commented Jun 20, 2018

googlebot commented Jun 20, 2018

russellhancox left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

alessandrogario commented Jun 28, 2018 •

edited

russellhancox commented Jun 29, 2018


		#pragma pack(push, 1)

		#pragma pack(pop)

google / santa

Join GitHub today

Add CMake support, implement fuzzers #284

Add CMake support, implement fuzzers #284

Conversation

alessandrogario commented Jun 20, 2018

googlebot commented Jun 20, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

alessandrogario commented Jun 20, 2018

googlebot commented Jun 20, 2018

russellhancox left a comment

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

russellhancox Jun 26, 2018 Collaborator

This comment has been minimized.

alessandrogario Jun 28, 2018 Author Contributor

alessandrogario commented Jun 28, 2018 • edited

russellhancox commented Jun 29, 2018

Essential cookies

Always active

Analytics cookies

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

russellhancox Jun 26, 2018
Collaborator

alessandrogario Jun 28, 2018
Author Contributor

alessandrogario commented Jun 28, 2018 •

edited