Skip to content

Commit

Permalink
Add check for fcontext lines containing only spaces
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cgzones authored and dburgener committed Apr 19, 2024
1 parent d8b309f commit 6ff3d00
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 12 deletions.
1 change: 1 addition & 0 deletions README
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions check_examples.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/check_hooks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down
10 changes: 7 additions & 3 deletions src/fc_checks.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 17 additions & 5 deletions src/parse_fc.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

#include <ctype.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
Expand Down Expand Up @@ -135,8 +136,7 @@ struct fc_entry *parse_fc_line(char *line)
} else {
out->context->range = NULL;
}
} else if (strcmp("<<none>>\n", pos) == 0
|| strcmp("<<none>>\r\n", pos) == 0) {
} else if (strcmp("<<none>>", pos) == 0) {
out->context = NULL;
} else {
out->context = parse_context(pos);
Expand Down Expand Up @@ -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");
Expand All @@ -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 ||
Expand All @@ -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;
Expand All @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions src/runner.c
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down
7 changes: 6 additions & 1 deletion tests/check_fc_checks.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
39 changes: 38 additions & 1 deletion tests/check_parse_fc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)";

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions tests/sample_policy_files/basic.fc
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions tests/sample_policy_files/none_context.fc
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/path/path <<none>>
/path/path2 <<none>>

0 comments on commit 6ff3d00

Please sign in to comment.