[apparmor] [PATCH v3] Convert aa-status to Python
Seth Arnold
seth.arnold at gmail.com
Tue May 31 08:16:31 UTC 2011
On Thu, May 26, 2011 at 5:37 PM, Marc Deslauriers
<marc.deslauriers at canonical.com> wrote:
Marc, thanks, I'm finally taking the time to give this a look, rather
than just commenting on the overall idea. :)
> +def cmd_enabled():
> + '''Returns error code if AppArmor is not enabled'''
> + if get_profiles() == {}:
> + sys.exit(2)
I think this code makes the assumption that _no profiles loaded_ means
_not enabled_. AppArmor might be enabled but have no profiles loaded.
(One might wonder if AppArmor is _really enabled_ if it isn't confining
anything, but there's a big difference between "enabled but doing
nothing" and "module not loaded".)
Some other mechanism to report enabled / empty would be nice.
> +def cmd_profiled():
> + '''Prints the number of loaded profiles'''
> + profiles = get_profiles()
> + sys.stdout.write("%d\n" % len(profiles))
> + if profiles == {}:
> + sys.exit(2)
Here, as well... incidentally, exiting with a status code seems a bit
abrupt for "no profiles loaded".
Some very dissapointing news:
http://docs.python.org/release/3.0.1/whatsnew/3.0.html#pep-3101-a-new-approach-to-string-formatting
The dirt-simple % format handling is being removed eventually. All these
nice simple % formats should be replaced with:
"{0}".format(len(profiles))
or
"{0} profiles are loaded.".format(len(profiles))
Sigh. I always thought the % operator was one of Python's nicest
features.
> +def cmd_enforced():
> + '''Prints the number of loaded enforcing profiles'''
> + profiles = get_profiles()
> + sys.stdout.write("%d\n" % len(filter_profiles(profiles, 'enforce')))
> + if profiles == {}:
> + sys.exit(2)
> +
> +def cmd_complaining():
> + '''Prints the number of loaded non-enforcing profiles'''
> + profiles = get_profiles()
> + sys.stdout.write("%d\n" % len(filter_profiles(profiles, 'complain')))
> + if profiles == {}:
> + sys.exit(2)
> +
> +def cmd_verbose():
> + '''Displays multiple data points about loaded profile set'''
> + global verbose
> + verbose = True
> + profiles = get_profiles()
> + processes = get_processes(profiles)
> +
> + stdmsg("%d profiles are loaded." % len(profiles))
> + for status in ('enforce', 'complain'):
> + filtered_profiles = filter_profiles(profiles, status)
> + stdmsg("%d profiles are in %s mode." % (len(filtered_profiles), status))
> + for item in filtered_profiles:
> + stdmsg(" %s" % item)
> +
> + stdmsg("%d processes have profiles defined." % len(processes))
> + for status in ('enforce', 'complain', 'unconfined'):
> + filtered_processes = filter_processes(processes, status)
> + if status == 'unconfined':
> + stdmsg("%d processes are unconfined but have a profile defined." % len(filtered_processes))
> + else:
> + stdmsg("%d processes are in %s mode." % (len(filtered_processes), status))
> + # Sort by name, and then by pid
> + filtered_processes.sort(key=lambda x: int(x[0]))
> + filtered_processes.sort(key=lambda x: x[1])
> + for (pid, process) in filtered_processes:
> + stdmsg(" %s (%s) " % (process, pid))
> +
> + if profiles == {}:
> + sys.exit(2)
... and seeing this block repeated for each of the cmd_foo() handlers
makes me think that it should be handled outside of the individual
cmd_foo() functions themselves.
> +def get_profiles():
> + '''Fetch loaded profiles'''
> +
> + profiles = {}
> +
> + if os.path.exists("/sys/module/apparmor"):
> + stdmsg("apparmor module is loaded.")
> + else:
> + errormsg("apparmor module is not loaded.")
> + sys.exit(1)
And if sysfs isn't loaded, or isn't mounted here? I wish we had a more
solid mechanism to determine if AppArmor is loaded.
> + apparmorfs = find_apparmorfs()
> + if not apparmorfs:
> + errormsg("apparmor filesystem is not mounted.")
> + sys.exit(3)
We _do_ pretty much need securityfs to be loaded, but this error message
doesn't help the user figure out how to do that. I suggest:
"Expected a securityfs filesystem to be mounted; /sys/kernel/security is common"
> + apparmor_profiles = os.path.join(apparmorfs, "profiles")
> + if not os.access(apparmor_profiles, os.R_OK):
> + errormsg("You do not have enough privilege to read the profile set.")
> + sys.exit(4)
Rather than exit, why not continue with the operations that _are_
allowed to the user? `ps auxwZ` contains a wealth of information
available to unprivileged users that the user might still wish to be
summarized.
> + for p in open(apparmor_profiles).readlines():
> + match = re.search("^([^\(]+)\s+\((\w+)\)$", p)
> + profiles[match.group(1)] = match.group(2)
> +
> + return profiles
This specifically is where I wish {} didn't mean two things. :)
> +def get_processes(profiles):
> + '''Fetch process list'''
> + processes = {}
> + contents = os.listdir("/proc")
> + for filename in contents:
> + if filename.isdigit():
> + try:
> + for p in open("/proc/%s/attr/current" % filename).readlines():
> + match = re.search("^([^\(]+)\s+\((\w+)\)$", p)
> + if match:
> + processes[filename] = { 'profile' : match.group(1), \
> + 'mode' : match.group(2) }
> + elif os.path.realpath("/proc/%s/exe" % filename) in profiles:
> + # keep only unconfined processes that have a profile defined
> + processes[filename] = { 'profile' : os.path.realpath("/proc/%s/exe" % filename), \
> + 'mode' : 'unconfined' }
> + except:
> + pass
> + return processes
> +
> +def filter_profiles(profiles, status):
> + '''Return a list of profiles that have a particular status'''
> + filtered = []
> + for key, value in list(profiles.items()):
> + if value == status:
> + filtered.append(key)
> + filtered.sort()
> + return filtered
> +
> +def filter_processes(processes, status):
> + '''Return a list of processes that have a particular status'''
> + filtered = []
> + for key, value in list(processes.items()):
> + if value['mode'] == status:
> + filtered.append([key, value['profile']])
> + return filtered
> +
Both these beg for list comprehensions. I'll let you decide if you think
these are clearer or not:
def filter_profiles(profiles, status):
'''Return a list of profiles that have a particular status'''
l = [k for k,v in profiles.items() if v == status]
l.sort()
return l
def filter_processes(processes, status):
'''Return a list of processes that have a particular status'''
l = [[fn,stat['profile']] for fn,stat in processes.items() if
stat['mode'] == status]
return l
> +def find_apparmorfs():
> + '''Finds AppArmor mount point'''
> + for p in open("/proc/mounts").readlines():
> + if p.split()[2] == "securityfs" and \
> + os.path.exists(os.path.join(p.split()[1], "apparmor")):
> + return os.path.join(p.split()[1], "apparmor")
> + return False
> +
> +def errormsg(message):
> + '''Prints to stderr if verbose mode is on'''
> + global verbose
> + if verbose:
> + sys.stderr.write(message + "\n")
> +
> +def stdmsg(message):
> + '''Prints to stdout if verbose mode is on'''
> + global verbose
> + if verbose:
> + sys.stdout.write(message + "\n")
> +
> +def print_usage():
> + '''Print usage information'''
> + sys.stdout.write('''Usage: %s [OPTIONS]
> +Displays various information about the currently loaded AppArmor policy.
> +OPTIONS (one only):
> + --enabled returns error code if AppArmor not enabled
> + --profiled prints the number of loaded policies
> + --enforced prints the number of loaded enforcing policies
> + --complaining prints the number of loaded non-enforcing policies
> + --verbose (default) displays multiple data points about loaded policy set
> + --help this message
> +''' % sys.argv[0])
> +
> +# Main
> +global verbose
> +verbose = False
> +
> +if len(sys.argv) > 2:
> + sys.stderr.write("Error: Too many options.\n")
> + print_usage()
> + sys.exit(1)
> +elif len(sys.argv) == 2:
> + cmd = sys.argv.pop(1)
> +else:
> + cmd = '--verbose'
> +
> +# Command dispatch:
> +commands = {
> + '--enabled' : cmd_enabled,
> + '--profiled' : cmd_profiled,
> + '--enforced' : cmd_enforced,
> + '--complaining' : cmd_complaining,
> + '--verbose' : cmd_verbose,
> + '-v' : cmd_verbose,
> + '--help' : print_usage,
> + '-h' : print_usage
> +}
> +
> +if cmd in commands:
> + commands[cmd]()
> + sys.exit(0)
> +else:
> + sys.stderr.write("Error: Invalid command.\n")
> + print_usage()
> + sys.exit(1)
> +
Ooooh, it'd sure be nice if these were easy to translate. :) I know the
Perl version wasn't i18ned, but it sure would be nice if Python's
gettext made it as easy...
Thanks again Marc!
More information about the AppArmor
mailing list