diff --git a/libraries/libapparmor/src/features.c b/libraries/libapparmor/src/features.c index 3f875beec..2b16dcf58 100644 --- a/libraries/libapparmor/src/features.c +++ b/libraries/libapparmor/src/features.c @@ -194,6 +194,8 @@ static int features_dir_cb(int dirfd, const char *name, struct stat *st, if (features_snprintf(fst, "%s {", name) == -1) return -1; + /* Handle symlink here. See _aa_dirat_for_each in private.c */ + if (S_ISREG(st->st_mode)) { ssize_t len; size_t remaining; diff --git a/libraries/libapparmor/src/policy_cache.c b/libraries/libapparmor/src/policy_cache.c index 1f3478746..7e840dc66 100644 --- a/libraries/libapparmor/src/policy_cache.c +++ b/libraries/libapparmor/src/policy_cache.c @@ -45,6 +45,8 @@ struct aa_policy_cache { static int clear_cache_cb(int dirfd, const char *path, struct stat *st, void *data unused) { + /* Handle symlink here. See _aa_dirat_for_each in private.c */ + if (S_ISREG(st->st_mode)) { /* remove regular files */ return unlinkat(dirfd, path, 0); diff --git a/libraries/libapparmor/src/private.c b/libraries/libapparmor/src/private.c index 3d23b1648..26b34c8d1 100644 --- a/libraries/libapparmor/src/private.c +++ b/libraries/libapparmor/src/private.c @@ -452,7 +452,8 @@ int _aa_overlaydirat_for_each(int dirfd[], int n, void *data, * * The cb function is called with the DIR in use and the name of the * file in that directory. If the file is to be opened it should - * use the openat, fstatat, and related fns. + * use the openat, fstatat, and related fns. If the file is a symlink + * _aa_dirat_for_each currently tries to traverse it for the caller * * Returns: 0 on success, else -1 and errno is set to the error code */ @@ -485,14 +486,34 @@ int _aa_dirat_for_each(int dirfd, const char *name, void *data, autofree struct dirent *dir = namelist[i]; struct stat my_stat; - if (rc) - continue; - - if (fstatat(cb_dirfd, dir->d_name, &my_stat, 0)) { + if (fstatat(cb_dirfd, dir->d_name, &my_stat, AT_SYMLINK_NOFOLLOW)) { PDEBUG("stat failed for '%s': %m\n", dir->d_name); rc = -1; continue; } + /* currently none of the callers handle symlinks, and this + * same basic code was applied to each. So for this patch + * just drop it here. + * + * Going forward we need to start handling symlinks as + * they have meaning. + * In the case of + * cache: they act as a place holder for files that have been + * combined into a single binary. This enables the + * file based cache lookup time find that relation + * and dedup, so multiple loads aren't done. + * profiles: just a profile in an alternate location, but + * should do dedup detection when doing dir reads + * so we don't double process. + */ + if (S_ISLNK(my_stat.st_mode)) { + /* just traverse the symlink */ + if (fstatat(cb_dirfd, dir->d_name, &my_stat, 0)) { + PDEBUG("symlink target stat failed for '%s': %m\n", dir->d_name); + rc = -1; + continue; + } + } if (cb(cb_dirfd, dir->d_name, &my_stat, data)) { PDEBUG("dir_for_each callback failed for '%s'\n", diff --git a/parser/parser_lex.l b/parser/parser_lex.l index 8aded956a..7d99cc0f2 100644 --- a/parser/parser_lex.l +++ b/parser/parser_lex.l @@ -141,6 +141,8 @@ static int include_dir_cb(int dirfd unused, const char *name, struct stat *st, return 0; } + /* Handle symlink here. See _aa_dirat_for_each in private.c */ + if (S_ISREG(st->st_mode)) { if (!(yyin = fopen(path,"r"))) yyerror(_("Could not open '%s' in '%s'"), path, d->filename); diff --git a/parser/parser_main.c b/parser/parser_main.c index 126df2485..5a6baf1cb 100644 --- a/parser/parser_main.c +++ b/parser/parser_main.c @@ -1450,6 +1450,8 @@ static int profile_dir_cb(int dirfd unused, const char *name, struct stat *st, { int rc = 0; + /* Handle symlink here. See _aa_dirat_for_each in private.c */ + if (!S_ISDIR(st->st_mode) && !is_blacklisted(name, NULL)) { struct dir_cb_data *cb_data = (struct dir_cb_data *)data; autofree char *path = NULL; @@ -1472,6 +1474,8 @@ static int binary_dir_cb(int dirfd unused, const char *name, struct stat *st, { int rc = 0; + /* Handle symlink here. See _aa_dirat_for_each in private.c */ + if (!S_ISDIR(st->st_mode) && !is_blacklisted(name, NULL)) { struct dir_cb_data *cb_data = (struct dir_cb_data *)data; autofree char *path = NULL; @@ -1664,7 +1668,7 @@ int main(int argc, char *argv[]) if ((retval = dirat_for_each(AT_FDCWD, profilename, &cb_data, cb))) { last_error = errno; - PDEBUG("Failed loading profiles from %s\n", + PERROR("There was an error while loading profiles from %s\n", profilename); if (abort_on_error) break; diff --git a/parser/tst/Makefile b/parser/tst/Makefile index a05838c0c..08d42f459 100644 --- a/parser/tst/Makefile +++ b/parser/tst/Makefile @@ -17,8 +17,8 @@ endif all: tests -.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching minimize equality valgrind -tests: error_output caching minimize equality parser_sanity +.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching minimize equality dirtest valgrind +tests: error_output caching minimize equality dirtest parser_sanity GEN_TRANS_DIRS=simple_tests/generated_x/ simple_tests/generated_perms_leading/ simple_tests/generated_perms_safe/ simple_tests/generated_dbus @@ -46,6 +46,9 @@ minimize: $(PARSER) equality: $(PARSER) LANG=C APPARMOR_PARSER="$(PARSER) $(PARSER_ARGS)" ./equality.sh +dirtest: $(PARSER) + LANG=C APPARMOR_PARSER="$(PARSER) $(PARSER_ARGS)" ./dirtest.sh + valgrind: $(PARSER) gen_xtrans gen_dbus LANG=C ./valgrind_simple.py -p "$(PARSER) $(PARSER_ARGS)" -v simple_tests