Enable -Werror -Wall on our build rules (#572)

* Reorder init lists for -Wreorder-init-lists

* Add nullability annotations to the rest of EndpointSecurityTestUtil

* Added fake uses for -Wunused-variable

* Corrected signed/unsigned int conversions in SNTPrefixTree

* Explicitly convert implicit conversions in Santacache

* Set bazelrc to -Werror -Wall
This commit is contained in:
Kent Ma 2021-08-13 11:31:41 -04:00 committed by GitHub
parent ac1f8ea1b8
commit 1edf6d9200
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 104 additions and 84 deletions

View File

@ -1,2 +1,5 @@
build --apple_generate_dsym --define=apple.propagate_embedded_extra_outputs=yes
build --host_force_python=PY2
build --copt=-Werror
build --copt=-Wall

View File

@ -16,27 +16,37 @@
#ifdef KERNEL
#include <libkern/locks.h>
#include "Source/common/SNTLogging.h"
#else
#include <mutex>
#include <string.h>
#define LOGD(format, ...) // NOP
#define LOGE(format, ...) // NOP
#include <mutex>
#define LOGD(format, ...) // NOP
#define LOGE(format, ...) // NOP
#define lck_rw_lock_shared(l) pthread_rwlock_rdlock(&l)
#define lck_rw_unlock_shared(l) pthread_rwlock_unlock(&l)
#define lck_rw_lock_exclusive(l) pthread_rwlock_wrlock(&l)
#define lck_rw_unlock_exclusive(l) pthread_rwlock_unlock(&l)
#define lck_rw_lock_shared_to_exclusive(l) ({ pthread_rwlock_unlock(&l); false; })
#define lck_rw_lock_exclusive_to_shared(l) ({ pthread_rwlock_unlock(&l); pthread_rwlock_rdlock(&l); })
#define lck_rw_lock_shared_to_exclusive(l) \
({ \
pthread_rwlock_unlock(&l); \
false; \
})
#define lck_rw_lock_exclusive_to_shared(l) \
({ \
pthread_rwlock_unlock(&l); \
pthread_rwlock_rdlock(&l); \
})
#define lck_mtx_lock(l) l->lock()
#define lck_mtx_unlock(l) l->unlock()
#endif // KERNEL
#endif // KERNEL
SNTPrefixTree::SNTPrefixTree(uint32_t max_nodes) {
root_ = new SantaPrefixNode();
@ -45,7 +55,8 @@ SNTPrefixTree::SNTPrefixTree(uint32_t max_nodes) {
#ifdef KERNEL
spt_lock_grp_attr_ = lck_grp_attr_alloc_init();
spt_lock_grp_ = lck_grp_alloc_init("santa-prefix-tree-lock", spt_lock_grp_attr_);
spt_lock_grp_ =
lck_grp_alloc_init("santa-prefix-tree-lock", spt_lock_grp_attr_);
spt_lock_attr_ = lck_attr_alloc_init();
spt_lock_ = lck_rw_alloc_init(spt_lock_grp_, spt_lock_attr_);
@ -57,9 +68,9 @@ SNTPrefixTree::SNTPrefixTree(uint32_t max_nodes) {
}
IOReturn SNTPrefixTree::AddPrefix(const char *prefix, uint64_t *node_count) {
// Serialize requests to AddPrefix. Otherwise one AddPrefix thread could overwrite whole
// branches of another. HasPrefix is still free to read the tree, until AddPrefix needs to
// modify it.
// Serialize requests to AddPrefix. Otherwise one AddPrefix thread could
// overwrite whole branches of another. HasPrefix is still free to read the
// tree, until AddPrefix needs to modify it.
lck_mtx_lock(spt_add_lock_);
// Don't allow an empty prefix.
@ -74,13 +85,13 @@ IOReturn SNTPrefixTree::AddPrefix(const char *prefix, uint64_t *node_count) {
lck_rw_lock_shared(spt_lock_);
SantaPrefixNode *node = root_;
for (int i = 0; i < len; ++i) {
for (size_t i = 0; i < len; ++i) {
// If there is a node in the path that is considered a prefix, stop adding.
// For our purposes we only care about the shortest path that matches.
if (node->isPrefix) break;
// Only process a byte at a time.
uint8_t value = prefix[i];
uint8_t value = (uint8_t)prefix[i];
// Create the child if it does not exist.
if (!node->children[value]) {
@ -103,7 +114,7 @@ IOReturn SNTPrefixTree::AddPrefix(const char *prefix, uint64_t *node_count) {
// Create the rest of the prefix.
while (i < len) {
value = prefix[i++];
value = (uint8_t)prefix[i++];
SantaPrefixNode *new_node = new SantaPrefixNode();
node->children[value] = new_node;
@ -159,7 +170,8 @@ bool SNTPrefixTree::HasPrefix(const char *string) {
SantaPrefixNode *node = root_;
// A well formed tree will always break this loop. Even if string doesn't terminate.
// A well formed tree will always break this loop. Even if string doesn't
// terminate.
const char *p = string;
while (*p) {
// Only process a byte at a time.
@ -193,8 +205,8 @@ void SNTPrefixTree::Reset() {
void SNTPrefixTree::PruneNode(SantaPrefixNode *target) {
if (!target) return;
// For deep trees, a recursive approach will generate too many stack frames. Make a "stack"
// and walk the tree.
// For deep trees, a recursive approach will generate too many stack frames.
// Make a "stack" and walk the tree.
auto stack = new SantaPrefixNode *[node_count_ + 1];
if (!stack) {
LOGE("Unable to prune tree!");
@ -206,7 +218,8 @@ void SNTPrefixTree::PruneNode(SantaPrefixNode *target) {
// Seed the "stack" with a starting node.
stack[count++] = target;
// Start at the target node and walk the tree to find and delete all the sub-nodes.
// Start at the target node and walk the tree to find and delete all the
// sub-nodes.
while (count) {
auto node = stack[--count];

View File

@ -51,7 +51,7 @@
*/
template <typename T>
uint64_t SantaCacheHasher(T const &t) {
return t * 11400714819323198549UL;
return (uint64_t)t * 11400714819323198549UL;
};
/**
@ -80,7 +80,7 @@ class SantaCache {
usage. Cannot be higher than 64 to try and ensure buckets don't overflow.
*/
SantaCache(uint64_t maximum_size = 10000, uint8_t per_bucket = 5) {
if (unlikely(per_bucket > maximum_size)) per_bucket = maximum_size;
if (unlikely(per_bucket > maximum_size)) per_bucket = (uint8_t)maximum_size;
if (unlikely(per_bucket < 1)) per_bucket = 1;
if (unlikely(per_bucket > 64)) per_bucket = 64;
max_size_ = maximum_size;
@ -214,7 +214,7 @@ class SantaCache {
}
uint16_t size = *array_size;
if (start + size > bucket_count_) size = bucket_count_ - start;
if (start + size > bucket_count_) size = (uint16_t)(bucket_count_ - start);
for (uint16_t i = 0; i < size; ++i) {
uint16_t count = 0;

View File

@ -17,7 +17,7 @@
#include <bsm/libbsm.h>
CF_EXTERN_C_BEGIN
es_string_token_t MakeStringToken(const NSString *s);
es_string_token_t MakeStringToken(const NSString *_Nonnull s);
CF_EXTERN_C_END
@interface ESResponse : NSObject
@ -25,17 +25,17 @@ CF_EXTERN_C_END
@property(nonatomic) bool shouldCache;
@end
typedef void (^ESCallback)(ESResponse *);
typedef void (^ESCallback)(ESResponse *_Nonnull);
// Singleton wrapper around all of the kernel-level EndpointSecurity framework functions.
@interface MockEndpointSecurity : NSObject
@property BOOL subscribed;
- (void)reset;
- (void)registerResponseCallback:(ESCallback)callback;
- (void)triggerHandler:(es_message_t *)msg;
- (void)registerResponseCallback:(ESCallback _Nonnull)callback;
- (void)triggerHandler:(es_message_t *_Nonnull)msg;
/// Retrieve an initialized singleton MockEndpointSecurity object
+ (instancetype)mockEndpointSecurity;
+ (instancetype _Nonnull)mockEndpointSecurity;
@end
API_UNAVAILABLE(ios, tvos, watchos)
@ -64,6 +64,6 @@ API_AVAILABLE(macos(10.15))
API_UNAVAILABLE(ios, tvos, watchos) es_return_t es_delete_client(es_client_t *_Nullable client);
API_AVAILABLE(macos(10.15))
API_UNAVAILABLE(ios, tvos, watchos) es_return_t
es_unsubscribe(es_client_t *_Nonnull client, const es_event_type_t *_Nonnull events,
uint32_t event_count);
API_UNAVAILABLE(ios, tvos, watchos)
es_return_t es_unsubscribe(es_client_t *_Nonnull client, const es_event_type_t *_Nonnull events,
uint32_t event_count);

View File

@ -19,10 +19,10 @@
#import "Source/santad/EventProviders/EndpointSecurityTestUtil.h"
CF_EXTERN_C_BEGIN
es_string_token_t MakeStringToken(const NSString *s) {
es_string_token_t MakeStringToken(const NSString *_Nonnull s) {
return (es_string_token_t){
.data = [s UTF8String],
.length = [s length],
.data = [s UTF8String],
};
}
CF_EXTERN_C_END
@ -65,11 +65,11 @@ CF_EXTERN_C_END
self.handler = handler;
}
- (void)triggerHandler:(es_message_t *)msg {
self.handler((__bridge es_client_t *)self.client, msg);
- (void)triggerHandler:(es_message_t *_Nonnull)msg {
self.handler((__bridge es_client_t *_Nullable)self.client, msg);
}
- (void)registerResponseCallback:(ESCallback)callback {
- (void)registerResponseCallback:(ESCallback _Nonnull)callback {
@synchronized(self) {
[self.responseCallbacks addObject:callback];
}
@ -89,7 +89,7 @@ CF_EXTERN_C_END
return ES_RESPOND_RESULT_SUCCESS;
};
+ (instancetype)mockEndpointSecurity {
+ (instancetype _Nonnull)mockEndpointSecurity {
static MockEndpointSecurity *sharedES;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{

View File

@ -42,6 +42,7 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
MockEndpointSecurity *mockES = [MockEndpointSecurity mockEndpointSecurity];
[mockES reset];
SNTEndpointSecurityManager *snt = [[SNTEndpointSecurityManager alloc] init];
(void)snt; // Make it appear used for the sake of -Wunused-variable
XCTestExpectation *expectation =
[self expectationWithDescription:@"Wait for santa's Auth dispatch queue"];
@ -60,26 +61,26 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
es_file_t dbFile = {.path = MakeStringToken(kEventsDBPath)};
es_file_t otherBinary = {.path = MakeStringToken(@"somebinary")};
es_process_t proc = {
.executable = &otherBinary,
.is_es_client = false,
.codesigning_flags = 570509313,
.session_id = 12345,
.group_id = 12345,
.ppid = 12345,
.original_ppid = 12345,
.group_id = 12345,
.session_id = 12345,
.codesigning_flags = 570509313,
.is_platform_binary = false,
.is_es_client = false,
.executable = &otherBinary,
};
es_event_unlink_t unlink_event = {.target = &dbFile};
es_events_t event = {.unlink = unlink_event};
es_message_t m = {
.version = 4,
.event_type = ES_EVENT_TYPE_AUTH_UNLINK,
.event = event,
.mach_time = 1234,
.action_type = ES_ACTION_TYPE_AUTH,
.deadline = 1234,
.process = &proc,
.seq_num = 1337,
.action_type = ES_ACTION_TYPE_AUTH,
.event_type = ES_EVENT_TYPE_AUTH_UNLINK,
.event = event,
};
[mockES triggerHandler:&m];
@ -105,6 +106,7 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
MockEndpointSecurity *mockES = [MockEndpointSecurity mockEndpointSecurity];
[mockES reset];
SNTEndpointSecurityManager *snt = [[SNTEndpointSecurityManager alloc] init];
(void)snt; // Make it appear used for the sake of -Wunused-variable
XCTestExpectation *expectation = [self expectationWithDescription:@"Wait for response from ES"];
__block ESResponse *got;
@ -116,26 +118,26 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
es_file_t dbFile = {.path = MakeStringToken(testPath)};
es_file_t otherBinary = {.path = MakeStringToken(@"somebinary")};
es_process_t proc = {
.executable = &otherBinary,
.is_es_client = false,
.codesigning_flags = 570509313,
.session_id = 12345,
.group_id = 12345,
.ppid = 12345,
.original_ppid = 12345,
.group_id = 12345,
.session_id = 12345,
.codesigning_flags = 570509313,
.is_platform_binary = false,
.is_es_client = false,
.executable = &otherBinary,
};
es_event_unlink_t unlink_event = {.target = &dbFile};
es_events_t event = {.unlink = unlink_event};
es_message_t m = {
.version = 4,
.event_type = ES_EVENT_TYPE_AUTH_UNLINK,
.event = event,
.mach_time = DISPATCH_TIME_NOW,
.action_type = ES_ACTION_TYPE_AUTH,
.deadline = DISPATCH_TIME_FOREVER,
.process = &proc,
.seq_num = 1337,
.action_type = ES_ACTION_TYPE_AUTH,
.event_type = ES_EVENT_TYPE_AUTH_UNLINK,
.event = event,
};
[mockES triggerHandler:&m];
@ -155,9 +157,9 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
MockEndpointSecurity *mockES = [MockEndpointSecurity mockEndpointSecurity];
[mockES reset];
SNTEndpointSecurityManager *snt = [[SNTEndpointSecurityManager alloc] init];
(void)snt; // Make it appear used for the sake of -Wunused-variable
XCTestExpectation *expectation = [self expectationWithDescription:@"Wait for response from ES"];
__block ESResponse *got;
[mockES registerResponseCallback:^(ESResponse *r) {
got = r;
@ -167,26 +169,26 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
es_file_t dbFile = {.path = MakeStringToken(@"/some/other/path")};
es_file_t otherBinary = {.path = MakeStringToken(@"somebinary")};
es_process_t proc = {
.executable = &otherBinary,
.is_es_client = true,
.codesigning_flags = 570509313,
.session_id = 12345,
.group_id = 12345,
.ppid = 12345,
.original_ppid = 12345,
.group_id = 12345,
.session_id = 12345,
.codesigning_flags = 570509313,
.is_platform_binary = false,
.is_es_client = true,
.executable = &otherBinary,
};
es_event_unlink_t unlink_event = {.target = &dbFile};
es_events_t event = {.unlink = unlink_event};
es_message_t m = {
.version = 4,
.event_type = ES_EVENT_TYPE_AUTH_UNLINK,
.event = event,
.mach_time = DISPATCH_TIME_NOW,
.action_type = ES_ACTION_TYPE_AUTH,
.deadline = DISPATCH_TIME_FOREVER,
.process = &proc,
.seq_num = 1337,
.action_type = ES_ACTION_TYPE_AUTH,
.event_type = ES_EVENT_TYPE_AUTH_UNLINK,
.event = event,
};
[mockES triggerHandler:&m];
@ -205,6 +207,7 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
MockEndpointSecurity *mockES = [MockEndpointSecurity mockEndpointSecurity];
[mockES reset];
SNTEndpointSecurityManager *snt = [[SNTEndpointSecurityManager alloc] init];
(void)snt; // Make it appear used for the sake of -Wunused-variable
XCTestExpectation *expectation = [self expectationWithDescription:@"Wait for response from ES"];
__block ESResponse *got;
@ -216,33 +219,33 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
es_file_t dbFile = {.path = MakeStringToken(testPath)};
es_event_rename_t renameEvent = {
.destination_type = ES_DESTINATION_TYPE_EXISTING_FILE,
.source = &otherFile,
.destination_type = ES_DESTINATION_TYPE_EXISTING_FILE,
.destination = {.existing_file = &dbFile},
};
es_file_t otherBinary = {.path = MakeStringToken(@"somebinary")};
es_process_t proc = {
.executable = &otherBinary,
.is_es_client = false,
.codesigning_flags = 570509313,
.session_id = 12345,
.group_id = 12345,
.ppid = 12345,
.original_ppid = 12345,
.group_id = 12345,
.session_id = 12345,
.codesigning_flags = 570509313,
.is_platform_binary = false,
.is_es_client = false,
.executable = &otherBinary,
};
es_events_t event = {.rename = renameEvent};
es_message_t m = {
.version = 4,
.event_type = ES_EVENT_TYPE_AUTH_RENAME,
.event = event,
.mach_time = DISPATCH_TIME_NOW,
.action_type = ES_ACTION_TYPE_AUTH,
.deadline = DISPATCH_TIME_FOREVER,
.process = &proc,
.seq_num = 1337,
.action_type = ES_ACTION_TYPE_AUTH,
.event_type = ES_EVENT_TYPE_AUTH_RENAME,
.event = event,
};
[mockES triggerHandler:&m];
@ -264,6 +267,7 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
MockEndpointSecurity *mockES = [MockEndpointSecurity mockEndpointSecurity];
[mockES reset];
SNTEndpointSecurityManager *snt = [[SNTEndpointSecurityManager alloc] init];
(void)snt; // Make it appear used for the sake of -Wunused-variable
XCTestExpectation *expectation = [self expectationWithDescription:@"Wait for response from ES"];
__block ESResponse *got;
@ -289,26 +293,26 @@ const NSString *const kRulesDBPath = @"/private/var/db/santa/rules.db";
es_file_t otherBinary = {.path = MakeStringToken(@"somebinary")};
es_process_t proc = {
.executable = &otherBinary,
.is_es_client = false,
.codesigning_flags = 570509313,
.session_id = 12345,
.group_id = 12345,
.ppid = 12345,
.original_ppid = 12345,
.group_id = 12345,
.session_id = 12345,
.codesigning_flags = 570509313,
.is_platform_binary = false,
.is_es_client = false,
.executable = &otherBinary,
};
es_events_t event = {.rename = renameEvent};
es_message_t m = {
.version = 4,
.event_type = ES_EVENT_TYPE_AUTH_RENAME,
.event = event,
.mach_time = DISPATCH_TIME_NOW,
.action_type = ES_ACTION_TYPE_AUTH,
.deadline = DISPATCH_TIME_FOREVER,
.process = &proc,
.seq_num = 1337,
.action_type = ES_ACTION_TYPE_AUTH,
.event_type = ES_EVENT_TYPE_AUTH_RENAME,
.event = event,
};
[mockES triggerHandler:&m];

View File

@ -85,14 +85,14 @@
NSString *binaryPath = [NSString pathWithComponents:@[ testPath, binaryName ]];
es_file_t binary = {.path = MakeStringToken(binaryPath)};
es_process_t proc = {
.executable = &binary,
.is_es_client = false,
.codesigning_flags = 570509313,
.session_id = 12345,
.group_id = 12345,
.ppid = 12345,
.original_ppid = 12345,
.group_id = 12345,
.session_id = 12345,
.codesigning_flags = 570509313,
.is_platform_binary = false,
.is_es_client = false,
.executable = &binary,
};
es_event_exec_t exec_event = {
.target = &proc,
@ -101,12 +101,12 @@
es_message_t m = {
.version = 4,
.mach_time = 181576143417379,
.event_type = ES_EVENT_TYPE_AUTH_EXEC,
.event = event,
.action_type = ES_ACTION_TYPE_AUTH,
.deadline = DISPATCH_TIME_FOREVER,
.process = &proc,
.seq_num = 1,
.action_type = ES_ACTION_TYPE_AUTH,
.event_type = ES_EVENT_TYPE_AUTH_EXEC,
.event = event,
};
[mockES triggerHandler:&m];