Skip to content

Commit

Permalink
Device Defender changes (#67)
Browse files Browse the repository at this point in the history 
Adjusts the code to allow custom metrics for Device Defender in the C++ V2 SDK.

Commit log:
* Modified device_defender to free the string lists from memory
* minor clang-format fix
* A really minor format issue I missed when manually fixing clang-format issue
* Added support for double numbers and double lists to Device Defender
* Fixed clang format issues and removed no longer needed header import
* Adjusted for numbers to always be doubles, removed unnecessary code, removed int returns on registering custom metrics
* Fixed crash with empty custom metric arrays
* Added some const back to functions to fix tests
* Fixed clang-format issues and adjusted tests to use doubles instead of int64_t
* Removed useless asserts in Device Defender tests
* Fixed a few missed DeviceDefender tests
* With all the fixes, I forgot a newline. Fixed
* Code review adjustments: added removed consts back and changed increment back to prefix style
* Slight modification after merge to use doubles with DeviceDefender again
* Fixed metrics not sending correctly
* Fixed clang-format issues, hopefully fixed memory leak in tests
* Further clang-format fixes and memory leak fixes in test
* Test to confirm the failure is related to the JSON module change
* Memory leak narrowing in: Testing to make sure cleanup is not leaking memory
* Memory leak narrowing: Added header back to test
* Fixed memory leak issue in test
* clang-format fix
* Test heap overflow issue - remove validation to see if that is causing problem
* Test heap overflow issue - comment out unused function
* Test heap overflow issue - make byte cursor from payload
* Test heap overflow issue - add back header validation and JSON parsing
* Test heap overflow issue - just create JSON report and free it
* Test heap overflow issue - send byte cursor directly
* Test heap overflow issue - test adding JSON validation back to normal metric test
* Test heap overflow issue - fix custom metric test and remove debug comments
* Test heap overflow issue - fix compile issue
  • Loading branch information
@TwistedTwigleg
TwistedTwigleg authored May 3, 2022
1 parent 1023bba commit 9a5d6a8
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 181 deletions.
32 changes: 12 additions & 20 deletions include/aws/iotdevice/device_defender.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,17 @@ typedef void(
*
* returns: AWS_OP_SUCCESS if the custom metric was successfully added to the task config
*/
typedef int(aws_iotdevice_defender_get_number_fn)(int64_t *const value, void *userdata);
typedef int(aws_iotdevice_defender_get_number_fn)(double *value, void *userdata);

/**
* User callback type invoked to retrieve a number list custom metric
*
* List provided will already be initialized and caller must push items into the list
* of type int64_t.
* of type double.
*
* returns: AWS_OP_SUCCESS if the custom metric was successfully added to the task config
*/
typedef int(aws_iotdevice_defender_get_number_list_fn)(struct aws_array_list *const number_list, void *userdata);
typedef int(aws_iotdevice_defender_get_number_list_fn)(struct aws_array_list *number_list, void *userdata);

/**
* User callback type invoked to retrieve a string list custom metric
Expand All @@ -95,7 +95,7 @@ typedef int(aws_iotdevice_defender_get_number_list_fn)(struct aws_array_list *co
*
* returns: AWS_OP_SUCCESS if the custom metric was successfully added to the task config
*/
typedef int(aws_iotdevice_defender_get_string_list_fn)(struct aws_array_list *const string_list, void *userdata);
typedef int(aws_iotdevice_defender_get_string_list_fn)(struct aws_array_list *string_list, void *userdata);

/**
* User callback type invoked to retrieve an ip list custom metric
Expand All @@ -106,7 +106,7 @@ typedef int(aws_iotdevice_defender_get_string_list_fn)(struct aws_array_list *co
*
* returns: AWS_OP_SUCCESS if the custom metric was successfully added to the task config
*/
typedef int(aws_iotdevice_defender_get_ip_list_fn)(struct aws_array_list *const ip_list, void *userdata);
typedef int(aws_iotdevice_defender_get_ip_list_fn)(struct aws_array_list *ip_list, void *userdata);

enum aws_iotdevice_defender_report_format { AWS_IDDRF_JSON, AWS_IDDRF_SHORT_JSON, AWS_IDDRF_CBOR };

Expand All @@ -116,10 +116,10 @@ enum aws_iotdevice_defender_report_format { AWS_IDDRF_JSON, AWS_IDDRF_SHORT_JSON
*/
enum defender_custom_metric_type {
DD_METRIC_UNKNOWN,
DD_METRIC_NUMBER, /* int64_t */
DD_METRIC_NUMBER_LIST, /* aws_array_list: int64_t */
DD_METRIC_NUMBER, /* double */
DD_METRIC_NUMBER_LIST, /* aws_array_list: double */
DD_METRIC_STRING_LIST, /* aws_array_list: struct aws_string */
DD_METRIC_IP_LIST /* aws_array_list: struct aws_string */
DD_METRIC_IP_LIST, /* aws_array_list: struct aws_string */
};

struct aws_iotdevice_defender_task;
Expand Down Expand Up @@ -259,11 +259,9 @@ int aws_iotdevice_defender_config_set_callback_userdata(
* \param[in] supplier callback function to produce the metric value when
* requested at report generation time
* \param[in] userdata specific callback data for the supplier callback function
* \returns AWS_OP_SUCCESS if the custom metric has been associated with the
* task configuration successfully
*/
AWS_IOTDEVICE_API
int aws_iotdevice_defender_config_register_number_metric(
void aws_iotdevice_defender_config_register_number_metric(
struct aws_iotdevice_defender_task_config *task_config,
const struct aws_byte_cursor *metric_name,
aws_iotdevice_defender_get_number_fn *supplier,
Expand All @@ -278,11 +276,9 @@ int aws_iotdevice_defender_config_register_number_metric(
* \param[in] supplier callback function to produce the metric value when
* requested at report generation time
* \param[in] userdata specific callback data for the supplier callback function
* \returns AWS_OP_SUCCESS if the custom metric has been associated with the
* task configuration successfully
*/
AWS_IOTDEVICE_API
int aws_iotdevice_defender_config_register_number_list_metric(
void aws_iotdevice_defender_config_register_number_list_metric(
struct aws_iotdevice_defender_task_config *task_config,
const struct aws_byte_cursor *metric_name,
aws_iotdevice_defender_get_number_list_fn *supplier,
Expand All @@ -297,11 +293,9 @@ int aws_iotdevice_defender_config_register_number_list_metric(
* \param[in] supplier callback function to produce the metric value when
* requested at report generation time
* \param[in] userdata specific callback data for the supplier callback function
* \returns AWS_OP_SUCCESS if the custom metric has been associated with the
* task configuration successfully
*/
AWS_IOTDEVICE_API
int aws_iotdevice_defender_config_register_string_list_metric(
void aws_iotdevice_defender_config_register_string_list_metric(
struct aws_iotdevice_defender_task_config *task_config,
const struct aws_byte_cursor *metric_name,
aws_iotdevice_defender_get_string_list_fn *supplier,
Expand All @@ -316,11 +310,9 @@ int aws_iotdevice_defender_config_register_string_list_metric(
* \param[in] supplier callback function to produce the metric value when
* requested at report generation time
* \param[in] userdata specific callback data for the supplier callback function
* \returns AWS_OP_SUCCESS if the custom metric has been associated with the
* task configuration successfully
*/
AWS_IOTDEVICE_API
int aws_iotdevice_defender_config_register_ip_list_metric(
void aws_iotdevice_defender_config_register_ip_list_metric(
struct aws_iotdevice_defender_task_config *task_config,
const struct aws_byte_cursor *metric_name,
aws_iotdevice_defender_get_ip_list_fn *supplier,
Expand Down
150 changes: 49 additions & 101 deletions source/device_defender.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ struct defender_custom_metric {
* Result of a custom metric's data collection callback function that needs to be
* populated into a report
*
* Data union only needs to physically point to a single number, and single list.
* Data union only needs to physically point to a single number and a single list.
*/
struct defender_custom_metric_data {
struct defender_custom_metric *metric;
union {
int64_t number;
double number;
struct aws_array_list list;
} data;
int callback_result;
Expand Down Expand Up @@ -300,7 +300,7 @@ static int s_get_metric_report_json(
}

struct aws_json_value *header = aws_json_value_new_object(allocator);
if (aws_json_value_add_to_object(root, aws_byte_cursor_from_c_str("header"), header) == false) {
if (aws_json_value_add_to_object(root, aws_byte_cursor_from_c_str("header"), header) == AWS_OP_ERR) {
goto cleanup;
}
if (aws_json_value_add_to_object(
Expand Down Expand Up @@ -347,18 +347,19 @@ static int s_get_metric_report_json(

struct aws_json_value *est_connections = aws_json_value_new_array(allocator);
if (aws_json_value_add_to_object(
established_tcp_conns, aws_byte_cursor_from_c_str("connections"), est_connections) == false) {
established_tcp_conns, aws_byte_cursor_from_c_str("connections"), est_connections) == AWS_OP_ERR) {
goto cleanup;
}

struct aws_json_value *listening_udp_ports = aws_json_value_new_object(allocator);
if (aws_json_value_add_to_object(metrics, aws_byte_cursor_from_c_str("listening_udp_ports"), listening_udp_ports) ==
false) {
AWS_OP_ERR) {
goto cleanup;
}

struct aws_json_value *udp_ports = aws_json_value_new_array(allocator);
if (aws_json_value_add_to_object(listening_udp_ports, aws_byte_cursor_from_c_str("ports"), udp_ports) == false) {
if (aws_json_value_add_to_object(listening_udp_ports, aws_byte_cursor_from_c_str("ports"), udp_ports) ==
AWS_OP_ERR) {
goto cleanup;
}

Expand All @@ -382,9 +383,11 @@ static int s_get_metric_report_json(
conn,
aws_byte_cursor_from_c_str("local_interface"),
aws_json_value_new_string(
allocator, aws_byte_cursor_from_c_str((char *)net_conn->local_interface))) == AWS_OP_ERR) {
allocator, aws_byte_cursor_from_c_str(aws_string_c_str(net_conn->local_interface)))) ==
AWS_OP_ERR) {
goto cleanup;
}

if (aws_json_value_add_to_object(
conn,
aws_byte_cursor_from_c_str("local_port"),
Expand Down Expand Up @@ -483,7 +486,8 @@ static int s_get_metric_report_json(

if (custom_metrics_len != 0) {
struct aws_json_value *custom_metrics = aws_json_value_new_object(allocator);
if (aws_json_value_add_to_object(root, aws_byte_cursor_from_c_str("custom_metrics"), custom_metrics) == false) {
if (aws_json_value_add_to_object(root, aws_byte_cursor_from_c_str("custom_metrics"), custom_metrics) ==
AWS_OP_ERR) {
goto cleanup;
}

Expand Down Expand Up @@ -516,7 +520,7 @@ static int s_get_metric_report_json(
if (aws_json_value_add_to_object(
item,
aws_byte_cursor_from_c_str("number"),
aws_json_value_new_number(allocator, (double)custom_metrics_data[metric_index].data.number)) ==
aws_json_value_new_number(allocator, custom_metrics_data[metric_index].data.number)) ==
AWS_OP_ERR) {
goto cleanup;
}
Expand All @@ -528,9 +532,9 @@ static int s_get_metric_report_json(
goto cleanup;
}
for (size_t num_index = 0; num_index < list_size; ++num_index) {
int64_t number = 0;
double number = 0;
aws_array_list_get_at(&custom_metrics_data[metric_index].data.list, &number, num_index);
json_value = aws_json_value_new_number(allocator, (double)number);
json_value = aws_json_value_new_number(allocator, number);
aws_json_value_add_array_element(json_list, json_value);
}
} else if (
Expand All @@ -545,7 +549,8 @@ static int s_get_metric_report_json(
}
} else {
json_list = aws_json_value_new_array(allocator);
if (aws_json_value_add_to_object(item, aws_byte_cursor_from_c_str("ip_list"), json_list) == false) {
if (aws_json_value_add_to_object(item, aws_byte_cursor_from_c_str("ip_list"), json_list) ==
AWS_OP_ERR) {
goto cleanup;
}
}
Expand Down Expand Up @@ -576,9 +581,6 @@ static int s_get_metric_report_json(
return_value = AWS_OP_ERR;
} else {
return_value = AWS_OP_SUCCESS;

struct aws_string *tmp = aws_string_new_from_cursor(allocator, &json_report_cursor);
aws_string_destroy_secure(tmp);
}
aws_byte_buf_clean_up_secure(&json_report_buf);

Expand Down Expand Up @@ -629,7 +631,7 @@ static int s_init_custom_metric_data(
break;
case DD_METRIC_NUMBER_LIST:
aws_array_list_init_dynamic(
&custom_metric_data[metric_index].data.list, allocator, 0, sizeof(int64_t));
&custom_metric_data[metric_index].data.list, allocator, 0, sizeof(double));
break;
case DD_METRIC_STRING_LIST:
/* fall through */
Expand Down Expand Up @@ -677,7 +679,9 @@ static void s_clean_up_custom_metric_data(
&custom_metrics_data[metric_index].data.list, (void *)&string_or_ip_entry, item_index);
aws_string_destroy(string_or_ip_entry);
}
/* fall through */
aws_array_list_clean_up(
&custom_metrics_data[metric_index].data.list); // Destory the list when finished
break;
case DD_METRIC_NUMBER_LIST:
aws_array_list_clean_up(&custom_metrics_data[metric_index].data.list);
break;
Expand Down Expand Up @@ -1263,7 +1267,7 @@ void aws_iotdevice_defender_task_clean_up(struct aws_iotdevice_defender_task *de
s_defender_task_clean_up(defender_task);
}

int aws_iotdevice_defender_config_register_number_metric(
void aws_iotdevice_defender_config_register_number_metric(
struct aws_iotdevice_defender_task_config *task_config,
const struct aws_byte_cursor *metric_name,
aws_iotdevice_defender_get_number_fn *supplier,
Expand All @@ -1274,30 +1278,16 @@ int aws_iotdevice_defender_config_register_number_metric(

struct aws_allocator *allocator = task_config->allocator;
struct defender_custom_metric *custom_metric = aws_mem_calloc(allocator, 1, sizeof(struct defender_custom_metric));
if (custom_metric) {
custom_metric->type = DD_METRIC_NUMBER;
custom_metric->metric_name = aws_string_new_from_cursor(allocator, metric_name);
custom_metric->supplier_fn.get_number_fn = supplier;
custom_metric->metric_cb_userdata = userdata;
custom_metric->type = DD_METRIC_NUMBER;
custom_metric->metric_name = aws_string_new_from_cursor(allocator, metric_name);
custom_metric->supplier_fn.get_number_fn = supplier;
custom_metric->metric_cb_userdata = userdata;

if (AWS_OP_SUCCESS != aws_array_list_push_back(&task_config->custom_metrics, &custom_metric)) {
AWS_LOGF_ERROR(
AWS_LS_IOTDEVICE_DEFENDER_TASK,
"id=%p: Failed to add number custom metric " PRInSTR,
(void *)task_config,
AWS_BYTE_CURSOR_PRI(*metric_name)); /* wrong subject */
aws_string_destroy(custom_metric->metric_name);
aws_mem_release(allocator, custom_metric);
return AWS_OP_ERR;
}

task_config->custom_metrics_len++;
return AWS_OP_SUCCESS;
}
return AWS_OP_ERR;
aws_array_list_push_back(&task_config->custom_metrics, &custom_metric);
task_config->custom_metrics_len++;
}

int aws_iotdevice_defender_config_register_number_list_metric(
void aws_iotdevice_defender_config_register_number_list_metric(
struct aws_iotdevice_defender_task_config *task_config,
const struct aws_byte_cursor *metric_name,
aws_iotdevice_defender_get_number_list_fn *supplier,
Expand All @@ -1308,30 +1298,16 @@ int aws_iotdevice_defender_config_register_number_list_metric(

struct aws_allocator *allocator = task_config->allocator;
struct defender_custom_metric *custom_metric = aws_mem_calloc(allocator, 1, sizeof(struct defender_custom_metric));
if (custom_metric) {
custom_metric->type = DD_METRIC_NUMBER_LIST;
custom_metric->metric_name = aws_string_new_from_cursor(allocator, metric_name);
custom_metric->supplier_fn.get_number_list_fn = supplier;
custom_metric->metric_cb_userdata = userdata;

if (AWS_OP_SUCCESS != aws_array_list_push_back(&task_config->custom_metrics, &custom_metric)) {
AWS_LOGF_ERROR(
AWS_LS_IOTDEVICE_DEFENDER_TASK,
"id=%p: Failed to add number list custom metric " PRInSTR,
(void *)task_config,
AWS_BYTE_CURSOR_PRI(*metric_name)); /* wrong subject */
aws_string_destroy(custom_metric->metric_name);
aws_mem_release(allocator, custom_metric);
return AWS_OP_ERR;
}
custom_metric->type = DD_METRIC_NUMBER_LIST;
custom_metric->metric_name = aws_string_new_from_cursor(allocator, metric_name);
custom_metric->supplier_fn.get_number_list_fn = supplier;
custom_metric->metric_cb_userdata = userdata;

task_config->custom_metrics_len++;
return AWS_OP_SUCCESS;
}
return AWS_OP_ERR;
aws_array_list_push_back(&task_config->custom_metrics, &custom_metric);
task_config->custom_metrics_len++;
}

int aws_iotdevice_defender_config_register_string_list_metric(
void aws_iotdevice_defender_config_register_string_list_metric(
struct aws_iotdevice_defender_task_config *task_config,
const struct aws_byte_cursor *metric_name,
aws_iotdevice_defender_get_string_list_fn *supplier,
Expand All @@ -1342,30 +1318,16 @@ int aws_iotdevice_defender_config_register_string_list_metric(

struct aws_allocator *allocator = task_config->allocator;
struct defender_custom_metric *custom_metric = aws_mem_calloc(allocator, 1, sizeof(struct defender_custom_metric));
if (custom_metric) {
custom_metric->type = DD_METRIC_STRING_LIST;
custom_metric->metric_name = aws_string_new_from_cursor(allocator, metric_name);
custom_metric->supplier_fn.get_string_list_fn = supplier;
custom_metric->metric_cb_userdata = userdata;

if (AWS_OP_SUCCESS != aws_array_list_push_back(&task_config->custom_metrics, &custom_metric)) {
AWS_LOGF_ERROR(
AWS_LS_IOTDEVICE_DEFENDER_TASK,
"id=%p: Failed to add string list custom metric " PRInSTR,
(void *)task_config,
AWS_BYTE_CURSOR_PRI(*metric_name)); /* wrong subject */
aws_string_destroy(custom_metric->metric_name);
aws_mem_release(allocator, custom_metric);
return AWS_OP_ERR;
}
custom_metric->type = DD_METRIC_STRING_LIST;
custom_metric->metric_name = aws_string_new_from_cursor(allocator, metric_name);
custom_metric->supplier_fn.get_string_list_fn = supplier;
custom_metric->metric_cb_userdata = userdata;

task_config->custom_metrics_len++;
return AWS_OP_SUCCESS;
}
return AWS_OP_ERR;
aws_array_list_push_back(&task_config->custom_metrics, &custom_metric);
task_config->custom_metrics_len++;
}

int aws_iotdevice_defender_config_register_ip_list_metric(
void aws_iotdevice_defender_config_register_ip_list_metric(
struct aws_iotdevice_defender_task_config *task_config,
const struct aws_byte_cursor *metric_name,
aws_iotdevice_defender_get_ip_list_fn *supplier,
Expand All @@ -1376,27 +1338,13 @@ int aws_iotdevice_defender_config_register_ip_list_metric(

struct aws_allocator *allocator = task_config->allocator;
struct defender_custom_metric *custom_metric = aws_mem_calloc(allocator, 1, sizeof(struct defender_custom_metric));
if (custom_metric) {
custom_metric->type = DD_METRIC_IP_LIST;
custom_metric->metric_name = aws_string_new_from_cursor(allocator, metric_name);
custom_metric->supplier_fn.get_string_list_fn = supplier;
custom_metric->metric_cb_userdata = userdata;

if (AWS_OP_SUCCESS != aws_array_list_push_back(&task_config->custom_metrics, &custom_metric)) {
AWS_LOGF_ERROR(
AWS_LS_IOTDEVICE_DEFENDER_TASK,
"id=%p: Failed to add IP list custom metric " PRInSTR,
(void *)task_config,
AWS_BYTE_CURSOR_PRI(*metric_name)); /* wrong subject */
aws_string_destroy(custom_metric->metric_name);
aws_mem_release(allocator, custom_metric);
return AWS_OP_ERR;
}
custom_metric->type = DD_METRIC_IP_LIST;
custom_metric->metric_name = aws_string_new_from_cursor(allocator, metric_name);
custom_metric->supplier_fn.get_string_list_fn = supplier;
custom_metric->metric_cb_userdata = userdata;

task_config->custom_metrics_len++;
return AWS_OP_SUCCESS;
}
return AWS_OP_ERR;
aws_array_list_push_back(&task_config->custom_metrics, &custom_metric);
task_config->custom_metrics_len++;
}

int aws_iotdevice_defender_config_set_task_cancelation_fn(
Expand Down
Loading

0 comments on commit 9a5d6a8

Please sign in to comment.