Commit graph

1154 commits

Author SHA1 Message Date
Christian Boltz
37edb354ff
Fix viewing a local inactive profile in aa-genprof
aa-genprof checks if one of the profiles in the extra profile dir
matches the binary, and proposes to use that profile as a starting
point.

Since 4d722f1839 the "(V)iew profile"
option to display the proposed profile was broken.

The easiest fix is to remember the filename in the extras directory, and
display the file from there.

Sidenote: when choosing to use the extra profile, it gets written to
disk without any problems, so this bug really only affected "(V)iew
profile" to preview the proposed extra profile.

(cherry picked from commit 8b4e76a7d5)
2018-11-18 21:41:48 +01:00
Christian Boltz
2296c30af5
serialize_profile(): Fix handling of options
In the 2.13 branch (and older), 'options' is not always a dict, but can
also be None or an empty string.

Adjust the if condition in serialize_profile() so that "View changes
between clean profiles" doesn't error out.
2018-11-11 18:49:42 +01:00
Christian Boltz
aa328cb058
Replace existing_profiles & fix minitools for named profiles
Technical stuff first:

Replace existing_profiles (a dict with the filenames for both active and
inactive profiles) with active_profiles and extra_profiles which are
ProfileList()s and store the active profiles and those in the extra
directory separately. Thanks to ProfileList, now also the relation
between attachments and filenames is easily available.

Also replace all usage of existing_profiles with active_profiles and
extra_profiles, and adjust it to the ProfileList syntax everywhere.

With this change, several bugs in aa-complain and the other minitools
get fixed:
- aa-complain etc. never found profiles that have a profile name
  (the attachment wasn't checked)
- even if the profile name was given as parameter to aa-complain, it
  first did "which $parameter" so it never matched on named profiles
- profile names with alternations (without attachment specification)
  also never matched because the old code didn't use AARE.

References: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882047#92
(search for "As usual" ;-)

Just for completeness - the matching still doesn't honor/expand
variables in the profile name.

(cherry picked from commit 4d722f1839)
2018-11-11 18:33:56 +01:00
Christian Boltz
1d183660d5
add ProfileList class to store list of profiles
ProfileList is meant to store the list of profiles (both name and
attachment) and in which files they live.

Also add unittests to make sure everything works as expected.

(cherry picked from commit 789c4658e2)
2018-11-11 18:33:56 +01:00
Christian Boltz
7b07832459
Move updating existing_profiles out of parse_profile_data()
parse_profile_data() returns the parsed profiles, but writes to
existing_profiles directly.

read_profiles() calls parse_profile_data() and already handles adding
the parsed profiles to aa, original_aa or extras, which means updating
existing_profiles there is a much better place.

This commit also includes a hidden change: Previously, when parsing
include files, they were also added to existing_profiles. This is
superfluous, only real profiles need to be stored there.

(cherry picked from commit 8809218ac8)
2018-11-11 18:33:56 +01:00
Christian Boltz
b6c96f3933
split off get_new_profile_filename()
... and call it from get_profile_filename_* if get_new is True
(= always with the current code)

(cherry picked from commit a6b8d14908)
2018-11-11 18:33:56 +01:00
Christian Boltz
ad236a59b8
split get_profile_filename into .._from_profile_name and .._from_attachment
Split get_profile_filename() into
- get_profile_filename_from_profile_name() (parameter: a profile name)
- get_profile_filename_from_attachment() (parameter: an attachment)

Currently both functions call get_profile_filename_orig() (formerly
get_profile_filename()) so the behaviour doesn't change yet.

The most important part of this commit is changing all
get_profile_filename() calls to use one of the new functions to make
clear if they specify a profile or an attachment/executable as
parameter.

As promised, the is_attachment parameter starts to get used in this
patch ;-)

Note: The get_new parameter (which I'll explain in the patch actually
using it) is set to True in all calls to the new functions.
The long term plan is to get rid of it in most cases (hence defaulting
to False), but that will need more testing.

(cherry picked from commit ec741424f8)
2018-11-11 18:33:55 +01:00
Christian Boltz
f8b95d036d
Add is_attachment parameter to write_profile
The minitools call write_profile(), write_profile_feedback_ui() and
serialize_profile() with the _attachment_ as parameter.

However, aa-logprof etc. call them with the _profile name_ as parameter.

This patch adds an is_attachment parameter to write_profile() and
write_profile_feedback_ui(). It also passes it through to
serialize_profile() via the options parameter.

If is_attachment is True, the parameter will be handled as attachment,
otherwise it is expected to be a profile name.

tools.py gets changed to set is_attachment to True when calling the
functions listed above to make clear that the parameter is an attachment.

Note: This patch only adds the is_attachment parameter/option, but
doesn't change any behaviour. That will happen in the next patch.

(cherry picked from commit bc783372b8)
2018-11-11 18:33:53 +01:00
Christian Boltz
1b32d764ef
delete serialize_profile_from_old_profile()
... which is unused since the last commit.

Note: unlike 0eb12a8cbd, this commit does
_not_ delete several write_* function that were only used by this
function. Verifying that these functions are really unused is not worth
the effort in the 2.13 branch.

(cherry picked from commit 0eb12a8cbd -
but only apply partially)
2018-11-11 15:20:14 +01:00
Christian Boltz
dd4c2b05ea
use serialize_profile() for the new profile in (V)iew Changes
... instead of serialize_profile_from_old_profile()

This will give a realistic preview of the changes (serialize_profile()
is also used when actually writing the profile) and replaces the
known-buggy serialize_profile_from_old_profile() with known-working
code.

It also fixes the issue reported in
    https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139
which means we no longer need the workaround of catching AttributeError
(verified in manual before/after test)

References:
- https://bugs.launchpad.net/apparmor/+bug/1394788
- https://bugs.launchpad.net/bugs/1528139
- https://bugs.launchpad.net/apparmor/+bug/1404893

(cherry picked from commit 469eb444de)
2018-11-11 15:14:24 +01:00
Christian Boltz
37e64d99d1 Merge branch 'aa-notify-manpage' into 'master'
aa-notify man page: update user's configuration file path

See merge request apparmor/apparmor!239

Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master

(cherry picked from commit f920915dd3)

2209e09a aa-notify man page: update user's configuration file path
2018-10-16 15:55:58 +00:00
Christian Boltz
6937123153 Add most abi/bad_*.sd tests to "exception not raised" list
Interestingly, abi/bad_6.sd is detected as invalid, and therefore not
added to the list.

PR: https://gitlab.com/apparmor/apparmor/merge_requests/238
(cherry picked from commit 5c54f66279)
Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-10-13 14:33:50 -07:00
Christian Boltz
c1dc77347c Merge branch 'cboltz-mergeprof-hasher-fun' into 'master'
Fix aa-mergeprof crash caused by accidentially initialzed hat

See merge request apparmor/apparmor!234

Acked-by: John Johansen <john.johansen@canonical.com>

(cherry picked from commit 93445ca02d)

bc492533 Fix aa-mergeprof crash caused by accidentially initialzed hat
2018-10-11 19:49:36 +00:00
nl6720
17d3831d2d aa-notify: Read user's configuration file from XDG_CONFIG_HOME
Legacy path ~/.apparmor/notify.conf is preferred if it exists, otherwise
$XDG_CONFIG_HOME/apparmor/notify.conf, with fallback to
~/.config/apparmor/notify.conf, is used.

PR: https://gitlab.com/apparmor/apparmor/merge_requests/215
Signed-off-by: nl6720 <nl6720@gmail.com>
(cherry picked from commit 1fb9acc59e)
Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-10-04 23:37:39 -07:00
Christian Boltz
420aea6262
Add basic support for abi rules to the tools
Add basic "understand and keep" support for abi rules, where
"understand" means to not error out when seeing an abi rule, and "keep"
simply means to keep the original abi rule when serializing a profile.

On the long term, abi rules should be parsed (similar to include rules),
but for now, this patch is the smallest possible changeset and easy to
backport.

Note that the only added test is via cleanprof_test.* which is used by
minitools_test.py - and does _not_ run if you do a 'make check'.
Oh, and of course the simple_tests/abi/ files also get parsed by
test-parser-simple-tests.py.

BTW: Even serialize_profile_from_old_profile() can handle abi rules :-)

This is a backport of 072d3e0451 / !202 to
2.13 (with some adjustments because that commit didn't appy cleanly)
2018-10-03 16:32:45 +02:00
Christian Boltz
5e4c68712f Merge branch 'zsh' into 'master'
add zsh to logprof.conf

See merge request apparmor/apparmor!201

Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master

(cherry picked from commit 7e22b0a894)

00871696 add zsh to logprof.conf
2018-09-24 17:35:10 +00:00
Christian Boltz
2f658e2422 add python3.7 to logprof.conf
(cherry picked from commit db096135eb)
Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-09-14 16:56:22 -07:00
Tyler Hicks
e27df656f0 utils: Point to the correct Profiles wiki page
The URL redirect ends up at a page in the new wiki that doesn't exist.
We have to link directly to the gitlab URL here since the current URL
redirect doesn't let us use a wiki.apparmor.net URL and still reach the
expected Profiles page.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
2018-09-13 11:45:59 -07:00
Tyler Hicks
2bef2e23d1 all: Use HTTPS links for apparmor.net
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
2018-09-13 11:45:59 -07:00
Christian Boltz
02ab39208b remove unused exception binding in sandbox.py
pyflakes 2.0 is more strict and found that 'e' is never used.

References: https://build.opensuse.org/request/show/629206 (comment
section)

PR: https://gitlab.com/apparmor/apparmor/merge_requests/178
(cherry picked from commit 51482c33f5)
Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-09-11 18:21:25 -07:00
John Johansen
fac81098fa Merge branch 'cboltz-fix-complain-named-profiles' into 'master'
set_profile_flags(): allow named profiles without attachment

See merge request apparmor/apparmor!142

Acked-by: John Johansen <john.johansen@canonical.com>


(cherry picked from commit c66a1a972c)

0dca959c set_profile_flags(): allow named profiles without attachment
2018-09-11 21:02:04 +00:00
Christian Boltz
65c1a6cae2
let change_profile_flags() change flags in child profiles
... instead of overwriting them with the flags of the main profile.

This fixes a longstanding issue with aa-complain, aa-enforce and
aa-audit which broke the flags of child profiles and hats if they
differed from the main profile.

It also fixes several issues documented in the tests (which obviously
need adjustment to match the fixed behaviour).

Also change the "no profile found" cases to AppArmorException - errors
in a profile are not worth triggering AppArmorBug ;-)

(cherry picked from commit b00aab0843)
2018-09-02 17:05:24 +02:00
Christian Boltz
529985973d
change_profile_flags: raise AppArmorBug on empty new flag
(cherry picked from commit d26ffbdd29)
2018-09-02 17:05:16 +02:00
Christian Boltz
7349a9cb03
merge set_profile_flags() into change_profile_flags()
(and adjust a few comments in profile_storage.py)

(cherry picked from commit c016fc6656)
2018-09-02 17:05:08 +02:00
Christian Boltz
fb7a5983bc
rewrite set_profile_flags() tests to use change_profile_flags()
All callers call change_profile_flags(), so it makes sense to test this
function instead of set_profile_flags().

Besides that, set_profile_flags() will be merged into
change_profile_flags() in the next commit ;-)

Note that this commit adds some '# XXX' notes to the tests. These will
be addressed in later commits.

(cherry picked from commit abd124c00d)
2018-09-02 17:04:58 +02:00
Christian Boltz
f4c722c739
change_profile_flags: use ', ' as flags delimiter
This looks better than a comma without whitespace.

Also adjust minitools_test.py to follow this change.

(cherry picked from commit 4a021ec203)
2018-09-02 17:04:49 +02:00
Christian Boltz
267c18e725
extend add_or_remove_flag() to handle str for old flags
If the old flags are given as str (or None), call split_flags() to
convert them to a list.

This allows to simplify change_profile_flags() which now doesn't need to
call split_flags() on its own.

Also add some tests with a str for the old flags

(cherry picked from commit e80caa130a +
 conflict resolution)
2018-09-02 17:04:29 +02:00
Christian Boltz
41eae89869
split off add_or_remove_flag() from change_profile_flags()
Also add some tests for add_or_remove_flag()

(cherry picked from commit 604004c2b6 +
 conflict resolution)
2018-09-02 17:00:55 +02:00
Christian Boltz
e13569fecb
move splitting flags into profile_storage split_flags() function
... and change change_profile_flags() to use it instead of doing it
itsself

Also add some tests for split_flags()

Cherry-picked from ce7ea062c5 + conflict
resolution
2018-09-02 16:55:45 +02:00
Christian Boltz
1c570118ed
activate_repo_profiles(): use change_profile_flags
... instead of set_profile_flags() to keep possibly existing flags like
attach_disconnected.

Note that this function is unused (meant to be used with the
no-longer-existing profile repo), therefore nobody noticed that
set_profile_flags() was called with the wrong number of parameters ;-)
2018-09-02 16:50:17 +02:00
Christian Boltz
5070ba61e1 aa-genprof: don't crash if setting printk_ratelimit fails
When running aa-genprof in a lxd instance, printk_ratelimit is readonly
and writing to it fails. Instead of crashing with a backtrace, only
print a warning.

References: https://bugs.launchpad.net/apparmor/+bug/1785391
(cherry picked from commit 961e69afe5)
Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-08-07 03:04:18 -07:00
Christian Boltz
acb40969b5 make message about notify-send package cross-distro compatible
PR: !144
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1100779
(cherry picked from commit 44ee1d5090)
Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-08-07 02:29:30 -07:00
Christian Boltz
7473044d41 Fix unsetting filename in get_profile()
When creating a new profile with aa-genprof, get_profile() searches for
an inactive ("extra") profile and, if it finds one, removes the filename
from that profile so that it gets stored in /etc/apparmor.d/ later.

However, it used .pop() to remove the filename, which explodes since
ProfileStorage is a class now.

This patch fixes this (tested manually).

PR: !140
(cherry picked from commit 73b33bdf36)
Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-08-07 02:13:47 -07:00
Christian Boltz
35522677d3 Merge branch 'cboltz-nested-child-error' into 'master'
parse_profile_start(): Error out on nested child profiles

See merge request apparmor/apparmor!136

Acked-by: John Johansen <john.johansen@canonical.com> for 2.10..master

(cherry picked from commit b7a4f37cbb)

8462c39b parse_profile_start(): Error out on nested child profiles
2018-06-21 10:20:20 +00:00
Christian Boltz
26a3351552
utis: fix writing alias rules
write_pair() ignored the 'tail' parameter, which resulted in writing
invalid alias rules (without the trailing comma).

Also add an alias to test/cleanprof.* to ensure it doesn't break again.

(cherry picked from commit ae4ab62855)

Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/119
2018-05-08 07:50:09 -07:00
Christian Boltz
28586f7309
utils: fix writing "link subset" rules
Writing a "link subset" rule missed a space, which resulted in something
like
  link subset/foo -> /bar,

Also add a test rule to tests/cleanprof.* to ensure this doesn't break
again.

(cherry picked from commit 514535608f)

Acked-by: Steve Beattie <steve@nxnw.org>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/117
2018-05-06 22:32:40 -07:00
John Johansen
9179b5cf17 Merge branch 'cboltz-utils-exclude-cache-d' into 'master'
is_skippable_dir(): add 'cache.d' to exclude list

See merge request apparmor/apparmor!110

Acked-by: John Johansen <john.johansen@canonical.com>


(cherry picked from commit 67d84c8959)

5b9497a8 is_skippable_dir(): add 'cache.d' to exclude list
2018-04-30 20:56:40 +00:00
John Johansen
aa7dc70de3 translations: sync from launchpad translations
Signed-off-by: John Johansen <john.johansen@canonical.com>
2018-04-15 06:54:44 -07:00
Christian Boltz
f472b6bb34 fix regression in {get,set}_profile_flags()
Since the latest change, calling {get,set}_profile_flags() with the
profile name failed when attachment was specified ("profile foo /bar").

Catched by the unittests.

Also fix a whitespace issue.
2018-04-14 21:26:34 +00:00
Christian Boltz
e28d0a922a
Merge branch 'goldwynr/apparmor-fixes'
See https://gitlab.com/apparmor/apparmor/merge_requests/91

Acked-by: Christian Boltz <apparmor@cboltz.de>
2018-04-14 21:16:16 +02:00
Goldwyn
5e187daa0b Set flags for profiles represented by a glob
Getting and Setting profile represented by a glob does not work correctly
because they are checked for equality. Use a glob match to check for them.
Also, add a warning stating that the profile being set represents multiple programs.

traceroute is an example whose profile name is represented as
/usr/{sbin/traceroute,bin/traceroute.db} and exhibits the issue:

Setting /usr/sbin/traceroute to enforce mode.

ERROR: /etc/apparmor.d/usr.sbin.traceroute contains no profile

Signed-off-by: Goldwyn <goldwyn@fiona.lan>
2018-04-12 05:57:27 -05:00
Christian Boltz
45922c6d21
make utils tests less verbose
Given the big number of tests, printing a dot for each test (instead of
multiple lines) is enough ;-)
2018-04-08 20:18:30 +02:00
Emerson Bernier
b4fa0cf9f6 Add ".dpkg-remove" to apparmor parser ignored list
References: https://bugs.debian.org/893974
2018-04-02 14:24:44 +00:00
Emerson Bernier
f0876ea92a Add .pacsave/.pacnew to apparmor parser ignored list
Currently there is a list of file extensions which apparmor parser
should ignore which contains rpm and dpkg backup files. The list could
be extended with extensions used by pacman package manager
(Archlinux/Manjaro/Antergos):

.pacsave

.pacnew

https://wiki.archlinux.org/index.php/Pacman/Pacnew_and_Pacsave

References: https://gitlab.com/apparmor/apparmor/issues/3
2018-04-02 14:24:25 +00:00
Christian Boltz
86ec3dd658
comment out use_group to remove group restrictions
use_group is only honored if it is defined.

The "real" permission check is reading the logfile - the group check
in aa-notify is just an annoying additional check, and the default
"admin" only works on Ubuntu (other distributions typically use
"wheel").

This commit comments out use_group in the default config, which allows
everybody to use aa-notify. Permissions for reading the log file are of
course still needed.

References: https://bugzilla.opensuse.org/show_bug.cgi?id=1058787
2018-03-18 19:56:29 +01:00
Christian Boltz
dc7c702188 utils tests: ignore tests for 'include if exists'
... and some exotic includes that are not supported by the tools yet
2018-03-16 21:37:17 +00:00
Christian Boltz
f9eb3fea0f ignore .git in is_skippable_dir()
References: https://bugs.launchpad.net/apparmor/+bug/1440273
2018-03-16 21:34:38 +00:00
Tyler Hicks
46f88f5f0d utils: Properly identify empty ouid/fsuid fields in logs
libaalogparse uses (unsigned long) -1 to indicate that a log entry does
not contain ouid and/or fsuid fields. The utils logparser was
incorrectly using 2^64 - 1 to detect such a condition but that wasn't
sufficient for 32 bit environments.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
2018-03-07 15:26:26 +00:00
John Johansen
ccef9161a8 Merge branch 'master' into 'master'
Add custom notification

See merge request apparmor/apparmor!72

Acked-by: John Johansen <john.johansen@canonical.com>
2018-03-06 07:42:37 +00:00
Kees Cook
735afbc947 aa-status: split profile from exec name
Right now, if you have a named profile with regular expressions to
match binaries, the profile will be shown in aa-status under the
"process list", which doesn't make sense. Instead, show the actual
executable name, and if the profile name differs, report it at the
end (or as a separate field in the json output mode).

Signed-off-by: Kees Cook <keescook@chromium.org>
2018-03-01 14:17:57 -08:00