[apparmor] GSoC review r63 and r64
Christian Boltz
apparmor at cboltz.de
Thu Sep 19 21:21:26 UTC 2013
Hello,
the review for r63 and r64 is attached.
I'd welcome an additional review by a native english speaker because
large parts of my review are wording changes in the manpages (including
changes to the text we currently have in bzr trunk). My english isn't
too bad, but a second pair of eyes won't hurt ;-)
Regards,
Christian Boltz
--
> I don't really know how nor why, but if a spellchecker is
> enabled on the wiki server, the edit wiki windows do
> colorize the mispelled words and this is very handy.
I have mixed feelings about using a spill chicken...
[> jdd and Peter Flodin in opensuse-wiki]
-------------- next part --------------
------------------------------------------------------------
revno: 64
committer: Kshitij Gupta <kgupta8592 at gmail.com
branch nick: apparmor-profile-tools
timestamp: Thu 2013-09-19 21:20:40 +0530
message:
Finally added the translations pot file for the current codebase
# a short README explaining how to create or update the pot file would be helpful.
# I reviewed all messages - having everything at one place uncovered some things we overlooked until now ;-)
# (Of course you'll have to do the changes in the py files and then re-generate the pot file.)
=== added directory 'Translate'
=== added file 'Translate/messages.pot'
--- Translate/messages.pot 1970-01-01 00:00:00 +0000
+++ Translate/messages.pot 2013-09-19 15:50:40 +0000
+#: ./../Tools/aa-genprof:67
+msgid "Can't find %s in the system path list. If the name of the application is correct, please run 'which %s' in another window in order to find the fully-qualified path and use the full path as parameter."
# seeing this overlong line - maybe you should add some linebreaks to this message in aa-genprof?
# (and the "in another window" part looks superfluous)
# maybe you can merge this text with the text in tools.py:105
+#: ./../Tools/aa-genprof:120
+msgid "Profiling"
# should that be changed to "Profiling %s"?
+#: aa.py:1252
+msgid ""
+"Launching processes in an unconfined state is a very\n"
+"dangerous operation and can cause serious security holes.\n"
+"\n"
+"Are you absolutely certain you wish to remove all\n"
+"AppArmor protection when executing : %s ?"
# "... when executing %s ?" (without ":") would sound better
+#: tools.py:105
+msgid ""
+"Can't find %s in the system path list. If the name of the application is correct, please run 'which %s' as a user with correct PATH environment set up in order to find the fully-qualified path.\n"
+"Please use the full path as parameter"
# overlong line - add some linebreaks in tools.py?
# maybe you can merge this with the text from aa-genprof (see above)
+#: ui.py:323
+msgid "Duplicate hotkey for"
# better: "... for %s: %s"
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py 2013-09-19 05:02:19 +0000
+++ apparmor/aa.py 2013-09-19 15:50:40 +0000
@@ -1239,18 +1239,9 @@
match = regex_optmode.search(ans).groups()[0]
exec_mode = str_to_mode(match)
px_default = 'n'
- px_msg = _('Should AppArmor sanitise the environment when\n' +
- 'switching profiles?\n\n' +
- 'Sanitising environment is more secure,\n' +
- 'but some applications depend on the presence\n' +
- 'of LD_PRELOAD or LD_LIBRARY_PATH.')
+ px_msg = _("""Should AppArmor sanitise the environment when\nswitching profiles?\n\nSanitising environment is more secure,\nbut some applications depend on the presence\nof LD_PRELOAD or LD_LIBRARY_PATH.""")
# does this and the similar other changes mean pygettext.py can't handle multi-line code?
vim:ft=diff
-------------- next part --------------
------------------------------------------------------------
revno: 63
committer: Kshitij Gupta <kgupta8592 at gmail.com
branch nick: apparmor-profile-tools
timestamp: Thu 2013-09-19 10:32:19 +0530
message:
Added manpages for the tools, fixes from rev 59..62, some fixes from rev 58
# I did a full review of all manpages, which means I'll also blame ;-) you for errors you just copied from the existing manpages.
# I know you are the wrong target, but it felt like a good time to do the review ;-)
=== added file 'Tools/manpages/aa-audit.pod'
--- Tools/manpages/aa-audit.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-audit.pod 2013-09-19 05:02:19 +0000
@@ -0,0 +1,39 @@
+=pod
+
+=head1 NAME
+
+aa-audit - set a AppArmor security profile to I<audit> mode.
# ... set a_n_ AppArmor ...
# (not worth backporting to the 2.8 branch ;-)
+=head1 DESCRIPTION
+
+B<aa-audit> is used to set the audit mode for one or more profiles to audit.
# that's one "audit" too much
# proposal:
# ... to set one or more profiles to audit mode.
+In this mode security policy is enforced and all access (successes and failures) are logged to the system log.
+
+The I<--remove> option can be used to remove the audit mode for the profile.
+
+=head1 BUGS
+
+If you find any bugs, please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
# (not sure if we keep using https://bugs.launchpad.net/apparmor-profile-tools/+filebug after merging your code in the official repo)
=== added file 'Tools/manpages/aa-autodep.pod'
--- Tools/manpages/aa-autodep.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-autodep.pod 2013-09-19 05:02:19 +0000
@@ -0,0 +1,66 @@
+B<-f --force>
+
+ Overrides any existing AppArmor profile for the executable with the generated minimal AppArmor profile.
# I'd prefer "Over_writes_" (instead of "Overrides") to make clear what happens
+The I<--force> option will override any existing profile for the executable with
# over_write_
+the newly generated minimal AppArmor profile.
+
+=head1 BUGS
+
+This program does not perform full static analysis of executables, so
+the profiles generated are necessarily incomplete. If you find any bugs,
+please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
=== added file 'Tools/manpages/aa-cleanprof.pod'
--- Tools/manpages/aa-cleanprof.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-cleanprof.pod 2013-09-19 05:02:19 +0000
@@ -0,0 +1,34 @@
+=head1 DESCRIPTION
+
+B<aa-cleanprof> is used to perform a cleanup on one or more profiles.
+The tool removes any existing superfluous rules, reorders the rules to group
# an explanation for "superfluous" would be nice, maybe something like
# "(matched by another rule)"
+similar rules together and removes all comments.
+
+=head1 BUGS
+
+If you find any bugs, please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
=== added file 'Tools/manpages/aa-complain.pod'
--- Tools/manpages/aa-complain.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-complain.pod 2013-09-19 05:02:19 +0000
@@ -0,0 +1,61 @@
+=head1 NAME
+
+aa-complain - set a AppArmor security profile to I<complain> mode.
# ... set a_n_ ...
+B<-r --remove>
+
+ Removes the complain mode for the profile.
# this should mention "sets to enforce mode"
+=head1 DESCRIPTION
+
+B<aa-complain> is used to set the enforcement mode for one or more profiles to
+complain. In this mode security policy is not enforced but rather access
# The first sentence is a bit confusing. What about this?
# aa-complain is used to set one or more profiles to complain mode.
+If you find any bugs, please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
=== added file 'Tools/manpages/aa-disable.pod'
--- Tools/manpages/aa-disable.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-disable.pod 2013-09-19 05:02:19 +0000
@@ -0,0 +1,62 @@
+=head1 DESCRIPTION
+
+B<aa-disable> is used to disable the enforcement mode for one or more
# It's not about enforcement mode, it completely disables the profile.
# Therefore the text should be:
# aa-disable is used to disable one or more profiles.
+profiles. This command will unload the profile from the kernel and
+prevent the profile from being loaded on AppArmor startup. The
+I<aa-enforce> and I<aa-complain> utilities may be used to to change this
+behavior.
+
+The I<--revert> option can be used to enable the profile.
+
+=head1 BUGS
+
+If you find any bugs, please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
=== added file 'Tools/manpages/aa-enforce.pod'
--- Tools/manpages/aa-enforce.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-enforce.pod 2013-09-19 05:02:19 +0000
@@ -0,0 +1,65 @@
+=head1 DESCRIPTION
+
+B<aa-enforce> is used to set the enforcement mode for one or more profiles
+to I<enforce>. This command is only relevant in conjunction with the
# the first sentence is confusing (like in aa-complain.pod). Proposal:
# aa-enforce is used to set one or more profiles to enforce mode.
+If you find any bugs, please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
=== added file 'Tools/manpages/aa-genprof.pod'
--- Tools/manpages/aa-genprof.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-genprof.pod 2013-09-19 05:02:19 +0000
+After the user finishes selecting profile entries based on violations
+that were detected during the program execution, aa-genprof will reload
+the updated profiles in complain mode and again prompt the user for (S)can and
+(D)one. This cycle can then be repeated as necessary until all application
# I'm quite sure it's (F)inish, not (D)one
+If you find any bugs, please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
=== added file 'Tools/manpages/aa-logprof.pod'
--- Tools/manpages/aa-logprof.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-logprof.pod 2013-09-19 05:02:19 +0000
+=head1 NAME
+
+aa-logprof - utility program for managing AppArmor security profiles
# what about
# aa-logprof - utility for updating AppArmor security profiles
+=head1 DESCRIPTION
+
+B<aa-logprof> is an interactive tool used to review AppArmor's
+complain mode output and generate new entries for AppArmor security
+profiles.
# it's not only about "complain mode output"
+=head2 Responding to AppArmor Events
+
+B<aa-logprof> will generate a list of suggested profile changes that
+the user can choose from, or they can create their own, to modifiy the
+permission set of the profile so that the generated access violation
+will not re-occur.
+
+The user is then presented with info about the access including profile,
+path, old mode if there was a previous entry in the profile for this path,
+new mode, the suggestion list, and given these options:
+
+ (A)llow, (D)eny, (N)ew, (G)lob last piece, (Q)uit
# "(I)gnore" is missing from this list.
# Also, a paragraph explaining the difference between (D)eny and (I)gnore would be nice.
# BTW: there might be a hotkey conflict between (I)gnore and (I)nherit
+If (Q)uit is selected at this point, aa-logprof will ignore all new pending
+capability and path accesses.
# "capability and path accesses" is incomplete.
# It would be better to just write "new pending accesses" than having an incomplete list IMHO.
+After all of the path accesses have been handled, logrof will write all
# "After all of the accesses" (remove "path")
+updated profiles to the disk and reload them if AppArmor is running.
+
+=head2 New Process (Execution) Events
+
+If there are unhandled x accesses generated by the execve(2) of a
+new process, aa-logprof will display the parent profile and the target
+program that's being executed and prompt the user to select and execute
+modifier. These modifiers will allow a choice for the target to: have it's
+own profile (px), inherit the parent's profile (ix), run unconstrained
+(ux), or deny access for the target. See apparmor.d(5) for details.
# child profile (cx) and named profile are missing
+=head2 ChangeHat Events
+
+If unknown aa_change_hat(2) events are found, the user is prompted to add a new
+hat, if the events should go into the default hat for this profile based
+on the corresponding entry in the defaulthat section of logprof.conf,
+or if the following events that run under that hat should be denied
+altogether.
+
+=head2 Capability Events
+
+If there are capability accesses, the user is shown each capability
+access and asked if the capability should be allowed, denied, or if the
+user wants to quit. See capability(7) for details.
# There should also be a section for other rule types. For example, "network" is missing.
+If you find any bugs, please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
=== added file 'Tools/manpages/aa-mergeprof.pod'
--- Tools/manpages/aa-mergeprof.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-mergeprof.pod 2013-09-19 05:02:19 +0000
@@ -0,0 +1,33 @@
+aa-mergeprof - merge AppArmor security profiles.
+
+=head1 SYNOPSIS
+
+B<aa-mergeprof I<E<lt>mineE<gt>> I<E<lt>userE<gt>> I<E<lt>otherE<gt>> [I<-d /path/to/profiles>]>
+
+=head1 OPTIONS
+
+B<-d --dir /path/to/profiles>
+
+ Specifies where to look for the AppArmor security profile set.
+ Defaults to /etc/apparmor.d.
+
+=head1 DESCRIPTION
+
+B<aa-mergeprof>
# the description is a bit too short ;-)
+If you find any bugs, please report them at
+L<https://bugs.launchpad.net/apparmor-profile-tools/+filebug>.
# Please use the "official" URL https://bugs.launchpad.net/apparmor/+filebug
=== added file 'Tools/manpages/aa-unconfined.pod'
--- Tools/manpages/aa-unconfined.pod 1970-01-01 00:00:00 +0000
+++ Tools/manpages/aa-unconfined.pod 2013-09-19 05:02:19 +0000
+B<aa-unconfined> must be run as root to retrieve the process executable
+link from the F</proc> filesystem. This program is susceptible to race
+conditions of several flavours: an unlinked executable will be mishandled;
+an executable started before a AppArmor profile is loaded will not
# before a_n_
+L<http://https://bugs.launchpad.net/apparmor/+filebug>.
# That's a "http://" too much ;-)
# Bugs noticed while testing (already discussed on IRC):
# python aa-logprof
# Traceback (most recent call last):
# File "aa-logprof", line 14, in <module>
# profiledir = args.d
# AttributeError: 'Namespace' object has no attribute 'd'
# -> now named "dir"?
# aa-unconfined lists only 3 processes with LANG=pt_BR
vim:ft=diff
More information about the AppArmor
mailing list