Re: [apparmor] Please review and merge updated Pidgin profile [Was: Updating the Pidgin profile]

2014-01-27 Thread Simon Deziel
Hi intrigeri,

On 14-01-26 06:40 AM, intrigeri wrote:
>>> I'd like someone else than me to test my current profile (attached),
>>> before I ask for inclusion again. Simon, do you want to test it?
> 
>> I'm not yet ready to move to Trusty/14.04 yet so I cannot test with
>> fresh packages. If/when I do, I'll post feedback.
> 
> OK, I'm then going ahead and asking for the update of the Pidgin
> profile with the attached one. Reviews and comments are welcome!

So I went ahead and installed 14.04 and tried your profile. I made some
changes:

* removed abstractions/dconf (does not exist as you said)
* added ~/.config/dconf/user
* dropped /{,var/}run/ compat (not needed anymore I think)
* re-added ~/.config/indicators/ (my previous problem is corrected so
  the blacklisting behaviour is fine
  and depends on the libnotify plugin)
* added /run/user/[0-9]*/orcexec.* rw, (sound notifications again)

All those changes were tested and the resulting profile is available
here [1]. In Ubuntu 14.04, this profile generates a ptrace denial even
if the "deny capability sys_ptrace" should make that silent. Since that
seems like an Apparmor problem, I've reported it [2].


Regards,
Simon


1:
https://github.com/simondeziel/aa-profiles/blob/master/14.04/usr.bin.pidgin
2: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1273518

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor


[apparmor] [patch] new profile tools - handling of "(F)inish"

2014-01-27 Thread Christian Boltz
Hello,

currently, selecting (F)inish in the new profile tools basically means 
aborting without saving anything. However, we already have Abo(r)t for 
that ;-)

(F)inish should ask the user if he wants to save the changed profiles 
before exiting.

The attached patch does this (except for two places, where I just added 
a TODO instead of real code ;-) - probably solvable by copy&paste)

I don't really like that I had to remove the central handling of 
(F)inish in ui.py and add it to several places in aa.py. OTOH calling 
confirm_and_finish() from ui.py a) isn't that easy and b) would mix 
program logic into ui.py - I'm not sure if this is a good idea.

Hmm, maybe we should also move the handling for Abo(r)t to aa.py and 
make ui.py totally dump to be consistent, even if this means to add some 
duplicated lines?

Or add a small function to aa.py as wrapper for UI_PromptUser that
- always adds (F)inish and Abo(r)t
- handles those two
- and returns any other answer to the calling function
?
This function could then be used for all user prompts, with the 
exception of the "save profile / view diff" prompt.

Opinions?


That all said:
Kshitij, this is your chance to write an evil review about a patch from 
me - don't waste it ;-)  (In the unlikely case that you like my patch, 
you can of course commit it ;-)


Regards,

Christian Boltz
-- 
Werbung lügt, Corporate Design sagt die Wahrheit. Naja,
alle _guten_ Komponenten der Wahrheit. :-)  [Ratti]
=== modified file 'Testing/runtests-py2.sh' (properties changed: -x to +x)
=== modified file 'Testing/runtests-py3.sh' (properties changed: -x to +x)
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py	2013-12-19 21:42:58 +
+++ apparmor/aa.py	2014-01-27 21:52:05 +
@@ -492,6 +492,7 @@
 
 ans = ''
 while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in ans:
+# TODO: handle FINISHED
 ans, arg = UI_PromptUser(q)
 p = profile_hash[options[arg]]
 q['selected'] = options.index(options[arg])
@@ -975,6 +979,8 @@
 
 ans = UI_PromptUser(q)
 
+# TODO: handle FINISHED
+
 transitions[context] = ans
 
 if ans == 'CMD_ADDHAT':
@@ -1236,6 +1242,11 @@
 q['functions'] += build_x_functions(default, options, exec_toggle)
 ans = ''
 continue
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_nx' or ans == 'CMD_nix':
 arg = exec_target
 ynans = 'n'
@@ -1535,6 +1546,11 @@
 done = False
 while not done:
 ans, selected = UI_PromptUser(q)
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 # Ignore the log entry
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
@@ -1779,6 +1795,10 @@
 
 ans, selected = UI_PromptUser(q)
 
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
 break
@@ -1930,6 +1950,11 @@
 done = False
 while not done:
 ans, selected = UI_PromptUser(q)
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
 break
@@ -2224,7 +2249,7 @@
 pass
 
 finishing = False
-# Check for finished
+# TODO: Check for finished
 save_profiles()
 
 ##if not repo_cfg['repository'].get('upload', False) or repo['repository']['upload'] == 'later':

=== modified file 'apparmor/ui.py'
--- apparmor/ui.py	2013-12-29 09:42:30 +
+++ apparmor/ui.py	2014-01-27 22:00:57 +
@@ -288,10 +288,10 @@
 if cmd == 'CMD_ABORT':
 confirm_and_abort()
 cmd = 'XXXINVALIDXXX'
-elif cmd == 'CMD_FINISHED':
-if not params:
-confirm_and_finish()
-cmd = 'XXXINVALIDXXX'
+#elif cmd == 'CMD_FINISHED':
+#if not params:
+#confirm_and_finish()
+#cmd = 'XXXINVALIDXXX'
 return (cmd, arg)
 
 def confirm_and_abort():
@@ -319,9 +319,9 @@
 })
 ypath, yarg = GetDataFromYast()
 
-def confirm_and_finish():
-sys.stdout.write(_('FINISHING...\n'))
-sys.exit(0)
+#def confirm_and_finish():
+#sys.stdout.write(_('FINISHING...