Browse code

clamonacc - improve inline and function level documentation

Mickey Sola authored on 2019/07/25 01:05:36
Showing 4 changed files
... ...
@@ -156,6 +156,14 @@ int onas_check_remote(struct onas_context  **ctx, cl_error_t *err) {
156 156
 	return ret;
157 157
 }
158 158
 
159
+/**
160
+ * @brief initialises a curl connection for the onaccess client; curl must be initialised globally before use
161
+ *
162
+ * @param curl pointer to the curl object to be used in the connection attempt
163
+ * @param ipaddr string which refers to either the TCPaddress or the local socket to connect to
164
+ * @param port the port to use in case of TCP connection, set to 0 if connecting to a local socket
165
+ * @param timeout time in ms to allow curl before timing out connection attempts
166
+ */
159 167
 CURLcode onas_curl_init(CURL **curl, const char *ipaddr, int64_t port, int64_t timeout) {
160 168
 
161 169
 	CURLcode curlcode = CURLE_OK;
... ...
@@ -382,6 +390,21 @@ int onas_get_clamd_version(struct onas_context **ctx)
382 382
     return 0;
383 383
 }
384 384
 
385
+/**
386
+ * @brief kick off scanning and return results
387
+ *
388
+ * @param tcpaddr   string string which refers to either the TCPaddress or the local socket to connect to
389
+ * @param portnum   the port to use in case of TCP connection, set to 0 if connecting to a local socket
390
+ * @param scantype  the type of scan to perform, e.g. fdpass, stream
391
+ * @param maxstream the max streamsize (in bytes) allowed across the socket per file
392
+ * @param fname     the name of the file to be scanned
393
+ * @param fd        the file descriptor for the file to be scanned, often (but not always) this is held by fanotify
394
+ * @param timeout   time in ms to allow curl before timing out connection attempts
395
+ * @param sb        variable to store and pass all of our stat info on the file so we don't have to access it multiple times (triggering multiple events)
396
+ * @param infected  return variable indincating whether daemon returned with an infected verdict or not
397
+ * @param err       return variable passed to the daemon protocol interface indicating how many things went wrong in the course of scanning
398
+ * @param ret_code  return variable passed to the daemon protocol interface indicating last known issue or success
399
+ */
385 400
 int onas_client_scan(const char *tcpaddr, int64_t portnum, int32_t scantype, uint64_t maxstream, const char *fname, int fd, int64_t timeout, STATBUF sb, int *infected, int *err, cl_error_t *ret_code)
386 401
 {
387 402
 	CURL *curl = NULL;
... ...
@@ -68,6 +68,28 @@ static struct onas_lnode *onas_listnode_init(void);
68 68
 
69 69
 static struct onas_hnode *onas_hashnode_init(void);
70 70
 
71
+/**
72
+ * The data structure described and implemented below is a hash table with elements that also act as relational nodes
73
+ * in a tree. This allows for average case constant time retrieval of nodes, and recursive operation on a node and all
74
+ * it's children and parents. The memory cost for this speed of relational retrieval is necessarily high, as every node
75
+ * must also keep track of it's children in a key-accessible way. To cut down on memory costs, children of nodes are not
76
+ * themselves key accessible, but must be combined with their parent in a constant-time operation to be retrieved from
77
+ * the table.
78
+ *
79
+ * Further optimization to retrieval and space management may include storing direct address to given children nodes, but
80
+ * such a design will create further complexitiy and time cost at insertion--which must also be as fast as possible in
81
+ * order to accomadate the real-time nature of security event processing.
82
+ *
83
+ * To date, the hashing function itself has not been well studied, and as such buckets were implemented from the start to
84
+ * help account for any potential collission issues in its design, as a measure to help offset any major time sinks during
85
+ * insertion.
86
+ *
87
+ * One last important note about this hash table is that to avoid massive slowdowns, it does not grow, but instead relies on
88
+ * buckets and a generous default size to distribute that load. Slight hit to retrieval time is a fair cost to pay to avoid
89
+ * total loss of service in a real-time system. Future work here might include automatically confiuguring initial hashtable
90
+ * size to align with the system being monitored, or max inotify watch points since that's our hard limit anyways.
91
+ */
92
+
71 93
 static inline uint32_t onas_hshift(uint32_t hash)
72 94
 {
73 95
 
... ...
@@ -84,6 +106,13 @@ static inline uint32_t onas_hshift(uint32_t hash)
84 84
     return hash;
85 85
 }
86 86
 
87
+/**
88
+ * @brief inline wrapper for onaccess inotify hashing function
89
+ *
90
+ * @param key       the string to be hashed
91
+ * @param keylen    size of the string
92
+ * @param size      the size of the hashtable
93
+ */
87 94
 static inline int onas_hash(const char *key, size_t keylen, uint32_t size)
88 95
 {
89 96
 
... ...
@@ -98,6 +127,9 @@ static inline int onas_hash(const char *key, size_t keylen, uint32_t size)
98 98
     return hash & (size - 1);
99 99
 }
100 100
 
101
+/**
102
+ * @brief initialises a bucketed hash table, pre-grown to the given size
103
+ */
101 104
 int onas_ht_init(struct onas_ht **ht, uint32_t size)
102 105
 {
103 106
 
... ...
@@ -177,7 +209,9 @@ static void onas_free_bucket(struct onas_bucket *bckt)
177 177
 
178 178
     return;
179 179
 }
180
-
180
+/**
181
+ * @brief the hash table uses buckets to store lists of key/value pairings
182
+ */
181 183
 struct onas_element *onas_element_init(struct onas_hnode *value, const char *key, size_t klen)
182 184
 {
183 185
 
... ...
@@ -258,7 +292,9 @@ static int onas_bucket_insert(struct onas_bucket *bckt, struct onas_element *ele
258 258
     return CL_SUCCESS;
259 259
 }
260 260
 
261
-/* Checks if key exists and optionally stores address to the element corresponding to the key within elem */
261
+/**
262
+ * @brief Checks if key exists and optionally stores address to the element corresponding to the key within elem
263
+ */
262 264
 int onas_ht_get(struct onas_ht *ht, const char *key, size_t klen, struct onas_element **elem)
263 265
 {
264 266
 
... ...
@@ -283,7 +319,9 @@ int onas_ht_get(struct onas_ht *ht, const char *key, size_t klen, struct onas_el
283 283
     return CL_SUCCESS;
284 284
 }
285 285
 
286
-/* Removes the element corresponding to key from the hashtable and optionally returns a pointer to the removed element. */
286
+/**
287
+ * @brief Removes the element corresponding to key from the hashtable and optionally returns a pointer to the removed element.
288
+ */
287 289
 int onas_ht_remove(struct onas_ht *ht, const char *key, size_t klen, struct onas_element **relem)
288 290
 {
289 291
     if (!ht || !key || klen <= 0) return CL_ENULLARG;
... ...
@@ -347,7 +385,9 @@ static int onas_bucket_remove(struct onas_bucket *bckt, struct onas_element *ele
347 347
 
348 348
 /* Dealing with hash nodes and list nodes */
349 349
 
350
-/* Function to initialize hashnode. */
350
+/**
351
+ * @brief Function to initialize hashnode, which is the data value we're storing in the hash table
352
+ */
351 353
 static struct onas_hnode *onas_hashnode_init(void)
352 354
 {
353 355
     struct onas_hnode *hnode = NULL;
... ...
@@ -381,7 +421,9 @@ static struct onas_hnode *onas_hashnode_init(void)
381 381
     return hnode;
382 382
 }
383 383
 
384
-/* Function to initialize listnode. */
384
+/**
385
+ * @brief Function to initialize listnodes, which ultimately allow us to traverse this datastructure like a tree
386
+ */
385 387
 static struct onas_lnode *onas_listnode_init(void)
386 388
 {
387 389
     struct onas_lnode *lnode = NULL;
... ...
@@ -397,7 +439,9 @@ static struct onas_lnode *onas_listnode_init(void)
397 397
     return lnode;
398 398
 }
399 399
 
400
-/* Function to free hashnode. */
400
+/**
401
+ * @brief Function to free hashnodes
402
+ */
401 403
 void onas_free_hashnode(struct onas_hnode *hnode)
402 404
 {
403 405
     if (!hnode) return;
... ...
@@ -416,7 +460,9 @@ void onas_free_hashnode(struct onas_hnode *hnode)
416 416
     return;
417 417
 }
418 418
 
419
-/* Function to free list of listnodes. */
419
+/**
420
+ * @brief Function to free list of listnode
421
+ */
420 422
 void onas_free_dirlist(struct onas_lnode *head)
421 423
 {
422 424
     if (!head) return;
... ...
@@ -432,7 +478,9 @@ void onas_free_dirlist(struct onas_lnode *head)
432 432
     return;
433 433
 }
434 434
 
435
-/* Function to free a listnode. */
435
+/**
436
+ * @brief Function to free a single listnode
437
+ */
436 438
 void onas_free_listnode(struct onas_lnode *lnode)
437 439
 {
438 440
     if (!lnode) return;
... ...
@@ -448,6 +496,9 @@ void onas_free_listnode(struct onas_lnode *lnode)
448 448
     return;
449 449
 }
450 450
 
451
+/**
452
+ * @brief Function to add a single value to a hashnode's listnode
453
+ */
451 454
 static int onas_add_hashnode_child(struct onas_hnode *node, const char *dirname)
452 455
 {
453 456
     if (!node || !dirname) return CL_ENULLARG;
... ...
@@ -463,7 +514,9 @@ static int onas_add_hashnode_child(struct onas_hnode *node, const char *dirname)
463 463
     return CL_SUCCESS;
464 464
 }
465 465
 
466
-/* Function to add a dir_listnode to a list */
466
+/**
467
+ * @brief Function to add a dir_listnode to a list
468
+ */
467 469
 int onas_add_listnode(struct onas_lnode *tail, struct onas_lnode *node)
468 470
 {
469 471
     if (!tail || !node) return CL_ENULLARG;
... ...
@@ -479,7 +532,9 @@ int onas_add_listnode(struct onas_lnode *tail, struct onas_lnode *node)
479 479
     return CL_SUCCESS;
480 480
 }
481 481
 
482
-/* Function to remove a listnode based on dirname. */
482
+/**
483
+ * @brief Function to remove a listnode based on dirname.
484
+ */
483 485
 int onas_rm_listnode(struct onas_lnode *head, const char *dirname)
484 486
 {
485 487
     if (!dirname || !head) return CL_ENULLARG;
... ...
@@ -505,7 +560,9 @@ int onas_rm_listnode(struct onas_lnode *head, const char *dirname)
505 505
 
506 506
 /*** Dealing with parent/child relationships in the table. ***/
507 507
 
508
-/* Determines parent and returns a copy based on full pathname. */
508
+/**
509
+ * @brief Determines parent of given directory and returns a copy based on full pathname.
510
+ */
509 511
 inline static char *onas_get_parent(const char *pathname, size_t len)
510 512
 {
511 513
     if (!pathname || len <= 1) return NULL;
... ...
@@ -530,7 +587,9 @@ inline static char *onas_get_parent(const char *pathname, size_t len)
530 530
     return ret;
531 531
 }
532 532
 
533
-/* Gets the index at which the name of directory begins from the full pathname. */
533
+/**
534
+ * @brief Gets the index at which the name of directory begins from the full pathname.
535
+ */
534 536
 inline static int onas_get_dirname_idx(const char *pathname, size_t len)
535 537
 {
536 538
     if (!pathname || len <= 1) return -1;
... ...
@@ -547,7 +606,15 @@ inline static int onas_get_dirname_idx(const char *pathname, size_t len)
547 547
     return idx;
548 548
 }
549 549
 
550
-/* Emancipates the specified child from the specified parent. */
550
+/**
551
+ * @brief Emancipates the specified child from the specified parent directory, typical done after a delete or move event
552
+ *
553
+ * @param ht        the hashtable structure
554
+ * @param prntpath  the full path of the parent director to be used hashed and used as a key to retrieve the corresponding entry from the table
555
+ * @param prntlen   the length of the parent path in bytes
556
+ * @param childpath the path of the child to be deassociated with the passed parent
557
+ * @param childlen  the length of the child path in bytes
558
+ */
551 559
 int onas_ht_rm_child(struct onas_ht *ht, const char *prntpath, size_t prntlen, const char *childpath, size_t childlen)
552 560
 {
553 561
 
... ...
@@ -569,7 +636,15 @@ int onas_ht_rm_child(struct onas_ht *ht, const char *prntpath, size_t prntlen, c
569 569
     return CL_SUCCESS;
570 570
 }
571 571
 
572
-/* The specified parent adds the specified child to its list. */
572
+/**
573
+ * @brief The specified parent adds the specified child to its list, typical done after a create, or move event
574
+ *
575
+ * @param ht        the hashtable structure
576
+ * @param prntpath  the full path of the parent director to be used hashed and used as a key to retrieve the corresponding entry from the table
577
+ * @param prntlen   the length of the parent path in bytes
578
+ * @param childpath the path of the child to be associated with the passed parent
579
+ * @param childlen  the length of the child path in bytes
580
+ */
573 581
 int onas_ht_add_child(struct onas_ht *ht, const char *prntpath, size_t prntlen, const char *childpath, size_t childlen)
574 582
 {
575 583
     if (!ht || !prntpath || prntlen <= 0 || !childpath || childlen <= 1) return CL_ENULLARG;
... ...
@@ -588,7 +663,9 @@ int onas_ht_add_child(struct onas_ht *ht, const char *prntpath, size_t prntlen,
588 588
 
589 589
 /*** Dealing with hierarchy changes. ***/
590 590
 
591
-/* Adds the hierarchy under pathname to the tree and allocates all necessary memory. */
591
+/**
592
+ * @brief Adds the hierarchy under pathname to the tree and allocates all necessary memory.
593
+ */
592 594
 int onas_ht_add_hierarchy(struct onas_ht *ht, const char *pathname)
593 595
 {
594 596
     if (!ht || !pathname) return CL_ENULLARG;
... ...
@@ -674,7 +751,9 @@ out:
674 674
     return CL_SUCCESS;
675 675
 }
676 676
 
677
-/* Removes the underlying hierarchy from the tree and frees all associated memory. */
677
+/**
678
+ * @brief Removes the underlying hierarchy from the tree and frees all associated memory.
679
+ */
678 680
 int onas_ht_rm_hierarchy(struct onas_ht *ht, const char *pathname, size_t len, int level)
679 681
 {
680 682
     if (!ht || !pathname || len <= 0) return CL_ENULLARG;
... ...
@@ -92,6 +92,9 @@ static int onas_ddd_init_ht(uint32_t ht_size)
92 92
     return onas_ht_init(&ddd_ht, ht_size);
93 93
 }
94 94
 
95
+/**
96
+ * @brief Initialize watch descriptor lookup table which we use alongside inotify to keep track of which open watchpoints correspond to which objects
97
+ */
95 98
 static int onas_ddd_init_wdlt(uint64_t nwatches)
96 99
 {
97 100
 
... ...
@@ -105,6 +108,9 @@ static int onas_ddd_init_wdlt(uint64_t nwatches)
105 105
     return CL_SUCCESS;
106 106
 }
107 107
 
108
+/**
109
+ * @brief Initialize watch descriptor lookup table which we use alongside inotify to keep track of which open watchpoints correspond to which objects
110
+ */
108 111
 static int onas_ddd_grow_wdlt()
109 112
 {
110 113
 
... ...
@@ -152,6 +158,9 @@ int onas_ddd_init(uint64_t nwatches, size_t ht_size)
152 152
     return CL_SUCCESS;
153 153
 }
154 154
 
155
+/**
156
+ * @brief convenience function for adding both inotify and fanotify watchpoints for a single path in one go
157
+ */
155 158
 static int onas_ddd_watch(const char *pathname, int fan_fd, uint64_t fan_mask, int in_fd, uint64_t in_mask)
156 159
 {
157 160
     if (!pathname || fan_fd <= 0 || in_fd <= 0) return CL_ENULLARG;
... ...
@@ -168,6 +177,15 @@ static int onas_ddd_watch(const char *pathname, int fan_fd, uint64_t fan_mask, i
168 168
     return CL_SUCCESS;
169 169
 }
170 170
 
171
+/**
172
+ * @brief recursively adds a hierarchy from the hash table and all watches of a single type to specified object
173
+ *
174
+ * @param pathname  the directory to start watching
175
+ * @param len       the size of pathname in bytes
176
+ * @param fd        the fanotify or inotify file descriptor
177
+ * @param mask      options for watching the path
178
+ * @param type      specifies whether or not to add inotify or fanotify watchpoints and the type of fd passed
179
+ */
171 180
 static int onas_ddd_watch_hierarchy(const char *pathname, size_t len, int fd, uint64_t mask, uint32_t type)
172 181
 {
173 182
 
... ...
@@ -204,6 +222,7 @@ static int onas_ddd_watch_hierarchy(const char *pathname, size_t len, int fd, ui
204 204
         return CL_EARG;
205 205
     }
206 206
 
207
+    /* recursively watch all children */
207 208
     struct onas_lnode *curr = hnode->childhead;
208 209
 
209 210
     while (curr->next != hnode->childtail) {
... ...
@@ -227,6 +246,9 @@ static int onas_ddd_watch_hierarchy(const char *pathname, size_t len, int fd, ui
227 227
     return CL_SUCCESS;
228 228
 }
229 229
 
230
+/**
231
+ * @brief convenience function for removing both inotify and fanotify watchpoints for a single path in one go
232
+ */
230 233
 static int onas_ddd_unwatch(const char *pathname, int fan_fd, int in_fd)
231 234
 {
232 235
     if (!pathname || fan_fd <= 0 || in_fd <= 0) return CL_ENULLARG;
... ...
@@ -243,6 +265,14 @@ static int onas_ddd_unwatch(const char *pathname, int fan_fd, int in_fd)
243 243
     return CL_SUCCESS;
244 244
 }
245 245
 
246
+/**
247
+ * @brief recursively removes a hierarchy from the hash table and drops all watches of a single type from linked objects
248
+ *
249
+ * @param pathname  the directory to stop watching
250
+ * @param len       the size of pathname in bytes
251
+ * @param fd        the fanotify or inotify file descriptor
252
+ * @param type      specifies whether or not to remove inotify or fanotify watchpoints and the type of fd passed
253
+ */
246 254
 static int onas_ddd_unwatch_hierarchy(const char *pathname, size_t len, int fd, uint32_t type)
247 255
 {
248 256
 
... ...
@@ -275,6 +305,7 @@ static int onas_ddd_unwatch_hierarchy(const char *pathname, size_t len, int fd,
275 275
         return CL_EARG;
276 276
     }
277 277
 
278
+    /* free all children recursively */
278 279
     struct onas_lnode *curr = hnode->childhead;
279 280
 
280 281
     while (curr->next != hnode->childtail) {
... ...
@@ -63,13 +63,14 @@ static void onas_scan_thread_exit(int sig)
63 63
 }
64 64
 
65 65
 /**
66
- * Safe-scan wrapper, originally used by inotify and fanotify threads, now exists for error checking/convenience.
67
- * Owned by scanthread to force multithreaded client archtiecture which better avoids kernel level deadlocks from
68
- * fanotify blocking/prevention
66
+ * @brief Safe-scan wrapper, originally used by inotify and fanotify threads, now exists for error checking/convenience.
67
+ *
68
+ * Owned by scanthread to try and force multithreaded client archtiecture which better avoids kernel level deadlocks from
69
+ * fanotify blocking/prevention.
69 70
  */
70 71
 static int onas_scan(struct onas_scan_event *event_data, const char *fname, STATBUF sb, int *infected, int *err, cl_error_t *ret_code)
71 72
 {
72
-    int ret             = 0;
73
+    int ret = 0;
73 74
     int i = 0;
74 75
     uint8_t retry_on_error = event_data->bool_opts & ONAS_SCTH_B_RETRY_ON_E;
75 76
 
... ...
@@ -108,7 +109,11 @@ static int onas_scan(struct onas_scan_event *event_data, const char *fname, STAT
108 108
 }
109 109
 
110 110
 /**
111
- * Thread-safe scan wrapper to ensure there's no processs contention over use of the socket.
111
+ * @brief Thread-safe scan wrapper to ensure there's no processs contention over use of the socket.
112
+ *
113
+ * This is noticeably slower, and I had no issues running smaller scale tests with it off, but better than sorry until more testing can be done.
114
+ *
115
+ * TODO: make this configurable?
112 116
  */
113 117
 static cl_error_t onas_scan_safe(struct onas_scan_event *event_data, const char *fname, STATBUF sb, int *infected, int *err, cl_error_t *ret_code) {
114 118
 
... ...
@@ -300,6 +305,11 @@ static cl_error_t onas_scan_thread_handle_file(struct onas_scan_event *event_dat
300 300
 	return ret;
301 301
 }
302 302
 
303
+/**
304
+ * @brief worker thread designed to work with the lovely c-thread-pool library to handle our scanning jobs after our queue thread consumes an event
305
+ *
306
+ * @param arg this should always be an onas_scan_event struct
307
+ */
303 308
 void *onas_scan_worker(void *arg) {
304 309
 
305 310
 	struct onas_scan_event *event_data = (struct onas_scan_event *) arg;
... ...
@@ -374,8 +384,16 @@ done:
374 374
 	return NULL;
375 375
 }
376 376
 
377
-/* Simple utility function for external interfaces to add relevant context information to scan_event struct;
378
- * doing this mapping cuts down significantly on memory overhead when queueing hundreds of these scan_event structs */
377
+/**
378
+ * @brief Simple utility function for external interfaces to add relevant context information to scan_event struct.
379
+ *
380
+ * Doing this mapping cuts down significantly on memory overhead when queueing hundreds of these scan_event structs
381
+ * especially vs using a copy of a raw context struct.
382
+ *
383
+ * Other potential design options include giving the event access to the "global" context struct address instead,
384
+ * to further cut down on space used, but (among other thread safety concerns) I'd prefer the worker threads not
385
+ * have the ability to modify it at all to keep down on potential maintenance headaches in the future.
386
+ */
379 387
 cl_error_t onas_map_context_info_to_event_data(struct onas_context *ctx, struct onas_scan_event **event_data) {
380 388
 
381 389
     if(NULL == ctx || NULL == event_data || NULL == *event_data) {