diff --git a/README b/README index fa5e3409..f501fd40 100644 --- a/README +++ b/README @@ -157,6 +157,7 @@ CHECK IDS C-005: Permissions in av rule or class declaration not ordered C-006: Declarations in require block not ordered C-007: Redudant type specification instead of self keyword + C-008: Violation of refpolicy style naming conventions S-001: Require block used instead of interface call S-002: File context file labels with type not declared in module diff --git a/src/Makefile.am b/src/Makefile.am index df108cd4..4a96cf75 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -13,7 +13,7 @@ # limitations under the License. bin_PROGRAMS = selint -selint_SOURCES = main.c lex.l parse.y tree.c tree.h selint_error.h parse_functions.c parse_functions.h maps.c maps.h runner.c runner.h parse_fc.c parse_fc.h template.c template.h file_list.c file_list.h check_hooks.c check_hooks.h fc_checks.c fc_checks.h util.c util.h if_checks.c if_checks.h selint_config.c selint_config.h string_list.c string_list.h startup.c startup.h te_checks.c te_checks.h ordering.c ordering.h color.c color.h perm_macro.c perm_macro.h +selint_SOURCES = main.c lex.l parse.y tree.c tree.h selint_error.h parse_functions.c parse_functions.h maps.c maps.h runner.c runner.h parse_fc.c parse_fc.h template.c template.h file_list.c file_list.h check_hooks.c check_hooks.h fc_checks.c fc_checks.h util.c util.h if_checks.c if_checks.h selint_config.c selint_config.h string_list.c string_list.h startup.c startup.h te_checks.c te_checks.h ordering.c ordering.h color.c color.h perm_macro.c perm_macro.h naming.c naming.h BUILT_SOURCES = parse.h AM_YFLAGS = -d -Wno-yacc -Werror=conflicts-rr -Werror=conflicts-sr diff --git a/src/check_hooks.h b/src/check_hooks.h index c5db22c7..57791ccc 100644 --- a/src/check_hooks.h +++ b/src/check_hooks.h @@ -27,6 +27,7 @@ enum convention_ids { C_ID_UNORDERED_PERM = 5, C_ID_UNORDERED_REQ = 6, C_ID_SELF = 7, + C_ID_NAMING = 8, C_END }; diff --git a/src/naming.c b/src/naming.c new file mode 100644 index 00000000..6eda0e13 --- /dev/null +++ b/src/naming.c @@ -0,0 +1,235 @@ +/* +* Copyright 2020 The SELint Contributors +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +#include "naming.h" + +#include +#include +#include + +static const char *const reason_invalid_char = "contains invalid character"; +static const char *const reason_start_underscore = "starts with an underscore"; +static const char *const reason_consecutive_underscores = "contains consecutive underscores"; +static const char *const reason_end_underscore = "ends with an underscore"; +static const char *const reason_type_postfix = "type does not end with type-specific postfix"; +static const char *const reason_role_postfix = "role does not end with role-specific postfix"; +static const char *const reason_user_postfix = "user does not end with user-specific postfix"; +static const char *const reason_if_no_mod_prefix = "interface has no module name prefix"; +static const char *const reason_if_no_postfix = "interface has no postfix"; +static const char *const reason_if_invalid_postfix = "interface has an invalid postfix"; +static const char *const reason_temp_no_mod_prefix = "template has no module name prefix"; +static const char *const reason_temp_no_postfix = "template has no postfix"; +static const char *const reason_temp_invalid_postfix = "template has an invalid postfix"; + + +static const char *generic_check(const char *name) +{ + const unsigned char *c = (const unsigned char *)name; + + if (*c == '_') { + return reason_start_underscore; + } + + unsigned short prev_is_underscore = 0; + while (*c != '\0') { + if (!islower(*c) && !isdigit(*c) && *c != '_') { + return reason_invalid_char; + } + + if (*c == '_') { + if (prev_is_underscore) { + return reason_consecutive_underscores; + } + + prev_is_underscore = 1; + } else { + prev_is_underscore = 0; + } + + c++; + } + + if (*(c-1) == '_') { + return reason_end_underscore; + } + + return NULL; +} + +static const char *type_check(const char *name) +{ + const size_t name_len = strlen(name); + if (name_len < 2 || name[name_len-2] != '_' || name[name_len-1] != 't') { + return reason_type_postfix; + } + + return NULL; +} + +static const char *role_check(const char *name) +{ + const size_t name_len = strlen(name); + if (name_len < 2 || name[name_len-2] != '_' || name[name_len-1] != 'r') { + return reason_role_postfix; + } + + return NULL; +} + +static const char *user_check(const char *name) +{ + const size_t name_len = strlen(name); + if (name_len < 2 || name[name_len-2] != '_' || name[name_len-1] != 'u') { + return reason_user_postfix; + } + + return NULL; +} + +const char *naming_decl_check(const char *name, enum decl_flavor flavor) +{ + // class and permission names are not in the scope of a policy writer + if (flavor == DECL_CLASS || flavor == DECL_PERM) { + return NULL; + } + + const char *res = generic_check(name); + if (res) { + return res; + } + + switch (flavor) { + case DECL_TYPE: + res = type_check(name); + break; + case DECL_ATTRIBUTE: + //TODO + break; + case DECL_ROLE: + res = role_check(name); + break; + case DECL_ATTRIBUTE_ROLE: + //TODO + break; + case DECL_USER: + res = user_check(name); + break; + case DECL_BOOL: + //TODO + break; + case DECL_CLASS: + case DECL_PERM: + break; + } + + if (res) { + return res; + } + + return NULL; +} + +static int mod_prefix_check(const char *name, const char *mod_name) +{ + const size_t mod_name_len = strlen(mod_name); + + if (0 == strncmp(name, mod_name, mod_name_len) && name[mod_name_len] == '_') { + return 0; + } + + static const char *const exceptions[][2] = { + { "fs" , "filesystem" }, + { "corecmd" , "corecommands" }, + { "seutil" , "selinuxutil" }, + { "libs" , "libraries" }, + { "dev" , "devices" }, + { "term" , "terminal" }, + { "corenet" , "corenetwork" }, + { "auth" , "authlogin" }, + { "userdom" , "userdomain" }, + { "sysnet" , "sysnetwork" }, + }; + + const char *prefix = strchr(name, '_'); + if (!prefix) { + return 1; + } + + const size_t prefix_len = (size_t)(prefix - name); + + size_t i; + for (i = 0; i < (sizeof exceptions / sizeof *exceptions); ++i) { + if (0 == strncmp(name, exceptions[i][0], prefix_len) && + 0 == strcmp(mod_name, exceptions[i][1])) { + return 0; + } + } + + return 1; +} + +const char *naming_if_check(const char *name, const char *module_name) +{ + const char *res = generic_check(name); + if (res) { + return res; + } + + if (mod_prefix_check(name, module_name)) { + return reason_if_no_mod_prefix; + } + + const char *postfix = strrchr(name, '_'); + if (!postfix) { + return reason_if_no_postfix; + } + + static const char *const invalid_postfixes[] = { + "_pattern", + }; + size_t i; + for (i = 0; i < (sizeof invalid_postfixes / sizeof *invalid_postfixes); ++i) { + if (0 == strcmp(postfix, invalid_postfixes[i])) { + return reason_if_invalid_postfix; + } + } + + return NULL; +} + +const char *naming_temp_check(const char *name, const char *module_name) +{ + const char *res = generic_check(name); + if (res) { + return res; + } + + if (mod_prefix_check(name, module_name)) { + return reason_temp_no_mod_prefix; + } + + const char *postfix = strrchr(name, '_'); + if (!postfix) { + return reason_temp_no_postfix; + } + + if (0 != strcmp(postfix, "_template") && + 0 != strcmp(postfix, "_admin")) { + return reason_temp_invalid_postfix; + } + + return NULL; +} diff --git a/src/naming.h b/src/naming.h new file mode 100644 index 00000000..123bd32f --- /dev/null +++ b/src/naming.h @@ -0,0 +1,49 @@ +/* +* Copyright 2020 The SELint Contributors +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +#ifndef NAMING_H +#define NAMING_H + +#include "tree.h" + +/********************************** +* Check the given name with the given kind of declaration for naming +* convention violations. +* name (in) - the name of the declaration +* flavor (in) - the kind of declaration +* Returns NULL if check passes, else a violation reason. +**********************************/ +const char *naming_decl_check(const char *name, enum decl_flavor flavor); + +/********************************** +* Check the given interface name for naming +* convention violations. +* name (in) - the name of the interface +* module_name (in) - the name of the containing module +* Returns NULL if check passes, else a violation reason. +**********************************/ +const char *naming_if_check(const char *name, const char *module_name); + +/********************************** +* Check the given template name for naming +* convention violations. +* name (in) - the name of the template +* module_name (in) - the name of the containing module +* Returns NULL if check passes, else a violation reason. +**********************************/ +const char *naming_temp_check(const char *name, const char *module_name); + +#endif diff --git a/src/runner.c b/src/runner.c index e185a918..f23ec487 100644 --- a/src/runner.c +++ b/src/runner.c @@ -146,6 +146,14 @@ struct checks *register_checks(char level, add_check(NODE_XAV_RULE, ck, "C-007", check_no_self); } + if (CHECK_ENABLED("C-008")) { + add_check(NODE_DECL, ck, "C-008", + check_naming); + add_check(NODE_TEMP_DEF, ck, "C-008", + check_naming); + add_check(NODE_INTERFACE_DEF, ck, "C-008", + check_naming); + } // FALLTHRU case 'S': if (CHECK_ENABLED("S-001")) { diff --git a/src/te_checks.c b/src/te_checks.c index eaea0420..168fe147 100644 --- a/src/te_checks.c +++ b/src/te_checks.c @@ -24,6 +24,7 @@ #include "ordering.h" #include "util.h" #include "perm_macro.h" +#include "naming.h" struct check_result *check_te_order(const struct check_data *data, const struct policy_node *node) @@ -167,7 +168,42 @@ struct check_result *check_no_self(__attribute__((unused)) const struct check_da return make_check_result('C', C_ID_SELF, "Recommend use of self keyword instead of redundant type"); +} + +struct check_result *check_naming(const struct check_data *data, + const struct policy_node *node) +{ + const char *reason; + + switch (node->flavor) { + case NODE_DECL: + // check only declarations in te files + if (data->flavor != FILE_TE_FILE) { + return NULL; + } + + reason = naming_decl_check(node->data.d_data->name, node->data.d_data->flavor); + break; + + case NODE_INTERFACE_DEF: + reason = naming_if_check(node->data.str, data->mod_name); + break; + + case NODE_TEMP_DEF: + reason = naming_temp_check(node->data.str, data->mod_name); + break; + + default: + return alloc_internal_error("Invalid node type for `check_naming`"); + } + + if (!reason) { + return NULL; + } + return make_check_result('C', C_ID_NAMING, + "Naming convention voliated: %s", + reason); } struct check_result *check_require_block(const struct check_data *data, diff --git a/src/te_checks.h b/src/te_checks.h index 4ee5ba51..056fea3d 100644 --- a/src/te_checks.h +++ b/src/te_checks.h @@ -58,6 +58,16 @@ struct check_result *check_unordered_perms(const struct check_data *data, struct check_result *check_no_self(const struct check_data *data, const struct policy_node *node); +/********************************************* +* Check for violations of refpolicy style naming conventions. +* Called on NODE_DECL nodes. +* data - metadata about the file currently being scanned +* node - the node to check +* returns NULL if passed or check_result for issue C-005 +*********************************************/ +struct check_result *check_naming(const struct check_data *data, + const struct policy_node *node); + /********************************************* * Check for the presence of require blocks in TE files. * Interface calls are to be preferred. diff --git a/tests/Makefile.am b/tests/Makefile.am index b1c31065..29c2b89a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -131,6 +131,8 @@ FUNCTIONAL_TEST_FILES=functional/end-to-end.bats \ functional/policies/check_triggers/c06.warn.if \ functional/policies/check_triggers/c07.te \ functional/policies/check_triggers/c07.if \ + functional/policies/check_triggers/c08.te \ + functional/policies/check_triggers/c08.if \ functional/policies/check_triggers/e02.fc \ functional/policies/check_triggers/e03e04e05.fc \ functional/policies/check_triggers/e06.te \ @@ -288,7 +290,7 @@ FC_CHECKS_OBJS=$(top_builddir)/src/fc_checks.o ${CHECK_HOOKS_OBJS} IF_CHECKS_HEADS=$(top_builddir)/src/if_checks.h ${CHECK_HOOKS_HEADS} ${UTIL_HEADS} IF_CHECKS_OBJS=$(top_builddir)/src/if_checks.o ${CHECK_HOOKS_OBJS} ${UTIL_OBJS} TE_CHECKS_HEADS=$(top_builddir)/src/te_checks.h ${CHECK_HOOKS_HEADS} ${UTIL_HEADS} -TE_CHECKS_OBJS=$(top_builddir)/src/te_checks.o ${CHECK_HOOKS_OBJS} $(top_builddir)/src/ordering.o ${UTIL_OBJS} +TE_CHECKS_OBJS=$(top_builddir)/src/te_checks.o ${CHECK_HOOKS_OBJS} $(top_builddir)/src/ordering.o $(top_builddir)/src/naming.o ${UTIL_OBJS} RUNNER_HEADS=$(top_builddir)/src/runner.h ${SELINT_ERROR_HEADS} ${CHECK_HOOKS_HEADS} ${PARSE_FUNCTIONS_HEADS} ${FILE_LIST_HEADS} RUNNER_OBJS=$(top_builddir)/src/runner.o ${CHECK_HOOKS_OBJS} ${PARSE_FUNCTIONS_OBJS} ${FILE_LIST_OBJS} ${FC_CHECKS_OBJS} ${IF_CHECKS_OBJS} ${TE_CHECKS_OBJS} ${PARSE_FC_OBJS} ${UTIL_OBJS} ${STARTUP_OBJS} ${PARSE_OBJS} ORDERING_HEADS=$(top_builddir)/src/ordering.h ${SELINT_ERROR_HEADS} ${TREE_HEADS} diff --git a/tests/functional/end-to-end.bats b/tests/functional/end-to-end.bats index b66cf5ae..998be35d 100644 --- a/tests/functional/end-to-end.bats +++ b/tests/functional/end-to-end.bats @@ -151,6 +151,11 @@ test_parse_error_impl() { test_one_check "C-007" "c07.if" } +@test "C-008" { + test_one_check_expect "C-008" "c08.te" 5 + test_one_check_expect "C-008" "c08.if" 4 +} + @test "S-001" { test_one_check "S-001" "s01.te" } diff --git a/tests/functional/policies/check_triggers/c08.if b/tests/functional/policies/check_triggers/c08.if new file mode 100644 index 00000000..0b1f5e9a --- /dev/null +++ b/tests/functional/policies/check_triggers/c08.if @@ -0,0 +1,16 @@ +#comment +interface(`cnot10_if1',` + #empty +') + +interface(`c10_pattern',` + #empty +') + +template(`cnot10_template',` + #empty +') + +template(`c10_something',` + #empty +') diff --git a/tests/functional/policies/check_triggers/c08.te b/tests/functional/policies/check_triggers/c08.te new file mode 100644 index 00000000..cfcbf2dd --- /dev/null +++ b/tests/functional/policies/check_triggers/c08.te @@ -0,0 +1,8 @@ +policy_module(c06, 1.0.0) + +type A; +type t__t; +type t_; +type t_f; + +role foo_t;