diff --git a/README b/README index bfd5be5c..a4f92be6 100644 --- a/README +++ b/README @@ -175,9 +175,9 @@ CHECK IDS S-009: Permission macro suffix does not match class name S-010: Permission macro usage suggested - W-001: Type or attribute referenced without explicit declaration - W-002: Type, attribute or role used but not listed in require block in interface - W-003: Unused type, attribute or role listed in require block + 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 + W-003: Unused type, attribute, role or userspace class listed in require block W-004: Potentially unescaped regex character in file contexts paths W-005: Interface call from module not in optional_policy block W-006: Interface call with empty argument diff --git a/src/if_checks.c b/src/if_checks.c index e7e4704c..e855482d 100644 --- a/src/if_checks.c +++ b/src/if_checks.c @@ -330,6 +330,13 @@ struct check_result *check_name_used_but_not_required_in_if(const struct flavor = "Role Attribute"; } else if (name_is_role(ndata) && look_up_in_decl_map(ndata->name, DECL_ROLE)) { flavor = "Role"; + } else if (name_is_class(ndata) && look_up_in_decl_map(ndata->name, DECL_CLASS)) { + /* Ignore kernel classes */ + if (!is_userspace_class(ndata->name, ndata->traits)) { + name_node = name_node->next; + continue; + } + flavor = "Class"; } else { // This is a string we don't recognize. Other checks and/or // the compiler catch invalid bare words @@ -374,6 +381,14 @@ struct check_result *check_name_required_but_not_used_in_if(const struct flavor = "Role Attribute"; } else if (dd->flavor == DECL_ROLE) { flavor = "Role"; + } else if (dd->flavor == DECL_CLASS) { + flavor = "Class"; + if (!is_userspace_class(dd->name, dd->attrs)) { + return make_check_result('W', + W_ID_UNUSED_REQ, + "Class %s is listed in require block but is not a userspace class", + dd->name); + } } else { return NULL; } @@ -400,7 +415,12 @@ struct check_result *check_name_required_but_not_used_in_if(const struct return NULL; } - struct name_list *names_to_check = get_names_in_node(node); + struct name_list *names_to_check; + if (dd->flavor == DECL_CLASS) { + names_to_check = name_list_create(dd->name, NAME_CLASS); + } else { + names_to_check = get_names_in_node(node); + } if (!names_to_check) { // This should never happen return alloc_internal_error( diff --git a/src/main.c b/src/main.c index e5bd5d86..7e234958 100644 --- a/src/main.c +++ b/src/main.c @@ -411,6 +411,7 @@ int main(int argc, char **argv) char *obj_perm_sets_path = NULL; char *access_vector_path = NULL; struct string_list *global_cond_files = NULL; + char *security_classes_path = NULL; while (file) { const char *suffix = (file->fts_pathlen > 3) ? (file->fts_path + file->fts_pathlen - 3) : NULL; @@ -462,6 +463,10 @@ int main(int argc, char **argv) && (!strcmp(file->fts_name, "global_booleans") || !strcmp(file->fts_name, "global_tunables"))) { // TODO: Make names configurable global_cond_files = concat_string_lists(global_cond_files, sl_from_str(file->fts_path)); + } else if (source_flag + && !strcmp(file->fts_name, "security_classes")) { + // TODO: Make access_vectors name configurable + security_classes_path = strdup(file->fts_path); } else { // Directories might get traversed twice: preorder and final visit. // Print only the final visit @@ -533,6 +538,10 @@ int main(int argc, char **argv) && !str_in_sl(file->fts_path, global_cond_files) && (0 == strcmp(file->fts_name, "global_booleans") || 0 == strcmp(file->fts_name, "global_tunables"))) { global_cond_files = concat_string_lists(global_cond_files, sl_from_str(file->fts_path)); + } else if (source_flag + && !security_classes_path + && 0 == strcmp(file->fts_name, "security_classes")) { + security_classes_path = strdup(file->fts_path); } file = fts_read(ftsp); } @@ -557,6 +566,17 @@ int main(int argc, char **argv) printf("%sWarning%s: Failed to locate access_vectors file.\n", color_warning(), color_reset()); } + if (security_classes_path) { + enum selint_error res = load_security_classes_source(security_classes_path); + if (res != SELINT_SUCCESS) { + printf("%sWarning%s: Failed to parse security_classes from %s: %d\n", color_warning(), color_reset(), security_classes_path, res); + } else { + print_if_verbose("Loaded security classes from %s\n", security_classes_path); + } + } else { + printf("%sWarning%s: Failed to locate security_classes file.\n", color_warning(), color_reset()); + } + if (modules_conf_path) { enum selint_error res = load_modules_source(modules_conf_path); @@ -630,6 +650,7 @@ int main(int argc, char **argv) free_file_list(context_if_files); free(obj_perm_sets_path); free(access_vector_path); + free(security_classes_path); free(modules_conf_path); free_string_list(global_cond_files); return EX_CONFIG; @@ -637,6 +658,7 @@ int main(int argc, char **argv) free(obj_perm_sets_path); free(access_vector_path); + free(security_classes_path); free(modules_conf_path); free_string_list(global_cond_files); diff --git a/src/maps.c b/src/maps.c index 6aebb244..62468797 100644 --- a/src/maps.c +++ b/src/maps.c @@ -38,6 +38,7 @@ static struct hash_elem *perm_map = NULL; static struct hash_elem *mods_map = NULL; static struct hash_elem *mod_layers_map = NULL; static struct if_hash_elem *interfaces_map = NULL; +static struct bool_hash_elem *userspace_class_map = NULL; static struct sl_hash_elem *permmacros_map = NULL; static struct template_hash_elem *template_map = NULL; @@ -273,6 +274,56 @@ unsigned int decl_map_count(enum decl_flavor flavor) } } +no_sanitize_unsigned_integer_ +void mark_userspace_class(const char *class_name) +{ + struct bool_hash_elem *userspace_class; + + HASH_FIND(hh_userspace_class, userspace_class_map, class_name, strlen(class_name), userspace_class); + + if (!userspace_class) { + userspace_class = malloc(sizeof(struct bool_hash_elem)); + userspace_class->key = strdup(class_name); + userspace_class->val = 1; + HASH_ADD_KEYPTR(hh_userspace_class, userspace_class_map, userspace_class->key, + strlen(userspace_class->key), userspace_class); + } else { + userspace_class->val = 1; + } +} + +no_sanitize_unsigned_integer_ +int is_userspace_class(const char *class_name, const struct string_list *permissions) +{ + struct bool_hash_elem *userspace_class; + HASH_FIND(hh_userspace_class, userspace_class_map, class_name, strlen(class_name), userspace_class); + if (userspace_class && userspace_class->val == 1) { + return 1; + } + + // the system class might be used by systemd depending on the permission + if (0 != strcmp(class_name, "system")) { + return 0; + } + + for (const struct string_list *p = permissions; p; p = p->next) { + // if permission is not one of the kernel ones + // treat system as userspace class + if (0 != strcmp(p->string, "ipc_info") && + 0 != strcmp(p->string, "syslog_read") && + 0 != strcmp(p->string, "syslog_mod") && + 0 != strcmp(p->string, "syslog_console") && + 0 != strcmp(p->string, "module_request") && + 0 != strcmp(p->string, "module_load") && + 0 != strcmp(p->string, "*") && + 0 != strcmp(p->string, "~")) { + return 1; + } + } + + return 0; +} + no_sanitize_unsigned_integer_ void mark_transform_if(const char *if_name) { @@ -583,6 +634,12 @@ unsigned int permmacros_map_count(void) free(cur_decl); \ } \ +#define FREE_BOOL_MAP(mn) HASH_ITER(hh_ ## mn, mn ## _map, cur_bool, tmp_bool) { \ + HASH_DELETE(hh_ ## mn, mn ## _map, cur_bool); \ + free(cur_bool->key); \ + free(cur_bool); \ +} \ + #define FREE_IF_MAP(mn) HASH_ITER(hh_ ## mn, mn ## _map, cur_if, tmp_if) { \ HASH_DELETE(hh_ ## mn, mn ## _map, cur_if); \ free(cur_if->name); \ @@ -619,6 +676,10 @@ void free_all_maps(void) FREE_IF_MAP(interfaces); + struct bool_hash_elem *cur_bool, *tmp_bool; + + FREE_BOOL_MAP(userspace_class); + struct sl_hash_elem *cur_sl, *tmp_sl; HASH_ITER(hh_permmacros, permmacros_map, cur_sl, tmp_sl) { diff --git a/src/maps.h b/src/maps.h index 93ca1f5a..5a5bf961 100644 --- a/src/maps.h +++ b/src/maps.h @@ -29,6 +29,12 @@ struct hash_elem { hh_class, hh_perm, hh_mods, hh_mod_layers; }; +struct bool_hash_elem { + char *key; + int val; + UT_hash_handle hh_userspace_class; +}; + struct sl_hash_elem { char *key; struct string_list *val; @@ -71,6 +77,10 @@ void insert_into_ifs_map(const char *if_name, const char *mod_name); const char *look_up_in_ifs_map(const char *if_name); +void mark_userspace_class(const char *class_name); + +int is_userspace_class(const char *class_name, const struct string_list *permissions); + void mark_transform_if(const char *if_name); int is_transform_if(const char *if_name); diff --git a/src/startup.c b/src/startup.c index 88f29ab6..bb79f1ea 100644 --- a/src/startup.c +++ b/src/startup.c @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include #include @@ -93,6 +94,75 @@ enum selint_error load_access_vectors_source(const char *av_path) return SELINT_SUCCESS; } +enum selint_error load_security_classes_source(const char *sec_class_path) +{ + FILE *fd = fopen(sec_class_path, "r"); + + if (!fd) { + return SELINT_IO_ERROR; + } + + enum selint_error ret = SELINT_SUCCESS; + + char *line = NULL; + ssize_t len_read = 0; + size_t buf_len = 0; + while ((len_read = getline(&line, &buf_len, fd)) != -1) { + if (len_read <= 1 || line[0] == '#') { + continue; + } + + const char *pos = line; + if (0 != strncmp(pos, "class", strlen("class"))) { + ret = SELINT_PARSE_ERROR; + break; + } + pos += strlen("class"); + + if (*pos != ' ' && *pos != '\t') { + ret = SELINT_PARSE_ERROR; + break; + } + + while (*pos == ' ' || *pos == '\t') { + pos++; + } + + if (!islower((unsigned char)*pos)) { + ret = SELINT_PARSE_ERROR; + break; + } + + const char *name_start = pos; + + while (*pos == '_' || islower((unsigned char)*pos) || isdigit((unsigned char)*pos)) { + pos++; + } + + const char *name_end = pos; + + while (isspace((unsigned char)*pos)) { + pos++; + } + + if (*pos != '\0' && *pos != '#') { + ret = SELINT_PARSE_ERROR; + break; + } + + if (*pos == '#' && NULL != strstr(pos, "userspace")) { + char *name = strndup(name_start, (size_t)(name_end - name_start)); + mark_userspace_class(name); + free(name); + } + } + + free(line); + fclose(fd); + + return ret; +} + void load_modules_normal(void) { diff --git a/src/startup.h b/src/startup.h index ef96f4d8..199b4a7d 100644 --- a/src/startup.h +++ b/src/startup.h @@ -25,6 +25,8 @@ enum selint_error load_access_vectors_kernel(const char *av_path); enum selint_error load_access_vectors_source(const char *av_path); +enum selint_error load_security_classes_source(const char *sec_class_path); + void load_modules_normal(void); enum selint_error load_modules_source(const char *modules_conf_path); diff --git a/src/te_checks.c b/src/te_checks.c index d1aff2d8..d86280bd 100644 --- a/src/te_checks.c +++ b/src/te_checks.c @@ -531,6 +531,13 @@ struct check_result *check_no_explicit_declaration(const struct check_data *data flavor = DECL_ATTRIBUTE; } else if (name_is_roleattr(ndata) && (mod_name = look_up_in_decl_map(ndata->name, DECL_ATTRIBUTE_ROLE))) { flavor = DECL_ATTRIBUTE_ROLE; + } else if (name_is_class(ndata) && look_up_in_decl_map(name->data->name, DECL_CLASS)) { + // Ignore kernel classes + if (!is_userspace_class(name->data->name, name->data->traits)) { + continue; + } + flavor = DECL_CLASS; + mod_name = NULL; // Do not check for roles: in refpolicy they are defined in the kernel module // and used in role modules (like unprivuser) } else { @@ -538,13 +545,20 @@ struct check_result *check_no_explicit_declaration(const struct check_data *data continue; } - if (0 != strcmp(data->mod_name, mod_name)) { + if (!mod_name || 0 != strcmp(data->mod_name, mod_name)) { // It may be required if (!has_require(node, ndata->name, flavor)) { // We didn't find a require block with this name - struct check_result *to_ret = make_check_result('W', W_ID_NO_EXPLICIT_DECL, - "No explicit declaration for %s from module %s. You should access it via interface call or use a require block.", - ndata->name, mod_name); + struct check_result *to_ret; + if (flavor == DECL_CLASS) { + to_ret = make_check_result('W', W_ID_NO_EXPLICIT_DECL, + "No explicit declaration for userspace class %s. You should access it via interface call or use a require block.", + ndata->name); + } else { + to_ret = make_check_result('W', W_ID_NO_EXPLICIT_DECL, + "No explicit declaration for %s from module %s. You should access it via interface call or use a require block.", + ndata->name, mod_name); + } free_name_list(names); return to_ret; } diff --git a/tests/Makefile.am b/tests/Makefile.am index 6664a663..2661d5c7 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -150,6 +150,7 @@ FUNCTIONAL_TEST_FILES=functional/end-to-end.bats \ functional/policies/check_triggers/e10.warn.te \ functional/policies/check_triggers/modules.conf \ functional/policies/check_triggers/obj_perm_sets.spt \ + functional/policies/check_triggers/security_classes \ functional/policies/check_triggers/s01.te \ functional/policies/check_triggers/s02.fc \ functional/policies/check_triggers/s02_other.te \ @@ -168,7 +169,11 @@ FUNCTIONAL_TEST_FILES=functional/end-to-end.bats \ functional/policies/check_triggers/w02_role.te \ functional/policies/check_triggers/w02.te \ functional/policies/check_triggers/w02.bad_if.if \ + functional/policies/check_triggers/w02_system_nowarn.if \ + functional/policies/check_triggers/w02_system_warn.if \ functional/policies/check_triggers/w03_alias.if \ + functional/policies/check_triggers/w03_system_nowarn.if \ + functional/policies/check_triggers/w03_system_warn.if \ functional/policies/check_triggers/w03.if \ functional/policies/check_triggers/w03_role.if \ functional/policies/check_triggers/w03_stub.if \ diff --git a/tests/functional/end-to-end.bats b/tests/functional/end-to-end.bats index e40007aa..e2e266a1 100644 --- a/tests/functional/end-to-end.bats +++ b/tests/functional/end-to-end.bats @@ -21,7 +21,7 @@ do_test() { local FILENAME=$2 local EXPECT=$3 local ARGS=$4 - run ${SELINT_PATH} -s -c tmp.conf ${ARGS} ./policies/check_triggers/${FILENAME} ./policies/check_triggers/modules.conf ./policies/check_triggers/obj_perm_sets.spt ./policies/check_triggers/access_vectors + run ${SELINT_PATH} -s -c tmp.conf ${ARGS} ./policies/check_triggers/${FILENAME} ./policies/check_triggers/modules.conf ./policies/check_triggers/obj_perm_sets.spt ./policies/check_triggers/access_vectors ./policies/check_triggers/security_classes echo $output [ "$status" -eq 0 ] count=$(echo ${output} | grep -o ${CHECK_ID} | wc -l) @@ -220,21 +220,25 @@ test_report_format_impl() { } @test "W-001" { - test_one_check_expect "W-001" "w01*" 5 + test_one_check_expect "W-001" "w01*" 6 } @test "W-002" { test_one_check_expect "W-002" "w02.*" 2 test_one_check "W-002" "w02_role.*" test_one_check_expect "W-002" "w02.bad_if.if" 0 + test_one_check_expect "W-002" "w02_system_nowarn.if" 0 + test_one_check_expect "W-002" "w02_system_warn.if" 1 } @test "W-003" { test_one_check "W-003" "w03.if" - test_one_check "W-003" "w03_role.if" + test_one_check_expect "W-003" "w03_role.if" 4 test_one_check_expect "W-003" "w03_ta.if" 0 test_one_check_expect "W-003" "w03_alias.if" 0 test_one_check_expect "W-003" "w03_stub.if" 0 + test_one_check_expect "W-003" "w03_system_nowarn.if" 0 + test_one_check_expect "W-003" "w03_system_warn.if" 1 } @test "W-004" { diff --git a/tests/functional/policies/check_triggers/access_vectors b/tests/functional/policies/check_triggers/access_vectors index 1b981eea..52be54dc 100644 --- a/tests/functional/policies/check_triggers/access_vectors +++ b/tests/functional/policies/check_triggers/access_vectors @@ -1,3 +1,7 @@ class file { read open } class dir { read open search } + +class system { ipc_info halt } + +class dbus { send_msg } diff --git a/tests/functional/policies/check_triggers/security_classes b/tests/functional/policies/check_triggers/security_classes new file mode 100644 index 00000000..fa1d6fd2 --- /dev/null +++ b/tests/functional/policies/check_triggers/security_classes @@ -0,0 +1,5 @@ +class file +class dir +class ipc +class system +class dbus # userspace diff --git a/tests/functional/policies/check_triggers/w01.te b/tests/functional/policies/check_triggers/w01.te index 38ac493b..d6fd7513 100644 --- a/tests/functional/policies/check_triggers/w01.te +++ b/tests/functional/policies/check_triggers/w01.te @@ -12,3 +12,6 @@ role foo_roles types bar_t; typeattribute bar_t foo_domain; roleattribute bar_r foo_roles; + +allow bar_t bar_t:dbus send_msg; +allow bar_t bar_t:system ipc_info; diff --git a/tests/functional/policies/check_triggers/w02_system_nowarn.if b/tests/functional/policies/check_triggers/w02_system_nowarn.if new file mode 100644 index 00000000..26d47c5a --- /dev/null +++ b/tests/functional/policies/check_triggers/w02_system_nowarn.if @@ -0,0 +1,4 @@ +#comment +interface(`foo_ipc_info',` + allow $1 $1:system ipc_info; +') diff --git a/tests/functional/policies/check_triggers/w02_system_warn.if b/tests/functional/policies/check_triggers/w02_system_warn.if new file mode 100644 index 00000000..6348ba3b --- /dev/null +++ b/tests/functional/policies/check_triggers/w02_system_warn.if @@ -0,0 +1,4 @@ +#comment +interface(`foo_systemd_halt',` + allow $1 $1:system halt; +') diff --git a/tests/functional/policies/check_triggers/w03_role.if b/tests/functional/policies/check_triggers/w03_role.if index 33d41a89..db156761 100644 --- a/tests/functional/policies/check_triggers/w03_role.if +++ b/tests/functional/policies/check_triggers/w03_role.if @@ -33,3 +33,33 @@ interface(`this_one_fails',` allow $1 foo_t:file read; ') + +# Comment +interface(`this_one_fails2',` + gen_require(` + type foo_t; + class file { read }; + ') + + allow $1 foo_t:file read; +') + +# Comment +interface(`this_one_fails3',` + gen_require(` + type foo_t; + class dir { read }; + ') + + allow $1 foo_t:file read; +') + +# Comment +interface(`this_one_fails4',` + gen_require(` + type foo_t; + class dbus { send_msg }; + ') + + allow $1 foo_t:file read; +') diff --git a/tests/functional/policies/check_triggers/w03_system_nowarn.if b/tests/functional/policies/check_triggers/w03_system_nowarn.if new file mode 100644 index 00000000..233a8508 --- /dev/null +++ b/tests/functional/policies/check_triggers/w03_system_nowarn.if @@ -0,0 +1,7 @@ +#comment +interface(`foo_halt_systemd',` + gen_require(` + class system { halt }; + ') + allow $1 $1:system halt; +') diff --git a/tests/functional/policies/check_triggers/w03_system_warn.if b/tests/functional/policies/check_triggers/w03_system_warn.if new file mode 100644 index 00000000..d1c98156 --- /dev/null +++ b/tests/functional/policies/check_triggers/w03_system_warn.if @@ -0,0 +1,7 @@ +#comment +interface(`foo_ipc_info',` + gen_require(` + class system { ipc_info }; + ') + allow $1 $1:system ipc_info; +')