On Wed, Jun 01, 2016 at 02:07:10AM +0200, Christian Boltz wrote:
> Hello,
>
> thanks to reading the wrong directory in read_inactive_profiles()
> (profile_dir instead of extra_profile_dir), aa-genprof never asked about
> using a profile from the extra_profile_dir.
>
> Sounds like an easy fix, right? ;-)
>
> After fixing this (last chunk), several other errors popped up, one
> after the other:
> - get_profile() missed a required parameter in a serialize_profile() call
> - when saving the profile, it was written to extra_profile_dir, not to
> profile_dir where it (as a now-active profile) should be. This is
> fixed by removing the filename from existing_profiles{} so that it can
> pick up the default name.
> - CMD_FINISHED (when asking if the extra profile should be used or a new
> one) behaved exactly like CMD_CREATE_PROFILE, but this is surprising
> for the user. Remove it to avoid confusion.
> - displaying the extra profile was only implemented in YaST mode
> - get_pager() returned None, not an actual pager. Since we have 'less'
> hardcoded at several places, also return it in get_pager()
>
> Finally, also remove CMD_FINISHED from the get_profile() test in
> test-translations.py.
>
>
> I propose this patch for 2.9, 2.10 and trunkAcked-by: Seth Arnold <[email protected]> for trunk Nice set of fixes; however I'm uncomfortable with making that large a change on 2.9, 2.10. I know it's only a wee tiny little code change but the semantic idea of the change is quite a bit larger, and it doesn't feel appropriate to me to make the change anywhere except trunk. (I won't object if someone else ACKs it for 2.9, 2.10.. I just don't feel comfortable myself.) I'm also confused by the removal of the CMD_FINISHED; how do you end an aa-genprof or aa-logprof session? And, final question: > + profile_data[pname][pname]['filename'] = None # will be stored in > /etc/apparmor.d when saving, so it shouldn't carry the extra_profile_dir > filename Is this correct? Should it instead be: del profile_data[pname][pname]['filename'] I'm not sure what else may depend upon this being present-but-None vs not-even-present. (Half of why I'm excited about the death of the hasher.. :) Thanks > (test-translations.py is only in trunk, therefore this part of the patch > is obviously trunk-only.) > > > [ 01-genprof-ask-for-extra-dir.diff ] > > === modified file ./utils/apparmor/aa.py > --- utils/apparmor/aa.py 2016-05-30 23:16:05.713448348 +0200 > +++ utils/apparmor/aa.py 2016-06-01 01:25:31.242505830 +0200 > @@ -578,8 +578,11 @@ > inactive_profile[prof_name][prof_name].pop('filename') > profile_hash[uname]['username'] = uname > profile_hash[uname]['profile_type'] = 'INACTIVE_LOCAL' > - profile_hash[uname]['profile'] = > serialize_profile(inactive_profile[prof_name], prof_name) > + profile_hash[uname]['profile'] = > serialize_profile(inactive_profile[prof_name], prof_name, None) > profile_hash[uname]['profile_data'] = inactive_profile > + > + existing_profiles.pop(prof_name) # remove profile filename from > list to force storing in /etc/apparmor.d/ instead of extra_profile_dir > + > # If no profiles in repo and no inactive profiles > if not profile_hash.keys(): > return None > @@ -604,18 +607,13 @@ > > q = aaui.PromptQuestion() > q.headers = ['Profile', prof_name] > - q.functions = ['CMD_VIEW_PROFILE', 'CMD_USE_PROFILE', > 'CMD_CREATE_PROFILE', > - 'CMD_ABORT', 'CMD_FINISHED'] > + q.functions = ['CMD_VIEW_PROFILE', 'CMD_USE_PROFILE', > 'CMD_CREATE_PROFILE', 'CMD_ABORT'] > q.default = "CMD_VIEW_PROFILE" > q.options = options > q.selected = 0 > > ans = '' > while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in ans: > - if ans == 'CMD_FINISHED': > - save_profiles() > - return > - > ans, arg = q.promptUser() > p = profile_hash[options[arg]] > q.selected = options.index(options[arg]) > @@ -627,12 +625,13 @@ > 'profile_type': p['profile_type'] > }) > ypath, yarg = GetDataFromYast() > - #else: > - # pager = get_pager() > - # proc = subprocess.Popen(pager, stdin=subprocess.PIPE) > + else: > + pager = get_pager() > + proc = subprocess.Popen(pager, stdin=subprocess.PIPE) > # proc.communicate('Profile submitted by %s:\n\n%s\n\n' % > # (options[arg], p['profile'])) > - # proc.kill() > + proc.communicate(p['profile'].encode()) > + proc.kill() > elif ans == 'CMD_USE_PROFILE': > if p['profile_type'] == 'INACTIVE_LOCAL': > profile_data = p['profile_data'] > @@ -683,6 +682,7 @@ > if not profile_data: > profile_data = create_new_profile(pname) > file = get_profile_filename(pname) > + profile_data[pname][pname]['filename'] = None # will be stored in > /etc/apparmor.d when saving, so it shouldn't carry the extra_profile_dir > filename > attach_profile_data(aa, profile_data) > attach_profile_data(original_aa, profile_data) > if os.path.isfile(profile_dir + '/tunables/global'): > @@ -1960,7 +1970,7 @@ > reload_base(profile_name) > > def get_pager(): > - pass > + return 'less' > > def generate_diff(oldprofile, newprofile): > oldtemp = tempfile.NamedTemporaryFile('w') > @@ -2204,7 +2214,7 @@ > except: > fatal_error(_("Can't read AppArmor profiles in %s") % > extra_profile_dir) > > - for file in os.listdir(profile_dir): > + for file in os.listdir(extra_profile_dir): > if os.path.isfile(extra_profile_dir + '/' + file): > if is_skippable_file(file): > continue > > === modified file 'utils/test/test-translations.py' > --- utils/test/test-translations.py 2016-05-14 11:25:15 +0000 > +++ utils/test/test-translations.py 2016-05-31 23:58:23 +0000 > @@ -24,7 +24,7 @@ > (['CMD_ALLOW', 'CMD_DENY', 'CMD_IGNORE_ENTRY', 'CMD_GLOB', > 'CMD_GLOBEXT', 'CMD_NEW', 'CMD_AUDIT_OFF', 'CMD_ABORT', 'CMD_FINISHED'], > True), # aa.py available_buttons() with CMD_AUDIT_OFF > (['CMD_ALLOW', 'CMD_DENY', 'CMD_IGNORE_ENTRY', 'CMD_GLOB', > 'CMD_GLOBEXT', 'CMD_NEW', 'CMD_AUDIT_NEW', 'CMD_ABORT', 'CMD_FINISHED'], > True), # aa.py available_buttons() with CMD_AUDIT_NEW > (['CMD_SAVE_CHANGES', 'CMD_SAVE_SELECTED', 'CMD_VIEW_CHANGES', > 'CMD_VIEW_CHANGES_CLEAN', 'CMD_ABORT'], True), > # aa.py save_profiles() > - (['CMD_VIEW_PROFILE', 'CMD_USE_PROFILE', 'CMD_CREATE_PROFILE', > 'CMD_ABORT', 'CMD_FINISHED'], True), > # aa.py get_profile() > + (['CMD_VIEW_PROFILE', 'CMD_USE_PROFILE', 'CMD_CREATE_PROFILE', > 'CMD_ABORT'], True), > # aa.py get_profile() > (['CMD_UPLOAD_CHANGES', 'CMD_VIEW_CHANGES', 'CMD_ASK_LATER', > 'CMD_ASK_NEVER', 'CMD_ABORT'], True), > # aa.py console_select_and_upload_profiles() > (['CMD_ix', 'CMD_pix', 'CMD_cix', 'CMD_nix', 'CMD_EXEC_IX_OFF', > 'CMD_ux', 'CMD_DENY', 'CMD_ABORT', 'CMD_FINISHED'], True), # > aa.py build_x_functions() with exec_toggle > (['CMD_ix', 'CMD_cx', 'CMD_px', 'CMD_nx', 'CMD_ux', > 'CMD_EXEC_IX_ON', 'CMD_DENY', 'CMD_ABORT', 'CMD_FINISHED'], > True), # aa.py build_x_functions() without exec_toggle > > >
signature.asc
Description: PGP signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
