Add utils/apparmor/rule/change_profile.py with the ChangeProfileRule and
ChangeProfileRuleset classes. These classes are meant to handle
change_profile rules.
In comparison to the current code in aa.py, ChangeProfileRule has some
added features:
- support for audit and allow/deny keywords (for which John promised a
parser patch really soon)
- support for change_profile rules with an exec condition
Also add the improved regex RE_PROFILE_CHANGE_PROFILE_2 to regex.py.
Acked-by: Steve Beattie <steve@nxnw.org>
It did this in the old 2.8 code, but didn't in 2.9.x (first there was a
broken hat regex, then I commented out the hat handling to avoid
breakage caused by the broken regex).
This patch makes sure the hat flags get set when setting the flags for
the main profile.
Also change RE_PROFILE_HAT_DEF to use more named matches
(leadingwhitespace and hat_keyword). Luckily all code that uses the
regex uses named matches already, which means adding another (...) pair
doesn't hurt.
Finally adjust the tests:
- change _test_set_flags to accept another optional parameter
expected_more_rules (used to specify the expected hat definition)
- add tests for hats (with '^foobar' and 'hat foobar' syntax)
- add tests for child profiles, one of them commented out (see below)
Remaining known issues (also added as TODO notes):
- The hat and child profile flags are *overwritten* with the flags used
for the main profile. (That's well-known behaviour from 2.8 :-/ but we
have more flags now, which makes this more annoying.)
The correct behaviour would be to add or remove the specified flag,
while keeping other flags unchanged.
- Child profiles are not handled/changed if you specify the 'program'
parameter. This means:
- 'aa-complain smbldap-useradd' or 'aa-complain /usr/sbin/smbldap-useradd'
_will not_ change the flags for the nscd child profile
- 'aa-complain /etc/apparmor.d/usr.sbin.smbldap-useradd' _will_ change
the flags for the nscd child profile (and any other profile and
child profile in that file)
Even with those remaining issues (which need bigger changes in
set_profile_flags() and maybe also in the whole flags handling), the
patch improves things and fixes the regression from the 2.8 code.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
A number of simple query tests based on read and write perms of files
and directories.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The test program was querying its own profile. Adjust the profile
generation so that a separate profile is generated and have query_label
query the separate profile.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Adjust the internal splitcon() function to strip a single trailing
newline character when the bool strip_newline argument is true.
aa_getprocattr_raw(2) needs to set strip_newline to true since the
kernel appends a newline character to the end of the AppArmor contexts
read from /proc/>PID>/attr/current.
aa_splitcon(3) also sets strip_newline to true since it is unknown
whether the context is originated from a location that appends a newline
or not.
aa_getpeercon_raw(2) does not set strip_newline to true since it is
unexpected for the kernel to append a newline to the the buffer returned
from getsockopt(2).
This patch also creates tests specifically for splitcon() and updates
the aa_splitcon(3) man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Test confinement context splitting, using aa_splitcon(3), with and
without a valid mode pointer.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Create a new libapparmor public function that allows external code to
split an AppArmor confinement context.
This is immediately useful for code that retrieves a D-Bus peer's
AppArmor confinement context using the
org.freedesktop.DBus.GetConnectionCredentials bus method.
https://launchpad.net/bugs/1430532
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The parse_confinement_mode() function returned NULL when a confinement
mode was not present (unconfined) and when it could not properly parse
the confinement context. The two situations should be differentiated
since the latter should be treated as an error.
This patch reworks parse_confinement_mode() to split a confinement
context and, optionally, assign the mode string. If a parsing error is
encountered, NULL is returned to indicate error.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Use the passed in confinement context string size to improve the
comparison by only doing the string comparison if the size matches and
removing the possibility of reading past the end of the buffer.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
When passing the size of the confinement context to
parse_confinement_mode(), don't include the NUL terminator byte in the
size.
It is confusing to count the NUL terminator as part of the string's
length. This change makes it so that, after a few additional changes,
parse_confinement_mode() can be exposed as part of libapparmor's public
API.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch modifies the socketpair.c test to verify the return value of
aa_getpeercon() based upon the expected label and expected mode lengths.
The test had to be changed slightly so that the returned mode, from
aa_getpeercon(), was preserved. It was being overwritten with the
special NO_MODE value.
This change helps to make sure that future changes to the code behind
aa_getpeercon() does not unintentionally change the function's return
value.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Samba 4.2 needs some more permissions for nmbd and winbindd.
To avoid overcomplicated profiles, change abstractions/samba to allow
/var/lib/samba/** rwk, (instead of **.tdb rwk) - this change already
fixes the nmbd profile.
winbindd additionally needs some more write permissions in /etc/samba/
(and also in /var/lib/samba/, which is covered by the abstractions/samba
change and also results in some profile cleanup)
References: https://bugzilla.opensuse.org/show_bug.cgi?id=921098 and
https://bugzilla.opensuse.org/show_bug.cgi?id=923201
Acked-by: Seth Arnold <seth.arnold@canonical.com>
I noticed "disconnected path" (run/nscd/*) events for ntpd while
updating to the latest openSUSE Tumbleweed.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.9.
aa-mergeprof failed to fail ;-) when it should raise an AppArmorException.
Instead, it failed with
AttributeError: 'module' object has no attribute 'AppArmorException'
I confirmed this bug in trunk and 2.9.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
quote_if_needed() will be used by the upcoming ChangeProfileRule class,
which means it must be moved out of aa.py to avoid an import loop.
rule/__init__.py looks like a better place.
Also re-import quote_if_needed() into aa.py because it's still needed
there by various functions.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
(might get re-used later ;-)
Also add two tests for profile names not starting with / - the quoted
version wasn't catched as invalid before, so this change is actually
also a bugfix.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Change aa-notify parse_message() to also honor complain mode log events.
This affects both modes - desktop notifications and the summary report.
Acked-by: Steve Beattie <steve@nxnw.org>
Add setUp() to AATest that sets "self.maxDiff = None" (unlimited).
This gives us unlimited array diffs everywhere where AATest is used.
Also rename several setUp() functions in test-regex_matches.py to
AASetup() to avoid that the shiny new AATest setUp() gets overwritten.
Acked-by: Steve Beattie <steve@nxnw.org>
As requested by Steve, also add an example AASetup() to test-example.py.
This ignores the sniplets generated by profiles/Makefile, but doesn't
ignore local/README because it doesn't have a dot in its name.
Acked-by: John Johansen <john.johansen@canonical.com>
Add several missing network DOMAINs to the apparmor.d manpage.
The list is based on the list that utils/vim/Makefile generates.
Acked-by: John Johansen <john.johansen@canonical.com>
reported by darix on IRC. This is needed if you have a bigger setup with
dovecot on a different (or multiple) machines
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Replace usage of RE_PROFILE_CAP and RE_PROFILE_NETWORK with
CapabilityRule.match() and NetworkRule.match() calls.
This also means aa.py doesn't need to import those regexes anymore.
As a side effect of this change, test-regex_matches.py needs a small
fix because it imported RE_PROFILE_CAP from apparmor.aa instead of
apparmor.regex.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Add match() and _match() class methods to rule classes:
- _match() returns a regex match object for the given raw_rule
- match() converts the _match() result to True or False
The primary usage is to get an answer to the question "is this raw_rule
your job?". (For a moment, I thought about naming the function
*Rule.myjob() instead of *Rule.match() ;-)
My next patch will change aa.py to use *Rule.match() instead of directly
using RE_*, which will make the import list much shorter and hide
another implementation detail inside the rule classes.
Also change _parse() to use _match() instead of the regex, and add some
tests for match() and _match().
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Change aa.py to use NetworkRule and NetworkRuleset instead of a
sub-hasher to store, check and write network rules. In detail:
- drop profile_known_network() and use is_known_rule() instead
- replace match_net_includes() usage with match_includes() calls
- drop delete_net_duplicates(), use the code in NetworkRule and
NetworkRuleset instead
- make match_net_includes() (still used by aa-mergeprof) a wrapper for
match_includes()
- drop all the network rule parsing from parse_profile_data() and
serialize_profile_from_old_profile() - instead, just call
NetworkRule.parse()
- now that write_net_rules() got fixed, drop it ;-)
- change write_netdomain to use NetworkRuleset
- drop netrules_access_check() - that's is_covered() now
- use 'network' instead of 'netdomain' as storage keyword (log events
still use 'netdomain')
Also update cleanprofile.py to use the NetworkRuleset class.
This also means to delete the (now superfluous) delete_net_duplicates()
function.
Finally, there are some changes in regex.py:
- change RE_PROFILE_NETWORK in regex.py to named matches and to use
RE_COMMA_EOL (not only RE_EOL)
- drop the no longer needed RE_NETWORK_FAMILY and RE_NETWORK_FAMILY_TYPE
(rule/network.py has regexes that check against the list of available
keywords)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Add utils/test/test-network.py with tests for NetworkRule and
NetworkRuleset.
The tests are hopefully self-explaining, so let me just mention the most
important things:
- I started to play with namedtuple, which looks very useful (see "exp")
- the test loops make the tests much more readable (compare with
test-capability.py!) and make it easy to add some more tests
- 100% coverage :-)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Add utils/apparmor/rule/network.py with the NetworkRule and
NetworkRuleset classes. These classes are meant to handle network rules.
In comparison to the existing code in aa.py, relevant news are:
- the keywords are checked against a list of allowed domains, types and
protocols (these lists are based on what the utils/vim/Makefile
generates - on the long term an autogenerated file with the keywords
for all rule types would be nice ;-)
- there are variables for domain and type_or_protocol instead of
first_param and second_param. (If someone is bored enough to map the
protocol "shortcuts" to their expanded meaning, that shouldn't be too
hard.)
- (obviously) more readable code because we have everything at one place
now
- some bugs are fixed along the way (for example, "network foo," will now
be kept, not "network foo bar," - see my last mail about
write_net_rules() for details)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
CleanProf.remove_duplicate_rules() didn't call
$profile['capability'].delete_duplicates()
because aa-cleanprof sets same_file=True.
Fix this by calling delete_duplicates(None) so that it
only checks the profile against itsself.
Note: this is only needed if the to-be-cleaned profile doesn't
contain any include rules - with includes present, the
"for inc in includes:" block already called delete_duplicates()
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Implement in-profile de-duplication in BaseRuleset (currently affects
"only" CapabilityRuleset, but will also work for all future *Ruleset
classes).
Also change 'deleted' to be a simple counter and add some tests that
verify the in-profile deduplication.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
test_parse_modifiers_invalid() uses a hand-broken ;-) regex to parse
only the allow/deny/audit keywords. This test applies to all rule types
and doesn't contain anything specific to capability or other rules,
therefore it should live in test-baserule.py
Moving that test also means to move the imports for parse_modifiers and
re around (nothing else in test-capability.py needs them).
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Add some tests for the Baserule class to cover the 3 functions that must
be re-implemented in each rule class. This means we finally get 100%
test coverage for apparmor/rule/__init__.py ;-)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Ensure nosetests sees all tests in the tests[] tuples. This requires
some name changes because nosetests thinks all function names containing
"test" are tests. (A "not a test" docorator would be an alternative, but
that would require some try/except magic to avoid a dependency on nose.)
To avoid nosetests thinks the functions are a test,
- rename setup_all_tests() to setup_all_loops()
- rename regex_test() to _regex_test() (in test-regex_matches.py)
Also add the module_name as parameter to setup_all_loops and always run
it (not only if __name__ == '__main__').
Known issue: nosetests errors out with
ValueError: no such test method in <class ...>: stub_test
when trying to run a single test generated out of tests[].
(debugging hint: stub_test is the name used in setup_test_loop().)
But that's still an improvement over not seeing those tests at all ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Assume you have a profile like
/bin/foo {
/etc/ r,
network,
/usr/ r,
}
(important: there must be be a non-path rule between the two path blocks)
Then run aa-logprof and add another path event. When choosing (V)iew changes,
it will crash with a misleading
File ".../utils/apparmor/aamode.py", line 205, in split_mode
other = mode - user
TypeError: unsupported operand type(s) for -: 'collections.defaultdict' and 'set'
The reason for this is our beloved hasher, which is playing funny games
another time.
The patch wraps the hasher usage with a check for the parent element to
avoid auto-creation of empty childs, which then lead to the above crash.
BTW: This is another issue uncovered by the LibreOffice profile ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Update the postfix-common abstraction to cope with signal and unix
socket mediation, update the access to the sasl library locations
in a multiarch compliant way, and allow access to limited bits
of the filesystem paths under which postfix chroots itself to
(/var/spool/postfix/ on Ubuntu).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
serialize_profile_from_old_profiles() calls store_list_var() with an
empty hasher. This fails for "+=" because in this case store_list_var()
expects a non-empty hasher with the variable already defined, and raises
an exception because of the empty hasher.
This patch sets "correct = False" if a "+=" operation appears, which
means the variable will be written in "clean" mode instead.
Adding proper support for "add to variable" needs big changes (like
storing a variable's "history" - where it was initially defined and what
got added where).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
the LibreOffice profile uncovered that handling of @{var} += is broken:
File ".../utils/apparmor/aa.py", line 3272, in store_list_var
var[list_var] = set(var[list_var] + vlist)
TypeError: unsupported operand type(s) for +: 'set' and 'list'
This patch fixes it:
- change separate_vars() to use and return a set instead of a list
(FYI: separate_vars() is only called by store_list_var())
- adoptstore_list_var() to expect a set
- remove some old comments in these functions
- explain the less-intuitive parameters of store_list_var()
Also add some tests for separate_vars() and store_list_var().
The tests were developed based on the old code, but not all of them
succeed with the old code.
As usual, the tests uncovered some interesting[tm] behaviour in
separate_vars() (see the XXX comments and tell me what the really
expected behaviour is ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Move the code that does the c -> a and d -> w replacement in denied_mask
and requested_mask so that it only runs for path and exec events, but not
for other events (like dbus and ptrace). The validate_log_mode() and
log_str_to_mode() calls are also moved.
Technically, this means moving code from parse_event() to the path
and exec sections in add_event_to_tree().
This also means aa-logprof no longer crashes if it hits a ptrace or
dbus event in the log.
The "if dmask:" and "if rmask:" checks are removed - if a path event
doesn't have these two, it is totally broken and worth a aa-logprof
crash ;-)
Also adjust the parse_event() tests to expect the "raw" mask instead of
a set.
This patch fixes
https://bugs.launchpad.net/apparmor/+bug/1426651 and
https://bugs.launchpad.net/apparmor/+bug/1243932
I manually tested that
- c and d log events are still converted to a and w
- aa-logprof handles exec events correctly
- ptrace events no longer crash aa-logprof
Note: add_event_to_tree() is not covered by tests.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
"capability foo".is_covered("deny capability foo") should return False
even if check_allow_deny is False.
Also add some tests with check_allow_deny=False.
Acked-by: Steve Beattie <steve@nxnw.org>
Also add libraries/libapparmor/swig/perl/Makefile.perle (noticed and
proposed by Steve)
With these changes, "bzr status" is clean again after "make distclean"
Acked-by: Steve Beattie <steve@nxnw.org>.
Acked-by: Tyler Hicks <tyhicks@canonical.com>
