[apparmor] [patch] require logfile only for aa-logprof and aa-genprof
Seth Arnold
seth.arnold at canonical.com
Fri Feb 20 19:51:03 UTC 2015
On Fri, Feb 20, 2015 at 08:23:02PM +0100, Christian Boltz wrote:
> Hello,
>
> this patch makes sure most tools (for example aa-complain) don't error
> out if no logfile can be found. (For obvious reasons, aa-logprof and
> aa-genprof will still require a logfile ;-)
>
> This is done by moving code from the global area in aa.py to the new
> function set_logfile(), which is called by aa-logprof and aa-genprof.
>
> While on it,
> - rename apparmor.filename to apparmor.logfile
> - move the error handling for user-specified logfile from aa-genprof
> and aa-logprof to aa.py set_logfile()
>
> Note: I'd have prefered to hand over the logfile as parameter to
> do_logprof_pass(), but that would break last_audit_entry_time() in
> aa-genprof which requires the log filename before do_logprof_pass() is
> called.
>
> Also note that the patch preserves all error messages. We could simplify
> if by using "The logfile %s does not exist." instead of using two
> different error messages.
>
>
> References: https://bugs.launchpad.net/apparmor/+bug/1423702
>
>
> I'm not sure if we should apply this patch to 2.9 or if trunk is
> enough - opinions?
It'd be nice to apply to 2.9 if it doesn't look like it breaks anything. I
looked around for possible name collisions with 'logfile' and possible
not-converted uses of 'filename' and .. it's tough to tell.
So perhaps just apply to trunk for now and we can cherry-pick the commit
back to 2.9 if it seems like it doesn't break anything.
Thanks
Acked-by: Seth Arnold <seth.arnold at canonical.com>
>
>
> [ set-logfile.diff ]
>
> === modified file 'utils/aa-genprof'
> --- utils/aa-genprof 2014-11-05 19:25:44 +0000
> +++ utils/aa-genprof 2015-02-20 19:03:39 +0000
> @@ -41,7 +41,7 @@
> f_out.write(str(value))
>
> def last_audit_entry_time():
> - out = subprocess.check_output(['tail', '-1', apparmor.filename])
> + out = subprocess.check_output(['tail', '-1', apparmor.logfile])
> logmark = None
> out = out.decode('ascii')
> if re.search('^.*msg\=audit\((\d+\.\d+\:\d+).*\).*$', out):
> @@ -61,16 +61,8 @@
>
> profiling = args.program
> profiledir = args.dir
> -filename = args.file
> -
> -
> -if filename:
> - if not os.path.exists(filename):
> - raise apparmor.AppArmorException(_('The logfile %s does not exist. Please check the path') % filename)
> - elif os.path.isdir(filename):
> - raise apparmor.AppArmorException(_('%s is a directory. Please specify a file as logfile') % filename)
> - else:
> - apparmor.filename = filename
> +
> +apparmor.set_logfile(args.file)
>
> aa_mountpoint = apparmor.check_for_apparmor()
> if not aa_mountpoint:
>
> === modified file 'utils/aa-logprof'
> --- utils/aa-logprof 2014-11-05 19:25:44 +0000
> +++ utils/aa-logprof 2015-02-20 19:04:51 +0000
> @@ -28,17 +28,9 @@
> args = parser.parse_args()
>
> profiledir = args.dir
> -filename = args.file
> logmark = args.mark or ''
>
> -
> -if filename:
> - if not os.path.exists(filename):
> - raise apparmor.AppArmorException(_('The logfile %s does not exist. Please check the path') % filename)
> - elif os.path.isdir(filename):
> - raise apparmor.AppArmorException(_('%s is a directory. Please specify a file as logfile') % filename)
> - else:
> - apparmor.filename = filename
> +apparmor.set_logfile(args.file)
>
> aa_mountpoint = apparmor.check_for_apparmor()
> if not aa_mountpoint:
>
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py 2015-02-04 12:16:29 +0000
> +++ utils/apparmor/aa.py 2015-02-20 19:02:43 +0000
> @@ -72,7 +72,7 @@
> sev_db = None
> # The file to read log messages from
> ### Was our
> -filename = None
> +logfile = None
>
> cfg = None
> repo_cfg = None
> @@ -2233,6 +2233,24 @@
>
> return newincludes
>
> +def set_logfile(filename):
> + ''' set logfile to a) the specified filename or b) if not given, the first existing logfile from logprof.conf'''
> +
> + global logfile
> +
> + if filename:
> + logfile = filename
> + else:
> + logfile = conf.find_first_file(cfg['settings']['logfiles']) or '/var/log/syslog'
> +
> + if not os.path.exists(logfile):
> + if filename:
> + raise AppArmorException(_('The logfile %s does not exist. Please check the path') % logfile)
> + else:
> + raise AppArmorException('Can\'t find system log "%s".' % (logfile))
> + elif os.path.isdir(logfile):
> + raise AppArmorException(_('%s is a directory. Please specify a file as logfile') % logfile)
> +
> def do_logprof_pass(logmark='', passno=0, pid=pid):
> # set up variables for this pass
> # t = hasher()
> @@ -2250,7 +2268,7 @@
> # skip = hasher() # XXX global?
> # filelist = hasher()
>
> - aaui.UI_Info(_('Reading log entries from %s.') % filename)
> + aaui.UI_Info(_('Reading log entries from %s.') % logfile)
>
> if not passno:
> aaui.UI_Info(_('Updating AppArmor profiles in %s.') % profile_dir)
> @@ -2264,7 +2282,8 @@
> ## repo_cfg = read_config('repository.conf')
> ## if not repo_cfg['repository'].get('enabled', False) or repo_cfg['repository]['enabled'] not in ['yes', 'no']:
> ## UI_ask_to_enable_repo()
> - log_reader = apparmor.logparser.ReadLog(pid, filename, existing_profiles, profile_dir, log)
> +
> + log_reader = apparmor.logparser.ReadLog(pid, logfile, existing_profiles, profile_dir, log)
> log = log_reader.read_log(logmark)
> #read_log(logmark)
>
> @@ -4572,10 +4591,6 @@
> if not os.path.isfile(parser) or not os.access(parser, os.EX_OK):
> raise AppArmorException('Can\'t find apparmor_parser')
>
> -filename = conf.find_first_file(cfg['settings']['logfiles']) or '/var/log/syslog'
> -if not os.path.isfile(filename):
> - raise AppArmorException('Can\'t find system log "%s".' % (filename))
> -
> ldd = conf.find_first_file(cfg['settings']['ldd']) or '/usr/bin/ldd'
> if not os.path.isfile(ldd) or not os.access(ldd, os.EX_OK):
> raise AppArmorException('Can\'t find ldd')
>
>
> Regards,
>
> Christian Boltz
> --
> In college, I wrote a TECO-like progamming language as a joke -
> one-letter statements, totally unreadable. Then I discovered sendmail,
> and stopped, because the joke had been done so much better than I ever
> could. [Mark 'Kamikaze' Hughes]
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150220/08a3f133/attachment.pgp>
More information about the AppArmor
mailing list