From f78fa3f07183d6a8d0214c07fabdeefdde92d6ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Fri, 23 Feb 2024 20:27:23 +0100 Subject: [PATCH] Add check for fcontext lines containing only spaces Currently file context definition lines containing only white spaces will result in an error: libraries.mod.fc: 130: (E): Bad file context format (E-002) Support parsing those lines and issue a new style check. --- README | 1 + check_examples.txt | 4 +++ src/check_hooks.h | 1 + src/fc_checks.c | 10 ++++-- src/parse_fc.c | 22 ++++++++++--- src/runner.c | 4 +++ tests/check_fc_checks.c | 7 +++- tests/check_parse_fc.c | 39 ++++++++++++++++++++++- tests/sample_policy_files/basic.fc | 4 +-- tests/sample_policy_files/none_context.fc | 1 + 10 files changed, 81 insertions(+), 12 deletions(-) diff --git a/README b/README index 88d1675b..a62a02cf 100644 --- a/README +++ b/README @@ -174,6 +174,7 @@ CHECK IDS S-008: Unquoted gen_require block S-009: Permission macro suffix does not match class name S-010: Permission macro usage suggested + S-011: File context line containing only white spaces W-001: Type, attribute or userspace class referenced without explicit declaration W-002: Type, attribute, role or userspace class used but not listed in require block in interface diff --git a/check_examples.txt b/check_examples.txt index c9ad1fa7..30730c50 100644 --- a/check_examples.txt +++ b/check_examples.txt @@ -87,6 +87,10 @@ S-010: allow foo_t bar_t:file { open read }; +S-011: + + # file context line containing only white spaces + Warning: W-001: diff --git a/src/check_hooks.h b/src/check_hooks.h index db4cd46d..b10b0324 100644 --- a/src/check_hooks.h +++ b/src/check_hooks.h @@ -48,6 +48,7 @@ enum style_ids { S_ID_UNQUOTE_GENREQ = 8, S_ID_PERM_SUFFIX = 9, S_ID_PERMMACRO = 10, + S_ID_FC_SPACES = 11, S_END }; diff --git a/src/fc_checks.c b/src/fc_checks.c index 5d0f0318..097c0f6d 100644 --- a/src/fc_checks.c +++ b/src/fc_checks.c @@ -170,11 +170,15 @@ struct check_result *check_file_context_error_nodes(__attribute__((unused)) cons *node) { - if (node->flavor != NODE_ERROR) { - return NULL; + if (node->flavor == NODE_ERROR) { + return make_check_result('E', E_ID_FC_ERROR, "Bad file context format"); + } + + if (node->flavor == NODE_EMPTY) { + return make_check_result('S', S_ID_FC_SPACES, "File context line containing only white spaces"); } - return make_check_result('E', E_ID_FC_ERROR, "Bad file context format"); + return alloc_internal_error("Invalid node type for `check_file_context_error_nodes`"); } struct check_result *check_file_context_users(__attribute__((unused)) const struct check_data *data, diff --git a/src/parse_fc.c b/src/parse_fc.c index 0b84b211..196fc663 100644 --- a/src/parse_fc.c +++ b/src/parse_fc.c @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include #include @@ -135,8 +136,7 @@ struct fc_entry *parse_fc_line(char *line) } else { out->context->range = NULL; } - } else if (strcmp("<>\n", pos) == 0 - || strcmp("<>\r\n", pos) == 0) { + } else if (strcmp("<>", pos) == 0) { out->context = NULL; } else { out->context = parse_context(pos); @@ -229,6 +229,12 @@ bool check_for_fc_macro(const char *line, const struct string_list *custom_fc_ma return false; } +static void rtrim(char *line, size_t len) +{ + while (len > 0 && isspace((unsigned char)line[len - 1])) + line[--len] = '\0'; +} + struct policy_node *parse_fc_file(const char *filename, const struct string_list *custom_fc_macros) { FILE *fd = fopen(filename, "re"); @@ -253,6 +259,10 @@ struct policy_node *parse_fc_file(const char *filename, const struct string_list if (len_read <= 1 || line[0] == '#') { continue; } + + // Drop trailing white spaces + rtrim(line, (size_t)len_read); + // Skip over m4 constructs if (strncmp(line, "ifdef", 5) == 0 || strncmp(line, "ifndef", 6) == 0 || @@ -262,8 +272,6 @@ struct policy_node *parse_fc_file(const char *filename, const struct string_list continue; } - // TODO: Right now whitespace parses as an error - // We may want to detect it and report a lower severity issue if (check_for_fc_macro(line, custom_fc_macros)) { continue; @@ -272,7 +280,11 @@ struct policy_node *parse_fc_file(const char *filename, const struct string_list struct fc_entry *entry = parse_fc_line(line); enum node_flavor flavor; if (entry == NULL) { - flavor = NODE_ERROR; + if (*line == '\0') { + flavor = NODE_EMPTY; + } else { + flavor = NODE_ERROR; + } } else { flavor = NODE_FC_ENTRY; } diff --git a/src/runner.c b/src/runner.c index 0dcedf66..804cf466 100644 --- a/src/runner.c +++ b/src/runner.c @@ -208,6 +208,10 @@ struct checks *register_checks(char level, add_check(NODE_AV_RULE, ck, "S-010", check_perm_macro_available); } + if (CHECK_ENABLED("S-011")) { + add_check(NODE_EMPTY, ck, "S-011", + check_file_context_error_nodes); + } // FALLTHRU case 'W': if (CHECK_ENABLED("W-001")) { diff --git a/tests/check_fc_checks.c b/tests/check_fc_checks.c index 2b44d507..d9493481 100644 --- a/tests/check_fc_checks.c +++ b/tests/check_fc_checks.c @@ -250,7 +250,12 @@ START_TEST (test_check_file_context_error_nodes) { struct check_result *res = check_file_context_error_nodes(data, node); - ck_assert_ptr_null(res); + ck_assert_ptr_nonnull(res); + ck_assert_int_eq(res->severity, 'F'); + ck_assert_int_eq(res->check_id, F_ID_INTERNAL); + ck_assert_ptr_nonnull(res->message); + + free_check_result(res); node->flavor = NODE_ERROR; diff --git a/tests/check_parse_fc.c b/tests/check_parse_fc.c index 227c412d..57c199e5 100644 --- a/tests/check_parse_fc.c +++ b/tests/check_parse_fc.c @@ -55,6 +55,23 @@ START_TEST (test_parse_context_missing_field) { } END_TEST +START_TEST (test_parse_fc_empty) { + + char line[] = ""; + + struct fc_entry *out = parse_fc_line(line); + + ck_assert_ptr_null(out); + + char line2[] = " "; + + out = parse_fc_line(line2); + + ck_assert_ptr_null(out); + +} +END_TEST + START_TEST (test_parse_fc_line_with_gen_context) { char line[] = "/usr/bin(/.*)? gen_context(system_u:object_r:bin_t, s0)"; @@ -147,11 +164,21 @@ START_TEST (test_parse_basic_fc_file) { cur = cur->next; + ck_assert_int_eq(cur->flavor, NODE_EMPTY); + ck_assert_ptr_nonnull(cur->next); + + cur = cur->next; + ck_assert_int_eq(cur->flavor, NODE_ERROR); ck_assert_ptr_nonnull(cur->next); cur = cur->next; + ck_assert_int_eq(cur->flavor, NODE_EMPTY); + ck_assert_ptr_nonnull(cur->next); + + cur = cur->next; + ck_assert_int_eq(cur->flavor, NODE_FC_ENTRY); ck_assert_ptr_nonnull(cur->next); @@ -202,12 +229,21 @@ START_TEST (test_parse_none_context) { struct policy_node *cur = ast->next; ck_assert_int_eq(cur->flavor, NODE_FC_ENTRY); - ck_assert_ptr_null(cur->next); + ck_assert_ptr_nonnull(cur->next); struct fc_entry *data = cur->data.fc_data; ck_assert_ptr_null(data->context); + cur = cur->next; + + ck_assert_int_eq(cur->flavor, NODE_FC_ENTRY); + ck_assert_ptr_null(cur->next); + + data = cur->data.fc_data; + + ck_assert_ptr_null(data->context); + free_policy_node(ast); } END_TEST @@ -236,6 +272,7 @@ static Suite *parse_fc_suite(void) { tcase_add_test(tc_core, test_parse_context); tcase_add_test(tc_core, test_parse_context_missing_field); + tcase_add_test(tc_core, test_parse_fc_empty); tcase_add_test(tc_core, test_parse_fc_line_with_gen_context); tcase_add_test(tc_core, test_parse_fc_line); tcase_add_test(tc_core, test_parse_fc_line_with_obj); diff --git a/tests/sample_policy_files/basic.fc b/tests/sample_policy_files/basic.fc index 7e4d4932..08abb967 100644 --- a/tests/sample_policy_files/basic.fc +++ b/tests/sample_policy_files/basic.fc @@ -1,8 +1,8 @@ /usr/bin/basic -- gen_context(system_u:object_r:basic_t, s0) /etc/basic(/.*)? gen_context(system_u:object_r:basic_conf_t, s0) - + /var/www/basic gen_this_is_an_error(system_u:obect_r:basic_conf_t, s0) - + /var/log/basic.log -- system_u:object_r:basic_log_t:s0 /var/www/basic/basic.conf -- gen_context(system_u:object_r:basic_conf_t, s0, c1) diff --git a/tests/sample_policy_files/none_context.fc b/tests/sample_policy_files/none_context.fc index a3dbbf48..2252e5ed 100644 --- a/tests/sample_policy_files/none_context.fc +++ b/tests/sample_policy_files/none_context.fc @@ -1 +1,2 @@ /path/path <> +/path/path2 <>