~tyhicks/apparmor/stacking : changes

~tyhicks/apparmor/stacking » Changes from revision 3389

From Revision 3389 to 3370

Rev	Summary	Authors	Date
3389	Fix wrong usage of write_prof_data in serialize_profile_from... Fix wrong usage of write_prof_data in serialize_profile_from_old_profile() write_prof_data[hat] is correct (it only contains one profile, see bug 1528139), write_prof_data[profile][hat] is not and returns an empty (sub)hasher. This affects RE_PROFILE_START and RE_PROFILE_BARE_FILE_ENTRY. Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.9 and 2.10	Christian Boltz	8 years ago
3388	parser: Clean up pivot_root target parsing parser: Clean up pivot_root target parsing Instead of reusing opt_named_transition and be forced to reconstruct the target path when is looks like ":odd:target", create simpler grammer rules that have nothing to do with named transitions and namespaces. Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: John Johansen <john.johansen@canonical.com>	Tyler Hicks	8 years ago
3387	Change log_dict to use profile_storage() and simplify log tr... Change log_dict to use profile_storage() and simplify log translation a) change log_dict to profile_storage() Change collapse_log() to initialize log_dict[aamode][profile][hat] as profile_storage() instead of a hasher(). This also means path events need to go into log_dict[aamode][profile][hat]['allow']['path'] instead of log_dict[aamode][profile][hat]['path'] to match the profile_storage() layout. b) Simplify log translation The translation from logparser.py's output to Rule events was more ugly than needed. This patch removes one step. Instead of translating log_dict to log_obj in ask_the_questions(), add Rule objects to log_dict and adjust ask_the_questions() to use log_dict instead of log_obj. This also means log_obj in ask_the_questions() is now superfluous and can be removed. c) Other small changes: - use is_known_rule() instead of .is_covered() for capability events, which means included files are also checked now. - remove the "if rule_obj.log_event != aamode:" check, because a) it depends on the content of *Rule.log_event (which means it ignores events with log_event != 'ALLOWING' or 'REJECTING' b) it's superfluous because the whole code section is wrapped in a "for aamode in sorted(log.dict.keys())" which means we have separate loops for enforce and complain mode already Acked-by: Kshitij Gupta <kgupta8592@gmail.com>	Christian Boltz	8 years ago
3386	aa.py get_output(): raise exception on non-executable or non... aa.py get_output(): raise exception on non-executable or non-existing programs If the program specified as get_output param isn't executable or doesn't exist at all, get_output() returns with ret = -1. Raising an exception looks like a better option, especially because other possible exec failures already raise an exception ("Unable to fork"). Note: get_output is only used by get_reqs() which also does the os.access() check for x permissions (and raises an exception), so in practise raising an exception in get_output() doesn't change anything. This change also allows to rewrite and simplify get_output() quite a bit. Another minor change (and fix) is in the removal of the last line. The old code removed the last line if output contained at least two items. This had two not-so-nice effects: - an empty output resulted in [''] instead of [] - if a command didn't add a \n on the last line, this line was deleted nevertheless The patch changes that to always remove the last line if it is empty, which fixes both issues mentioned above. Also add a test to ensure the exception is really raised, and adjust the test that expects an empty stdout. Acked-by: Kshitij Gupta <kgupta8592@gmail.com>	Christian Boltz	8 years ago
3385	Add tests for aa.py get_output() and get_reqs() Add tests for aa.py get_output() and get_reqs() To make these tests independent from the underlaying system, add a fake_ldd script that provides hardcoded ldd output for the "known" executables and libraries. To avoid interferences with the real system (especially symlinks), all paths in fake_ldd have '/AATest' prepended. Acked-by: Kshitij Gupta <kgupta8592@gmail.com>	Christian Boltz	8 years ago
3384	Add more ruletypes to the cleanprof test profiles Add more ruletypes to the cleanprof test profiles To ensure aa-cleanprof works as expected (and writing the rules works as expected), add some rules for every rule class to the cleanprof.in and cleanprof.out test profiles. Acked-by: Kshitij Gupta <kgupta8592@gmail.com>	Christian Boltz	8 years ago
3383	Make sure 'x' log events always come with type 'exec' Make sure 'x' log events always come with type 'exec' According to a discussion with John on IRC, denied_mask="x" can only happen for 'exec' log events. This patch raises an exception if John is wrong ;-) Acked-by: Kshitij Gupta <kgupta8592@gmail.com>	Christian Boltz	8 years ago
3382	handle_binfmt: resolve symlinks in library paths handle_binfmt: resolve symlinks in library paths This should happen rarely, but nevertheless it can happen - and since AppArmor needs the symlink target in the profile, we have to resolve all symlinks. Acked-by: Kshitij Gupta <kgupta8592@gmail.com>	Christian Boltz	8 years ago
3381	Drop unused function split_name() in aa.py Drop unused function split_name() in aa.py Acked-by: Kshitij Gupta <kgupta8592@gmail.com>	Christian Boltz	8 years ago
3380	Prevent crash caused by serialize_profile_from_old_profile() Prevent crash caused by serialize_profile_from_old_profile() If a profile file contains multiple profiles and one of those profiles contains a rule managed by a *Ruleset class, serialize_profile_from_old_profile() crashes with an AttributeError. This happens because profile_data / write_prof_data contain only one profile with its hats, which explodes if a file contains multiple profiles, as reported in lp#1528139 Fixing this would need lots of write_prof_data[hat] -> write_prof_data[profile][hat] changes (and of course also a change in the calling code) or, better option, a full rewrite of serialize_profile_from_old_profile(). Unfortunately I don't have the time to do the rewrite at the moment (I have other things on my TODO list), and changing write_prof_data[hat] -> write_prof_data[profile][hat] is something that might introduce more breakage, so I'm not too keen to do that. Therefore this patch wraps the serialize_profile_from_old_profile() call in try/except. If it fails, the diff will include an error message and recommend to use 'View Changes b/w (C)lean profiles' instead, which is known to work. Note: I know using an error message as 'newprofile' isn't an usual way to display an error message, but I found it more intuitive than displaying it as a warning (without $PAGER). References: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139 Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.10	Christian Boltz	8 years ago
3379	dovecot-lda profile: allow tempfiles and executing sendmail dovecot-lda profile: allow tempfiles and executing sendmail dovecot-lda needs to read and write /tmp/dovecot.lda.*. It also needs to be able to execute sendmail to send sieve vacation mails. For now, I'm using a child profile for sendmail to avoid introducing a new profile with possible regressions. This child profile is based on the usr.sbin.sendmail profile in extras and should cover both postfix' and sendmail's sendmail. I also mixed in some bits that were needed for (postfix) sendmail on my servers, and dropped some rules that were obsolete (directory rules not ending with a /) or covered by an abstraction. In the future, we might want to provide a stand-alone profile for sendmail (based on this child profile) and change the rule in the dovecot-lda profile to Px. References: https://bugzilla.opensuse.org/show_bug.cgi?id=954959 https://bugzilla.opensuse.org/show_bug.cgi?id=954958 Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9.	Christian Boltz	8 years ago
3378	Add simple_tests/profile/profile_ns_bad8.sd to utils test ex... Add simple_tests/profile/profile_ns_bad8.sd to utils test exception list parser/tst/simple_tests/profile/profile_ns_bad8.sd was added in r3376 (trunk) / r3312 (2.10 branch) and contains the profile name ':ns/t' which misses the terminating ':' for the namespace. Unfortunately the tools don't understand namespaces yet and just use the full profile name. This also means this test doesn't fail as expected when tested against the utils code. This patch adds profile_ns_bad8.sd to the exception list of test-parser-simple-tests.py. Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.10.	Christian Boltz	8 years ago
3377	parser: Properly parse named transition targets parser: Properly parse named transition targets https://launchpad.net/bugs/1540666 Reuse the new parse_label() function to initialize named_transition structs so that transition targets, when used with change_profile, are properly seperated into a profile namespace and profile name. Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: John Johansen <john.johansen@canonical.com>	Tyler Hicks	8 years ago
3376	parser: Allow the profile keyword to be used with namespaces parser: Allow the profile keyword to be used with namespaces https://launchpad.net/bugs/1544387 Don't split namespaces from profile names using YACC grammar. Instead, treat the entire string as a label in the grammer. The label can then be split into a namespace and a profile name using the new parse_label() function. This fixes a bug that caused the profile keyword to not be used with a label containing a namespace in the profile declaration. Fixing this bug uncovered a bad parser test case at simple_tests/profile/profile_ns_ok1.sd. The test case mistakenly included two definitions of the :foo:unattached profile despite being marked as expected to pass. I've adjusted the name of one of the profiles to :foo:unattached2. Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: John Johansen <john.johansen@canonical.com>	Tyler Hicks	8 years ago
3375	parser: Allow AF_UNSPEC family in network rules parser: Allow AF_UNSPEC family in network rules https://launchpad.net/bugs/1546455 Don't filter out AF_UNSPEC from the list of valid protocol families so that the parser will accept rules such as 'network unspec,'. There are certain syscalls, such as socket(2), where the LSM hooks are called before the protocol family is validated. In these cases, AppArmor was emitting denials even though socket(2) will eventually fail. There may be cases where AF_UNSPEC sockets are accepted and we need to make sure that we're mediating those appropriately. Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Suggested-by: Steve Beattie <steve@nxnw.org> Acked-by: John Johansen <john.johansen@canonical.com> [cboltz: Add 'unspec' to the network domain keywords of the utils]	Tyler Hicks	8 years ago
3374	Fix aa-mergeprof crash with files containing multiple profil... Fix aa-mergeprof crash with files containing multiple profiles If a profile file contains multiple profiles, aa-mergeprof crashes on saving in write_profile() because the second profile in the file is not listed in 'changed'. (This happens only if the second profile didn't change.) This patch first checks if 'changed' contains the profile before pop()ing it. Reproducer: copy utils/test/cleanprof_test.in to your profile directory and run aa-mergeprof utils/test/cleanprof_test.out. Then just press 's' to save the profile. Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.10 and 2.9	Christian Boltz	8 years ago
3373	Remove pname to bin_name mapping in autodep() Remove pname to bin_name mapping in autodep() If autodep() is called with a pname starting with / (which can happen for (N)amed exec depending on the user input), this pname is mapped to bin_name. This might look like a good idea, however if the given pname doesn't exist as file on-disk, autodep() returns None instead of a (mostly empty) profile. (Reproducer: choose (N)amed, enter "/foo/bar") Further down the road, this results in two things: a) the None result gets written as empty profile file (with only a "Last modified" line) b) a crash if someone chooses to add an abstraction to the None, because None doesn't support the delete_duplicates() method for obvious reasons ;-) Unfortunately this patch also introduces a regression - aa-logprof now fails to follow the exec and doesn't ask about the log events for the exec target anymore. However this doesn't really matter because of a) - asking and saving to /dev/null vs. not asking isn't a real difference ;-) Actually the patch slightly improves things - it creates a profile for the exec target, but only with the depmod() defaults (abstractions/base) and always in complain mode. I'd prefer a patch that also creates a complete profile for the exec target, but that isn't as easy as fixing the issues mentioned above and therefore is something for a future fix. To avoid we forget it, I opened https://bugs.launchpad.net/apparmor/+bug/1545155 Note: 2.9 "only" writes an empty file and doesn't crash - but writing an empty profile is still an improvement. Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.10 and 2.9	Christian Boltz	8 years ago
3372	Handle quoted peers when parsing ptrace rules Handle quoted peers when parsing ptrace rules This patch adds handling for quoted ptrace peer values and two testcases for it. Acked-by: Kshitij Gupta <kgupta8592@gmail.com>	Christian Boltz	8 years ago
3371	apparmor.d.pod: document 'deny x' apparmor.d.pod: document 'deny x' deny rules don't allow ix, Px, Ux etc. - only 'deny /foo x,' is allowed. Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.10 Note: Seth mentioned in the mail that he doesn't like the 'deny x' section too much, but we didn't find a better solution when discussing it on IRC. Therefore I keep the patch unchanged, but will happily review a follow-up patch if someone sends one ;-)	Christian Boltz	8 years ago
3370	parser: Move failing test to TODO parser: Move failing test to TODO This test causes `make check` to fail but it is known bug so mark it as a TODO test. Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: Steve Beattie <steve@nxnw.org>	Tyler Hicks	8 years ago

Older »