Re: [apparmor] Fwd: GSoC r13, r14, r15 review

2013-07-09 Thread Christian Boltz
 happens if someone is not using utf-8?
 
 # The program's output needs to be utf-8 encoded else they get
 some weird stuff, on encoding look below.

This means you have to set the encoding somewhere ;-)
Or set   LANG=C   and you should have pure ASCII ;-)

 +ret = e.returncode
 +else:
 +ret = 0
 +output = output.decode('utf-8').split('\n')
 +# Remove the extra empty string caused due to \n if present
 +if len(output)  1:
 +output.pop()
 +return (ret, output)
 
 +def get_reqs(file):
 +Returns a list of paths from ldd output
 +pattern1 = re.compile('^\s*\S+ = (\/\S+)')
 +pattern2 = re.compile('^\s*(\/\S+)')
 +reqs = []
 +ret, ldd_out = get_output(ldd, file)
 
 ### ldd is None - you should fill it before using it ;-)
 ### also, I'm not sure if ldd needs to be a global variable
 
 # The setter method is yet to come. ;-)
 # Will fix it when refractoring the module later as I planned. :-)

OK.

 === added file 'apparmor/common.py'
 --- apparmor/common.py1970-01-01 00:00:00 +
 +++ apparmor/common.py2013-07-03 23:34:04 +
 
 
 +def cmd_pipe(command1, command2):
 +'''Try to pipe command1 into command2.'''
 +try:
 +sp1 = subprocess.Popen(command1, stdout=subprocess.PIPE)
 +sp2 = subprocess.Popen(command2, stdin=sp1.stdout)
 +except OSError as ex:
 +return [127, str(ex)]
 +
 +if sys.version_info[0] = 3:
 +out = sp2.communicate()[0].decode('ascii', 'ignore')
 
 ### is ascii correct here, or should it be the system locale?
 
 # From aa-enforce, havent used it yet. However, utf-8 is default
 for Python3, so maybe we can use it?

If we use it everywhere, then I'd say yes.

 +def open_file_read(path):
 +'''Open specified file read-only'''
 +try:
 +orig = codecs.open(path, 'r', UTF-8)
 
 # once more - is hardcoding utf-8 a good idea?
 
 # Again from aa-enforce, and we can discuss using utf-8 or system
 locale.

Sounds like something we should discuss in the meeting later ;-)



Regards,

Christian Boltz
-- 
Guten Tag, ich möchte gerne einen Tisch reservieren.
Gerne, auf welchen Namen denn?
31337 /-/ /\ X0R!
[Jens Benecke in suse-linux zum Thema Realnames]


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


Re: [apparmor] GSoC r13, r14, r15 review

2013-07-09 Thread Christian Boltz
Hello,

Am Dienstag, 9. Juli 2013 schrieb Kshitij Gupta:
  I somehow doubt this is intentional - the test should catch this
  exception ;-)
 
 As it turns this issue has got something to do with the Python3's new
 feature called Exception Chaining.
 Its another of those Python2-3 problem.
 An additional line needs to be incorporated to check if Python2 or 3
 and raise accordingly. :/

:-(

  It was inside the try except block of test, I got it out to try how
  exception looked.
  Turns out \n\t aren't working yet. :-\
  
  Also note that the last line contains \n\t - this should become a
  real line break and tab in the output...
 
 I did not notice until later, but the AppArmorException uses repr() to
 print the description and hence its not possible to print the nicely
 formatted exception with the line number in new line.

I noticed you removed repr() in r16 - does this fix the issue?
(a short test tells me the answer is probably no)

 I feel I should use the error(), it will print something like this and
 terminate program:
 
 ERROR: No severity value present in file: severity_broken.db
 [Line 14]: CAP_SYS_MODULE
 What would you say? move to error() instead of raising
 AppArmorException?

Can error() be catched with try/except like an exception? For example, 
we'll need this to display a nice error dialog in YaST. 
(If yes, then using error() is OK.)


Regards,

Christian Boltz
-- 
The normal user is happy with openSUSE because: [...]
  - openSUSE isn't a religion (like a few others);
What users blame us is for lack of customization :)
[Nelson Marques in opensuse-factory]


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


Re: [apparmor] apparmor policy versioning

2013-07-11 Thread Christian Boltz
.

With an additional copy of the profiles, we'll end up in a maintenance 
hell - and users will kill us because they have to update two profiles 
instead of one if they want to switch kernels.


Regards,

Christian Boltz

[1] does this name remind you to something? ;-)

[2] to be exact, cleanup of code duplication in PostfixAdmin
-- 
Comic Sans
Man möge mir verzeihen, aber ich möchte mich im Rahmen dieses Essays in
erster Linie mit Schriften auseinandersetzen, nicht mit Krankheiten.
[http://praegnanz.de/essays/136/typo-im-web-html-schriften-unter-der-lupe]


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


[apparmor] GSoC review r17..22

2013-07-18 Thread Christian Boltz
Hello,

the attached files contain my review notes for r17..22.

In case you miss the files for r19 and r20: that's intentional, those 
commits look so good that I don't need to comment on them ;-)


Regards,

Christian Boltz
-- 
Jungs. Mit dem Argument kann ich kaputte Autos verkaufen und dann
erklären, daß Fahrradfahren eh viel gesünder ist.
[Peer Heinlein in postfixbuch-users]
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py	2013-07-08 22:16:26 +
+++ apparmor/aa.py	2013-07-17 15:08:13 +
@@ -619,28 +624,36 @@
 def set_profile_flags(prof_filename, newflags):
 Reads the old profile file and updates the flags accordingly
 regex_bin_flag = re.compile('^(\s*)((??\/.+???)|(profile\s+(??.+???)))\s+(flags=\(.+\)\s+)*\{\s*$/')
-regex_hat_flag = re.compile('^(\s*\^\S+)\s+(flags=\(.+\)\s+)*\{\s*$')
+regex_hat_flag = re.compile('^([a-z]*)\s+([A-Z]*)((\s+#\S*)*)\s*$')

### this looks superfluous because it's overwritten two lines below

+a=re.compile('^([a-z]*)\s+([A-Z]*)((\s+#\S*)*)\s*$')

### a is not used anywhere in the function

+regex_hat_flag = re.compile('^(\s*\^\S+)\s+(flags=\(.+\)\s+)*\{\s*(#*\S*)$')

### (regex_hat_flag overwritten here)

 if os.path.isfile(prof_filename):
 with open_file_read(prof_filename) as f_in:
-with open_file_write(prof_filename + '.new') as f_out:
+tempfile = tempfile.NamedTemporaryFile('w', prefix=prof_filename , delete=False, dir='/etc/apparmor.d/')

### Looks quite good. A small detail is missing - please add   suffix='~'   to make sure 
### a profile in a tempfile is not loaded when someone calls rcapparmor reload

@@ -654,3 +667,240 @@
+def sync_profile():
+def submit_created_profiles(new_profiles):
+def submit_changed_profiles(changed_profiles):
+def yast_select_and_upload_profiles(title, message, profiles_up):
+def console_select_and_upload_profiles(title, message, profiles_up):
+def set_profile_local_only(profs):

### those functions are about the profile repo and currently unused.
### a comment saying that would be nice
### (or move them to a separate file, maybe profilerepo.py)

+def handle_children(profile, hat, root):
+entries = root[:]
+for entry in entries:
+

### looks like the code to handle the children is still missing ;-)


=== added file 'apparmor/ui.py'
--- apparmor/ui.py	1970-01-01 00:00:00 +
+++ apparmor/ui.py	2013-07-17 15:08:13 +
@@ -0,0 +1,255 @@
+def init_localisation():
+locale.setlocale(locale.LC_ALL, '')
+#cur_locale = locale.getlocale()
+filename = 'res/messages_%s.mo' % locale.getlocale()[0][0:2]

### [0:2] means locales like pt_BR won't be used (only pt, which might be different).
### I doubt this is intentional.

+def UI_Info(text):
+if DEBUGGING:
+debug_logger.info(text)

### debug_logger should check DEBUGGING itsself to avoid all the if DEBUGGING: conditions
### even if this means you'll need a small wrapper around debug_logger - but on the long term it makes the code more readable

+def UI_YesNo(text, default):
+if DEBUGGING:
+debug_logger.debug('UI_YesNo: %s: %s %s' %(UI_mode, text, default))
+ans = default
+if UI_mode == 'text':
+yes = '(Y)es'
+no = '(N)o'
+usrmsg = 'PromptUser: Invalid hotkey for'
+yeskey = 'y'
+nokey = 'n'

### can you honor translations please?
### for example, in german it should be (j)a / (n)ein

+sys.stdout.write('\n' + text + '\n')
+if default == 'y':

### better:   if default == yeskey   - avoids problems when using translations

+sys.stdout.write('\n[%s] / %s\n' % (yes, no))
+else:
+sys.stdout.write('\n%s / [%s]\n' % (yes, no))
+ans = readkey()
+if ans:
+ans = ans.lower()
+else:
+ans = default
+else:
+SendDataTooYast({
+ 'type': 'dialog-yesno',
+ 'question': text
+ })
+ypath, yarg = GetDataFromYast()
+ans = yarg['answer']
+if not ans:
+ans = default
+return ans

### ans can be anything, not just yeskey and nokey
### if something else (= invalid key) is entered, the function should ask again
### (see UI_YesNoCancel which does exactly that)

+def UI_YesNoCancel(text, default):
+if DEBUGGING:
+debug_logger.debug('UI_YesNoCancel: %s: %s %s' % (UI_mode, text, default))
+
+if UI_mode == 'text':
+yes = '(Y)es'
+no = '(N)o'
+cancel = '(C)ancel'
+yeskey = 'y'
+nokey = 'n'
+cancelkey = 'c'

### should honor translations


+CMDS = {
+'CMD_ALLOW': '(A)llow',
+'CMD_OTHER': '(M)ore',

### do all those options honor translations?

=== added file 'apparmor/yasti.py'
--- apparmor/yasti.py	1970-01-01 00:00:00 +
+++ apparmor/yasti.py	2013-07-17 15:08:13 +
@@ -0,0 +1,82 @@
+def SendDataToYast(data):
+for line in sys.stdin:
[...]
+Return

[apparmor] GSoC review r23

2013-07-20 Thread Christian Boltz
Hello,

see the attachment for r23 review. The commit looks quite good, but I 
found some small issues nevertheless ;-)


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]
=== modified file 'Testing/severity_test.py'
--- Testing/severity_test.py	2013-07-18 19:14:55 +
+++ Testing/severity_test.py	2013-07-19 22:49:07 +

def testRank_Test(self):
	z = severity.Severity()

### (not from this commit, nevertheless)
### z seems to be unused - remove it?


### BTW:
#### python3 severity_test.py 
###severity_test.py:59: ResourceWarning: unclosed file _io.BufferedReader name='severity_broken.db'
###[...]


=== modified file 'apparmor/aa.py'
--- apparmor/aa.py	2013-07-17 23:59:54 +
+++ apparmor/aa.py	2013-07-19 22:49:07 +
@@ -434,40 +434,30 @@
 hashbang = head(localfile)
 if hashbang.startswith('#!'):
 interpreter = get_full_path(hashbang.lstrip('#!').strip())
-try:
-local_profile[localfile]['allow']['path'][interpreter]['audit'] |= 0
-except TypeError:
-local_profile[localfile]['allow']['path'][interpreter]['audit'] = 0
+
+local_profile[localfile]['allow']['path'][localfile]['mode'] = local_profile[localfile]['allow']['path'][localfile].get('mode', str_to_mode('r')) | str_to_mode('r')

### that looks already better than the try/except
### nevertheless it will cause some superfluous typing
### the ideal syntax would be something like
### local_profile.add('allow', 'path', '/bin/foo', 'r', no_audit)   # no_audit could be an alias of false
### (then let all the magic happen inside local_profile, where you need the code only once)

@@ -894,5 +885,183 @@
 
 def handle_children(profile, hat, root):
 entries = root[:]

### the code uses  entry[:3], entry[:5] etc. quite often, so a comment explaining the structure
### (or pointing to the explanation elsewhere) would be very useful

+regex_nullcomplain = re.compile('null(-complain)*-profile')
 for entry in entries:

+elif type == 'unknown_hat':
[...]
+if ans == 'CMD_ADDHAT':
+hat = uhat
+aa[profile][hat]['flags'] = aa[profile][profile]['flags']
+elif ans == 'CMD_USEDEFAULT':
+hat = default_hat
+elif ans == 'CMD_DENY':
+return None

### shouldn't CMD_DENY add a deny rule? (or is this handled elsewhere?)


+elif type == 'path' or type == 'exec':
[...]
+# Give Execute dialog if x access requested for something that's not a directory 
+# For directories force an 'ix' Path dialog
+do_execute = False
+exec_target = detail
+
+if mode  str_to_mode('x'):
+if os.path.isdir(exec_target):

### this test might fail if someone sends you his log and you try to update a profile based on the log
### (does the log contain directories as /foo/bar/? If yes, testing for the trailing / might work)

+if mode  AA_MAY_LINK:
+regex_link = re.compile('^from (.+) to (.+)$')

### I can imagine some funny filenames to break that regex ;-)
### for example /home/cb/I'm from germany and go to the next train station.txt
### (not sure if this is too relevant in real world, and what's the best way to prevent it)


=== modified file 'apparmor/severity.py'
--- apparmor/severity.py	2013-07-18 19:14:55 +
+++ apparmor/severity.py	2013-07-19 22:49:07 +
@@ -184,16 +184,20 @@
 
 def load_variables(self, prof_path):
 Loads the variables for the given profile
+regex_include = re.compile('include (\S*)')

### looks better, but could get false positives and false negatives, for example
### /foo/bar r, # instead of #include abstractions/foobar --- regex will match comment
### #include abstractions/foo --- will not match because there's more than one space after #include
### a better regex would be
### ^\s*#?include\s+(\S*)


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


[apparmor] GSoC review r24

2013-07-23 Thread Christian Boltz
Hello,

see the attachment for the r24 review.


Regards,

Christian Boltz
-- 
Dominian There is always room for improvement
Dominian to seek perfection is to drive yourself insane.
Dominian except suseROCKs, he's already insane.
[from #opensuse-project]
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py	2013-07-19 22:49:07 +
+++ apparmor/aa.py	2013-07-22 23:05:51 +
@@ -449,7 +450,7 @@
 local_profile[localfile]['include']['abstractions/python'] = True
 elif 'ruby' in interpreter:

### hmm, just using ruby might lead to false positives
### better check for the full path or at least the full basename

 local_profile[localfile]['include']['abstractions/ruby'] = True
-elif interpreter in ['/bin/bash', '/bin/dash', '/bin/sh']:
+elif re.search('/bin/(bash|dash|sh)', interpreter):

### if you use a regex, don't forget ^...$  ;-)



 
+# If profiled program executes itself only 'ix' option
+if exec_target == profile:
+options = 'i'

### ix is a good default, but there may be (rare) cases where a program executes itsself with different options etc.
### so I'd still allow the other x options

+# Don't allow hats to cx?
+options.replace('c', '')

### child profiles can't be nested, but one child profile can call another
### at the moment the workaround is to do (assuming we are in /bin/foo//bin/bar child profile) Px - /bin/foo//bin/baz
### having an option use another child profile (which makes clear it won't be nested as /bin/foo//bin/bar//bin/baz) would be nice

+# Add deny to options
+options += 'd'
+# Define the default option
+default = None
+if 'p' in options and os.path.exists(get_profile_filename(exec_target)):
+default = 'CMD_px'

### while we are at it - displaying a note target profile exists would be helpful




+elif ans == 'CMD_ux':
+exec_mode = str_to_mode('ux')
+ynans = UI_YesNo(gettext('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 ?' % exec_target, 'n')

### even if this question is absolutely correct, a never ask again option would be welcome ;-)

+if exec_mode  str_to_mode('i'):
+if 'perl' in exec_target:
+aa[profile][hat]['include']['abstractions/perl'] = True

### oh, does executing /bin/perlentaucher need abstractions/perl?
### (the code will happily add it ;-)

+elif '/bin/bash' in exec_path or '/bin/sh' in exec_path:

### similar question here for /usr/bin/bin/shorewall ;-)
### (in other words: using in can have quite some side effects)

+if 'perl' in interpreter:
+aa[profile][hat]['include']['abstractions/perl'] = True
+elif '/bin/bash' in interpreter or '/bin/sh' in interpreter:
+aa[profile][hat]['include']['abstractions/bash'] = True

### some more in...

### that all said - this is at least the second time where the code contains interpreter - abstraction information
### better store it at one place - for example like this:
### def abstraction_for_interpreter(interpreter)
### # TODO: interpreter_basename = remove ^(/usr)?/bin/ from interpreter
### if interpreter_basename == 'bash'
### return 'abstractions/bash'
### elif interpreter_basename == 'perl'
### [...]



=== modified file 'apparmor/severity.py'
--- apparmor/severity.py	2013-07-19 22:49:07 +
+++ apparmor/severity.py	2013-07-22 23:05:51 +
@@ -184,7 +184,7 @@
 
 def load_variables(self, prof_path):
 Loads the variables for the given profile
-regex_include = re.compile('include (\S*)')
+regex_include = re.compile('^#?include\s*(\S*)')

### nearly correct ;-) - you should allow whitespace at the beginning of the line

 if os.path.isfile(prof_path):
 with open_file_read(prof_path) as f_in:
 for line in f_in:

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


Re: [apparmor] [parser patch] fix apparmor cache tempfile location to use passed arg

2013-07-23 Thread Christian Boltz
Hello,

Am Dienstag, 23. Juli 2013 schrieb Steve Beattie:
 The second issue is that it would generate the temporary file that it
 stores the cache file in [basedir]/cache even if an alternate cache
 location was specified on the command line. This causes a problem
 if [basedir]/cache is on a separate device than the alternate cache
 location, because the rename() of the tempfile into the final location
 would fail 

Looks like it was a good idea to package /etc/apparmor/cache as a 
symlink to /var/cache/apparmor on openSUSE ;-)

 (which the parser would not check the return code of).

Nice[tm].

That said - your patch looks like something that should be backported to 
the 2.8 branch (even if it isn't needed for openSUSE thanks to the 
symlink).


Regards,

Christian Boltz
-- 
Aren't most of SUSE-employed community members part of the
ResearchDestroy department? [Sascha Peilicke in opensuse-project]


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


[apparmor] GSoC review r26 and r27

2013-07-27 Thread Christian Boltz
Hello,

see the attached file for r26 and r27 review notes.

@John: I'm still waiting for your answer about
# ix implies m, so we don't need to add m if ix is present

I have some profiles that contain mrix (for example sbin.dhclient and 
usr.sbin.ntpd), so either the old logprof was buggy or the comment is 
wrong ;-)


Regards,

Christian Boltz
-- 
[Grundrechte] Natürlich gibt's da auch das berühme Recht auf freie
Entfaltung.  Andererseits:  setzt das nicht auch zwingend vorraus,
daß man vorher auch gehörig zusammengefaltet wurde? ;-)
[Gerard Jensen in suse-linux]
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py	2013-07-24 16:42:34 +
+++ apparmor/aa.py	2013-07-27 10:02:12 +
-elif 'python' in interpreter:
+elif interpreter == 'python':
 local_profile[localfile]['include']['abstractions/python'] = True

### 'python3' should also be allowed


+def ask_the_question():
[...]
+elif ans == 'CMD_DENY':
+aa[profile][hat]['deny']['capability'][capability]['set'] = True
+changed[profile] = True

### sidenote: an additional ignore option would be nice to just ignore a log event
### without adding anything to the profile.
### (usecase: starting sshd the very first time will write /etc/ssh/ssh_host_key, but you probably don't want
### to have write permissions for it in the profile) (distro-shipped profile may be an exception of course)

+# If we get an mmap request, check if we already have it in allow_mode
+if mode  AA_EXEC_MMAP:
+# ix implies m, so we don't need to add m if ix is present

### hmm, some of my profiles disagree and have mrix...
### (John, any comments on this?)

+
+if not mode_contains(allow_mode, mode):
+default_option = 1
+options = []
+newincludes = []
+include_valid = False
+
+for incname in include.keys():
+include_valid = False
+# If already present skip
+if aa[profile][hat][incname]:
+continue
+
+if cfg['settings']['custom_includes']:
+for incn in cfg['settings']['custom_includes'].split():
+if incn in incname:
+include_valid = True
+
+if 'abstraction' in incname:
+include_valid = True

### so I can add abstractions/doesnotexist and foobar/abstraction? ;-)
### (same for custom_includes)
### hints:
### - check if the file exists
### - use startswith 'abstractions/' instead of in

+if not include_valid:
+continue

### does this mean it's impossible to add an #include for files not in abstractions/ or custom_includes?
### I'd prefer not to forbid that
### (of course, check if the file exists etc.)

   
+elif ans == 'CMD_ALLOW':
+path = options[selected]
+done = True
+match = re.search('^#include\s+(.+)$', path)

### wrong regex:
### - space after #include is optional
### - fails with inline comments
### - if path is not trimmed, it fails with leading space
### (hint: a function re_match_include(path) would be a good idea, you already have the correct regex somewhere else)

+elif ans == 'CMD_GLOB':
+newpath = options[selected].strip()
+if not newpath.startswith('#include'):
+if newpath[-1] == '/':
+if newpath[-4:] == '/**/' or newpath[-3:] == '/*/':
+# collapse one level to /**/
+newpath = re.sub('/[^/]+/\*{1,2}$/', '/\*\*/', newpath)
+else:
+newpath = re.sub('/[^/]+/$', '/\*/', newpath)

### what happens with /foo/bar**/ ?
### (should glob to /foo/**/ - but the code looks like it will be globbed to /foo/*/ )

+else:
+if newpath[-3:] == '/**' or newpath[-2:] == '/*':
+newpath = re.sub('/[^/]+/\*{1,2}$', '/\*\*', newpath)
+elif re.search('/\*\*[^/]+$', newpath):
+newpath = re.sub

[apparmor] GSoC meeting

2013-07-28 Thread Christian Boltz
Hello,

this week, the GSoC IRC meeting will be a day earlier than usual because
I'll be away on tuesday.

This means the meeting is on monday (= tomorrow) at 19:00 UTC.

Besides the usual topics, we'll also discuss the to-be-written mergeprof.


Regards,

Christian Boltz
-- 
AV is homeopathy for computers: This software was in contact with mal-
ware in the past, so it'll recognize other malware in the future. $79
[https://twitter.com/thegrugq/status/297177182848049152]



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


[apparmor] GSoC review r30

2013-08-01 Thread Christian Boltz
Hello,

the review for r30 is attached - it had lots of new code (and
interesting[tm] regexes) - therefore I have several notes about it ;-)

@John: The review contains some questions for you - can you please answer
them?


Regards,

Christian Boltz
-- 
  My calendar shows May 12th to be a Friday, not a Thursday?
 I meant 11th ;-(.
With all the delays, perhaps mentioning the year would also be
a good idea. ;-) [ Andreas Jaeger and houghi in opensuse]


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


Re: [apparmor] GSoC review r30

2013-08-02 Thread Christian Boltz
Hello,

John Johansen wrote:
 On 08/01/2013 02:59 PM, Christian Boltz wrote:

 ### a check if the hat already exists might be useful to avoid duplicate
 hat names (which might get merged on write, but I doubt that's intended
 behaviour)

 ### interestingly, the parser does _not_ complain about duplicate hats.
 ### John, is this a bug or intentional?

 That should fail, there is an explicit test for this in the parser. And in
 my quick testing I get

   Multiple definitions for hat foo in profile (null) exist,bailing out.

 so a bug in the output but the check worked, can you forward an example
 where it is not working correctly

It happened with some echo $whatever | apparmor_parser -p /dev/stdin
and checking my bash history showed I accidently deleted the pipe when I
hit the so-called bug. In other words: it works as it should (and I get
the correct error message for duplicate hat names) - sorry for the false
alarm!

 +# Below is not required I'd say

 ### hmm, not sure - John?

 +if not do_include:
 +for hatglob in cfg['required_hats'].keys():
 +for parsed_prof in sorted(parsed_profiles):
 +if re.search(hatglob, parsed_prof):
 +for hat in cfg['required_hats'][hatglob].split():
 +if not profile_data[parsed_prof].get(hat, False):
 +profile_data[parsed_prof][hat] = hasher()

 err, I am going to have to get back to you on this one. I need to dive
 in and get more context first

;-)

 ### we should discuss if we want to keep writing in sorted() order (which
 can be helpful, but also annoying)
 ### or if we want to keep the original order of a profile whenever
 possible
 ### (see discussion about writing config files)
 ### - topic for the next meeting?

 I prefer original order when possible, possibly with an option to tell it
 to order and clean up the profile.

Yes, that sounds like a good method, even if it means a bit more work.
(Hint: we already do something similar when writing config files ;-)

For the clean up option - don't read the old profile while writing the new
one ;-)

 Basically it comes down to ordering
 destroys semantic/logical groupings and commenting.

Yes.


Regards,

Christian Boltz
-- 
Chance is irrelevant.  We will succeed.-- Seven of Nine



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


[apparmor] GSoC r31 review

2013-08-04 Thread Christian Boltz
Hello,

the GSoC review for r31 is attached.


Regards,

Christian Boltz
-- 
 My 2 cents,
tja, Stundenlohn wird nach Eignung, Leistung und Befähigung gezahlt
[ Claus Reheis und Detlef Reichelt in opensuse-de]


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


[apparmor] GSoC review r34

2013-08-09 Thread Christian Boltz
Hello,

one more (quite harmless) review ;-)


Regards,

Christian Boltz
-- 
[Windows krepiert nach Update]  Habt Ihr eine Idee, was ich tun könnte?
Vermutlich ein Computervirus. Besorg etwas Aciclovir aus der Apotheke,
oeffne das Rechnergehaeuse und troepfle das Mittel auf alle roten oder
geschwollenen Bauteile. [Mirko Liss]

revno: 34
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Tue 2013-08-06 01:53:28 +0530
message:
  Some code for logprof

=== added directory 'Tools'
=== added file 'Tools/aa-logprof.py'
--- Tools/aa-logprof.py 1970-01-01 00:00:00 +
+++ Tools/aa-logprof.py 2013-08-05 20:23:28 +
@@ -0,0 +1,13 @@

+os.environb.putenv('PATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin')

# what's the reason for overriding $PATH ?


=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-08-05 13:25:34 +
+++ apparmor/aa.py  2013-08-05 20:23:28 +
@@ -3361,17 +3365,25 @@
 
+def store_list_var(var, list_var, value, var_operation):
...
+if var_operation == '=': 
+if not var.get(list_var, False):
+var[list_var] = set(vlist)
+else:
+raise AppArmorException('An existing variable redefined')

# better error message please - it's easy to include the variable name

+else:
+if var.get(list_var, False):
+var[list_var] = set(var[list_var] + vlist)
+else:
+raise AppArmorException('An existing variable redefined')

# better error message please - it's easy to include the variable name

 
@@ -3829,8 +3841,8 @@
 
 def get_include_data(filename):
 data = []
-if os.path.exists(profile_dir + '/' + filename):
-with open_file_read(profile_dir + '/' + filename) as f_in:
+if os.path.exists(filename):
+with open_file_read(filename) as f_in:

# this means get_include_data() needs the full path as parameter

@@ -3844,6 +3856,7 @@
 incfile = load_includeslist.pop(0)
 data = get_include_data(incfile)
 incdata = parse_profile_data(data, incfile, True)
+print(incdata)

# forgotten debug code?



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


[apparmor] GSoC review r35..39

2013-08-09 Thread Christian Boltz
Hallo,

the reviews for r35..r39 are attached.

I have no complaints about the revisions with even numbers ;-)


Regards,

Christian Boltz
-- 
Aus der Beschreibung entnehme ich, daß deine Fonts nach Typ 3
konvertiert werden (Finger im Hals) und deine Bilder auf Screen-
Qualität (Fuß zum Finger dazusteck...) [Ratti in suse-linux]

revno: 35
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Wed 2013-08-07 14:43:17 +0530
message:
  certain fixes

=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-08-05 20:23:28 +
+++ apparmor/aa.py  2013-08-07 09:13:17 +
@@ -1337,7 +1337,7 @@
 if not 
os.path.exists(get_profile_filename(exec_target)):
 ynans = 'y'
 if exec_mode  str_to_mode('i'):
-ynans = UI_YesNo(_('A profile for ') + 
str(exec_target) + gettext(' doesnot exist.\nDo you want to create one?'), 'n')
+ynans = UI_YesNo(_('A profile for ') + 
str(exec_target) + _(' doesnot exist.\nDo you want to create one?'), 'n')

# this text should use %s instead of two split texts

@@ -1596,6 +1596,7 @@
 #last = None
 #event_type = None
 try:
+print(filename)

# forgotten debug code?

 log_open = open_file_read(filename)
 except IOError:
 raise AppArmorException('Can not read AppArmor logfile: ' + filename)
 
@ -3371,7 +3376,9 @@
 if not var.get(list_var, False):
 var[list_var] = set(vlist)
 else:
-raise AppArmorException('An existing variable redefined')
+print('Ignoring: New definition for variable for:',list_var,'=', 
value, 'operation was:',var_operation,'old value=', var[list_var])
+pass
+#raise AppArmorException('An existing variable redefined')

# variable redefinition should be an error, not a warning
# (apparmor_parser also errors out IIRC)


vim:ft=diff

revno: 37
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Fri 2013-08-09 11:04:32 +0530
message:
  fixed debugglogger


=== modified file 'apparmor/common.py'
--- apparmor/common.py  2013-08-05 13:25:34 +
+++ apparmor/common.py  2013-08-09 05:34:32 +
@@ -188,3 +189,22 @@
 new_reg =  new_reg + '$'
 return new_reg
 
+class DebugLogger:
+def __init__(self, module_name=__name__):
+logging.basicConfig(filename='/var/log/apparmor/logprof.log')
+self.logger = logging.getLogger(module_name)
+self.debugging = True
+if os.getenv('LOGPROF_DEBUG', False):
+self.debugging = True

# maybe we could make LOGPROF_DEBUG an integer (instead of on/off) to define 
the log level?
# (0 = none, 1=error, 2=error+info, 3=error+info+debug

+def error(self, msg):
+if self.debugging:
+self.logger.error(msg)
+def info(self, msg):
+if self.debugging:
+self.logger.info(msg)
+def debug(self, msg):
+if self.debugging:
+self.logger.debug(msg)
+def shutdown(self):
+logging.shutdown()
+#logging.shutdown([self.logger])



vim:ft=diff

revno: 39
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Sat 2013-08-10 01:17:00 +0530
message:
  working logger


=== modified file 'apparmor/common.py'
--- apparmor/common.py  2013-08-09 11:19:01 +
+++ apparmor/common.py  2013-08-09 19:47:00 +
@@ -191,16 +191,17 @@
 
 class DebugLogger:
 def __init__(self, module_name=__name__):
-#logging.basicConfig(filename='/var/log/apparmor/logprof.log')
+logging.basicConfig(filename='/var/log/apparmor/logprof.log', 
level=logging.DEBUG, format='%(asctime)s - %(name)s - %(message)s\n')   
 self.logger = logging.getLogger(module_name)
-self.debugging = True
+self.debugging = False
 if os.getenv('LOGPROF_DEBUG', False):
 self.debugging = True
 def error(self, msg):
 if self.debugging:
+logging.error(msg)
 self.logger.error(msg)
 def info(self, msg):
+logging.info(msg)
 if self.debugging:
 self.logger.info(msg)
 def debug(self, msg):

# hmm, for info, logging.info() is always called, even if debugging is off - 
intentional?


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


Re: [apparmor] GSoC review r34

2013-08-10 Thread Christian Boltz
Hello,

Am Samstag, 10. August 2013 schrieb Christian Boltz:
 one more (quite harmless) review ;-)

I noticed two additional small issues, see the [v2] in the updated 
review.


Regards,

Christian Boltz
-- 
dU hAsT nAtUeRlIcH rEcHt. MaN mUsS sIcH bEiM lEsEn NuR dArAn GeWoEhNeN.
mAcHt DaNn KeInEn UnTeRsChIeD mEhR.   [Andreas Kneib in suse-linux]

revno: 34
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Tue 2013-08-06 01:53:28 +0530
message:
  Some code for logprof

=== added directory 'Tools'
=== added file 'Tools/aa-logprof.py'
--- Tools/aa-logprof.py 1970-01-01 00:00:00 +
+++ Tools/aa-logprof.py 2013-08-05 20:23:28 +
@@ -0,0 +1,13 @@

+os.environb.putenv('PATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin')

# what's the reason for overriding $PATH ?


=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-08-05 13:25:34 +
+++ apparmor/aa.py  2013-08-05 20:23:28 +
@@ -3361,17 +3365,25 @@
 
+def store_list_var(var, list_var, value, var_operation):
...
+if var_operation == '=': 
+if not var.get(list_var, False):
+var[list_var] = set(vlist)
+else:
+raise AppArmorException('An existing variable redefined')

# better error message please - it's easy to include the variable name

+else:

# [v2] a comment saying else means += would be nice
# [v2] or, better, explicitely check for += and add an else: that errors out 
saying invalid var_operation

+if var.get(list_var, False):
+var[list_var] = set(var[list_var] + vlist)
+else:
+raise AppArmorException('An existing variable redefined')

# [v2] wrong error message, should be attemped to append to non-existing 
variable
 
# better error message please - it's easy to include the variable name

@@ -3829,8 +3841,8 @@
 
 def get_include_data(filename):
 data = []
-if os.path.exists(profile_dir + '/' + filename):
-with open_file_read(profile_dir + '/' + filename) as f_in:
+if os.path.exists(filename):
+with open_file_read(filename) as f_in:

# this means get_include_data() needs the full path as parameter



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


[apparmor] GSoC review r40

2013-08-10 Thread Christian Boltz
Hello,

the review for r40 is attached. I also included the r34 [v2] comments, 
so you can skip the mail with the updated r34 review ;-)

@John: it also includes a question for you (the same I asked on IRC, but 
you didn't respond yet ;-)


Regards,

Christian Boltz
-- 
Sagt mal ehrlich: Ist mein Rechner geisteskrank
[Harald Katzer in suse-linux]

revno: 40
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Sat 2013-08-10 12:46:22 +0530
message:
  fixes from rev 32..39 and fixed(?) flags=(..)thing


=== modified file 'Testing/common_test.py'
--- Testing/common_test.py  2013-08-05 13:25:34 +
+++ Testing/common_test.py  2013-08-10 07:16:22 +
@@ -4,72 +4,23 @@
+tests=apparmor.config.Config('ini')
+tests.CONF_DIR = '.'
+regex_tests = tests.read_config('regex_tests.ini')
+for regex in regex_tests.sections():
+parsed_regex = re.compile(apparmor.common.convert_regexp(regex))
+for regex_testcase in regex_tests.options(regex):
+self.assertEqual(bool(parsed_regex.search(regex_testcase)), 
eval(regex_tests[regex][regex_testcase]), 'Incorrectly Parsed regex')

# the error message should print out which regex failed

# besides that, this + regex_tests.ini are much better readable :-) 


=== modified file 'Tools/aa-logprof.py'
--- Tools/aa-logprof.py 2013-08-07 09:13:17 +
+++ Tools/aa-logprof.py 2013-08-10 07:16:22 +
@@ -7,9 +7,9 @@
 import argparse
 
 if sys.version_info  (3,0):
-os.environ['PATH'] = '/bin/:/sbin/:/usr/bin/:/usr/sbin'
+os.environ['AAPATH'] = '/bin/:/sbin/:/usr/bin/:/usr/sbin'
 else:
-os.environb.putenv('PATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin')
+os.environb.putenv('AAPATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin')
 
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-08-09 11:19:01 +
+++ apparmor/aa.py  2013-08-10 07:16:22 +
@@ -215,7 +215,7 @@
 
 def which(file):
 Returns the executable fullpath for the file, None otherwise
-env_dirs = os.getenv('PATH').split(':')
+env_dirs = os.getenv('AAPATH').split(':')

# renaming the variable doesn't change much ;-)
# the question is if we want to override PATH or if we should use the existing 
dirs in PATH
# (which users might called expected behaviour, but could also include a small 
security risk)

# John?

 for env_dir in env_dirs:
 env_path = env_dir + '/' + file
 # Test if the path is executable or not

@@ -1751,6 +1748,7 @@
 
 def ask_the_questions():
 found = None
+print(log_dict)

# debug code?

 for aamode in sorted(log_dict.keys()):
 # Describe the type of changes
 if aamode == 'PERMITTING':
@@ -2955,10 +2953,9 @@
 repo_data = None
 parsed_profiles = []
 initial_comment = ''
-RE_PROFILE_START = 
re.compile('^((??\/.+???)|(profile\s+(??.+???)))\s+((flags=)?\((.+)\)\s+)*\{\s*(#.*)?$')
+RE_PROFILE_START = 
re.compile('^((??\/.+???)|(profile\s+(??.+???)))\s+((flags=)?\((.+)\)\s+)?\{\s*(#.*)?$')
  ^
# regex_bin_flag does not have this \ - which of them is correct? ;-)
# (it would be even better to share the regex)

@@ -3373,12 +3361,12 @@
 else:
 print('Ignored: New definition for variable for:',list_var,'=', 
value, 'operation was:',var_operation,'old value=', var[list_var])
 pass
-#raise AppArmorException('An existing variable redefined')
+#raise AppArmorException('An existing variable redefined: %s' 
%list_var)

# (r35 comment)
# variable redefinition should be an error, not a warning
# (apparmor_parser also errors out IIRC)


 else:

# (r34 [v2] comment)
# a comment saying else means += would be nice
# or, better, explicitely check for += and add an else: that errors out 
saying invalid var_operation


 if var.get(list_var, False):
 var[list_var] = set(var[list_var] + vlist)
 else:
-raise AppArmorException('An existing variable redefined')
+raise AppArmorException('An existing variable redefined: %s' 
%list_var)
 
# (r34 [v2] comment)
# wrong error message, should be something like attemped to append to 
non-existing variable


=== modified file 'apparmor/common.py'
--- apparmor/common.py  2013-08-09 19:47:00 +
+++ apparmor/common.py  2013-08-10 07:16:22 +
@@ -165,13 +165,12 @@
 #new_reg = new_reg.replace('}', '}')
 #new_reg = new_reg.replace(',', '|')
 
-while re.search('{.*,.*}', new_reg):
-match = re.search('(.*){(.*),(.*)}(.*)', new_reg).groups()
+while re.search('{[^}]*,[^}]*}', new_reg):
+match = re.search('(.*){([^}]*)}(.*)', new_reg).groups()

# I'd use the same regex for while and match (hint: use the regex from match) 
and store it in a variable

# I'd also use ^...$ to be on the safe side

@@ -190,18 +189,25 @@
 
 class

Re: [apparmor] GSoC review r35..39

2013-08-10 Thread Christian Boltz
Hello,

Am Samstag, 10. August 2013 schrieb Kshitij Gupta:
 Sorry, I was lost in deep thought about modes (read as: taking a nap
 ;) ) when you pinged me on IRC.

No problem, and not surprising given the time ;-)

 I shamelessly made a few tiny pushes since you told me you extract out
 diff using a script and dont mind email spam ;)

I even prefer tiny pushes, thanks for doing it this way!

 convert_regex thing regarding [^}] was useful i stumbled on a testcase
 which needed it to be used. :)

;-)


Regards,

Christian Boltz
-- 
 Was ist das, Nacht?
Das ist der Zeitraum, in dem Du effektiv administrieren kannst. Weil
anscheinend die User alle total faul sind, und sich ausgeloggt haben.
[Wilfried Kramer]


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


Re: [apparmor] [PATCH] apparmor: add the ability to report a crypto hash of loaded policy

2013-08-10 Thread Christian Boltz
Hello,

Am Donnerstag, 8. August 2013 schrieb John Johansen:
 Provide userspace the ability to validate what policy is loaded via
 an exported crypto hash value.

Just a small issue:

 diff --git a/security/apparmor/policy_unpack.c
 b/security/apparmor/policy_unpack.c index 80050b3..bb7acc7 100644
 --- a/security/apparmor/policy_unpack.c
 +++ b/security/apparmor/policy_unpack.c

 @@ -66,6 +67,7 @@ struct aa_ext {
   u32 version;
  };
 
 +
  /* audit callback for unpack fields */
  static void audit_cb(struct audit_buffer *ab, void *va)
  {

Was adding the second empty line intentional?


Regards,

Christian Boltz
-- 
 Ich hab da nochma ne Frage! :o) Was is eigentlich en DAU? Ich mein ihr
 sagt mir zwar die ganze Zeit das ich das bin, aber was das is wes ich
 ni! *heul* Ich rate ganz einfach ma!;o) Die Allercoolste Userin? Isses
 das? Ohhh danke danke!
Du solltest doch nicht so viele von diesen bunten Pillen auf einmal
schlucken... [Mickimausi und Axel Woelke in dag°]


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


Re: [apparmor] [RFC] handling XDG user directories

2013-08-10 Thread Christian Boltz
Hello,

your proposal looks very good. 

Nevertheless, let me add some notes (as usual ;-)

Am Dienstag, 6. August 2013 schrieb John Johansen:
 On 08/05/2013 03:59 PM, Jamie Strandboge wrote:
  and users/admins can adjust /etc/apparmor.d/tunables/xdg-dirs or
  drop files into /etc/apparmor.d/tunables/xdg-dirs.d, providing a
  welcome convenience[2].

 I know that people like the drop in dir bits, but quite bluntly I
 don't, for most things, its a way of papering over real problems (of
 course I consider treating profiles the way we do with packaging as a
 problems so ...)

Tell us a better way ;-)

  This of course doesn't solve everything. Because users can modify
  theirrs ~/.config/user-dirs.dirs file at will and have it point
  anywhere, so we can't examine those files and do anything automatic
  there (when we have user policy we can revisit this). This proposal
  handles translations well though and use of
 This depends on what you are trying to enforce. If its a system level
 policy that is anything but advisory we can't ever let the user
 control the location.

Exactly - otherwise things will become funny when a user sets 
XDG_DOWNLOAD_DIR=/ (wow, my browser can write everywhere! :-)

If we allow the user config to be read, then it needs some restrictions:
- only inside @{HOME} (and no ../ allowed)  (even if I can guarantee 
  you that users will complain about this ;-)
- there must be a config option to disable reading ~/.config/, maybe
  we should even disable it by default

 If its a system level policy enforcing an advisory location for the
 user circumsribed by some larger control eg.
   @{HOME/**  intersected with user defined @{HOME}/@{XDG_DESKTOP_DIR}

yes, something like that - and also at least 
#include abstractions/private-files  # or ...-strict

 If you mean that a user can set their translations sure, but again
 that doesn't currently reflect what system policy does. The sys admin
 can choose the set of local translations that are acceptable to
 system policy

good point, see below

   * apparmor-xdg-dirs.py: this takes the output of 'locale -a' and

I'm afraid this will result in a bit too much ;-)

On my system, locale -a gives me 270 locales from aa_DJ to zu_ZA 
(and I even dropped suffixes like @euro or .utf-8 - with them, I get 460 
locales) [1]

In other words: this should be configurable:
a) autogenerate for all installed languages (which would be a lot on my 
   system)
b) autogenerate for all languages in $config_option
c) similar to b), but somehow automated (on openSUSE, you can choose to 
   install for example all german translations in YaST - this should 
   also add the german XDG dirs to apparmor)
d) do not autogenerate anything

Option a) might even result in too many permissions - I'm quite sure in 
one of the 270 locales I have, for example ~/downloads translates to a 
directory name I have, and that should not be accessible ;-)

The perfect solution would be to only allow the directory names in each 
user's language (so the profile would have /home/cb/Dokumente/ and 
/home/english/documents/ for example) - but I know that's not really 
easy to implement ;-)


Regards,

Christian Boltz

[1] I have no idea why this happens, the only thing I can imagine is 
that openSUSE has some *-lang packages that contains all non-english 
texts, and locale -a picks up all of the languages from them.

-- 
 [...] is currently down due to a failure in the NAS system.
 [...]
 your NAS (network attached storage)
Oh. I thought it stood for Networked Adrian Schröter :D
[ Adrian Schröter and Jean Delvare in opensuse-buildservice]


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


[apparmor] GSoC review r41..45

2013-08-11 Thread Christian Boltz
Hello,

the review for r41..45 is attached (merged into one review).

BTW: Following the moved code was quite interesting[tm], but still 
easier than completely reviewing the new aamode.py and logparser.py ;-)


Regards,

Christian Boltz
-- 
Seit einiger Zeit ist ftp://mirrors.mathematik.uni-bi*l*f*ld.de down
Offenbar. [...] Aber warum machst du dir Sorgen? Bi*l*f*ld existiert
nicht, wieso sollte es dort dann ein Ftp-Server geben?
[ Al Bogner und David Haller in suse-linux]

revno: 45
revno: 44
revno: 43
revno: 42
revno: 41




=== modified file 'Testing/aa_test.py'
--- Testing/aa_test.py  2013-08-09 11:19:01 +
+++ Testing/aa_test.py  2013-08-11 13:00:01 +
 
 def test_modes_to_string(self):
-MODE_TEST = {'x': apparmor.aa.AA_MAY_EXEC,
...
+MODE_TEST = {'x': apparmor.aamode.AA_MAY_EXEC, 
...

 def test_string_to_modes(self):
-MODE_TEST = {'x': apparmor.aa.AA_MAY_EXEC,
...
+MODE_TEST = {'x': apparmor.aamode.AA_MAY_EXEC, 

# I still think test_modes_to_string() and test_string_to_modes() should share 
MODE_TEST ;-)



=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-08-10 07:16:22 +
+++ apparmor/aa.py  2013-08-11 17:46:05 +
@@ -244,7 +184,7 @@
 return os.path.realpath(path)
 
 def find_executable(bin_path):
-Returns the full executable path for the binary given, None otherwise
+Returns the full executable path for the given, None otherwise

# was this comment change intentional?
# (hmm, maybe Returns the full path for the given executable given, None 
otherwise?)


@@ -3296,7 +2747,7 @@
 profile_data[profile][hat]['initial_comment'] = initial_comment
 initial_comment = ''
 if filelist[file]['profiles'][profile].get(hat, False):
-raise AppArmorException('Error: Multiple definitions for hat 
%s in profile %s.' %(hat, profile))
+pass#raise AppArmorException('Error: Multiple definitions for 
hat %s in profile %s.' %(hat, profile))
 filelist[file]['profiles'][profile][hat] = True

# huh? what's wrong with raising an exception here?

@@ -3831,6 +3284,7 @@
 
 def get_include_data(filename):
 data = []
+filename = profile_dir + '/' + filename
 if os.path.exists(filename):

# includes do not have to be inside the profile_dir
# see the IRC log at the end of this file for details

# short version: there's also
# #include foo- relative to base dir (typically /etc/apparmor.d)
# #include /path/to/foo   - absolute path
# (both with optional quotes around the filename)


=== added file 'apparmor/aamode.py'
--- apparmor/aamode.py  1970-01-01 00:00:00 +
+++ apparmor/aamode.py  2013-08-11 17:46:05 +
@@ -0,0 +1,268 @@

+AA_MAY_EXEC = set('x')
...
+AA_EXEC_UNSAFE = set('z')

# 'z' is currently unused, but nevertheless there's a small risk here (it will 
break if we ever introduce 'z' permissions, whatever that will be)

+AA_EXEC_INHERIT = set('i')
+AA_EXEC_UNCONFINED = set('U')
+AA_EXEC_PROFILE = set('P')
+AA_EXEC_CHILD = set('C')

# I'd directly use ix/Ux/Px/Cx, so you don't need to always add AA_MAY_EXEC

+AA_EXEC_NT = set('N')

# similar problem as with 'z'

+AA_LINK_SUBSET = set('ls')

# similar problem (for the 's' part) as with 'z'


=== modified file 'apparmor/common.py'
--- apparmor/common.py  2013-08-10 07:16:22 +
+++ apparmor/common.py  2013-08-11 17:46:35 +
@@ -194,13 +195,21 @@
 self.debug_level = logging.DEBUG   
 if os.getenv('LOGPROF_DEBUG', False):
 self.debugging = os.getenv('LOGPROF_DEBUG')
+try:
+self.debugging = int(self.debugging)
+except:
+self.debugging = False
+if self.debugging not in range(1,4):
+sys.stderr.out('Environment Variable: LOGPROF_DEBUG contains 
invalid value: %s' %os.getenv('LOGPROF_DEBUG'))

# I'd also allow 0 (for people who fail to unset the variable and set it to 0 
instead) ;-)


=== added file 'apparmor/logparser.py'
--- apparmor/logparser.py   1970-01-01 00:00:00 +
+++ apparmor/logparser.py   2013-08-11 09:52:07 +
@@ -0,0 +1,379 @@

+def parse_event(self, msg):
...
+# Map c (create) to a and d (delete) to w, logprof doesn't support c 
and d
+if rmask:
+rmask = rmask.replace('c', 'a')
+rmask = rmask.replace('d', 'w')
+if not validate_log_mode(hide_log_mode(rmask)):
+pass#fatal_error(_('Log contains unknown mode %s') % rmask)

# I'd at least make that a debug notice or warning

+if dmask:
+dmask = dmask.replace('c', 'a')
+dmask = dmask.replace('d', 'w')
+if not validate_log_mode(hide_log_mode(dmask)):
+pass #fatal_error(_('Log contains unknown mode %s') % dmask)

# I'd at least make

[apparmor] GSoC r46..47 review

2013-08-12 Thread Christian Boltz
Hello,

see /dev/null for the r46 and r47 review.

(In other words: looks good, I don't have anything to complain ;-)


Regards,

Christian Boltz
-- 
 cat /inhalt/der/mail | mail -s mein subject [...]
Ist der Useless Use of Cat Award diese Woche schon vergeben? ;-)
[ Andreas Feile und Martin Schmitz in suse-linux]


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


[apparmor] GSoC - remaining parts of old reviews

2013-08-13 Thread Christian Boltz
Hello,

the attached files contain the remaining parts of old reviews.

I always remove things that are done, and also checked the remaining 
parts in the r40 code (that's why most files have checked-in-r40) - 
but even with this suffix, they should be up to date with r47.

I hope the number of files doesn't shock you too much - most of the 
files are quite small ;-)

If you have questions or think some things need to be discussed, just 
ask ;-)


Regards,

Christian Boltz
-- 
und *echte* Männer benutzen Linux -- wegen der langen Kommandozeilen
(Meine ist länger als deine!). Dann muss man nicht mehr Krieg spielen,
um zu zeigen, wie hart man ist. [S. Lauterkorn in suse-talk]
review of r13, r14 and r15


=== added file 'apparmor/aa.py'
--- apparmor/aa.py  1970-01-01 00:00:00 +
+++ apparmor/aa.py  2013-07-06 13:27:06 +
+CONFDIR = '/etc/apparmor'

### It's just a path, but you should keep it at _one_ place (config.py).
### (besides that, CONFDIR is unused in aa.py)

+unimplemented_warning = False

### set this to true in debugging mode?
### (right now, unimplemented_warning is unused)

+aa = dict()  # Profiles originally in sd, replace by aa
+original_aa =  dict()

### aa and original_aa aren't very self-explaining variable names - can you 
give them a better name?
# They basically contain the list of profiles present version and
# the one at the time of loading

# ### Nice, but the variable name should represent that. What about 
profile_list and original_profile_list?



+def get_profile_filename(profile):
+Returns the full profile name
+filename = profile
+if filename.startswith('/'):
+# Remove leading /
+filename = filename[1:]
+else:
+filename = profile_ + filename
+filename.replace('/', '.')

### should we replace more chars?
### just imagine someone has ~/bin/bad $$ script ? for * stars
### (ok, extreme example - but you want a profile for this for sure ;-)
### 
### I'm not sure about backward compability if we change this (John?)
### but people could also have a profile for /bin/foo in /etc/apparmor.d/mybar
### so we can't really expect the /bin/foo profile in bin.foo

#(r45) get_profile_filename is now in logparser.py



+def complain(path):
+Sets the profile to complain mode if it exists
+filename, name = name_to_prof_filename(path)

### see above - you can't rely on having the proifle for /bin/foo in bin.foo
### (IIRC there's even an open bugreport about this)

+def enforce(path):
+Sets the profile to complain mode if it exists
+filename, name = name_to_prof_filename(path)

### same here

+def head(file):
+Returns the first/head line of the file
+first = ''
+if os.path.isfile(file):
+with open_file_read(file) as f_in:
+first = f_in.readline().rstrip()
+return first

### should head() raise an error if file doesn't exist?
### (or does open_file_read() do this already?)

+def get_output(params):
+Returns the return code output by running the program with the args 
given in the list
+program = params[0]
+args = params[1:]
+ret = -1
+output = []
+# program is executable
+if os.access(program, os.X_OK):
+try:
+# Get the output of the program
+output = subprocess.check_output(params)
+except OSError as e:
+raise  AppArmorException(Unable to fork: %s\n\t%s %(program, 
str(e)))
+# If exit-codes besides 0
+except subprocess.CalledProcessError as e:
+output = e.output
+output = output.decode('utf-8').split('\n')

### what happens if someone is not using utf-8?

+ret = e.returncode
+else:
+ret = 0
+output = output.decode('utf-8').split('\n')
+# Remove the extra empty string caused due to \n if present
+if len(output)  1:
+output.pop() 
+return (ret, output)   

+def get_reqs(file):
+Returns a list of paths from ldd output
+pattern1 = re.compile('^\s*\S+ = (\/\S+)')
+pattern2 = re.compile('^\s*(\/\S+)')
+reqs = []
+ret, ldd_out = get_output(ldd, file)

### ldd is None - you should fill it before using it ;-)
### also, I'm not sure if ldd needs to be a global variable

# The setter method is yet to come. ;-)
# Will fix it when refractoring the module later as I planned. :-)




=== added file 'apparmor/common.py'
--- apparmor/common.py  1970-01-01 00:00:00 +
+++ apparmor/common.py  2013-07-03 23:34:04 +


+def cmd_pipe(command1, command2):
+'''Try to pipe command1 into command2.'''
+try:
+sp1 = subprocess.Popen(command1, stdout=subprocess.PIPE)
+sp2 = subprocess.Popen(command2, stdin=sp1.stdout)
+except OSError as ex:
+return [127, str(ex)]
+
+if sys.version_info[0] = 3:
+out = sp2.communicate()[0].decode('ascii', 'ignore')

### is ascii correct here, or should it be the system locale

[apparmor] GSoC review r48..51

2013-08-22 Thread Christian Boltz
Hello,

the review for r48, 49, 50 and 51 is attached.

Of course feedback to all the code is always welcome (I don't have a 
monopoly on reviewing GSoC code ;-) but there's a detail I'd like to 
discuss:

aa-genprof.py has:

if os.path.exists('/var/log/audit/audit.log'):
syslog = False

I'm not sure if audit.log exists is the best way to choose the logfile 
but I have to admit that I don't have a better method ;-)

Does someone have any better ideas? Or is the current way ok?


Regards,

Christian Boltz
-- 
But you are probably also complaining if local root exploits in the
kernel are fixed, because now you no longer can use that to become root
easily... [Stefan Seyfried in opensuse-factory]


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


[apparmor] GSoC review r52 and r53

2013-08-26 Thread Christian Boltz
Hello,

the review for r52 and r53 is attached.

For the flags handling, I have an interesting question: 
Should we prefer to add the flags (for example complain) to the 
profile as we do now, or should we prefer to create a symlink in 
/etc/apparmor.d/force-complain/ ?

The flags=(...) in the profile has the advantage that people are used to 
it, OTOH creating a symlink means we don't need to modify the profile.

Opinions?

(We'll have to contunue supporting both ways, the question is what
aa-complain, aa-audit etc. should do.)


Regards,

Christian Boltz
-- 
Der fünfte apokalyptische Reiter der Digitalen Inkompatibilität wird
dich mit den hornigen Hufen des Workarounds niedertrampeln, und niemand
wird in der Nacht deine Fehlermeldungen lesen. 
Wir aber werden an einem warmen Feuer sitzen, in dem deine Sourcen
verbrennen, und uns der Kommandozeile erfreuen. So.
[Ratti in fontlinge-devel]

revno: 52
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Mon 2013-08-26 00:23:59 +0530
message:
  Merged aa-audit, aa-autodep, aa-complain, aa-disable, aa-enforce to share the
  common code into a tools.py module. Added -r/--remove feature to aa-complain,
  aa-enforce, aa-audit and -r/--revert feature to aa-disable. Some other fixes
  from review 48..51

revno: 53
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Mon 2013-08-26 00:41:15 +0530
message:
  aa-cleanprof tool



=== modified file 'Tools/aa-audit.py'
--- Tools/aa-audit.py   2013-08-21 05:56:09 +
+++ Tools/aa-audit.py   2013-08-25 18:53:59 +
+audit.check_profile_dir()

# $tool.check_profile_dir should be part of apparmor/tools.py
# besides that, I like the new mini-tools :-)

=== modified file 'Tools/aa-autodep.py'
--- Tools/aa-autodep.py 2013-08-21 05:56:09 +
+++ Tools/aa-autodep.py 2013-08-25 18:53:59 +
-parser = argparse.ArgumentParser(description='Disable the profile for the 
given programs')
+parser = argparse.ArgumentParser(description='')

# a description would be nice ;-)

+autodep.check_profile_dir()

# $tool.check_profile_dir should be part of apparmor/tools.py

=== added file 'Tools/aa-cleanprof.py'
--- Tools/aa-cleanprof.py   1970-01-01 00:00:00 +
+++ Tools/aa-cleanprof.py   2013-08-25 18:53:59 +

# aa-cleanprof should also use apparmor/tools.py
# (the code looks quite similar, it even has the wrong error message if the 
profiledir doesn't exist ;-)

+if os.path.exists(program):

# This check means aa-cleanprof works only if you have the program on your 
computer -
# you can't cleanup unused profiles.
# That's not the best idea ;-)

# you should check for profile for program exists instead.


=== modified file 'Tools/aa-complain.py'
--- Tools/aa-complain.py2013-08-21 05:56:09 +
+++ Tools/aa-complain.py2013-08-25 18:53:59 +
+complain.check_profile_dir()

# $tool.check_profile_dir should be part of apparmor/tools.py

=== modified file 'Tools/aa-disable.py'
--- Tools/aa-disable.py 2013-08-21 05:56:09 +
+++ Tools/aa-disable.py 2013-08-25 18:53:59 +

+disable.check_profile_dir()
+disable.check_disable_dir()

# $tool.check_profile_dir should be part of apparmor/tools.py
# (and also check_disable_dir() (wrapped with an if condition) to have all code 
at the same place)

=== modified file 'Tools/aa-enforce.py'
--- Tools/aa-enforce.py 2013-08-21 05:56:09 +
+++ Tools/aa-enforce.py 2013-08-25 18:53:59 +
+parser.add_argument('-r', '--remove', action='store_true', help='remove 
enforce mode')

# maybe switch to complain mode would be a better help text

+enforce.check_profile_dir()

# $tool.check_profile_dir should be part of apparmor/tools.py
# (did I already mention that I love copypaste reviews? ;-)

+args = parser.parse_args()
+
+enforce = aa_tools('enforce', args)

# enforce is not a real flag - basically it translates to not complain.
# live would be easier in apparmor/tools.py if you do
#
# args.remove = not args.remove  # invert the remove flag
# enforce = aa_tools('complain', args)


=== modified file 'Tools/aa-unconfined.py'
--- Tools/aa-unconfined.py  2013-08-21 05:56:09 +
+++ Tools/aa-unconfined.py  2013-08-25 18:53:59 +
@@ -48,22 +48,22 @@
 if not attr:
-if re.search('^(/usr)?/bin/(python|perl|bash)', prog):
-cmdline = re.sub('\0', ' ', cmdline)
+if re.search('^(/usr)?/bin/(python|perl|bash|sh)', prog):

# this still doesn't cover the etc. part of sh etc. ;-)
# (hint: apparmor/aa.py contains some more shells, and it would be a good idea 
to have the list of shells at _one_ place)
# besides that, the regex will also cover /bin/pythonhunter and /usr/bin/shit 
;-)

+cmdline = re.sub('\x00', ' ', cmdline)
 cmdline = re.sub('\s+$', '', cmdline).strip()
+if 'perl' in cmdline

Re: [apparmor] [patch] make __init__.py GSoC-ready

2013-09-12 Thread Christian Boltz
Hello,

Am Donnerstag, 12. September 2013 schrieb Christian Boltz:
 to make testing Kshitij's new tools easier, I propose to merge his
 code in utils/apparmor/__init__.py - that's the only filename
 conflict (at least in the 2.8 branch). If we do this, we can ship his
 new tools in a testing package that can be installed on top of the
 2.8.x packages without problems [1].
 
 The following patch is for trunk and the 2.8 branch:

As usual ;-) I found a way to break it - using LANG=C caused an 
exception because C is shorter than what filename = ...[0:2] expects.

Here's an updated patch. Changes:
- filename = ... moved into the try block
- catch all exceptions, not only IOError

I know it's not perfect, but worst thing that can happen is to get the
non-translated interface. That's much better than an exception ;-)


=== modified file 'utils/apparmor/__init__.py'
--- utils/apparmor/__init__.py  2012-05-08 05:37:48 +
+++ utils/apparmor/__init__.py  2013-09-12 15:10:50 +
@@ -1,9 +1,25 @@
 # --
 #
 #Copyright (C) 2011-2012 Canonical Ltd.
+#Copyright (C) 2013 Kshitij Gupta
 #
 #This program is free software; you can redistribute it and/or
 #modify it under the terms of version 2 of the GNU General Public
 #License published by the Free Software Foundation.
 #
 # --
+
+import gettext
+import locale
+
+def init_localisation():
+locale.setlocale(locale.LC_ALL, '')
+#cur_locale = locale.getlocale()
+try:
+filename = '/usr/share/locale/%s/LC_MESSAGES/apparmor-utils.mo' % 
locale.getlocale()[0][0:2]
+trans = gettext.GNUTranslations(open( filename, 'rb'))
+except: # IOError:
+trans = gettext.NullTranslations()
+trans.install()
+
+init_localisation()

 Note that the Canonical copyright header in __init.py__ was
 basically for the empty file - I'm not sure if it makes sense to keep
 it ;-) (but I won't remove it unless explicitely asked to do so to
 avoid Canonical will sue me for stealing an (besides the copyright
 header) empty file ;-)
 
 Also note that trunk also needs changes in utils/apparmor/common.py -
 Kshitij added several functions there. But that's another story and
 not too urgent. However we should do it before the 3.0 release.

 [1] We'll need to re-add the .py suffix for the tools in the testing
 package, but that's another story ;-)

BTW: I'll also commit this patch to openSUSE Factory which has feature 
freeze today ;-)


Regards,

Christian Boltz
-- 
 Jan, I can write scripts too :)
rebuild_logic=coolo
[ Stephan Kulow and Marcus Meissner in opensuse-factory]


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


[apparmor] [patch] utils/po/de.po update

2013-09-13 Thread Christian Boltz
Hello,

after seeing a very strange translation, I did a round of proofreading 
on utils/po/de.po.

The most strange translation was:

 msgid Reading log entries from %s.
-msgstr %s Mailserver-Domains werden eingelesen...
 
+msgstr Protokolleinträge von %s werden eingelesen.

but there were more things that were mistranslated (but not as bad as 
the one quoted above ;-) or needed minor adjustments.

I also marked some translations as fuzzy because I'm not too happy with 
them, but need to think about better alternatives. 

The most interesting question is if capability should be translated to 
Funktion - I somehow doubt...

See the attached patch for all changes.

I propose this patch for trunk and the 2.8 tree.


Regards,

Christian Boltz

PS: Note to myself: avoid to use lokalize - it changes unmodified 
texts to multiline format even if I didn't touch them, which results
in an unreadable diff :-/
The attached patch is of course cleaned up ;-)

-- 
Erstes Gesetz WWW: 
 Du mögest trennen die Spinnen und Indianer von den Usern und jedem
 sein eigen Grund und Heim zuteilen auf das der eine nicht neidisch
 werde auf den anderen und begehre dessen Heim und Gut. *lach*
 [Thomas Templin in suse-linux]
=== modified file 'utils/po/de.po'
--- utils/po/de.po	2011-02-09 00:29:59 +
+++ utils/po/de.po	2013-09-13 19:12:39 +
@@ -1,19 +1,23 @@
 # Copyright (C) 2006 SuSE Linux Products GmbH, Nuernberg
+# Copyright (C) 2013 Christian Boltz
 # This file is distributed under the same license as the package.
 #
 msgid 
 msgstr 
 Project-Id-Version: apparmor-utils\n
 Report-Msgid-Bugs-To: apparmor-gene...@forge.novell.com\n
 POT-Creation-Date: 2008-09-22 22:56-0700\n
-PO-Revision-Date: 2009-02-05 13:38\n
-Last-Translator: Novell Language langu...@novell.com\n
+PO-Revision-Date: 2013-09-13 21:05+0200\n
+Last-Translator: Christian Boltz appar...@cboltz.de\n
 Language-Team: Novell Language langu...@novell.com\n
 MIME-Version: 1.0\n
 Content-Type: text/plain; charset=UTF-8\n
 Content-Transfer-Encoding: 8bit\n
+Language: de\n
+Plural-Forms: nplurals=2; plural=(n != 1);\n
 
 #: ../genprof:69
+#, fuzzy
 msgid Please enter the program to profile: 
 msgstr Geben Sie das Programm fÃŒr das Profil ein: 
 
@@ -52,12 +57,12 @@
 #: ../logprof:72
 #, perl-format
 msgid usage: %s [ -d /path/to/profiles ] [ -f /path/to/logfile ] [ -m \mark in log to start processing after\
-msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ -f /pfad/zu/protokolldatei ] [ -m \markierng im protokoll, nach der die verarbeitung gestartet werden soll\
+msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ -f /pfad/zu/protokolldatei ] [ -m \Markierng im Protokoll, nach der die Verarbeitung gestartet werden soll\
 
 #: ../autodep:63
 #, perl-format
 msgid Can't find AppArmor profiles in %s.
-msgstr In %s wurden keine UnterdomÀnenprofile gefunden.
+msgstr In %s wurden keine AppArmor-Profile gefunden.
 
 #: ../autodep:71
 msgid Please enter the program to create a profile for: 
@@ -86,7 +91,7 @@
 #: ../audit:131
 #, perl-format
 msgid usage: %s [ -d /path/to/profiles ] [ program to switch to audit mode ]
-msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ programm, das in den prÃŒfmodus versetzt werden soll ]
+msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ Programm, das in den PrÃŒfmodus versetzt werden soll ]
 
 #: ../complain:64
 msgid Please enter the program to switch to complain mode: 
@@ -100,7 +105,7 @@
 #: ../complain:131
 #, perl-format
 msgid usage: %s [ -d /path/to/profiles ] [ program to switch to complain mode ]
-msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ programm, das in den meldungsmodus versetzt werden soll ]
+msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ Programm, das in den Meldungsmodus versetzt werden soll ]
 
 #: ../enforce:64
 msgid Please enter the program to switch to enforce mode: 
@@ -109,12 +114,12 @@
 #: ../enforce:105 ../AppArmor.pm:592
 #, perl-format
 msgid Setting %s to enforce mode.
-msgstr Einstellungen %s fÃŒr Erwzingungsmodus
+msgstr %s wird in den Erwzingen-Modus versetzt.
 
 #: ../enforce:131
 #, perl-format
 msgid usage: %s [ -d /path/to/profiles ] [ program to switch to enforce mode ]
-msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ programm, das in den erzwingen-modus versetzt werden soll ]
+msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ Programm, das in den Erzwingen-Modus versetzt werden soll ]
 
 #: ../unconfined:50
 #, perl-format
@@ -193,7 +198,7 @@
 
 #: ../AppArmor.pm:1159
 msgid Select which of the changed profiles you would like to upload\nto the repository
-msgstr WÀhlen Sie die geÀnderten Profile aus, die Sie an das Repository \nhochladen möchten
+msgstr WÀhlen Sie die geÀnderten Profile aus, die Sie in das Repository \nhochladen möchten
 
 #: ../AppArmor.pm:1161
 msgid Changed profiles
@@ -210,7 +215,7 @@
 #: ../AppArmor.pm:1236 ../AppArmor.pm:1316
 #, perl-format
 msgid WARNING: An error occured while

[apparmor] GSoC review r58

2013-09-15 Thread Christian Boltz
Hello,

the attached file contains the review for r58 and also some bugs I found 
while testing.


Regards,

Christian Boltz
-- 
Stell dein cron auch deine Rechneruhr? Ja? 
Dann würde ich ihm nicht allzuviel mehr anvertrauen - er scheint leicht 
überlastet und strebt in Riesenschritten die Rente an ;-)
[Matthias Houdek in suse-linux zu einer Mail aus der Zukunft]

revno: 58
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Thu 2013-09-12 14:42:15 +0530
message:
  Updated __init_.py tested with de_DE and hi_IN translations using old
  apparmor-utils.mo file, not pushing remainder of files for their lack of
  beauty



### I've pushed the tested and fixed __init__.py file however an early
### pressed Enter also ended up pushing some other files. Try ignoring
### them for now. They probably won't make much sense.


=== modified file 'Testing/minitools_test.py'
--- Testing/minitools_test.py   2013-08-31 12:18:40 +
+++ Testing/minitools_test.py   2013-09-12 09:12:15 +
@@ -2,74 +2,74 @@

+test_path = '/usr/sbin/ntpd'
+local_profilename = None
+
+python_interpreter = 'python'
+if sys.version_info = (3,0):
+python_interpreter = 'python3'

 def test_audit(self):
 #Set ntpd profile to audit mode and check if it was correctly set
-subprocess.check_output('python ./../Tools/aa-audit -d ./profiles 
ntpd', shell=True)
-local_profilename = 
apparmor.get_profile_filename(apparmor.get_full_path(apparmor.which('ntpd')))
+subprocess.check_output('%s ./../Tools/aa-audit -d ./profiles 
ntpd'%python_interpreter, shell=True)

# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename

+local_profilename = apparmor.get_profile_filename(test_path)

# you should use the result for the aa-audit call ;-)
# Besides that, local_profilename is already set in the global part of the 
script - no need to do it here again

 #Remove audit mode from ntpd profile and check if it was correctly 
removed
+subprocess.check_output('%s ./../Tools/aa-audit -d ./profiles -r 
ntpd'%python_interpreter, shell=True)
 
# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename
 
 def test_complain(self):
 #Set ntpd profile to complain mode and check if it was correctly set
+subprocess.check_output('%s ./../Tools/aa-complain -d ./profiles 
ntpd'%python_interpreter, shell=True)

# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename

 #Set ntpd profile to enforce mode and check if it was correctly set
+subprocess.check_output('%s ./../Tools/aa-complain -d ./profiles -r 
ntpd'%python_interpreter, shell=True)
 
# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename

 # Set audit flag and then complain flag in a profile
+subprocess.check_output('%s ./../Tools/aa-audit -d ./profiles 
ntpd'%python_interpreter, shell=True)
+subprocess.check_output('%s ./../Tools/aa-complain -d ./profiles 
ntpd'%python_interpreter, shell=True)
 
# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename

 #Remove complain flag first i.e. set to enforce mode
+subprocess.check_output('%s ./../Tools/aa-complain -d ./profiles -r 
ntpd'%python_interpreter, shell=True)
 
# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename

 #Remove audit flag
-subprocess.check_output('python ./../Tools/aa-audit -d ./profiles -r 
ntpd', shell=True)
+subprocess.check_output('%s ./../Tools/aa-audit -d ./profiles -r 
ntpd'%python_interpreter, shell=True)
 
# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename

 def test_enforce(self):
 #Set ntpd profile to complain mode and check if it was correctly set
+subprocess.check_output('%s ./../Tools/aa-enforce -d ./profiles -r 
ntpd'%python_interpreter, shell=True)
 
# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename
 
 #Set ntpd profile to enforce mode and check if it was correctly set
+subprocess.check_output('%s ./../Tools/aa-enforce -d ./profiles 
ntpd'%python_interpreter, shell=True)
 
# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename
 
 def test_disable(self):
 #Disable the ntpd profile and check if it was correctly disabled
+subprocess.check_output('%s ./../Tools/aa-disable -d ./profiles 
ntpd'%python_interpreter, shell=True)
 
# this will still fail if ntpd is not installed - use the full path to ntpd or 
the profile filename
 
 #Enable the ntpd profile and check

[apparmor] [patch] ntpd needs read access to openssl.cnf

2013-09-16 Thread Christian Boltz
Hello,

I just received the following patch and propose it for 2.8 and trunk:



Patch-Author: Stefan Seyfried seife+...@b1-systems.com

After this change in ntp:

* Mo Aug 19 2013 crrodrig...@opensuse.org
- Build with -DOPENSSL_LOAD_CONF , ntp must respect and use
  the system's openssl configuration.

we need to read openssl.cnf or starting of ntpd will fail silently(!)



Patch v2 by Christian Boltz: use abstractions/openssl instead of
allowing /etc/ssl/openssl.cnf directly


=== modified file 'profiles/apparmor.d/usr.sbin.ntpd'
--- profiles/apparmor.d/usr.sbin.ntpd   2011-08-08 20:16:06 +
+++ profiles/apparmor.d/usr.sbin.ntpd   2013-09-16 20:28:39 +
@@ -14,6 +14,7 @@
 /usr/sbin/ntpd {
   #include abstractions/base
   #include abstractions/nameservice
+  #include abstractions/openssl
   #include abstractions/xad
 
   capability dac_override,





Regards,

Christian Boltz
-- 
No need to use Windows -- it's easier to go through the door.
[author unknown]


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


Re: [apparmor] [patch] ntpd needs read access to openssl.cnf

2013-09-16 Thread Christian Boltz
Hello,

Am Montag, 16. September 2013 schrieb Steve Beattie:
 On Mon, Sep 16, 2013 at 10:39:13PM +0200, Christian Boltz wrote:
  I just received the following patch and propose it for 2.8 and
  trunk:
  
  Patch-Author: Stefan Seyfried seife+...@b1-systems.com
  
  After this change in ntp:
  
  * Mo Aug 19 2013 crrodrig...@opensuse.org
  - Build with -DOPENSSL_LOAD_CONF , ntp must respect and use
  
the system's openssl configuration.
  
  we need to read openssl.cnf or starting of ntpd will fail
  silently(!)

 Though ntpd failing silently sounds like an ntpd bug to me.

Indeed - I'll ask Seife if he can open a bugreport about it (probably in 
ntp upstream).


Regards,

Christian Boltz
-- 
Ich bin beeindruckt!
Windows startet nicht mehr - Problem gelöst.
Ich wünschte, ich könnte meine Probleme auch so befriedigend lösen.
[Sandy Drobic in suse-linux]


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


[apparmor] Revert r1225 mistranslations (utils/po/*.po)

2013-09-17 Thread Christian Boltz
Hello,

during the last days, we (as in: the usual people in #apparmor) 
discovered that the r1225 translation update introduced _lots_ of 
mistranslations in various languages, even for texts that had correct 
translations before. Also, most of the changes I commited for de.po 
recently fixed things that were introduced in r1225.

Therefore I propose to completely [1] revert r1225 in trunk and also in 
the 2.8 branch.


Note that r1225 is a commit from 2009, so it's likely that we'll get 
some merge conflicts. We can easily solve conflicts by removing the 
translated text so that translators get a second chance.


The alternative is to mark _all_ texts in _all_ languages as fuzzy, so 
that translators see them as please check this. The disadvantages of 
this way are
- we keep the mistranslations until translators have checked the 
  translations - not the best idea because some translations are really 
  bad (and I can only judge on en_* and de - maybe the other languages 
  are even more funny[tm])
- it's probably more work for translators than re-translating a few 
  texts that are a victim of revert/merge conflicts

The only advantage of marking everything as fuzzy is that we'll get 
proofreading for all texts which might also catch mistranslations from 
other commits.


Opinions? Objections?


(if you want to see the patch for this proposal: bzr diff -r1224..1225, 
then swap + and -)


Regards,

Christian Boltz

[1] except de.po because I fixed it already
-- 
Zwar sind CSS-Bugs kein Alleinstellungsmerkmal des Internet Explorers, 
jedoch beansprucht Microsoft seit vielen Jahren die Marktführerschaft.
[Dirk Jesse auf
http://www.highresolution.info/spotlight/entry/was_sie_ueber_css-frameworks_wissen_sollten/]


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


Re: [apparmor] [patch] utils/*.pod: fix broken URL

2013-09-19 Thread Christian Boltz
Hello,

Am Donnerstag, 19. September 2013 schrieb Steve Beattie:
 On Thu, Sep 19, 2013 at 08:52:19PM +0200, Christian Boltz wrote:
  the following patch fixes broken URLs in various utils/*.pod files.
  (The broken URLs were introduced in r1582.)
  
  I propose this patch for trunk and for the 2.8 branch.
 
 Acked-by: Steve Beattie st...@nxnw.org for trunk and 2.8
 
 There are a bunch of other instances to fix as well. I haven't
 verified if all are available in 2.8, but a similar sed -i conversion
 should happen there, too.

Acked-by: Christian Boltz appar...@cboltz.de for trunk and 2.8

I should really use bzr log -v -r$rev when checking regressions to 
find the full breakage ;-)

As discussed on #apparmor, I included your patch in my commit.

BTW: your patch also worked in the 2.8 branch, with the exception that 
apparmor.vim.pod still lives in parser/ there.


Regards,

Christian Boltz
-- 
[tables vs. css layout] please - we should not start another religious
war here, unless the GNOME vs KDE and emacs vs vi wars are fought out
;-)).   [Frank Sundermeyer in opensuse-wiki]


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


[apparmor] GSoC review r66 and r67

2013-09-20 Thread Christian Boltz
Hello,

the review for r67 is attached. It looks big, but mostly contains minor 
text changes ;-)

r66 looks good - no need for a review file.


Regards,

Christian Boltz
-- 
 [submit-request #65647 declined by saschpe:]
   description is 400 lines, too long :-)
Where is a limit documented?
[Stephan Kulow in opensuse-packaging]

revno: 67
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Sat 2013-09-21 01:08:34 +0530
message:
  fixes from rev65

=== modified file 'Translate/README'
--- Translate/README2013-09-20 13:50:41 +
+++ Translate/README2013-09-20 19:38:34 +

# might need another update to use the easier available ;-) xgettext
#
# Syntax:
# xgettext --language=Python --keyword=_ --output=Translate/messages.pot 
apparmor/*.py Tools/aa-*
# (run from the main directory, not inside Translate)

# xgettext will also give you a list of all texts containing multiple %s ;-)


=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-09-20 13:50:41 +
+++ apparmor/aa.py  2013-09-20 19:38:34 +
@@ -340,10 +343,12 @@
 Modifies the profile to add the requirements
 reqs_processed = dict()
 reqs = get_reqs(path)
+print(reqs)

# debug code?

@@ -2572,7 +2575,7 @@
+raise AppArmorException(_('%s profile in %s contains 
syntax errors in line: %s.') % (profile, file, lineno+1))

# line %s (without : )

@@ -2624,7 +2627,7 @@
 elif RE_PROFILE_END.search(line):
 # If profile ends and we're not in one
 if not profile:
-raise AppArmorException('Syntax Error: Unexpected End of 
Profile reached in file: %s line: %s' % (file, lineno+1))
+raise AppArmorException(_('Syntax Error: Unexpected End of 
Profile reached in file: %s line: %s') % (file, lineno+1))

# file %s line %s (without : )

# while at it, you should use %(file)s and %(lineno + 1)s - not sure if 
%(lineno + 1)s works...
# (also in the following copypaste review notes, even if I didn't paste this 
note everywhere ;-)

@@ -2639,7 +2642,7 @@
 matches = RE_PROFILE_CAP.search(line).groups()
 
 if not profile:
-raise AppArmorException('Syntax Error: Unexpected capability 
entry found in file: %s line: %s' % (file, lineno+1))
+raise AppArmorException(_('Syntax Error: Unexpected capability 
entry found in file: %s line: %s') % (file, lineno+1))
 
# file %s line %s (without : )

@@ -2658,7 +2661,7 @@
 matches = RE_PROFILE_LINK.search(line).groups()
 
 if not profile:
-raise AppArmorException('Syntax Error: Unexpected link entry 
found in file: %s line: %s' % (file, lineno+1))
+raise AppArmorException(_('Syntax Error: Unexpected link entry 
found in file: %s line: %s') % (file, lineno+1))
 
# file %s line %s (without : )
@@ -2686,7 +2689,7 @@
 matches = RE_PROFILE_CHANGE_PROFILE.search(line).groups()
 
 if not profile:
-raise AppArmorException('Syntax Error: Unexpected change 
profile entry found in file: %s line: %s' % (file, lineno+1))
+raise AppArmorException(_('Syntax Error: Unexpected change 
profile entry found in file: %s line: %s') % (file, lineno+1))
 
# file %s line %s (without : )
@@ -2708,7 +2711,7 @@
 matches = RE_PROFILE_RLIMIT.search(line).groups()
 
 if not profile:
-raise AppArmorException('Syntax Error: Unexpected rlimit entry 
found in file: %s line: %s' % (file, lineno+1))
+raise AppArmorException(_('Syntax Error: Unexpected rlimit 
entry found in file: %s line: %s') % (file, lineno+1))
 
# file %s line %s (without : )

@@ -2719,7 +2722,7 @@
 matches = RE_PROFILE_BOOLEAN.search(line)
 
 if not profile:
-raise AppArmorException('Syntax Error: Unexpected boolean 
definition found in file: %s line: %s' % (file, lineno+1))
+raise AppArmorException(_('Syntax Error: Unexpected boolean 
definition found in file: %s line: %s') % (file, lineno+1))
 
# file %s line %s (without : )

@@ -2759,7 +2762,7 @@
 matches = RE_PROFILE_PATH_ENTRY.search(line).groups()
 
 if not profile:
-raise AppArmorException('Syntax Error: Unexpected path entry 
found in file: %s line: %s' % (file, lineno+1))
+raise AppArmorException(_('Syntax Error: Unexpected path entry 
found in file: %s line: %s') % (file, lineno+1))
 
# file %s line %s (without : )

@@ -2783,10 +2786,10 @@
 try:
 re.compile(p_re)
 except:
-raise AppArmorException('Syntax Error: Invalid Regex %s in 
file: %s line: %s' % (path, file, lineno+1

[apparmor] GSoC review r68 and r69

2013-09-21 Thread Christian Boltz
Hello,

the review for r68 is attached - it contains some small issues and a 
set of fresh bugs I found while doing some tests.

r69 passed my review without complaints ;-)


@John (and whoever else wants to answer)

I also have an interesting question about behaviour of 
aa-audit /bin/ping

The profile starts with
/{usr/,}bin/ping {
so it's, strictly speaking, two profiles in one.

The interesting question is what the correct behaviour is because 
besides setting the profile for /bin/ping to audit mode (expected)
it would also set the profile for /usr/bin/ping to audit mode (not expected)

OTOH, the current behaviour of aa-audit (error message saying that 
/bin/ping does not exist) also is not expected and will terribly confuse 
users.

What should aa-audit /bin/ping do in this case?

(aa-audit is just an example - the same question applies to all aa-* tools
that change profile flags.)


Regards,

Christian Boltz
-- 
 which camera is this?
Marcus, this is my bug :)
[Marcus Meissner and Stephan Kulow in
https://bugzilla.novell.com/show_bug.cgi?id=217731]

revno: 68
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Sat 2013-09-21 12:36:51 +0530
message:
  Fixed flag  reader and writer to be able to set unset flag for a specific
  target program also fixed tests for mini tools to be independent of existence
  of ntpd



=== modified file 'Tools/aa-complain'
--- Tools/aa-complain   2013-09-19 05:02:19 +
+++ Tools/aa-complain   2013-09-21 07:06:51 +
@@ -11,5 +11,5 @@
 complain = apparmor.tools.aa_tools('complain', args)
-
+print(args)

# debugging code?


=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-09-20 19:38:34 +
+++ apparmor/aa.py  2013-09-21 07:06:51 +
-def get_profile_flags(filename):
+def get_profile_flags(filename, program):
 # To-Do
 # XXX If more than one profile in a file then second one is being ignored 
XXX
 # Do we return flags for both or 

# with the change below, the To-Do seems solved

# you'll also need to fix
# Tools/aa-genprof:apparmor.helpers[program] = 
apparmor.get_profile_flags(profile_filename)
# to use two parameters

@@ -564,13 +564,17 @@
 with open_file_read(filename) as f_in:
 for line in f_in:
 if RE_PROFILE_START.search(line):
-flags = RE_PROFILE_START.search(line).groups()[6]
-return flags
+matches = RE_PROFILE_START.search(line).groups()
+profile = matches[1] or matches[3]
+flags = matches[6]
+if profile == program:
+return flags
 
 raise AppArmorException(_('%s contains no profile')%filename)

# with the above change, the error message should probably be
# '%(filename)s contains no profile for %(program)s'



-def change_profile_flags(filename, flag, set_flag):
-old_flags = get_profile_flags(filename)
+def change_profile_flags(filename, program, flag, set_flag):
+old_flags = get_profile_flags(filename, program)
+print(old_flags)

# debugging code?






# Testing results:

# logprof doesn't expand aliases correctly:
# (sorry for the german parts, but I'm quite sure you'll understand what is 
happening ;-)


Profil:  /{usr/,}bin/ping
Pfad:/home/sys-var/run/nscd/dbVSXZwf
Modus:   r
Schweregrad: 4

 [1 - /home/sys-var/run/nscd/dbVSXZwf]
  2 - /home/*/run/nscd/dbVSXZwf 
(A)llow / [(D)eny] / (I)gnore / (G)lob / Glob with (E)xtension / (N)ew / 
Abo(r)t / (F)inish / (M)ore

Neuen Pfad eingeben: /var/run/nscd/db*

The specified path does not match this log entry:

  Log Entry: /home/sys-var/run/nscd/dbVSXZwf
  Entered Path:  /var/run/nscd/db*
Do you really want to use this path?

(J)a / [(N)ein]

Profil:  /{usr/,}bin/ping
Pfad:/home/sys-var/run/nscd/dbVSXZwf
Modus:   r
Schweregrad: 4

  1 - /home/sys-var/run/nscd/dbVSXZwf 
  2 - /home/*/run/nscd/dbVSXZwf 
 [3 - /var/run/nscd/db*]
(A)llow / [(D)eny] / (I)gnore / (G)lob / Glob with (E)xtension / (N)ew / 
Abo(r)t / (F)inish / (M)ore
Adding /var/run/nscd/db* r to profile

Profil:  /{usr/,}bin/ping
Pfad:/home/sys-var/run/nscd/dbwvkZpy
Modus:   r
Schweregrad: 4

# # cat /etc/apparmor.d/tunables/alias
# alias /var/ - /home/sys-var/,
# alias /tmp/ - /home/sys-tmp/,
#
# - logprof should not ask /home/sys-var/run/nscd/dbwvkZpy because it's 
covered by /var/run/nscd/db* (with the alias applied)
# 
# in fact, it should not ask ask for /home/sys-var/run/nscd/db* at all because 
the /bin/ping profile includes abstractions/nameservice
# which already contains   /{,var/}run/nscd/db*  rmix,





# let's write one of the changed profiles...

= Changed Local Profiles =

The following local profiles were changed. Would you like to save them?

 [1 - /home/cb/bin/lj_make_galerie.sh]
  2 - /usr/lib/Adobe/Reader9/Reader/intellinux/bin/acroread 
  3 - /{usr/,}bin/ping 
(S)ave Changes / Save

[apparmor] GSoC - updated reviews

2013-09-21 Thread Christian Boltz
Hello,

I just reviewed all old reviews. The attached tar.gz contains everything 
that is still not fixed. (If you disagree with something, you can of 
course just tell me that I'm wrong ;-)

To make things more interesting, I added some small issues that are 
related to the old reviews ;-)

The attached review-r69 (I needed a filename ;-) contains another small 
bug - it's just a missing space, but causes invalid profiles ;-)


Regards,

Christian Boltz
-- 
Look at Debian... its stable, works on a variety of platforms and
development is racing along at the speed of a turtle with 3 broken legs.
[Joseph M. Gaffney in opensuse]


reviews.tar.gz
Description: application/compressed-tar
# (not really related to r69, but I needed a filename ;-)


(apparmor/aa.py)
def write_path_rules(prof_data, depth, allow):
...   
while user or other:
ownerstr = ''
tmpmode = 0
tmpaudit = False
if user - other:
# if no other mode set 
ownerstr = 'owner'

# causes broken profiles - should be 'owner ' (space added)

tmpmode = user - other
tmpaudit = user_audit
user = user - tmpmode
else:
if user_audit - other_audit  user:
ownerstr = 'owner '


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


[apparmor] GSoC review r70..72

2013-09-22 Thread Christian Boltz
Hello,

the (quite small) reviews for r70 and r72 are attached. The r70 review 
also contains two bugs I noticed.

For r71, I have no reason to complain ;-)


Regards,

Christian Boltz
-- 
 Das Autofahrersyndrom: Prüft Euren Ton.
*anschlag*
*bonk*
Stimmt, der Ton ist nicht sonderlich...
[ Peter Lipp und David Haller in suse-linux]

revno: 70
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Sun 2013-09-22 15:01:34 +0530
message:
  So that closes the first proper version of aa-cleanprof with testcases added,
  fixed profile writer to work on multiple profiles at once, please use the
  view clean changes option in logprof and genprof, the comment preserver
  version needs tweaking that version wont be written anyways. Plus a few other
  changes


=== modified file 'Tools/aa-cleanprof'
--- Tools/aa-cleanprof  2013-09-19 05:02:19 +
+++ Tools/aa-cleanprof  2013-09-22 09:31:34 +
@@ -7,6 +7,7 @@
 parser = argparse.ArgumentParser(description='Cleanup the profiles for the 
given programs')
 parser.add_argument('-d', '--dir', type=str, help='path to profiles')
 parser.add_argument('program', type=str, nargs='+', help='name of program')
+parser.add_argument('-s', '--silent', action='store_true', help='Silently 
over-write with cleanprofile')

# ...with clean profile


=== modified file 'Tools/manpages/aa-cleanprof.pod'
--- Tools/manpages/aa-cleanprof.pod 2013-09-20 19:38:34 +
+++ Tools/manpages/aa-cleanprof.pod 2013-09-22 09:31:34 +
   
+B-s --silent
+
+   Silently over-writes the profile without user prompt.

# overwrites






# cleanprof test results (cleaning up the nptd profile)
# network inet stream was removed, but network inet6 stream was not (both 
covered by abstractions/nameservice, so both should have been removed)
# besides that, the cleaned profile looks good
# (well, it shouldn't contain the allow keyword in all lines, but that's a 
known issue in the write method) 


# from testing logprof:
# I used -d ./profiles, but when saving a profile (with the Save Selec(t)ed 
Profile option), it was written to /etc/apparmor.d/


vim:ft=diff

revno: 72
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Sun 2013-09-22 15:25:20 +0530
message:
  Added help messages to translate strings and a few other minor fixes


=== modified file 'Tools/aa-cleanprof'
--- Tools/aa-cleanprof  2013-09-22 09:31:34 +
+++ Tools/aa-cleanprof  2013-09-22 09:55:20 +
 
-parser.add_argument('-s', '--silent', action='store_true', help='Silently 
over-write with cleanprofile')
+parser.add_argument('-s', '--silent', action='store_true', help=_('Silently 
over-write with a clean profile'))

# overwrite
 
=== modified file 'Tools/aa-mergeprof'
--- Tools/aa-mergeprof  2013-09-19 05:02:19 +
+++ Tools/aa-mergeprof  2013-09-22 09:55:20 +
 
 ##parser.add_argument('profiles', type=str, nargs=3, help='MINE BASE OTHER')
-parser.add_argument('-auto', action='store_true', help='Automatically merge 
profiles, exits incase of *x conflicts')
+parser.add_argument('-auto', action='store_true', help=_('Automatically merge 
profiles, exits incase of *x conflicts'))

# should it be '--auto'?


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


[apparmor] GSoC review r75

2013-09-22 Thread Christian Boltz
Hello,

the review for r75 is attached, with two bugs and a To-Do note included.


Regards,

Christian Boltz
-- 
you are spending too much time in web forums or with apache guys if you
are using +1 and -1 :-) [Stefan Seyfried in opensuse-factory]

revno: 75
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Mon 2013-09-23 02:14:11 +0530
message:
  Fixed the netrule persistence issue in cleanprof, some elementary work for 
mergeprof


=== modified file 'Tools/aa-mergeprof'
--- Tools/aa-mergeprof  2013-09-22 18:19:19 +
+++ Tools/aa-mergeprof  2013-09-22 20:44:11 +
@ -17,13 +16,12 @@
 
 profiles = [args.mine, args.base, args.other]
 
 print(profiles)

# debugging code?




# bugs noticed:

(from aa-cleanprof /usr/sbin/ntpd)
[23:08:29] cboltz I'm afraid there is a real bug in cleanprof
[23:08:35] cboltz it said Deleted 4 rules
[23:08:47] cboltz but manually diffing the profile shows that it removed 5 
rules ;-)
[23:08:56] kshitij8 damn! you noticed that :P
[23:09:35] kshitij8 I noticed that after the commit.




# python aa-mergeprof /etc/apparmor.d/usr.sbin.ntpd ./profiles/usr.sbin.ntpd 
/dev/null 
['/etc/apparmor.d/usr.sbin.ntpd', 'profiles/usr.sbin.ntpd', '/dev/null']
Traceback (most recent call last):
  File aa-mergeprof, line 72, in module
main()
  File aa-mergeprof, line 24, in main
mergeprofiles.clear_common()
  File aa-mergeprof, line 56, in clear_common
user_other = cleanprofile.CleanProf(False, user, other)
NameError: global name 'user' is not defined




[23:22:53] cboltz oh, mergeprof can now at least print --help output (no 
syntax error anymore ;-)
[23:23:24] cboltz it seems to enforce 3 parameters
[23:23:46] cboltz I'd like to also have a way to merge only 2 profiles
[23:25:32] kshitij8 that shouldn't be hard. I'll make the third param 
optional.
[23:26:01] kshitij8 and other changes about it ofcourse.



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


[apparmor] GSoC review r76..79

2013-09-23 Thread Christian Boltz
Hello,

the reviews for r76..79 are attached. (No complaints about r76 and r78.)


Regards,

Christian Boltz
-- 
 Microsoft-Compatible Spongiforme Encephalitis?
 Setzt das nicht Hirn voraus?
Irgendwo müssen doch all die Beschwörungsformeln hin, die man als
MCSE auswendig lernen muß. Ein schwammförmiges Gehirn scheint mir
dafür durchaus geeignet ...
[Aran Kuntze, Gerhard Schromm und Martin Bienwald in dasr]

revno: 79
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Mon 2013-09-23 21:00:36 +0530
message:
  (no message)


=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-09-23 14:02:25 +
+++ apparmor/aa.py  2013-09-23 15:30:36 +
@@ -3030,8 +3030,12 @@
 def set_allow_str(allow):
 if allow == 'deny':
 return 'deny '
-else:
+elif allow == 'allow':
 return 'allow '
+elif allow == '':
+return ''
+else:
+raise AppArmorException(_(Invalid allow string: %(allow)s))
 
# looks good (in theory), but in practise it seems all function calls pass in 
'allow', not ''
# which means we still get all lines prefixed with 'allow ' :-(
#
# Please change it to

elif allow == 'allow':
 return ''

# (better no allow than having allow everywhere ;-)



vim:ft=diff

revno: 77
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Mon 2013-09-23 19:32:25 +0530
message:
  Added first version of aa-mergeprof, does not include the check for 
conflicting ix rules yet


=== modified file 'apparmor/tools.py'
--- apparmor/tools.py   2013-09-22 17:21:30 +
+++ apparmor/tools.py   2013-09-23 14:02:25 +
@@ -123,7 +123,7 @@
 q = apparmor.hasher()
 q['title'] = 'Changed Local Profiles'
 q['headers'] = []
-q['explanation'] = _('The following local profiles were 
changed. Would you like to save them?')
+q['explanation'] = _('The local profile for %s in file %s was 
changed. Would you like to save it?') %(program, filename)

# good idea, but
# - you should use %(program)s and %(filename)s instead of %s
# - a \n wouldn't hurt ;-)




# review of aa-mergeprof postponed until ask_the_question() is re-integrated 
into aa.py (as discussed on IRC)
# 
# in other words: ignore everything below this line ;-)



=== modified file 'Tools/aa-mergeprof'
--- Tools/aa-mergeprof  2013-09-22 22:17:15 +
+++ Tools/aa-mergeprof  2013-09-23 14:02:25 +
@@ -16,39 +18,70 @@
 def main():
 mergeprofiles = Merge(profiles)
 #Get rid of common/superfluous stuff
 mergeprofiles.clear_common()

+if not args.auto:
+mergeprofiles.ask_the_questions('other')
+
+mergeprofiles.clear_common()
+print(base)
+mergeprofiles.ask_the_questions('base')
+
+q = apparmor.aa.hasher()
+q['title'] = 'Changed Local Profiles'
+q['headers'] = []
+q['explanation'] = _('The following local profiles were changed. Would 
you like to save them?')
+q['functions'] = ['CMD_SAVE_CHANGES', 'CMD_VIEW_CHANGES', 'CMD_ABORT']
+q['default'] = 'CMD_VIEW_CHANGES'
+q['options'] = []
+q['selected'] = 0
+p =None
+ans = ''
+arg = None
+programs = list(mergeprofiles.user.aa.keys())
+program = programs[0]
+while ans != 'CMD_SAVE_CHANGES':
+ans, arg = apparmor.aa.UI_PromptUser(q)
+if ans == 'CMD_SAVE_CHANGES':
+apparmor.aa.write_profile_ui_feedback(program)
+apparmor.aa.reload_base(program)
+elif ans == 'CMD_VIEW_CHANGES':
+for program in programs:
+apparmor.aa.original_aa[program] = 
apparmor.aa.deepcopy(apparmor.aa.aa[program])
+#oldprofile = 
apparmor.serialize_profile(apparmor.original_aa[program], program, '')
+newprofile = 
apparmor.aa.serialize_profile(mergeprofiles.user.aa[program], program, '')
+
apparmor.aa.display_changes_with_comments(mergeprofiles.user.filename, 
newprofile)
+
 
@@ -64,453 +97,526 @@
+def ask_the_questions(self, other):
+if other == 'other':
+other = self.other
+else:
+other = self.base
+#print(other.aa)
+
+#Add the file-wide includes from the other profile to the user profile
+done = False
+options = list(map(lambda inc: '#include %s' %inc, 
sorted(other.filelist[other.filename]['include'].keys(
+q = apparmor.aa.hasher()
+q['options'] = options
+default_option = 1
+q['selected'] = default_option - 1
+q['headers'] = [_('File includes'), _('Select the ones you wish to 
add')]
+q['functions'] = ['CMD_ALLOW', 'CMD_IGNORE_ENTRY', 'CMD_ABORT', 
'CMD_FINISHED

[apparmor] GSoC review r80..84

2013-09-23 Thread Christian Boltz
Hello,

the review for r80 is attached. Maybe I'll add some comments on the UI 
later after actually testing aa-mergeprof ;-)

r81..84 look fine :-)


Regards,

Christian Boltz
-- 
http://www1.giga.de/gigahelp/index_gigahelp/0,3597,,00.html
| Leider scheint Euer Browser den Aufbau von Frames zu unterstützen ...
*Leider?* :)
Tut Lynx doch gar nicht. :)   [Andreas Kneib in suse-linux]

revno: 80
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Mon 2013-09-23 23:05:25 +0530
message:
  Fixes netrule deletion for includes


=== modified file 'Tools/aa-mergeprof'
--- Tools/aa-mergeprof  2013-09-23 14:02:25 +
+++ Tools/aa-mergeprof  2013-09-23 17:35:25 +
@@ -97,6 +97,36 @@
 base_other = cleanprofile.CleanProf(False, self.base, self.other)
 deleted += user_base.compare_profiles()
 
+def conflict_mode(self, profile, hat, allow, path, mode, new_mode, 
old_mode):
+conflict_modes = set('uUpPcCiIxX')

# uppercase I should never appear (but it can't hurt to check for it 
nevertheless ;-)
# also, I'm not aware of uppercase X

+conflict_x= (old_mode | mode)  conflict_modes
+if conflict_x:
+#We may have conflicting x modes
+if conflict_x  set('x'):
+conflict_x.remove('x')
+if conflict_x  set('X'):
+conflict_x.remove('X')
+if len(conflict_x)  1:
+q = apparmor.aa.hasher()
+q['headers'] = [_('Path'), path]
+q['headers'] += [_('Select the appropriate mode'), '']
+options = []
+options.append('%s: %s' %(mode, path, 
apparmor.aa.mode_to_str_user(apparmor.aa.flatten_mode((old_mode | new_mode) - 
(old_mode  conflict_x)
+options.append('%s: %s' %(mode, path, 
apparmor.aa.mode_to_str_user(apparmor.aa.flatten_mode((old_mode | new_mode) - 
(new_mode  conflict_x)
+q['options'] = options
+q['functions'] = ['CMD_ALLOW', 'CMD_ABORT']

# I'll probably add a comment for the user interface after testing it, but it 
looks ok for now

+done = False
+while not done:
+ans, selected = apparmor.aa.UI_PromptUser(q)
+if ans == 'CMD_ALLOW':
+if selection == 0:
+self.user.aa[profile][hat][allow][path][mode] = 
(old_mode | new_mode) - (old_mode  conflict_x)
+elif selection == 1:
+self.user.aa[profile][hat][allow][path][mode] = 
(old_mode | new_mode) - (new_mode  conflict_x)
+else:
+raise apparmor.aa.AppArmorException(_('Unknown 
selection'))
+done = True



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


[apparmor] force-complain symlinks break cache?

2013-09-25 Thread Christian Boltz
Hello,

it seems using force-complain/ symlinks breaks cache usage:

root@beta:/etc/apparmor.d/force-complain time rcapparmor reload
redirecting to systemctl reload apparmor

real0m0.791s
user0m0.004s
sys 0m0.000s

Now let's create force-complain symlinks for all profiles:

root@beta:/etc/apparmor.d/force-complain ln -s ../* .

root@beta:/etc/apparmor.d/force-complain time rcapparmor reload
redirecting to systemctl reload apparmor

real0m17.267s
user0m0.000s
sys 0m0.004s
root@beta:/etc/apparmor.d/force-complain time rcapparmor reload
redirecting to systemctl reload apparmor

real0m17.250s
user0m0.000s
sys 0m0.004s


This is a server with openSUSE 13.1 beta with AppArmor 2.8.2.


Regards,

Christian Boltz
-- 
Hier gibt es zB eine Adress-DB für einige Leute und allein schon die
gleichzeitige Verwendung dieser DB ist eher die Ausnahme.
Wahrscheinlich verdienen die Datenbanken hier die Bezeichnung gar nicht.
Wenn du willst, kannst du auch dazu gemeinsam genutzter strukturierter
Notizzettel sagen. [Al Bogner in suse-linux]


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


[apparmor] [Patch] cleanup usr.sbin.ntpd profile

2013-09-30 Thread Christian Boltz
Hello,

this patch removes some rules from the ntpd profile that are already 
covered by abstractions:
- the network rules are in abstractions/nameservice
- /etc/gai.conf is also in abstractions/nameservice
- @{PROC}/sys/kernel/ngroups_max is in abstractions/base

I found those superfluous rules with aa-cleanup :-) but merged the 
changes manually to keep comments and rule sorting.

@Kshitij: it would be nice if aa-cleanup would have an option to only 
delete superfluous rules _without_ removing comments and sorting the 
remaining rules ;-)


=== modified file 'profiles/apparmor.d/usr.sbin.ntpd'
--- profiles/apparmor.d/usr.sbin.ntpd   2013-09-16 22:23:32 +
+++ profiles/apparmor.d/usr.sbin.ntpd   2013-09-30 15:36:51 +
@@ -27,10 +27,6 @@
   capability sys_time,
   capability sys_nice,
 
-  network inet dgram,
-  network inet stream,
-  network inet6 stream,
-
   /drift/ntp.drift rwl,
   /drift/ntp.drift.TEMP rwl,
   /etc/ntp.conf r,
@@ -39,7 +35,6 @@
   /etc/ntp/step-tickers r,
   /etc/ntpd.conf r,
   /etc/ntpd.conf.tmp r,
-  /etc/gai.conf r,
 
   /tmp/ntp* rwl,
   /usr/sbin/ntpd rmix,
@@ -60,7 +55,6 @@
   /{,var/}run/ntpd.pid w,
   /var/tmp/ntp* rwl,
   @{PROC}/@{pid}/net/if_inet6 r,
-  @{PROC}/sys/kernel/ngroups_max r,
 
   # allow access for when chrooted
   /var/lib/ntp/@{PROC}/@{pid}/net/if_inet6 r,



Regards,

Christian Boltz
-- 
[GUI vs. Command-Line] Einen ähnlichen Streit wird es in 20 Jahren
auch geben, wenn die 2D-Screenfanatiker auf die VR Fans losgehen
und wieder ein Streit vom Zaun bricht der an Sinnfreiheit kaum zu
überbieten ist.   [Phillip Richdale in suse-linux]


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


Re: [apparmor] [patch 05/13] parser - rewrite caching tests in python unittest

2013-10-10 Thread Christian Boltz
Hello,

[sorry for the slightly broken quoting - KMail needs some improvement 
when quoting overlong lines ;-) ]

Am Donnerstag, 10. Oktober 2013 schrieb Steve Beattie:
 --- /dev/null
 +++ b/parser/tst/caching.py

 +from optparse import OptionParser# deprecated, should move to
 argparse eventually 

I love it when a patch introduces a new file that already comes with a 
deprecated notice. What about just switching to argparse? ;-)


Lots of tests (all?) have

 +rc, report = testlib.run_cmd(cmd)
 +self.assertEquals(rc, 0, Got return code %d, expected
 0\nOutput: %s % (rc, report)) 

testlib.run_cmd should have a parameter expected_returncode (default 0) 
and fail the test if it doesn't match.

If you don't want to mix execution and result testing in one function, a 
separate function run_cmd_checkstatus() that calls testlib.run_cmd() and 
checks the return code also works. 

Even if it only saves one line per test - you have lots of tests ;-) 
It would remove some noise from all tests and makes sure the returncode 
is checked in every test. (With the current way, I wouldn't be too 
surprised if one of the tests forgets to check the returncode.)

 +def test_cache_not_loaded_when_features_differ(self):
 +'''test cache is not loaded when features file differs'''
 +
 +self._generate_cache_file()
 +
 +with open(os.path.join(self.cache_dir, '.features'), 'w+') as
 f: 
 +f.write('monkey\n')

It seems you have lots of bananas ;-) - adding the monkeys to the 
features file is also worth its own function IMHO.

An even better alternative is to add an optional monkey parameter to 
_generate_cache_file()

After reading the second half of the patch, I noticed that you also add 
monkeys to other files, so maybe (untested!)

def add_monkey(filename):
banana = os.path.join(self.cache_dir, filename)
with open(banana, 'w+') as f:
f.write('monkey')

would be another good solution.

 +def test_cache_writing_updates_cache_file(self):
...
 +with open(cache_file, 'rb') as f:
 +cache_contents = f.read()
 +new_size = os.fstat(f.fileno()).st_size
 +# We check sizes here rather than whether the string monkey
 is +# in cache_contents because of the difficulty coercing
 cache +# file bytes into strings in python3
 +self.assertNotEquals(orig_size, new_size, 'Expected cache
 file to be updated, got: \n%s' % cache_contents) 

Wait a minute - first you are afraid of diffing the file content because 
of handling binary stuff in python, and then you dump cache_contents 
(which contains the binary cache of the profile) to the terminal if the 
test fails.

I'm quite sure nobody wants to have his terminal filled up with binary 
data, and I'm also sure nobody can read the binary dump without using 
tools.

Instead, you should print both file sizes or just file size differs.


This is the only critical thing - everything else is just cleanup 
which can go into a follow-up patch.

With dumping the binary stuff to the terminal removed, and a promise to 
do the cleanups in a follow-up patch [1],
Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz

[1] you know I'm good at reminding you ;-)
-- 
Ugly doesn't even begin to describe the knoppix init script system. [..]
Some people should just be strung up by their short hairs and made to
walk in the steps of those who must follow them before being allowed to
code such monstrosities again.   [Robison, Jonathon (M.)]


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


Re: [apparmor] [patch 13/13] parser - update README information

2013-10-10 Thread Christian Boltz
Hello,

Am Donnerstag, 10. Oktober 2013 schrieb Steve Beattie:
 The README in the parser directory was woefully out of date; this
 patch updates the information to contain the current mail list, wiki,
 and bug tracking locations.

That was an easy one to proofread ;-)

Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
Aber doch ... Woast Bub, ich denk bei sowas immer willkürlich an 
den Worst-case. Nämlich das das nicht ein Gscheidle wie du macht,
sondern daß das irgendeiner hier oder in irgendeinem Forum
aufschnappt. [David Haller in opensuse-de]


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


Re: [apparmor] [patch 03/13] parser - add simple file deny rule tests

2013-10-11 Thread Christian Boltz
Hello,

Am Donnerstag, 10. Oktober 2013 schrieb Steve Beattie:
 Our simple language tests did not include any file deny rule tests.
 This patch adds a few simple ones.

Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
Oder kannst du dir ein AUto vorstellen das erst mit einem 
Benzinmotor fabriziert wird und dann wenn du es mit Diesel 
betankst auch noch fährt. *lach* [Thomas Templin in suse-linux]


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


Re: [apparmor] [patch 05/13] parser - rewrite caching tests in python unittest

2013-10-15 Thread Christian Boltz
Hello,

Am Dienstag, 15. Oktober 2013 schrieb Steve Beattie:
 On Fri, Oct 11, 2013 at 10:08:51PM +0200, Christian Boltz wrote:
  We'll see if you still like this in some months...
 
 While I reserve the right to flake out^W^W change my mind, I help

;-)

 maintain and improve other codebases that don't get reviews before
 commits... and they sure could use it.

;-)

  Maybe it would be even easier with break_file instead of
  add_monkey as function name. OTOH, I'm quite sure people _will_
  check what add_monkey() does, but nobody will read the code of
  break_file() ;-)
 The thing is, I often need the full path for the subsequent
 verification check as well, so pushing the
 os.path.join(self.cache_dir, ...)  call into the helper function both
 limits the generality of the helper function (in some cases, that's
 okay) and doesn't save me very much because I need to do it again
 later. So in this case, I created a write_file() function that takes
 a path and a string and writes that string to the path. It's more
 general but means the path join occurs in the test function.

What about this?

def write_file(directory, filename, contents):
'''write contents to path'''
path = os.path.join(directory, path)
with open(path, 'w+') as f:
f.write(contents)
return path

This makes the tests a bit more readable, and if you need the filename 
later, you can use
filename = write_file(...)

   Though, what I'd really like is to somehow set self.do_cleanup to
   False when any test fails, so that for test cases that fail, the
   temporary directory is left behind, to make diagnosing why it
   failed
   easier to do. I'll think about whether there's a reasonable way to
   do
   that.
  
  Looks like it isn't really nice or easy, but at least possible:
  
  http://stackoverflow.com/questions/4414234/getting-pythons-unittest-  
  results-in-a-teardown-method
  
  http://www.piware.de/2012/10/python-unittest-show-log-on-test-failur
  e/ (the comments are also interesting)
 
 Thanks. A lot of those solutions are specific to adding information
 to the logged output, which is fine, but not what I'm after; I want
 to be able to potentially re-run the test manually to see why it's
 failing with a minimum of effort.
 
 That said, I was able to make the decorator function approach work.


 Patch history:
   v1: - initial version
   v2: - create template base class
   - add keep_on_fail() decorator to keep temporary test files
 around after a test fails

I'd prefer if you can do this on a global level instead of having it on 
every test.

google says it's possible, see 
http://stackoverflow.com/questions/6695854/writing-a-class-decorator-that-applies-a-decorator-to-all-methods
and 
http://stackoverflow.com/questions/3467526/attaching-a-decorator-to-all-functions-within-a-class
and some more, see 
https://www.google.com/search?q=python+decorator+all+functionsie=UTF-8

   - create run_cmd_check wrapper to run_cmd that adds an assertion
 check based on whether return code matches the expected rc
   - similarly, add a check to run_cmd_check for verifying the
 output contains a specific string, also simplifying many of the
 caching tests.

Looks good :-)


I'd say commit the patch now and then do a follow-up patch for the 
things listed above. That's easier to review ;-)

Assuming the patch consists of the previous patch + the changes listed 
in your next mail, 
Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
 Was ist das, Nacht?
Das ist der Zeitraum, in dem Du effektiv administrieren kannst. Weil
anscheinend die User alle total faul sind, und sich ausgeloggt haben.
[Wilfried Kramer]


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


Re: [apparmor] [patch] updated usr.sbin.smbd profile

2013-10-16 Thread Christian Boltz
Hello,

looks like the patch needs one additional line (inserted below), see 
https://bugzilla.novell.com/show_bug.cgi?id=845867#c4

Am Dienstag, 15. Oktober 2013 schrieb Christian Boltz:
 Am Dienstag, 15. Oktober 2013 schrieb Christian Boltz:
  some samba *.dat files were moved, and a new library needs to be
  loaded by smbd.
 
 It turns out more changes are needed for samba, also in the nmbd and
 winbindd profile. The reason is probably a major version update -
 openSUSE 13.1 ships samba 4.1, while 12.3 came with samba 3.6.
 
 Also fix /usr/lib*/samba/{lowercase,upcase,valid}.dat r,
 which should be lowcase instead of lowercase.
 Google didn't find any samba-related lowercase.dat and my
 ARCHIVES.gz archive shows that openSUSE 11.4 already used
 lowcase.dat, so removing lowercase shouldn't cause any problems.
 Nevertheless, I'll not remove lowercase in the 2.8 branch to be on
 the safe side.
 
 References: https://bugzilla.novell.com/show_bug.cgi?id=845867
 References: https://bugzilla.novell.com/show_bug.cgi?id=846054
 
 I propose this patch for trunk and the 2.8 branch, with the little
 difference for lowercase mentioned above.
 
 I also noticed that the winbindd profile does not use
 abstractions/samba (which would simplify the profile a lot), but
 that's something for another patch ;-)
 
 
 === modified file 'profiles/apparmor.d/abstractions/samba'
 --- profiles/apparmor.d/abstractions/samba  2011-08-26 23:52:27
 + +++ profiles/apparmor.d/abstractions/samba  2013-10-15
 19:54:07 + @@ -11,6 +11,7 @@
 
/etc/samba/* r,
/usr/share/samba/*.dat r,
 +  /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r,
/var/lib/samba/**.tdb rwk,
/var/log/samba/cores/ rw,
/var/log/samba/cores/** rw,
 
 === modified file 'profiles/apparmor.d/usr.sbin.nmbd'
 --- profiles/apparmor.d/usr.sbin.nmbd   2013-01-02 23:31:01 +
 +++ profiles/apparmor.d/usr.sbin.nmbd   2013-10-15 19:54:34 +
 @@ -12,6 +12,7 @@
/usr/sbin/nmbd mr,
 
/var/{cache,lib}/samba/browse.dat* rw,
 +  /var/{cache,lib}/samba/gencache.dat rw,
/var/{cache,lib}/samba/wins.dat* rw,
/var/{cache,lib}/samba/smb_krb5/ rw,
/var/{cache,lib}/samba/smb_krb5/krb5.conf* rw,
 
 === modified file 'profiles/apparmor.d/usr.sbin.smbd'
 --- profiles/apparmor.d/usr.sbin.smbd   2013-10-09 20:42:41 +
 +++ profiles/apparmor.d/usr.sbin.smbd   2013-10-15 19:54:27 +
 @@ -29,7 +29,8 @@
/usr/lib*/samba/vfs/*.so mr,
/usr/lib*/samba/charset/*.so mr,
/usr/lib*/samba/auth/script.so mr,
 -  /usr/lib*/samba/{lowercase,upcase,valid}.dat r,
 +  /usr/lib*/samba/pdb/*.so mr,
 +  /usr/lib*/samba/{lowcase,upcase,valid}.dat r,
/usr/sbin/smbd mr,
/usr/sbin/smbldap-useradd Px,
/var/cache/samba/** rwk,
 @@ -38,6 +39,7 @@
/{,var/}run/cups/cups.sock rw,
/{,var/}run/dbus/system_bus_socket rw,
/{,var/}run/samba/** rk,
 +  /{,var/}run/samba/ncalrpc/ rw,

+  /{,var/}run/samba/ncalrpc/** rw,

/{,var/}run/samba/smbd.pid rw,
/var/spool/samba/** rw,
 
 
 === modified file 'profiles/apparmor.d/usr.sbin.winbindd'
 --- profiles/apparmor.d/usr.sbin.winbindd   2012-11-06 22:19:46
 + +++ profiles/apparmor.d/usr.sbin.winbindd   2013-10-15
 19:56:45 + @@ -1,4 +1,3 @@
 -# Last Modified: Mon Mar 26 20:28:18 2012
  #include tunables/global
 
  /usr/sbin/winbindd {
 @@ -13,6 +12,8 @@
/usr/lib*/samba/idmap/*.so mr,
/usr/lib*/samba/nss_info/*.so mr,
/usr/sbin/winbindd mr,
 +  /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r,
 +  /var/cache/samba/netsamlogon_cache.tdb rw,
/var/lib/samba/account_policy.tdb rwk,
/var/lib/samba/gencache.tdb rwk,
/var/lib/samba/gencache_notrans.tdb rwk,
 @@ -20,7 +21,7 @@
/var/lib/samba/messages.tdb rwk,
/var/lib/samba/netsamlogon_cache.tdb rwk,
/var/lib/samba/serverid.tdb rwk,
 -  /var/lib/samba/winbindd_cache.tdb rwk,
 +  /var/lib/samba/winbindd_cache.tdb* rwk,
/var/lib/samba/winbindd_privileged/pipe w,
/var/log/samba/cores/ rw,
/var/log/samba/cores/winbindd/ rw,
 @@ -28,6 +29,7 @@
/var/log/samba/log.wb-* w,
/var/log/samba/log.winbindd rw,
/{var/,}run/samba/winbindd.pid rwk,
 +  /{var/,}run/samba/winbindd/ rw,
 
# Site-specific additions and overrides. See local/README for
 details. #include local/usr.sbin.winbindd


Regards,

Christian Boltz
-- 
Die Borg sind einfach eine Allegorie auf M$: gross, toll und voller
endloser Featuritis - aber wenn es ernst wird, sterben sie an einer
Schutzverletzung. [Andreas Pohlke in drsst]


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


Re: [apparmor] [patch 1/8] parser caching tests - remove unused value

2013-10-24 Thread Christian Boltz
Hello,

Am Mittwoch, 23. Oktober 2013 schrieb Steve Beattie:
 Remove unused report value where it's not used.
 
 Signed-off-by: Steve Beattie st...@nxnw.org
 ---
  parser/tst/caching.py |   22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)

Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
 $ rpm -q --queryformat %{name}-%{version} %{buildtime:date} mod_php
 mod_php-3.0.11 Fri 23 Jul 1999 03:25:43 PM CEST
 -dn'*SCNR*'h
Jaja.
| grep root /etc/aliases
kaiser_willem:root
[ David Haller und Ratti in suse-linux]


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


Re: [apparmor] [patch 8/8] parser testlib - use metaclass to mark all test functions keep_on_fail

2013-10-24 Thread Christian Boltz
Hello,

Am Mittwoch, 23. Oktober 2013 schrieb Steve Beattie:
 This patch adds a python metaclass to wrap the test methods in the
 subclasses of the template class AATestTemplate with the keep_on_fail
 function, which sets the do_cleanup attribute to False when a testcase
 failure occurs (i.e. an Exception is raised), and removes the
 manually applied decorators to the caching tests that made use of
 this.
 
 The downside to this approach is that the way metaclasses are declared
 changed between python 2 and python 3 in an incompatible way. Since
 python 3 is The Future™, I chose that approach and made the caching
 and valgrind tests which use testlib be python3 (until this change,
 they would have worked under either python 2 or python 3).

I think requiring py3 for the parser tests is OK because it doesn't 
restrict what we test[1], and a BuildRequires: python3 is acceptable 
when it makes things easier on the programming side.

Therefore:
Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz

[1] Things would be different if you'd test _python code_ only with py3, 
even if it's written to run with py2 also
-- 
Ich glaube nicht, daß es je einem Briefkasten gelungen ist durch
Namenslosigkeit oder Pseudonym den Werbeblättchen des Supermarkts
auf der grünen Wiese oder denen einer politischen Partei direkt vor
Wahl verschont zu bleiben. [Helga Fischer in suse-linux]


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


[apparmor] [patch] parser/po/de.po fixes

2013-10-26 Thread Christian Boltz
Hello,

this patch fixes some minor issues in parser/po/de.po

I propose this patch for trunk only - fortunately nothing is critical 
enough for a backport to 2.8;-)


=== modified file 'parser/po/de.po'
--- parser/po/de.po 2009-02-07 12:16:03 +
+++ parser/po/de.po 2013-10-26 22:01:40 +
@@ -235,7 +235,7 @@
 #: ../parser_misc.c:473 ../parser_misc.c:480 ../parser_misc.c:487
 #: ../parser_misc.c:484 ../parser_misc.c:491
 msgid Conflict 'a' and 'w' perms are mutually exclusive.
-msgstr Konfliktive permanente Werte für 'a' und 'W' schließen sich 
gegenseitig aus.
+msgstr Berechtigungskonflikt - 'a' und 'w' schließen sich gegenseitig aus.
 
 #: ../parser_misc.c:497 ../parser_misc.c:504 ../parser_misc.c:508
 msgid Exec qualifier 'i' invalid, conflicting qualifier already specified
@@ -335,16 +335,16 @@
 
 #: parser_yacc.y:640 parser_yacc.y:637
 msgid Assert: 'local_profile rule' returned NULL.
-msgstr Assert: 'llocal_profile rulel' hat NULL zurückgegeben.
+msgstr Assert: 'local_profile rule' hat NULL zurückgegeben.
 
 #: parser_yacc.y:772
 #, c-format
 msgid Unset boolean variable %s used in if-expression
-msgstr In Bedingungssatz verwendete Boolesche Variable '%s' deaktivieren
+msgstr In Bedingungssatz verwendete Boolsche Variable '%s' deaktivieren
 
 #: parser_yacc.y:830
 msgid subset can only be used with link rules.
-msgstr Teilmenge kann nur mit Link-Regeln verwendet werden.
+msgstr subset kann nur mit Link-Regeln verwendet werden.
 
 #: parser_yacc.y:832
 msgid link and exec perms conflict on a file rule using -
@@ -356,7 +356,7 @@
 
 #: parser_yacc.y:850
 msgid unsafe rule missing exec permissions
-msgstr Unsichere Exec-Berechtigungen bei fehlender Regel
+msgstr Fehlende Exec-Berechtigungen bei unsicherer Regel
 
 #: parser_yacc.y:853
 msgid link perms are not allowed on a named profile transtion.\n
@@ -374,7 +374,7 @@
 #: parser_yacc.y:1064 parser_yacc.y:1072 parser_yacc.y:1053 parser_yacc.y:1061
 #, c-format
 msgid Invalid capability %s.
-msgstr Ungültige Funktion %s.
+msgstr Ungültige Capability %s.
 
 #: parser_yacc.y:1089 parser_yacc.y:1078
 #, c-format
@@ -389,7 +389,7 @@
 #: ../parser_regex.c:283
 #, c-format
 msgid %s: Illegal open {, nesting groupings not allowed\n
-msgstr %s: Öffnen ungültig {, verschachtelte Gruppierungen sind nicht 
zulässig\n
+msgstr %s: Ungültige öffnende {, verschachtelte Gruppierungen sind nicht 
zulässig\n
 
 #: ../parser_regex.c:303
 #, c-format
@@ -454,7 +454,7 @@
 #: ../parser_policy.c:298 ../parser_policy.c:304
 #, c-format
 msgid ERROR expanding variables for profile %s, failed to load\n
-msgstr FEHLER bei der Erweiterung von Variablen für Profil '%s'. Fehler beim 
Laden\n
+msgstr FEHLER beim Expandieren von Variablen für Profil '%s'. Fehler beim 
Laden\n
 
 #: ../parser_policy.c:481 ../parser_policy.c:486
 #, c-format




Regards,

Christian Boltz
-- 
Wieviele Leute braucht es, um Windows zu installieren? - Sieben!
Einen, der sich die CD leisten kann, drei die ausdiskutieren, wie man
das wohl macht, zwei Priester, die beten, dass es funktioniert und einen
Psychiater, der die Nervenzusammenbrüche behandelt!!!


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


Re: [apparmor] Revert r1225 mistranslations (utils/po/*.po)

2013-10-26 Thread Christian Boltz
Hello,

(summing up an IRC discussion from some hours ago for those who missed 
it)

Am Dienstag, 17. September 2013 schrieb Christian Boltz:
 during the last days, we (as in: the usual people in #apparmor)
 discovered that the r1225 translation update introduced _lots_ of
 mistranslations in various languages, even for texts that had correct
 translations before. Also, most of the changes I commited for de.po
 recently fixed things that were introduced in r1225.
 
 Therefore I propose to completely [1] revert r1225 in trunk and also
 in the 2.8 branch.
 
 Note that r1225 is a commit from 2009, so it's likely that we'll get
 some merge conflicts. 

Kshitij tried to revert r1225 today, and got about 30% rejects (line 
count of patch vs. line count of *.rej files). 

Additionally he noticed that hi.po contains some mistranslations from 
r1114. To make things even more interesting, the infamous Mailserver-
Domains mistranslation in de.po was initially introduced in r198 (de.po 
was added in this revision), fixed in r1114 and broken again in a later 
revision.

In other words: It looks like lots of revisions potentially introduced 
mistranslations, so...

 The alternative is to mark _all_ texts in _all_ languages as fuzzy, so
 that translators see them as please check this. 

... this is the only way to be sure everything gets checked and fixed.
I'd even add a notice (or send a mail) to the translators to warn them 
to do the fuzzy checking extra carefully because we know there are some 
mistranslations.


I also propose to completely drop the en_GB and en_US translations 
because I see no real value in them, but several bugs.

utils/po/en_GB.po basically contains:

msgid Abort
msgstr Aborted

 

msgid (C)ancel
msgstr Cancel 

 

msgid Severity
msgstr Security

and utils/po/en_US.po contains (only quoting the best stuff):

msgid Can't find AppArmor profiles in
msgstr Updating AppArmor profiles in %s.

msgid Are you sure you want to exit?
msgstr Do you really want to use this path?

msgid 
# Filter: %s, Value: %s\n
\n
msgstr # Period: %s-%s\n

msgid Setting %s to audit mode.
msgstr Setting %s to complain mode.

(and probably some more - the above were enough fun for today ;-)


The parser/po/en_*.po files are not that funny, but I'd also consider 
them superfluous.

(If there are real differences between en_GB and en_US that we should 
keep, please speak up ;-)


Regards,

Christian Boltz
-- 
weitere Indizien deuten ja auf KMail2:
- [...]
- KMail2 ist immer kaputt, warum nicht auch hier? ;)
[Roman Fietze in opensuse-de]


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


[apparmor] [patch] dnsmasq profile - update for libvirt files

2013-10-30 Thread Christian Boltz
Hello,

dnsmasq needs read access to more files in /var/lib/libvirt/dnsmasq/
(at least *.conf and *.addnhosts)

Since this directory contains only files that are intended for dnsmasq 
(also confirmed by Jim Fehlig, the SUSE libvirt maintainer), the best 
way is to just allow /var/lib/libvirt/dnsmasq/* r,

References: https://bugzilla.novell.com/show_bug.cgi?id=848215

I propose this patch for trunk and the 2.8 branch.


=== modified file 'profiles/apparmor.d/usr.sbin.dnsmasq'

 
--- profiles/apparmor.d/usr.sbin.dnsmasq2013-08-20 22:52:22
+++ profiles/apparmor.d/usr.sbin.dnsmasq2013-10-30 19:33:18
@@ -43,10 +43,10 @@
   @{TFTP_DIR}/ r,
   @{TFTP_DIR}/** r,
 
-  # libvirt lease and hosts files for dnsmasq
+  # libvirt config, lease and hosts files for dnsmasq
   /var/lib/libvirt/dnsmasq/r,
+  /var/lib/libvirt/dnsmasq/*r,
   /var/lib/libvirt/dnsmasq/*.leases rw,
-  /var/lib/libvirt/dnsmasq/*.hostsfile r,
 
   # libvirt pid files for dnsmasq
   /{,var/}run/libvirt/network/  r,


Regards,

Christian Boltz
-- 
Die Borg sind einfach eine Allegorie auf M$: gross, toll und voller
endloser Featuritis - aber wenn es ernst wird, sterben sie an einer
Schutzverletzung. [Andreas Pohlke in drsst]


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


[apparmor] [patch] ntpd profile update

2013-11-14 Thread Christian Boltz
Hello,

ntpd needs access to /var/lib/ntp/drift/driftfile and  
/var/lib/ntp/drift/driftfile.TEMP

References: https://bugzilla.novell.com/show_bug.cgi?id=850374

I propose this patch for 2.8 and trunk.

=== modified file 'profiles/apparmor.d/usr.sbin.ntpd'
--- profiles/apparmor.d/usr.sbin.ntpd   2013-10-03 13:35:56 +
+++ profiles/apparmor.d/usr.sbin.ntpd   2013-11-14 20:36:47 +
@@ -40,6 +40,8 @@
   /usr/sbin/ntpd rmix,
   /var/lib/ntp/drift rwl,
   /var/lib/ntp/drift.TEMP rwl,
+  /var/lib/ntp/drift/driftfile rw,
+  /var/lib/ntp/drift/driftfile.TEMP rw,
   /var/lib/ntp/drift/ntp.drift rw,
   /var/lib/ntp/drift/ntp.drift.TEMP rw,
   /var/lib/ntp/etc/* r,



Regards,

Christian Boltz
-- 
 Subscribers don't receive messages from authors,
 they receive messages from listservs.
I've never seen a list server write a message :-)
[Felix Miata and jdd in opensuse-factory]


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


[apparmor] [patch] Update samba profiles for samba 4.x

2013-11-19 Thread Christian Boltz
Hello,

this is an updated version (actually v4) of my patches for smbd and nmbd 
which I sent some weeks ago ([patch] updated usr.sbin.smbd profile), 
and is already included in the packages for the just released 
openSUSE 13.1.

The patch includes changes needed for Samba 4.x, which also includes 
some small abstraction updates.

References: https://bugzilla.novell.com/show_bug.cgi?id=845867
References: https://bugzilla.novell.com/show_bug.cgi?id=846054

I propose the patch for 2.8 and trunk (the patch is for 2.8, but it 
should apply to trunk without problems)

Note: I'm intentionally not including the winbindd profile in this mail.
I received another bugreport for it today, so I'll wait some days and 
will then hopefully be able to send a more complete patch ;-)


=== modified file 'profiles/apparmor.d/abstractions/samba'
--- profiles/apparmor.d/abstractions/samba  2011-08-26 23:52:27 +
+++ profiles/apparmor.d/abstractions/samba  2013-10-15 20:36:33 +
@@ -11,6 +11,7 @@
 
   /etc/samba/* r,
   /usr/share/samba/*.dat r,
+  /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r,
   /var/lib/samba/**.tdb rwk,
   /var/log/samba/cores/ rw,
   /var/log/samba/cores/** rw,

=== modified file 'profiles/apparmor.d/abstractions/kerberosclient'
--- profiles/apparmor.d/abstractions/kerberosclient.orig2011-03-23 
20:24:11.0 +0100
+++ profiles/apparmor.d/abstractions/kerberosclient 2013-11-02 
15:04:27.267448981 +0100
@@ -20,7 +20,7 @@
   /usr/lib/@{multiarch}/krb5/plugins/preauth/ r,
   /usr/lib/@{multiarch}/krb5/plugins/preauth/* mr,
 
-  /etc/krb5.keytabr,
+  /etc/krb5.keytabrk,
   /etc/krb5.conf  r,
 
   # config files found via strings on libs

=== modified file 'profiles/apparmor.d/usr.sbin.nmbd'
--- profiles/apparmor.d/usr.sbin.nmbd   2011-08-27 18:50:42 +
+++ profiles/apparmor.d/usr.sbin.nmbd   2013-10-20 11:54:48 +
@@ -11,7 +11,9 @@
 
   /usr/sbin/nmbd mr,
 
+  /var/cache/samba/gencache.tdb rwk,
   /var/{cache,lib}/samba/browse.dat* rw,
+  /var/{cache,lib}/samba/gencache.dat rw,
   /var/{cache,lib}/samba/wins.dat* rw,
   /var/{cache,lib}/samba/smb_krb5/ rw,
   /var/{cache,lib}/samba/smb_krb5/krb5.conf* rw,

=== modified file 'profiles/apparmor.d/usr.sbin.smbd'
--- profiles/apparmor.d/usr.sbin.smbd   2012-01-10 18:06:24 +
+++ profiles/apparmor.d/usr.sbin.smbd   2013-10-15 20:36:33 +
@@ -29,16 +29,21 @@
   /usr/lib*/samba/vfs/*.so mr,
   /usr/lib*/samba/charset/*.so mr,
   /usr/lib*/samba/auth/script.so mr,
-  /usr/lib*/samba/{lowercase,upcase,valid}.dat r,
+  /usr/lib*/samba/pdb/*.so mr,
+  /usr/lib*/samba/{lowercase,lowcase,upcase,valid}.dat r,   # [1]
   /usr/sbin/smbd mr,
   /usr/sbin/smbldap-useradd Px,
   /var/cache/samba/** rwk,
   /var/cache/samba/printing/printers.tdb mrw,
   /var/lib/samba/** rwk,
   /var/lib/samba/printers/** rw,
+  /var/lib/sss/mc/passwd r,
+  /var/lib/sss/pubconf/kdcinfo.* r,
   /{,var/}run/cups/cups.sock rw,
   /{,var/}run/dbus/system_bus_socket rw,
   /{,var/}run/samba/** rk,
+  /{,var/}run/samba/ncalrpc/ rw,
+  /{,var/}run/samba/ncalrpc/** rw,
   /{,var/}run/samba/smbd.pid rw,
   /var/log/samba/cores/smbd/ rw,
   /var/log/samba/cores/smbd/** rw,


[1] for trunk, this line will be
+  /usr/lib*/samba/{lowcase,upcase,valid}.dat r,   # [1]
because (quoting myself from Oct 15th):
Also fix /usr/lib*/samba/{lowercase,upcase,valid}.dat r,
which should be lowcase instead of lowercase.
Google didn't find any samba-related lowercase.dat and my ARCHIVES.gz 
archive shows that openSUSE 11.4 already used lowcase.dat, so removing
lowercase shouldn't cause any problems. 
Nevertheless, I'll not remove lowercase in the 2.8 branch to be on the 
safe side.


Regards,

Christian Boltz
-- 
  .domain.intern smpt:[mx.domain.intern]
 Du meinst sicher smtp und nicht smpt. :-)
Du kennst den Senseless Mailinglist Protocol Typo nicht? ;-)
[ Michael Neufing und () Gregor Hermens in postfixbuch-users]


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


Re: [apparmor] [patch] Update samba profiles for samba 4.x

2013-11-19 Thread Christian Boltz
Hello,

Am Dienstag, 19. November 2013 schrieb Seth Arnold:
 On Tue, Nov 19, 2013 at 10:28:28PM +0100, Christian Boltz wrote:

  === modified file 'profiles/apparmor.d/usr.sbin.nmbd'
  --- profiles/apparmor.d/usr.sbin.nmbd   2011-08-27 18:50:42 +
  +++ profiles/apparmor.d/usr.sbin.nmbd   2013-10-20 11:54:48 +
  @@ -11,7 +11,9 @@
  
 /usr/sbin/nmbd mr,
  
  +  /var/cache/samba/gencache.tdb rwk,

 The missing {,lib} on the first line makes my OCD tingle; is this
 correct?

abstractions/samba contains
  /var/lib/samba/**.tdb rwk,
so it's (more or less) correct that the nmbd profile doesn't include 
{,lib}.

We might need to include the /var/cache/ path in abstractions/samba, but 
that's something for another patch ;-)  (The smbd/nmbd/winbind profiles 
need a bit of cleanup anyway.)


Regards,

Christian Boltz
-- 
[package naming] Another funny example is libreoffice, 
it actually collides with our shared library policy ;-) 
[Sascha Peilicke in opensuse-packaging]


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


[apparmor] [patch] abstractions/ssl_certs update

2013-11-24 Thread Christian Boltz
Hello,

add /var/lib/ca-certificates/ to abstractions/ssl_certs.

update-ca-certificates (from ca-certificates-1_201310161709-1.1.noarch) 
stores certs in this directory now.

References: https://bugzilla.novell.com/show_bug.cgi?id=852018

I propose this patch for trunk and the 2.8 branch.


=== modified file 'profiles/apparmor.d/abstractions/ssl_certs'
--- profiles/apparmor.d/abstractions/ssl_certs  2011-08-08 20:22:03 
+++ profiles/apparmor.d/abstractions/ssl_certs  2013-11-24 16:14:18 
@@ -17,3 +17,5 @@
   /usr/share/ssl/certs/ca-bundle.crt  r,
   /usr/local/share/ca-certificates/ r,
   /usr/local/share/ca-certificates/** r,
+  /var/lib/ca-certificates/ r,
+  /var/lib/ca-certificates/** r,



Regards,

Christian Boltz
-- 
Wenn das Teil unter Windows CE oder Pocket PC 2000 läuft, ist Synce Dein
Fall.  Zu finden auf Sourceforge, wenn ich mich nicht irre, und ich irre
mich nie wenn ich mich nicht irre.[Michael Karges in suse-linux]


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


Re: [apparmor] [PATCH 1/4] security: add security_path_chdir hook

2013-11-28 Thread Christian Boltz
Hello,

Am Donnerstag, 28. November 2013 schrieb Seth Arnold:
 On Tue, Nov 05, 2013 at 05:34:58AM -0800, John Johansen wrote:

  diff --git a/fs/open.c b/fs/open.c
  index d420331..9505fc5 100644
  --- a/fs/open.c
  +++ b/fs/open.c
  @@ -387,6 +387,10 @@ retry:
  if (error)
  goto out;
  
  +   error = security_path_chdir(path);
  +   if (error)
  +   goto dput_and_out;
  +
  
  error = inode_permission(path.dentry-d_inode, MAY_EXEC |
  MAY_CHDIR);
  if (error)
  goto dput_and_out;

Hmm, does that mean that first the AppArmor permissions are checked and 
then the file/directory permissions?

I reported some time ago that the audit.log contains stuff that would be 
denied by file/directory permissions anyway (which also means logging it 
more confusing than useful ;-) and the answer was that this (IMHO buggy) 
behaviour is caused by the kernel.

It might be a good idea to check the file/directory permissions first, 
and, _if_ access would be allowed, ask AppArmor via the security hook.

  @@ -419,6 +423,10 @@ SYSCALL_DEFINE1(fchdir, unsigned int, fd)
  
  if (!S_ISDIR(inode-i_mode))
  goto out_putf;
  
  +   error = security_path_chdir(f.file-f_path);
  +   if (error)
  +   goto out_putf;
  +
  
  error = inode_permission(inode, MAY_EXEC | MAY_CHDIR);

Same here.


Regards,

Christian Boltz
-- 
Machen wir einen Club utf-8 geplagte Perl-Programmierer auf?
[Bernhard Walle in suse-programming]


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


Re: [apparmor] [PATCH 3/4] security: add security_path_access hook

2013-11-29 Thread Christian Boltz
Hello,

basically what we are just discussing in
[PATCH 1/4] security: add security_path_chdir hook
also applies here:

Am Donnerstag, 28. November 2013 schrieb Seth Arnold:
 On Tue, Nov 05, 2013 at 05:35:00AM -0800, John Johansen wrote:
  diff --git a/fs/open.c b/fs/open.c
  index 9505fc5..f3e276e 100644
  --- a/fs/open.c
  +++ b/fs/open.c
  @@ -343,6 +343,10 @@ retry:
  goto out_path_release;
  }
  
  +   res = security_path_access(path, mode | MAY_ACCESS);
  +   if (res)
  +   goto out_path_release;
  +
  
  res = inode_permission(inode, mode | MAY_ACCESS);
  /* SuS v2 requires we report a read only fs too */
  if (res || !(mode  S_IWOTH) || special_file(inode-i_mode))

Please insert the hook _after_ checking the file/directory permissions, 
not before.


Regards,

Christian Boltz
-- 
 Ich hab letztens nen Film gesehen, in dem sich zwei Irre unterhalten 
 haben. Da hat der eine den anderen auch nicht verstanden.
Stimmt, hast Recht. Wann haben wir übrigens wieder Freigang? ;)
[ Martin Borchert und Bernd Brodesser in suse-linux]


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


Re: [apparmor] [patch 02/12] parser: mark valgrind test target as phony

2013-12-03 Thread Christian Boltz
Hello,

Am Dienstag, 3. Dezember 2013 schrieb Steve Beattie:
 Signed-off-by: Steve Beattie st...@nxnw.org
 ---
  parser/tst/Makefile |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 Index: b/parser/tst/Makefile
 ===
 --- a/parser/tst/Makefile
 +++ b/parser/tst/Makefile
 @@ -13,7 +13,7 @@ endif
 
  all: tests
 
 -.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching
 minimize equality +.PHONY: tests error_output gen_dbus gen_xtrans
 parser_sanity caching minimize equality valgrind tests: error_output
 caching minimize equality parser_sanity
 
  GEN_TRANS_DIRS=simple_tests/generated_x/
 simple_tests/generated_perms_leading/
 simple_tests/generated_perms_safe/ simple_tests/generated_dbus

Acked-By: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
Früher habe ich auch gerne CDs gekauft [...] Aber ich habe gelernt, daß
ich damit nicht Musiker fördere, sondern nur koksende Sony-Spacken, die
mir zum Dank für meine Investition [...] ein Rootkit auf meine Karre
installieren, gleich neben den Staatstrojaner.
[http://blog.koehntopp.de/archives/3154-Nicht-Urheberrecht-ist-das-Kernthema.html]


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


Re: [apparmor] [patch 08/12] parser: add test case for empty character class regex

2013-12-03 Thread Christian Boltz
Hello,

Am Dienstag, 3. Dezember 2013 schrieb Steve Beattie:
 This patch adds a test that verifies the parser considers an emty
 character class regex as a parse arror.
 
 Signed-off-by: Steve Beattie st...@nxnw.org
 ---
  parser/tst/simple_tests/file/bad_re_brace_1.sd |8 
  1 file changed, 8 insertions(+)
 
 Index: b/parser/tst/simple_tests/file/bad_re_brace_1.sd
 ===
 --- /dev/null
 +++ b/parser/tst/simple_tests/file/bad_re_brace_1.sd
 @@ -0,0 +1,8 @@
 +#
 +#=DESCRIPTION regex with empty character class (brace)
 +#=EXRESULT FAIL
 +#
 +/usr/bin/foo {
 +  /alpha/[]beta rw,
 +}
 +

Good idea!

Acked-By: Christian Boltz appar...@cboltz.de

BTW: Do we already have a similar test for empty alternations, like
/foo{}/bar rw,
?


Regards,

Christian Boltz
-- 
 The kernel will stay the same between SUSE Linux 10.1 and SLE10 -
 it just might be that we release them at different days,
Good. Let the SLED customers test it for us first ;)
[ Andreas Jaeger and Martin Schlander in opensuse]


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


Re: [apparmor] [patch 10/12] parser: add basic alternation tests, along with their file and owner equivalents. (v2)

2013-12-03 Thread Christian Boltz
Hello,

Am Dienstag, 3. Dezember 2013 schrieb Steve Beattie:
 This patch verifies basic alternation usage.
 
 Patch history:
   v1: initial revision
   v2: mark nested alternation tests as passing, as it was deemed a bug
 that the parser didn't support them.

 Signed-off-by: Steve Beattie st...@nxnw.org
 ---
  parser/tst/simple_tests/file/file/ok_alternations_1.sd  |7
 +++ parser/tst/simple_tests/file/file/ok_alternations_2.sd  |   
 7 +++ parser/tst/simple_tests/file/ok_alternations_1.sd   |  
  7 +++ parser/tst/simple_tests/file/ok_alternations_2.sd   | 
   7 +++ parser/tst/simple_tests/file/owner/ok_alternations_1.sd |
7 +++ parser/tst/simple_tests/file/owner/ok_alternations_2.sd
 |7 +++ 6 files changed, 42 insertions(+)

Acked-By: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
Ach so, ein 64-Bit-System... Tja, es gibt da wohl zwei Tendenzen: 
die einen benutzen /lib für 64 Bit (weil es ein 64-Bit-System ist)
und /lib32 für den alten Kram, die anderen /lib für 32 Bit (weil
das schon immer so war) und /lib64 für das neue Zeugs.
Mir reichen schon 2 Bit ;-) [Werner Flamme in postfixbuch-users]


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


[apparmor] [patch] fix broken english in parser_yacc.y

2013-12-06 Thread Christian Boltz
Hello,

I think the patch (and $SUBJECT) speaks for itsself ;-)

=== modified file 'parser/parser_yacc.y'
--- parser/parser_yacc.y2013-09-28 00:26:39 +
+++ parser/parser_yacc.y2013-12-06 18:52:41 +
@@ -657,7 +657,7 @@
 rules:  rules opt_prefix mnt_rule
{
if ($2.owner)
-   yyerror(_(owner prefix not allow on mount rules));
+   yyerror(_(owner prefix not allowed on mount rules));
if ($2.deny  $2.audit) {
$3-deny = 1;
} else if ($2.deny) {
@@ -674,7 +674,7 @@
 rules:  rules opt_prefix dbus_rule
{
if ($2.owner)
-   yyerror(_(owner prefix not allow on dbus rules));
+   yyerror(_(owner prefix not allowed on dbus rules));
if ($2.deny  $2.audit) {
$3-deny = 1;
} else if ($2.deny) {
@@ -701,7 +701,7 @@
 rules:  rules opt_prefix capability
{
if ($2.owner)
-   yyerror(_(owner prefix not allow on capability 
rules));
+   yyerror(_(owner prefix not allowed on capability 
rules));
 
if ($2.deny)
$1-caps.deny |= $3;



Regards,

Christian Boltz
-- 
 Am Besten wäre natürlich, den Owner von /dev/usbkabel ;-) zu
 überprüfen *g*
Dieses Device ist IMHO aber erst im neuen Kernel vorgesehen. Hast
Du da etwa schon einen Patch für den SuSE-Kernel? ;-)
[ Christian Boltz und Harald Krause in suse-linux]


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


[apparmor] monthly meeting

2013-12-08 Thread Christian Boltz
Hello,

as I already mentioned in the last IRC meeting, I won't be online on 
tuesday for the monthly meeting. I'll let it up to you if we move it [1] 
or if you do the meeting without me ;-)


Regards,

Christian Boltz

[1] I'm also away on wednesday and saturday
-- 
   Nochmal: Insgesamt macht das PDF einen guten Eindruck!
  Gell? Bin ja auch stolz wie Oskar :=)
 Zu Recht!
Oh, danke! *erroet* *verbeug*
[ Christian Boltz und David Haller in suse-linux-faq]


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


Re: [apparmor] [patch] can ?not fix

2013-12-08 Thread Christian Boltz
Hello,

Am Donnerstag, 5. Dezember 2013 schrieb Seth Arnold:
 On Thu, Dec 05, 2013 at 10:50:56PM +0100, Christian Boltz wrote:
  as discussed on #apparmor yesterday, here's the most important patch
  we've ever seen ;-)
  
  References: https://bugzilla.novell.com/show_bug.cgi?id=853661

 Ha! The best part about this is that the entire section needs to be
 re-written, as it is several years out of date:

Well, it's the first time I touched this file. You know what this means? ;-)

 So, while the patch itself looks good, there's bigger problems that
 need to be fixed. :)

I was afraid of that ;-)

Here's an updated (and much bigger) patch that
- removes the note about can ?not mknod
- also removes mount and umount from the can ?not list which are covered
  by mount rules now (are the remaining parts still valid?)
- updates the example audit.log lines to the current log format
- updates the description of the log format
  BTW: Is the   
  (Nameis in quotes, because the process name is limited to 15 bytes; 
[...]
  part still valid?


=== modified file 'parser/apparmor.pod'
--- parser/apparmor.pod 2010-12-20 20:29:10 +
+++ parser/apparmor.pod 2013-12-08 14:32:51 +
@@ -6,6 +6,9 @@
 #Copyright (c) 2010
 #Canonical Ltd. (All rights reserved)
 #
+#Copyright (c) 2013
+#Christian Boltz (All rights reserved)
+#
 #This program is free software; you can redistribute it and/or
 #modify it under the terms of version 2 of the GNU General Public
 #License published by the Free Software Foundation.
@@ -89,43 +92,46 @@
 cannot call the following system calls:
 
create_module(2) delete_module(2) init_module(2) ioperm(2)
-   iopl(2) mount(2) umount(2) ptrace(2) reboot(2) setdomainname(2)
+   iopl(2) ptrace(2) reboot(2) setdomainname(2)
sethostname(2) swapoff(2) swapon(2) sysctl(2)
 
-A confined process can not call mknod(2) to create character or block devices.
-
 =head1 ERRORS
 
 When a confined process tries to access a file it does not have permission
 to access, the kernel will report a message through audit, similar to:
 
-   audit(1148420912.879:96): REJECTING x access to /bin/uname
- (sh(6646) profile /tmp/sh active /tmp/sh)
-
-   audit(1148420912.879:97): REJECTING r access to /bin/uname
- (sh(6646) profile /tmp/sh active /tmp/sh)
-
-   audit(1148420944.837:98): REJECTING access to capability
- 'dac_override' (sh(6641) profile /tmp/sh active /tmp/sh)
-
-
-The permissions requested by the process are immediately after
-REJECTING. The name and process id of the running program are reported,
-as well as the profile name and any hat that may be active. (Name
+   audit(1386511672.612:238): apparmor=DENIED operation=exec 
+ parent=7589 profile=/tmp/sh name=/bin/uname pid=7605 
+ comm=sh requested_mask=x denied_mask=x fsuid=0 ouid=0
+
+   audit(1386511672.613:239): apparmor=DENIED operation=open 
+ parent=7589 profile=/tmp/sh name=/bin/uname pid=7605 
+ comm=sh requested_mask=r denied_mask=r fsuid=0 ouid=0
+
+   audit(1386511772.804:246): apparmor=DENIED operation=capable
+ parent=7246 profile=/tmp/sh pid=7589 comm=sh pid=7589 
+ comm=sh capability=2  capname=dac_override
+
+The permissions requested by the process are described in the operation=
+and denied_mask= (for files - capabilities etc. use a slightly different
+log format).
+The name and process id of the running program are reported,
+as well as the profile name including any hat that may be active, 
+separated by //. (Name
 is in quotes, because the process name is limited to 15 bytes; it is the
-same as reported through the Berkeley process accounting.) If no hat is
-active (see aa_change_hat(2)) then the profile name is printed for active.
+same as reported through the Berkeley process accounting.)
 
 For confined processes running under a profile that has been loaded in 
 complain mode, enforcement will not take place and the log messages 
 reported to audit will be of the form:
 
-   audit(1146868287.904:237): PERMITTING r access to
- /etc/apparmor.d/tunables (du(3811) profile /usr/bin/du active
- /usr/bin/du)
+   audit(1386512577.017:275): apparmor=ALLOWED operation=open
+ parent=8012 profile=/usr/bin/du name=/etc/apparmor.d/tunables/
+ pid=8049 comm=du requested_mask=r denied_mask=r fsuid=1000 
ouid=0
 
-   audit(1146868287.904:238): PERMITTING r access to /etc/apparmor.d
- (du(3811) profile /usr/bin/du active /usr/bin/du)
+   audit(1386512577.017:276): apparmor=ALLOWED operation=open
+ parent=8012 profile=/usr/bin/du name=/etc/apparmor.d/tunables/
+ pid=8049 comm=du requested_mask=r denied_mask=r fsuid=1000 
ouid=0
 
 
 If the userland auditd is not running, the kernel will send audit events




Regards,

Christian Boltz
-- 
Was spricht gegen einen Punkt im Expertenmodus:
[ ] Ich weiß nicht, was eine Partition ist.
Wenn

[apparmor] dovecot profiles

2013-12-16 Thread Christian Boltz
Hello,

attached to this mail is a set of profiles for dovecot 2.x. Some are 
already in bzr in an older version (see the diff file for the 
changes), others are completely new.

I'm also introducing tunables/dovecot which contains 
@{DOVECOT_MAILSTORE} containing the location of the mailboxes, which is 
needed in several profiles (and replaces quite some lines in the already 
existing dovecot profiles.)

References: https://bugzilla.novell.com/show_bug.cgi?id=851984

I doubt if this is the final version, but nevertheless I'd welcome 
comments ;-)  (I'll propose the profiles to be added to 
profiles/apparmor.d/ when they are finished, and also release them as 
update for at least openSUSE 13.1.)

Note: some profiles don't have the #include local/... - that's on my 
TODO list. Also the paperwork (copyright headers) is still missing.


Regards,

Christian Boltz
-- 
* mrdocs wonders when darix sleeps
sshaw mrdocs: robots don't need sleep
[from #opensuse-buildservice]
=== modified file 'profiles/apparmor.d/usr.lib.dovecot.deliver'
--- profiles/apparmor.d/usr.lib.dovecot.deliver	2012-01-06 16:34:44 +
+++ profiles/apparmor.d/usr.lib.dovecot.deliver	2013-12-15 14:56:00 +
@@ -1,6 +1,8 @@
 # Author: Dulmandakh Sukhbaatar dulmand...@gmail.com
 
 #include tunables/global
+#include tunables/dovecot
+
 /usr/lib/dovecot/deliver {
   #include abstractions/base
   #include abstractions/nameservice
@@ -8,20 +10,16 @@
   capability setgid,
   capability setuid,
 
+  @{DOVECOT_MAILSTORE}/ rw,
+  @{DOVECOT_MAILSTORE}/** rwkl,
+
   # http://www.postfix.org/SASL_README.html#server_dovecot
   /etc/dovecot/dovecot.conf r,
   /etc/dovecot/{auth,conf}.d/*.conf r,
   /etc/dovecot/dovecot-postfix.conf r,
 
   @{HOME} r,
-  @{HOME}/Maildir/ rw,
-  @{HOME}/Maildir/** klrw,
-  @{HOME}/mail/ rw,
-  @{HOME}/mail/* klrw,
-  @{HOME}/mail/.imap/** klrw,
   /usr/lib/dovecot/deliver mr,
-  /var/mail/* klrw,
-  /var/spool/mail/* klrw,
 
   # Site-specific additions and overrides. See local/README for details.
   #include local/usr.lib.dovecot.deliver

=== modified file 'profiles/apparmor.d/usr.lib.dovecot.imap'
--- profiles/apparmor.d/usr.lib.dovecot.imap	2011-08-26 23:12:10 +
+++ profiles/apparmor.d/usr.lib.dovecot.imap	2013-12-13 12:48:02 +
@@ -1,6 +1,8 @@
 # Author: Kees Cook k...@ubuntu.com
 
 #include tunables/global
+#include tunables/dovecot
+
 /usr/lib/dovecot/imap {
   #include abstractions/base
   #include abstractions/nameservice
@@ -8,18 +10,11 @@
   capability setgid,
   capability setuid,
 
-  @{HOME} r,
-  @{HOME}/Maildir/ rw,
-  @{HOME}/Maildir/** klrw,
-  @{HOME}/Mail/ rw,
-  @{HOME}/Mail/* klrw,
-  @{HOME}/Mail/.imap/** klrw,
-  @{HOME}/mail/ rw,
-  @{HOME}/mail/* klrw,
-  @{HOME}/mail/.imap/** klrw,
+  @{DOVECOT_MAILSTORE}/ rw,
+  @{DOVECOT_MAILSTORE}/** rwkl,
+
+  @{HOME} r, # ???
   /usr/lib/dovecot/imap mr,
-  /var/mail/* klrw,
-  /var/spool/mail/* klrw,
 
   # Site-specific additions and overrides. See local/README for details.
   #include local/usr.lib.dovecot.imap

=== modified file 'profiles/apparmor.d/usr.lib.dovecot.pop3'
--- profiles/apparmor.d/usr.lib.dovecot.pop3	2011-08-26 23:12:10 +
+++ profiles/apparmor.d/usr.lib.dovecot.pop3	2013-12-13 12:49:33 +
@@ -1,6 +1,8 @@
 # Author: Kees Cook k...@ubuntu.com
 
 #include tunables/global
+#include tunables/dovecot
+
 /usr/lib/dovecot/pop3 {
   #include abstractions/base
   #include abstractions/nameservice
@@ -8,13 +10,10 @@
   capability setgid,
   capability setuid,
 
-  /var/mail/* klrw,
-  /var/spool/mail/* klrw,
-  @{HOME} r,
-  @{HOME}/mail/* klrw,
-  @{HOME}/mail/.imap/** klrw,
-  @{HOME}/Maildir/ rw,
-  @{HOME}/Maildir/** klrw,
+  @{DOVECOT_MAILSTORE}/ rw,
+  @{DOVECOT_MAILSTORE}/** rwkl,
+
+  @{HOME} r, # ???
   /usr/lib/dovecot/pop3 mr,
 
   # Site-specific additions and overrides. See local/README for details.

=== modified file 'profiles/apparmor.d/usr.sbin.dovecot'
--- profiles/apparmor.d/usr.sbin.dovecot	2013-01-02 23:34:38 +
+++ profiles/apparmor.d/usr.sbin.dovecot	2013-12-13 12:56:25 +
@@ -1,6 +1,8 @@
 # Author: Kees Cook k...@ubuntu.com
 
 #include tunables/global
+#include tunables/dovecot
+
 /usr/sbin/dovecot {
   #include abstractions/authentication
   #include abstractions/base
@@ -9,29 +11,42 @@
   #include abstractions/ssl_keys
 
   capability chown,
+  capability dac_override,
+  capability fsetid,
+  capability kill,
   capability net_bind_service,
   capability setgid,
   capability setuid,
   capability sys_chroot,
-  capability fsetid,
+
+
+
+  @{DOVECOT_MAILSTORE}/ rw,
+  @{DOVECOT_MAILSTORE}/** rwkl,
 
   /etc/dovecot/** r,
   /etc/mtab r,
   /etc/lsb-release r,
   /etc/SuSE-release r,
   @{PROC}/@{pid}/mounts r,
+  /usr/bin/doveconf rix,
+  /usr/lib/dovecot/anvil Px,
+  /usr/lib/dovecot/auth Px,
+  /usr/lib/dovecot/config Px,
   /usr/lib/dovecot/dovecot-auth Pxmr,
   /usr/lib/dovecot/imap Pxmr,
   /usr/lib/dovecot/imap-login Pxmr,
+  /usr/lib/dovecot/log Px,
+  /usr/lib/dovecot/managesieve Px,
+  /usr/lib

Re: [apparmor] [PATCH] profiles: rw file perms are now needed on AF_UNIX socket files

2013-12-22 Thread Christian Boltz
Hello,

Am Donnerstag, 19. Dezember 2013 schrieb Tyler Hicks:
 The AppArmor kernel now checks for both read and write permissions
 when a process calls connect() on a UNIX domain socket.
 
 The patch updates a four abstractions that were found to be needing
 changes after the kernel change.

Does this affect all sockets?

There are some more candidates I found while grepping through the profiles:

# grep -r ' w,' . |grep -v '/ w,'   # pid files, logs etc. manually removed 
from the list
./abstractions/nameservice:  /{,var/}run/avahi-daemon/socket w,
./abstractions/base:  /dev/log   w,
./abstractions/mdns:  /{,var/}run/mdnsd w,
./abstractions/apparmor_api/change_profile:@{PROC}/@{tid}/attr/{current,exec} w,
./abstractions/apache2-common:  @{PROC}/@{pid}/attr/current 
   w,
./abstractions/X:  /tmp/.X11-unix/*   w,
./usr.lib.dovecot.dovecot-auth:  /var/spool/postfix/private/dovecot-auth w,
./usr.sbin.winbindd:  /var/lib/samba/winbindd_privileged/pipe w,
./usr.sbin.winbindd:  /var/log/samba/log.winbindd-idmap w,
./usr.sbin.winbindd:  /{var/,}run/samba/winbindd/pipe w,
./sbin.syslogd:  /dev/tty* w,
./sbin.syslog-ng:  /dev/log w,
./sbin.syslog-ng:  /dev/syslog w,
./sbin.syslog-ng:  @{CHROOT_BASE}/var/lib/*/dev/log w,
./usr.sbin.nscd.orig:  /{,var/}run/avahi-daemon/socket w,
./usr.sbin.dovecot:  /var/spool/postfix/private/* w,
./usr.sbin.avahi-daemon:  /{,var/}run/avahi-daemon/socket w,

Do you think some of them need to be changed from w to rw? If yes, which ones?


Regards,

Christian Boltz
-- 
Gegen nachhaltige Zweifel, ob die SSL-Verschlüsselung in Windows
wirklich noch den erwarteten Schutz vor unerwünschten Lauschern bieten
kann, hilft damit letztlich nur der Wechsel des Betriebssystems.
[http://www.heise.de/ct/artikel/Microsofts-Hintertuer-1921730.html]


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


[apparmor] [patch] allow samba to create /var/run/samba/

2013-12-22 Thread Christian Boltz
Hello,

samba (nmbd and smbd) need to create /var/run/samba at startup
(at least on systems where /var/run is on a tmpfs)

References: https://bugzilla.novell.com/show_bug.cgi?id=856651

I propose this patch for trunk and 2.8

=== modified file 'profiles/apparmor.d/abstractions/samba'
--- profiles/apparmor.d/abstractions/samba  2013-11-20 00:11:01 +
+++ profiles/apparmor.d/abstractions/samba  2013-12-22 15:50:18 +
@@ -16,5 +16,6 @@
   /var/log/samba/cores/ rw,
   /var/log/samba/cores/** rw,
   /var/log/samba/log.* w,
+  /{,var/}run/samba/ w,
   /{,var/}run/samba/*.tdb rw,
 


Regards,

Christian Boltz
-- 
Du kannst dir einen Kernel so geschwaetzig eingestellt kompilieren, dass
die HDD kaum noch mit dem loggen hinterherkommt (was wiederum Bugs im
HDD-Treiber ausloesen koennte ;)) [David Haller in suse-linux]


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


Re: [apparmor] [patch] allow samba to create /var/run/samba/

2013-12-23 Thread Christian Boltz
Hello,

Am Sonntag, 22. Dezember 2013 schrieb Christian Boltz:
 samba (nmbd and smbd) need to create /var/run/samba at startup
 (at least on systems where /var/run is on a tmpfs)

It also needs to create /var/cache/samba/

 References: https://bugzilla.novell.com/show_bug.cgi?id=856651
 
 I propose this patch for trunk and 2.8

Updated patch:

=== modified file 'profiles/apparmor.d/abstractions/samba'
--- profiles/apparmor.d/abstractions/samba  2013-11-20 00:11:01 
+++ profiles/apparmor.d/abstractions/samba  2013-12-23 12:28:06 
@@ -12,9 +12,11 @@
   /etc/samba/* r,
   /usr/share/samba/*.dat r,
   /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r,
+  /var/cache/samba/ w,
   /var/lib/samba/**.tdb rwk,
   /var/log/samba/cores/ rw,
   /var/log/samba/cores/** rw,
   /var/log/samba/log.* w,
+  /{,var/}run/samba/ w,
   /{,var/}run/samba/*.tdb rw,
 

Regards,

Christian Boltz
-- 
  Wieso ich, ich habe 1,5 Mbit anbindung, ständig.
 Du? Oder dein Rechner? :-)
Was meinst du wie ich nach 1,500,000 Bit aussehe, glaubst du,
dann könnte ich noch schreiben, geschweige denn programmieren?
[ Ratti und Gerald Goebel in fontlinge-devel]


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


Re: [apparmor] AppArmor profile for LibreOffice

2013-12-25 Thread Christian Boltz
Hello,

Am Mittwoch, 25. Dezember 2013 schrieb Jonathan Davies:
 On 25/12/2013 16:23, Christian Boltz wrote:
  Am Mittwoch, 25. Dezember 2013 schrieb Jonathan Davies:
  I have created an AppArmor profile for LibreOffice and I would like
  to see it placed into the 14.04 packages.
  
  I had a short look at it. Some notes:
  audit deny network bluetooth,
  
  It seems this isn't allowed by any abstractions. What's the reason
  to
  explicitely deny it?
 
 I didn't want LibreOffice to talk on bluetooth, and it seems to open
 up a service there by default.

Sounds reasonable - and leaves me with the question if audit makes 
sense. (You already know it wants to do that, and you deny it - so why 
fill the logs?)

# abstractions/private-files-strict is in force from above.
owner @{HOME}/**   rwk,
  
  The usual problem of having an application with a save as...
  dialog ;-)
  
  I know there's some work done on a file dialog helper going (to
  avoid
  the need for such rules), but I don't know the details and if it's
  useable already.
 
 I don't see an issue here - I'm allowing full access to the home
 folder of the user, while private-files-strict is disallowing access
 to places such as ~/.{ssh,gnupg,mozilla}/*, etc. Trying opening or
 saving a file there and you'll find that access is denied.

The issue is that it allows full access to the home (with the private-
files-strict exceptions). It's the best we can do currently - I just 
wanted to mention that there might be a better solution in the future.

deny @{HOME}/.exec*   rwmx,
  
  What's the reason for this denial? Should it be part of an
  abstraction instead of having it in the profile?
 
 LibreOffice seems to try to write to these files but does nothing with
 them - so I decided to block it.

Ah, ok.

/usr/bin/bluetooth-sendto rmUx,
/usr/bin/lpr  rmUx,
/usr/bin/paperconfrmix,
/usr/bin/xdg-open rmUx,
  
  I'd recommend rmPUx instead of rmUx - if someone has a profile for
  one of them, it should be used.
 
 Someone needs to update the manpage, it says that this kind of mode
 mixing is incompatible.

PUx means: if a profile exists, use it (so Px) - but if no profile 
exists, fall back to Ux.

You are right - the apparmor.d manpage doesn't explain those fallback 
modes yet :-(  

I just submitted https://bugs.launchpad.net/apparmor/+bug/1264178 
to make sure it doesn't get lost in the holiday season ;-)


Regards,

Christian Boltz
-- 
Nicht das ich frei von Paranoia Schueben waere ;), aber wenn Dir das
passiert spiel sofort Lotto, bei dem Glueck bekommst Du bestimmt 4
Wochen den 6er mit Superzahl.   [Maik Holtkamp in suse-linux]


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


[apparmor] [patch] add FIPS support to abstractions/openssl

2014-01-03 Thread Christian Boltz
Hello,

patch description stolen from Lars Vogdt

The /proc/sys/crypto/fips_enabled r, should IMHO be integrated in the
upstream abstractions/openssl as this is not critical if you run without 
FIPS, but it will produce a lot of log entries on systems like SLES that 
are FIPS aware.

/stolen patch description

References: https://bugzilla.novell.com/show_bug.cgi?id=857122#c2


=== modified file 'profiles/apparmor.d/abstractions/openssl'
--- profiles/apparmor.d/abstractions/openssl2011-08-08 20:22:03
+++ profiles/apparmor.d/abstractions/openssl2014-01-03 18:07:23
@@ -10,4 +10,5 @@
 
   /etc/ssl/openssl.cnf r,
   /usr/share/ssl/openssl.cnf r,
+  @{PROC}/sys/crypto/fips_enabled r,
 


Regards,

Christian Boltz
-- 
I wonder how we ended up with baseurl and extra_url, now we are missing
one with a - like data-dir to violate consistency and the principle
of least surprise in all possible ways. 
[Duncan Mac-Vicar Prett in bnc#449842]


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


Re: [apparmor] aa-logprof doesn't check if user is root

2014-01-15 Thread Christian Boltz
Hello,

Am Mittwoch, 15. Januar 2014 schrieb Aaron Lewis:
 aa-logprof doesn't check if user is root
 
 Can someone add the verification please? just like aa-status and
 others

Well, not always ;-)

aa-logprof doesn't always need root permissions (well, except for 
reloading the profiles). You can easily run it as user when using -f 
/my/logfile -d /path/to/profiles/ (assuming the user has access to both 
/my/logfile and /path/to/profiles/). 

I know this isn't the typical usecase, but still something that should 
be possible. (However, maybe we should think about having the root check 
enabled by default, and add an option --no-profile-reload that also 
skips the root check.)

That said - feel free to test the rewritten tools available at 
https://code.launchpad.net/apparmor-profile-tools


Regards,

Christian Boltz
-- 
Weißt Du, man soll ja eigentlich keine Leute auf öffentlichen
Mailinglisten beschimpfen, sie kratzen oder ihnen Tiernamen geben.
Aber die traumwandlerische Sicherheit, mit der Du den relevanten Teil
des Logs weggeschnitten hast, ist schon beeindruckend.
Also, Du Hängebauchschwein, fühl Dich beschimpft und gekratzt ;-)
[Stefan Förster in postfixbuch-users]


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


Re: [apparmor] [patch 12/18] parser: add rlimit language acceptance tests

2014-01-16 Thread Christian Boltz
Hello,,

Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie:
 The parser was lacking language tests for rlimits. This test adds
 several, one for each rlimit type.
 
 Signed-off-by: Steve Beattie st...@nxnw.org

Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
Erstes Gesetz WWW: 
 Du mögest trennen die Spinnen und Indianer von den Usern und jedem
 sein eigen Grund und Heim zuteilen auf das der eine nicht neidisch
 werde auf den anderen und begehre dessen Heim und Gut. *lach*
 [Thomas Templin in suse-linux]


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


Re: [apparmor] [patch 13/18] parser: add rttime rlimit support

2014-01-16 Thread Christian Boltz
Hello,

Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie:
 This patch adds support for the rttime rlimit (aka RLIMIT_RTTIME),
 available since the 2.6.25 kernel, according to the getrlimit(2)
 man page; see that man page for more details on this rlimit.
 An acceptance test is also added.

 Index: b/parser/tst/simple_tests/rlimits/ok_rlimit_18.sd
 ===

 +profile rlimit {
 +  set rlimit rttime = 60minutes,
 +}

Does this also need an addition for apparmor.vim.in?
(and BTW, did you test if apparmor.vim displays all tests from 12/18 
correctly?)


Regards,

Christian Boltz
-- 
[SuSE 9.1] Und utf-8 saugt tote Hamster durch Strohhalme, selbst wenn
es funktioniert. [...] Und das alles nur, damit ich Klingonisch native
verarbeiten kann in meinem Rechner.
[http://blog.koehntopp.de/archives/317_Die+schlimmste+aller+Susen.html]


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


Re: [apparmor] [patch 15/18] utils: remove unneeded imports from a-easyprof and aa-sandbox

2014-01-16 Thread Christian Boltz
Hello,

Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie:
 Found by running pyflakes on these scripts.
 
 Signed-off-by: Steve Beattie st...@nxnw.org

Acked-by: Christian Boltz appar...@cboltz.de
(assuming pyflakes was right - and even if not, we'll notice the 
failures quickly ;-)


Regards,

Christian Boltz
-- 
 Ich dachte schon vor 2 Jahren, das Niveau ließe sich nicht mehr weiter
 senken, doch offensichtlich geht es tatsächlich auch unterirdisch noch
 weiter ...
Du hast den Nullpunkt flshca angesetzt.
[ Ralf Corsepius und Florian Gross in suse-linux über die Liste]


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


Re: [apparmor] [patch 16/18] utils: address pep8 complaints

2014-01-16 Thread Christian Boltz
Hello,

Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie:
 This patch eliminates the complaints from running:
 
   pep8 --ignore=E501 aa-easyprof vim/
 
 (E501 is 'line too long', which I'm not too chuffed about.
 )
 Mostly, it's a lot of whitespace touchups, with a few conversions from
 '==' to 'is'.
 
 Signed-off-by: Steve Beattie st...@nxnw.org
 ---
  utils/aa-easyprof|3
  utils/vim/create-apparmor.vim.py |  133
 +++ 2 files changed, 68
 insertions(+), 68 deletions(-)
 


 Index: b/utils/vim/create-apparmor.vim.py
 ===
 --- a/utils/vim/create-apparmor.vim.py
 +++ b/utils/vim/create-apparmor.vim.py
 @@ -73,28 +74,28 @@ for af_pair in af_pairs:
...
  aa_regex_map = {
  'FILENAME': filename,
 -'FILE':
 r'\v^\s*(audit\s+)?(deny\s+|allow\s+)?(owner\s+)?' + filename +
 r'\s+', # Start of a file rule +'FILE':
 r'\v^\s*(audit\s+)?(deny\s+|allow\s+)?(owner\s+)?' + filename +
 r'\s+',  # Start of a file rule # (whitespace_+_, owner etc. flag_?_,
 filename pattern, whitespace_+_) -'DENYFILE':
 r'\v^\s*(audit\s+)?deny\s+(owner\s+)?' + filename + r'\s+', # deny,
 otherwise like FILE +'DENYFILE':
 r'\v^\s*(audit\s+)?deny\s+(owner\s+)?' + filename + r'\s+',  # deny,
 otherwise like FILE 'auditdenyowner':  
 r'(audit\s+)?(deny\s+|allow\s+)?(owner\s+)?', -   
 'audit_DENY_owner': r'(audit\s+)?deny\s+(owner\s+)?', # must include
 deny, otherwise like auditdenyowner +'audit_DENY_owner':
 r'(audit\s+)?deny\s+(owner\s+)?',  # must include deny, otherwise
 like auditdenyowner 'auditdeny':   
 r'(audit\s+)?(deny\s+|allow\s+)?',
 -'EOL':  r'\s*,(\s*$|(\s*#.*$)\@=)', # End of a line
 (whitespace_?_, comma, whitespace_?_ comment.*) +'EOL':  
r'\s*,(\s*$|(\s*#.*$)\@=)',  # End of a line (whitespace_?_,
 comma, whitespace_?_ comment.*) 'TRANSITION':  
 r'(\s+-\\s+\S+)?',

Sorry for the terrible quoting, anyway:
Does it really make sense to have two spaces in front of # ?

 +#syn match  sdEntryM /@@DENYFILE@@(r|mk|x)+@@EOL@@/ 
 contains=sdGlob,sdComment 
 nextgroup=@sdEntry,sdComment,sdError,sdInclude
 -#syn match  sdEntryM /@@DENYFILE@@(r|m|k|x)+@@EOL@@/
 contains=sdGlob,sdComment 
 nextgroup=@sdEntry,sdComment,sdError,sdInclude

NAK - you broke the regex here (first search for the difference 
yourself, then have a look at [1] ;-)
(and yes, I'd like to have this un-broken even if it's only a comment)

With that fixed, 
Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz

[1] It probably becomes more obvious with the lines shortened:

 +#syn match  sdEntryM /@@DENYFILE@@(r|mk|x)+@@EOL@@/
 -#syn match  sdEntryM /@@DENYFILE@@(r|m|k|x)+@@EOL@@/ 
^^^
-- 
 ...womit die Geschichte wieder von vorn losgeht...
Jaha, aber nun ist das eine vollkommen andere Situation: Jetzt hast Du
nämlich eine von Suse generierte kdmrc für ein von Suse geliefertes KDE,
und KDE ist eine vom Installationssupport abgedeckte Komponente.
Also ist das ein meldefähiger und supportberechtigter Bug! :-)
[ Gerald Martin und Kristian Köhntopp in suse-linux]


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


Re: [apparmor] [patch] utils: fix apparmor.vim rlimits support (was Re: [patch 13/18] parser: add rttime rlimit support)

2014-01-17 Thread Christian Boltz
Hello,

Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie:
 On Fri, Jan 17, 2014 at 12:45:27AM +0100, Christian Boltz wrote:
  (and BTW, did you test if apparmor.vim displays all tests from 12/18
  correctly?)
 
 Apparently I missed all the incorrect highlighting vim gave me while
 creating those test cases, because no, apparmor.vim does not display
 many of them correctly. The following is a patch to address the
 shortcomings I found:
 
 Subject: utils: fix apparmor.vim rlimits support
 
 The rlimits syntax checking support in apparmor.vim was broken in
 various unhelpful ways:
[...]

Thanks!

Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
Yeah, life always gets in the way of the important stuff :-)
[Per Jessen in opensuse-project]


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


[apparmor] [patch] update winbindd profile

2014-01-19 Thread Christian Boltz
Hello,

this patch includes several updates for the winbindd profile that the 
openSUSE package collected over the last months.

- add abstractions/samba to usr.sbin.winbindd profile
  (and cleanup things that are included in the abstraction - the cleanup 
  part is not in the openSUSE package)
- add capabilities ipc_lock and setuid to usr.sbin.winbindd profile 
  (bnc#851131)
- updates for samba 4.x and kerberos (bnc#846586#c12 and #c15, 
  bnc#845867, bnc#846054)
- drop always-outdated Last Modified comment

References: see the bnc# above (they are bug numbers at 
bugzilla.novell.com)



=== modified file 'profiles/apparmor.d/usr.sbin.winbindd'
--- profiles/apparmor.d/usr.sbin.winbindd   2012-11-06 22:19:46
+++ profiles/apparmor.d/usr.sbin.winbindd   2014-01-19 15:56:00
@@ -1,33 +1,32 @@
-# Last Modified: Mon Mar 26 20:28:18 2012
 #include tunables/global
 
 /usr/sbin/winbindd {
   #include abstractions/base
   #include abstractions/nameservice
-
-  /etc/samba/dhcp.conf r,
+  #include abstractions/samba
+
+  deny capability block_suspend,
+
+  capability ipc_lock,
+  capability setuid,
+
   /etc/samba/passdb.tdb rwk,
   /etc/samba/secrets.tdb rwk,
   @{PROC}/sys/kernel/core_pattern r,
   /tmp/.winbindd/ w,
+  /tmp/krb5cc_* rwk,
   /usr/lib*/samba/idmap/*.so mr,
   /usr/lib*/samba/nss_info/*.so mr,
+  /usr/lib*/samba/pdb/*.so mr,
   /usr/sbin/winbindd mr,
-  /var/lib/samba/account_policy.tdb rwk,
-  /var/lib/samba/gencache.tdb rwk,
-  /var/lib/samba/gencache_notrans.tdb rwk,
-  /var/lib/samba/group_mapping.tdb rwk,
-  /var/lib/samba/messages.tdb rwk,
-  /var/lib/samba/netsamlogon_cache.tdb rwk,
-  /var/lib/samba/serverid.tdb rwk,
-  /var/lib/samba/winbindd_cache.tdb rwk,
-  /var/lib/samba/winbindd_privileged/pipe w,
-  /var/log/samba/cores/ rw,
-  /var/log/samba/cores/winbindd/ rw,
-  /var/log/samba/cores/winbindd/** rw,
-  /var/log/samba/log.wb-* w,
+  /var/cache/samba/*.tdb rwk,
+  /var/lib/samba/smb_krb5/krb5.conf.* rw,
+  /var/lib/samba/smb_tmp_krb5.* rw,
+  /var/lib/samba/winbindd_cache.tdb* rwk,
   /var/log/samba/log.winbindd rw,
   /{var/,}run/samba/winbindd.pid rwk,
+  /{var/,}run/samba/winbindd/ rw,
+  /{var/,}run/samba/winbindd/pipe w,
 
   # Site-specific additions and overrides. See local/README for 
details.
   #include local/usr.sbin.winbindd





Regards,

Christian Boltz
-- 
 auf meinem Rechen Suse 8.2 KDE 3.1.1, [...]
Hey, man kann SuSE inzwischen sogar auf einem Rechen installieren?
Wow, da muss ich morgen mal im Garten vorbei schauen... :-))
[ Bernhard Schimanski und Thomas Hertweck in suse-linux]


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


[apparmor] [patch] profiles/Makefile: make sure all profiles have #include local/...

2014-01-19 Thread Christian Boltz
Hello,

the following patch makes sure all profiles have a line
  #include local/...

BTW: All profiles pass this test :-)  but having an additional check 
can never hurt.


=== modified file 'profiles/Makefile'
--- profiles/Makefile   2013-01-05 00:33:41 +
+++ profiles/Makefile   2014-01-19 19:22:56 +
@@ -44,6 +44,7 @@
for profile in ${TOPLEVEL_PROFILES}; do \
fn=$$(basename $$profile); \
echo # Site-specific additions and overrides for '$$fn'  
${PROFILES_SOURCE}/local/$$fn; \
+   grep include\\s\\s*local/$$fn $$profile /dev/null || { 
echo $$profile doesn't contain #include local/$$fn ; exit 1; } ; \
done; \
 
 .PHONY: install



Regards,

Christian Boltz
-- 
   116: Programm
  Sobald eine Datei von einem Virus infiziert werden kann, ist
  es ein Programm. (Markus Kuhn)


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


Re: [apparmor] [patch 1/3] dovecot profiles: introduce tunables/dovecot

2014-01-23 Thread Christian Boltz
Hello,

Am Donnerstag, 23. Januar 2014 schrieb John Johansen:
 On 01/19/2014 08:58 AM, Christian Boltz wrote:
  this patch introduces tunables/dovecot (with @{DOVECOT_MAILSTORE})
  and replaces the mail storage location in various dovecot-related
  profiles with this variable.
  
  It also adds nice copyright headers (I hope I got the bzr log right
  ;-)
 a few comments inline
 
  === added file 'profiles/apparmor.d/tunables/dovecot'
  --- profiles/apparmor.d/tunables/dovecot1970-01-01 00:00:00 +
  +++ profiles/apparmor.d/tunables/dovecot2014-01-19 16:08:06
...
  +# @{DOVECOT_MAILSTORE} is a space-separated list of all directories
  +# where dovecot is allowed to store and read mails
  +#
  +# The default value is quite broad to avoid breaking existing
  setups. +# Please change @{DOVECOT_MAILSTORE} to (only) contain the
  directory +# you use, and remove everything else.
  +
  +@{DOVECOT_MAILSTORE}=@{HOME}/Maildir/ @{HOME}/mail/ @{HOME}/Mail/
  /var/vmail/ /var/mail/ /var/spool/mail/




  === modified file 'profiles/apparmor.d/usr.lib.dovecot.imap'
  --- profiles/apparmor.d/usr.lib.dovecot.imap2011-08-26 23:12:10
  + +++ profiles/apparmor.d/usr.lib.dovecot.imap  2014-01-19
...
  -  @{HOME} r,
  -  @{HOME}/Maildir/ rw,
  -  @{HOME}/Maildir/** klrw,
  -  @{HOME}/Mail/ rw,
  -  @{HOME}/Mail/* klrw,
  -  @{HOME}/Mail/.imap/** klrw,
  -  @{HOME}/mail/ rw,
  -  @{HOME}/mail/* klrw,
  -  @{HOME}/mail/.imap/** klrw,
  +  @{DOVECOT_MAILSTORE}/ rw,
  +  @{DOVECOT_MAILSTORE}/** rwkl,
 
 so this is slightly wider perms than
 
  -  @{HOME}/{m,M}ail/* klrw,
  -  @{HOME}/{m,M}ail/.imap/** klrw,
 
 is this what we want?

The idea of @{DOVECOT_MAILSTORE} is to allow the directories that were 
allowed in the old profile, and getting all profiles in sync so that for 
example IMAP and POP3 allow access to the same directory.

I know the list got quite long - but that's what you get from checking 
the current dovecot-related profiles. (Maybe also a location from a 
bugreport on bnc sneaked in, I'd have to check that ;-)

I'd happily shorten the list to just /var/vmail/, but I'm sure users 
would kill me for doing it ;-)

The perfect solution would be to auto-generate @{DOVECOT_MAILSTORE} from 
the dovecot config, but unfortunately that isn't as easy as it looks. 
(Proposals and scripts welcome ;-)


  +  @{HOME} r, # ???
 
 why the ???, not sure if this rule is required

above, you'll find
  -  @{HOME} r,

I'm also not sure if it's required (that's why I added ???), but 
wanted to keep it for backwards compability (there must be a reason why 
it's there ;-)

(If you are sure we can remove it, this should be a separate patch 
titled break the profile or so ;-)

 /usr/lib/dovecot/imap mr,
  
  -  /var/mail/* klrw,
  -  /var/spool/mail/* klrw,
 
 again a slight widening of permissions

Yes, see above.

  === modified file 'profiles/apparmor.d/usr.lib.dovecot.pop3'
  --- profiles/apparmor.d/usr.lib.dovecot.pop32011-08-26 23:12:10
  + +++ profiles/apparmor.d/usr.lib.dovecot.pop3  2014-01-19
  16:08:30 + @@ -1,6 +1,18 @@
...
  -  @{HOME} r,
  -  @{HOME}/mail/* klrw,
  -  @{HOME}/mail/.imap/** klrw,
  -  @{HOME}/Maildir/ rw,
  -  @{HOME}/Maildir/** klrw,
  +  @{DOVECOT_MAILSTORE}/ rw,
  +  @{DOVECOT_MAILSTORE}/** rwkl,
  +
  +  @{HOME} r, # ???
 
 again the change in allowed permissions

again see above ;-)


Regards,

Christian Boltz
-- 
how to use the -b   parameter ? 
You... type it in.
[ Jun Hu and Jan Engelhardt in opensuse-packaging]


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


Re: [apparmor] [patch 01/11] mod_apparmor: fix logging [v3]

2014-01-23 Thread Christian Boltz
Hello,

Am Donnerstag, 23. Januar 2014 schrieb Steve Beattie:
 On Thu, Jan 23, 2014 at 03:04:53AM -0800, John Johansen wrote:
  Looks good, though I did find myself wishing for a patch to rename
  immunix to apparmor.
 
 Yeah, as well as a patch to fix up some of the whitespace quirks (lots
 of trailing whitespace for one). But I wanted functional code changes

That, and also a funny mix of tabs and spaces in several lines.

 to land first before doing any of that, to make it easier to merge to
 2.8, if need be.

Personally, I'd like to have the fixes applied[1] to 2.8 ;-)  (maybe 
except the change to using aa_change_hatv to be very sure nothing 
breaks?)

Nevertheless, I'll probably take the risk and test 2.8 with the latest 
mod_apparmor.c as soon as you commit your patches to trunk. (I want one 
big patch, not copypaste from 11 mails all changing the same file ;-)

BTW: will the updated mod_apparmor also need 2.8 r2111? (libapparmor: 
fix aa_change_hat token format string)


That all said - how many lines are _not_ touched by your patch series? 
;-)


Regards,

Christian Boltz

[1] no need to write backport - the patches should apply without 
problems ;-)
-- 
Grundsaetzlich ist es so, dass was unter Linux als Alpha Bezeichnet
wird, einem Release unter Windows entspricht, die Beta Versionen
ungefaehrt Service Pack 2 entsprechen und Stabil unter linux keinen
Gegenpart in der Windows Welt hat .. :) [Stefan Onken in suse-linux]


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


Re: [apparmor] [patch 09/11] mod_apparmor: add logging for AAHatName/AADefaultHatName policy misconfig

2014-01-23 Thread Christian Boltz
Hello,

Am Donnerstag, 23. Januar 2014 schrieb Steve Beattie:
 It kind of points to a minor deficiency in aa_change_hatv()'s
 interface, in that you know you successfully changed to hat or not,
 but not which one.

That sounds like we should find a way to change that ;-)

Does aa_change_hatv internally know to which hat it switched?
If yes, I'd propose to add another function similar to aa_change_hatv 
that indicates the selected hat in its return value.

(aa_change_hat(2) says the return value is zero on success, so changing 
the return code of the existing function isn't a real option.)


Regards,

Christian Boltz
-- 
So... Hm... ich bin etwas aufgeschmissen.
How to troubleshoot without trouble?
[Ratti in fontlinge-devel]


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


Re: [apparmor] [patch 1/3] dovecot profiles: introduce tunables/dovecot

2014-01-26 Thread Christian Boltz
Hello,

Am Donnerstag, 23. Januar 2014 schrieb John Johansen:
 On 01/23/2014 06:37 AM, Christian Boltz wrote:
  Am Donnerstag, 23. Januar 2014 schrieb John Johansen:
  On 01/19/2014 08:58 AM, Christian Boltz wrote:
  this patch introduces tunables/dovecot (with @{DOVECOT_MAILSTORE})
  and replaces the mail storage location in various dovecot-related
  profiles with this variable.
  
  It also adds nice copyright headers (I hope I got the bzr log
  right
  ;-)
  
  a few comments inline
  
  === added file 'profiles/apparmor.d/tunables/dovecot'
  --- profiles/apparmor.d/tunables/dovecot  1970-01-01 00:00:00
  +++ profiles/apparmor.d/tunables/dovecot  2014-01-19 16:08:06
  
  ...
  
  +# @{DOVECOT_MAILSTORE} is a space-separated list of all
  directories
  +# where dovecot is allowed to store and read mails
  +#
  +# The default value is quite broad to avoid breaking existing
  setups. +# Please change @{DOVECOT_MAILSTORE} to (only) contain
  the
  directory +# you use, and remove everything else.
  +
  +@{DOVECOT_MAILSTORE}=@{HOME}/Maildir/ @{HOME}/mail/ @{HOME}/Mail/
  /var/vmail/ /var/mail/ /var/spool/mail/
  
  
  
  
  === modified file 'profiles/apparmor.d/usr.lib.dovecot.imap'
  --- profiles/apparmor.d/usr.lib.dovecot.imap  2011-08-26 
23:12:10
  + +++ profiles/apparmor.d/usr.lib.dovecot.imap2014-01-19
  
  ...
  
  -  @{HOME} r,
  -  @{HOME}/Maildir/ rw,
  -  @{HOME}/Maildir/** klrw,
  -  @{HOME}/Mail/ rw,
  -  @{HOME}/Mail/* klrw,
  -  @{HOME}/Mail/.imap/** klrw,
  -  @{HOME}/mail/ rw,
  -  @{HOME}/mail/* klrw,
  -  @{HOME}/mail/.imap/** klrw,
  +  @{DOVECOT_MAILSTORE}/ rw,
  +  @{DOVECOT_MAILSTORE}/** rwkl,
  
  so this is slightly wider perms than
  
  -  @{HOME}/{m,M}ail/* klrw,
  -  @{HOME}/{m,M}ail/.imap/** klrw,
  
  is this what we want?
  
  The idea of @{DOVECOT_MAILSTORE} is to allow the directories that
  were allowed in the old profile, and getting all profiles in sync
  so that for example IMAP and POP3 allow access to the same
  directory.
  
  I know the list got quite long - but that's what you get from
  checking the current dovecot-related profiles. (Maybe also a
  location from a bugreport on bnc sneaked in, I'd have to check that
  ;-)
  
  I'd happily shorten the list to just /var/vmail/, but I'm sure users
  would kill me for doing it ;-)
  
  The perfect solution would be to auto-generate @{DOVECOT_MAILSTORE}
  from the dovecot config, but unfortunately that isn't as easy as it
  looks. (Proposals and scripts welcome ;-)
 
 Not quite what I meant by widening of the perms (though I would love
 to have the list generated from the config too). The list for
 @{DOVECOT_MAILSTORE} is fine.
 
 What I meant was that for the case of @{HOME}/mail/ and @{HOME}/Mail/
 we used to have
   @{HOME}/{m,M}ail/* klrw,
   @{HOME}/{m,M}ail/.imap/** klrw,
 
 but we now have
   @{HOME}/{m,M}ail/** klrw,
 
 the difference being that we only allowed the recursive ** for the
 .imap dir.
 
 I just wanted to make sure this widening of permissions was
 intentional

More or less ;-)
I'd call it the price we have to pay to get it configurable in 
@{DOVECOT_MAILSTORE} - which also means permissions for ~/{m,M}ail can 
easily be removed. 

Besides that, only allowing the .imap/** subdirectory doesn't match the 
other allowed directories, especially ~/Maildir/** which we already have 
in the profile.


Regards,

Christian Boltz
-- 
 if ( ! ifdef $root ) { [...] }
ifdef?
Da hat einer zusammengerollte Makefiles geraucht...
[ Christian Boltz und Ratti in fontlinge-devel]


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


[apparmor] [patch] /usr/lib/dovecot/auth and mysql

2014-01-26 Thread Christian Boltz
Hello,

this patch is an interesting one - /usr/lib/dovecot/auth reads the mysql 
config files, which is not covered by abstractions/mysql.

Now the interesting question is where we should add this.

a) add it to abstractions/mysql because it belongs to mysql even if 
   /usr/lib/dovecot/auth is the only one that needs it 

b) add it to usr.lib.dovecot.auth because only /usr/lib/dovecot/auth
   is the only one that needs it

At the moment, I tend to b) to avoid superfluous permissions for other 
programs with abstractions/mysql, but I'd like to hear your opinions ;-)


=== modified file 'profiles/apparmor.d/usr.lib.dovecot.auth'
--- profiles/apparmor.d/usr.lib.dovecot.auth2014-01-26 21:46:51
+++ profiles/apparmor.d/usr.lib.dovecot.auth2014-01-26 22:36:47
@@ -23,6 +23,10 @@
   capability setgid,
   capability setuid,
 
+  /etc/my.cnf r,
+  /etc/my.cnf.d/ r,
+  /etc/my.cnf.d/*.cnf r,
+
   /etc/dovecot/dovecot-database.conf.ext r,
   /etc/dovecot/dovecot-sql.conf.ext r,
   /usr/lib/dovecot/auth mr,


Regards,

Christian Boltz
-- 
chliEßlichle sendi emeiSt Enleut ehier mehralsdreIpo Stingsa Mtag sOd
Asesdoch et. Waserm üdentwärdenkahnimmerrattentsumÜßenw aßIrge
nDeinezUs Ahmäst ell unkvonbU chst, abensagenw iel ;-) 
[Tilman Ahr in dcoulm zum Thema Rechtschreibfehler stoeren doch nicht]


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


[apparmor] [patch] usr.bin.dovecot profile

2014-01-26 Thread Christian Boltz
Hello,

after testing the dovecot profiles on a new server, I noticed 
/usr/sbin/dovecot needs some more permissions:
-mysql access
- execution permissions for /usr/lib/dovecot/dict and lmtp
- write access to some postfix sockets, used to
  - provide SMTP Auth via dovecot
  - deliver mails to dovecot via LMTP 
- and read access to /proc/filesystems


=== modified file 'profiles/apparmor.d/usr.sbin.dovecot'
--- profiles/apparmor.d/usr.sbin.dovecot2014-01-26 21:48:02 +
+++ profiles/apparmor.d/usr.sbin.dovecot2014-01-26 23:18:44 +
@@ -15,6 +15,7 @@
 /usr/sbin/dovecot {
   #include abstractions/authentication
   #include abstractions/base
+  #include abstractions/mysql
   #include abstractions/nameservice
   #include abstractions/ssl_certs
   #include abstractions/ssl_keys
@@ -33,13 +34,16 @@
   /etc/lsb-release r,
   /etc/SuSE-release r,
   @{PROC}/@{pid}/mounts r,
+  @{PROC}/filesystems r,
   /usr/bin/doveconf rix,
   /usr/lib/dovecot/anvil Px,
   /usr/lib/dovecot/auth Px,
   /usr/lib/dovecot/config Px,
+  /usr/lib/dovecot/dict Px,
   /usr/lib/dovecot/dovecot-auth Pxmr,
   /usr/lib/dovecot/imap Pxmr,
   /usr/lib/dovecot/imap-login Pxmr,
+  /usr/lib/dovecot/lmtp Px,
   /usr/lib/dovecot/log Px,
   /usr/lib/dovecot/managesieve Px,
   /usr/lib/dovecot/managesieve-login Pxmr,
@@ -50,6 +54,8 @@
   /usr/sbin/dovecot mrix,
   /var/lib/dovecot/ w,
   /var/lib/dovecot/* rwkl,
+  /var/spool/postfix/private/auth w,
+  /var/spool/postfix/private/dovecot-lmtp w,
   /{,var/}run/dovecot/ rw,
   /{,var/}run/dovecot/** rw,
   link /{,var/}run/dovecot/** - /var/lib/dovecot/**,



Regards,

Christian Boltz
-- 
Sorry, mit java kenne ich mich gar nicht aus, das ist mir einfach zu
unportabel.   [Thorsten Kukuk in suse-linux]


-- 
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 copypaste)

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...\n

[apparmor] systemd AppArmorProfile=

2014-02-01 Thread Christian Boltz
Hello,

I was just pointed to 
https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg15717.html
which contains a set of patches that seem to implement aa-exec-like
behaviour in systemd (and a note that there will be a fresh patch soon).

@Michael: 
I hope the mail address I found is the right one. If not, sorry for 
disturbing ;-)

Feel free to send your (updated) patches to the AppArmor mailinglist 
(in CC) to get a review of the AppArmor side of your patches.

BTW: It looks like your patch requires the profiles to be loaded already.
Do you have any plans for loading, reloading or removing profiles via 
systemd?


@all: 
Can someone have a look at those patches, please? (Even if it's clear 
that there will be a v2 ;-)


Regards,

Christian Boltz
-- 
 Manfred, Du solltest so spaet keine Emails mehr schreiben :-)
Danke für die Berichtigung, werd mir den Tipp hinter die Ohren schreiben
und nur noch Mailen, wenn ich die Augen zumindestens zu einem drittel 
aufkriege. [ Thomas Hertweck und Manfred Tremmel in suse-linux]


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


[apparmor] review r93..95

2014-02-01 Thread Christian Boltz
Hello,

the review for r93..95 of the new profile tools is attached.

Your commits fixed several bugs, but there are also cases where you 
replaced one bug with another ;-)

I also answered your question about trace handling in the aa-* tools, 
even if my answer where to handle it won't be too surprising ;-)


Regards,

Christian Boltz
-- 
Well, I guess, Stephan knows very well, what the fuzz is about: it's
about hundreds of patches, which will have to be regenerated, done as
an employment-creation measure for this lazy gang of packagers. 
[Hans-Peter Jansen in opensuse-packaging]

revno: 93
committer: Kshitij Gupta kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Sat 2014-02-01 06:14:05 +0530
message:
  Some bugfixes for UIYesNo to deny invalid keys, fix autodep when creating new 
profiles


=== modified file 'Tools/aa-audit'
--- Tools/aa-audit  2013-10-21 21:36:23 +
+++ Tools/aa-audit  2014-02-01 00:44:05 +


#[merged review for aa-audit r93..95]
#
# r95 message:
#   Fixed the sample --trace feature. Opinions on using it? and should it be 
implemented in every tool separately?

# not shocking users with a backtrace is a good idea.

# maybe you should only hide AppArmor exceptions (they could be called 
expected, and the error message in them is usually enough)
# and still display other (unexpected) exceptions (because we want to know 
where something fails unexpectedly)

# This should of course ;-) be handled in a central place (tools.py?) instead 
of adding it to every aa-*



=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2013-12-19 21:42:58 +
+++ apparmor/aa.py  2014-02-01 00:44:05 +
@ -418,7 +418,8 @@
 local_profile[hat]['flags'] = 'complain'
 
 created.append(localfile)
-
+changed.append(localfile)

# looks good, but...

# python Tools/aa-genprof ~cb/linuxtag/apparmor/scripts/hello 
# Traceback (most recent call last):
#   File Tools/aa-genprof, line 102, in module
# apparmor.autodep(program)
#   File /usr/lib/python2.7/site-packages/apparmor/aa.py, line 569, in autodep
# profile_data = create_new_profile(pname)
#   File /usr/lib/python2.7/site-packages/apparmor/aa.py, line 421, in 
create_new_profile
# changed.append(localfile)
# AttributeError: 'dict' object has no attribute 'append'

# hint (aa.py around line 90)
# changed = dict()
# created = []

@@ -865,34 +869,42 @@
 
 def build_x_functions(default, options, exec_toggle):
 ret_list = []
+fallback_toggle = False
 if exec_toggle:
 if 'i' in options:
 ret_list.append('CMD_ix')
 if 'p' in options:
 ret_list.append('CMD_pix')
-ret_list.append('CMD_EXEC_IX_OFF')
+fallback_toggle = True
-elif 'c' in options:
+if 'c' in options:
 ret_list.append('CMD_cix')
-ret_list.append('CMD_EXEC_IX_OFF')
+fallback_toggle = True
-elif 'n' in options:
+if 'n' in options:
 ret_list.append('CMD_nix')
+fallback_toggle = True
+if fallback_toggle:
 ret_list.append('CMD_EXEC_IX_OFF')

# placing CMD_EXEC_IX_OFF here has a small disadvantage - it will be offered 
left of CMD_ux (but should be right of it)

-elif 'u' in options:
+if 'u' in options:
 ret_list.append('CMD_ux')



=== modified file 'apparmor/common.py'
--- apparmor/common.py  2013-12-29 09:42:30 +
+++ apparmor/common.py  2014-02-01 00:44:05 +
@@ -201,9 +201,16 @@
 
+def user_perm(prof_dir):

# the function name is not too helpful
# what about is_writeable()?
# (or must_be_writeable() and let it raise an exception if prof_dir is not 
writeable, see below)

+if not os.access(prof_dir, os.R_OK):

# [should check for W_OK, already fixed in r94]

+sys.stdout.write(Cannot write to profile directory.\n +
+ Please run as a user with appropriate permissions. )

# please make the error message translatable
# I'd also mention prof_dir in the message.
# To make the function useable for everything (not just profile dir), change 
the message to   Cannot write to %s




 class DebugLogger(object):
 def __init__(self, module_name=__name__):
...   
+try:
+logging.basicConfig(filename=self.logfile, 
level=self.debug_level,
+format='%(asctime)s - %(name)s - 
%(message)s\n')
+except OSError:
# looks good, but fails with py2
# - use except IOError: instead, that works with py2 and py3

+# Unable to open the default logfile, so create a temporary 
logfile and tell use about it
# ... tell use_r_ about it
+import tempfile
+templog = tempfile.NamedTemporaryFile('w', prefix='apparmor', 
suffix='.log' ,delete=False

Re: [apparmor] systemd AppArmorProfile=

2014-02-02 Thread Christian Boltz
Hello,

Am Sonntag, 2. Februar 2014 schrieb Michael Scherer:
 Le samedi 01 février 2014 à 18:18 +0100, Christian Boltz a écrit :

  BTW: It looks like your patch requires the profiles to be loaded
  already. Do you have any plans for loading, reloading or removing
  profiles via systemd?
 
 I had plan to look on how Suse is doing this, but the only way i found
 after a quick look was t run a external binary, and I think that's
 something that should be avoided at least in systemd. I also didn't
 found a potential C library to do that.

The current way is the /etc/init.d/boot.apparmor initscript, which calls 
code in /lib/apparmor/rc.apparmor.functions, which finally loads the 
profiles using apparmor_parser.

AppArmor 3.0 (not released yet) will make it a bit easier - 
apparmor_parser will be able to load all profiles in /etc/apparmor.d/ at 
once, instead of having to load one profile after the other. This means 
(re)loading all profiles can be done with
apparmor_parser -r /etc/apparmor.d/
Maybe you need some additional options, but you should get the point. 
Also note that this way didn't get much testing yet.

I slightly ;-) doubt if it's a good idea to re-invent apparmor_parser 
inside systemd, and calling it as external binary doesn't sound too bad 
to me. (Hey, it worked without problems for the last 10 years ;-)

If you really want a library, the best way is probably to convert most 
of apparmor_parser into a library. However, I'm afraid this will need 
some[tm] time.

 Well, I have the v2 already, i just didn't found time to really test
 it with a VM before sending it.

Ah, the usual ENOTIME ;-)


Regards,

Christian Boltz
-- 
Please resolve this as NOT A BUG and USER SHOULD HAVE MORE COFFEE BEFORE
FILING BUGS.  I apologize for taking up valuable developer time!
[Jon Nelson in https://bugzilla.novell.com/show_bug.cgi?id=776271#c2]


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


Re: [apparmor] [Branch ~apparmor-dev/apparmor/master] Rev 2363: Move short_options next to long_options to make them easier to keep in sync

2014-02-05 Thread Christian Boltz
Hello,

Am Mittwoch, 5. Februar 2014 schrieben Sie:
 
 revno: 2363
 committer: John Johansen john.johan...@canonical.com
 branch nick: apparmor
 timestamp: Tue 2014-02-04 20:56:17 -0500
 message:
   Move short_options next to long_options to make them easier to keep
 in sync 


 === modified file 'parser/parser_main.c'
[...]

 === modified file 'parser/parser_regex.c'
 --- parser/parser_regex.c 2014-01-24 18:47:42 +
 +++ parser/parser_regex.c 2014-02-05 01:56:17 +
 @@ -493,8 +493,6 @@
   if ((entry-mode  AA_USER_SHIFT)  AA_EXEC_INHERIT)
   entry-mode |= AA_EXEC_MMAP  AA_USER_SHIFT;
 
 - /* relying on ptrace and change_profile not getting merged earlier
 */ -
   /* the link bit on the first pair entry should not get masked
* out by a deny rule, as both pieces of the link pair must
* match.  audit info for the link is carried on the second
 @@ -556,19 +554,6 @@
   if (!aare_add_rule_vec(dfarules, 0, AA_ONEXEC, 0, index, vec,
 dfaflags)) return FALSE;
   }
 - if (entry-mode  (AA_USER_PTRACE | AA_OTHER_PTRACE)) {
 - int mode = entry-mode  (AA_USER_PTRACE | AA_OTHER_PTRACE);
 - if (entry-ns) {
 - const char *vec[2];
 - vec[0] = entry-ns;
 - vec[1] = entry-name;
 - if (!aare_add_rule_vec(dfarules, 0, mode, 0, 2, vec, 
dfaflags))
 - return FALSE;
 - } else {
 -   if (!aare_add_rule(dfarules, entry-name, 0, mode, 0, 
dfaflags))
 - return FALSE;
 - }
 - }
   return TRUE;
  }

This part doesn't look related to short options ;-)


Regards,

Christian Boltz
-- 
 Henne, did you actually test this before closing the bug as invalid?
of course i did not test it. do you think i'm bored?
[ Christian Boltz and Hendrik Vogelsang in 
 https://bugzilla.novell.com/show_bug.cgi?id=420972]


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


[apparmor] new profile tools: preserve full initial comment

2014-02-05 Thread Christian Boltz
Hello,

while playing with aa-cleanprof, I noticed only the last line of the 
initial comment was preserved.

This patch
- preserves the complete initial comment
- makes sure whitespace inside the comment is kept (except leading 
  whitespace - line.trim() is still applied).
- no longer removes the # vim:syntax line

Note: I didn't test if handling the REPOSITORY line still works (in
theory it should), but without a working repo, I don't care too much ;-)
BTW: It might be a good idea to use a different variable name for the 
result of line.split() to avoid confusion.


I also noticed you didn't commit my patch for handling (F)inish I sent
a week ago.

Kshitij, if you like my patches, please commit them - I'd like to have 
a more readable bzr diff again soon ;-)


=== modified file 'apparmor/aa.py'
--- apparmor/aa.py  2014-02-01 00:44:05 +
+++ apparmor/aa.py  2014-02-05 22:30:08 +
@@ -2937,10 +2966,12 @@
 elif line[0] == '#':
 # Handle initial comments
 if not profile:
-if line.startswith('# vim:syntax') or line.startswith('# Last 
Modified:'):
+#if line.startswith('# vim:syntax') or line.startswith('# Last 
Modified:'):
+if line.startswith('# Last Modified:'):
 continue
-line = line.split()
-if len(line)  1 and line[1] == 'REPOSITORY:':
+elif line.startswith('# REPOSITORY'): # TODO: allow any number 
of spaces/tabs
+#if len(line)  1 and line[1] == 'REPOSITORY:':
+line = line.split()
 if len(line) == 3:
 repo_data = {'neversubmit': True}
 elif len(line) == 5:
@@ -2948,7 +2979,7 @@
  'user': line[3],
  'id': line[4]}
 else:
-initial_comment = ' '.join(line) + '\n'
+initial_comment = initial_comment + line + '\n'
 
 else:
 raise AppArmorException(_('Syntax Error: Unknown line found in 
file: %s line: %s') % (file, lineno+1))




Regards,

Christian Boltz
-- 
cboltz jjohansen: you are making it too easy for kshitij8 ;-)
jjohansen cboltz: oops sorry, now I'll have to come up with a new task
to make him suffer :)
sarnold review the c++11 conversion? :)
* sarnold runs
jjohansen haha, sarnold I said suffer, not drive him to commit suicide
[from #apparmor]


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


Re: [apparmor] [patch 1/8] chromium-browser profile

2014-02-12 Thread Christian Boltz
Hello,

Am Dienstag, 11. Februar 2014 schrieb Seth Arnold:
 Author: Jamie Strandboge ja...@canonical.com
 Description: chromium-browser profile
 Forwarded: yes
 
 ---
  profiles/apparmor.d/usr.bin.chromium-browser |  221

Just to make sure I understand this correct - you propose to add this 
profile to bzr trunk to the set of default profiles, right?

Short summary: The profile contains some restrictions that will result 
in quite some annoyed users (especially the restriction to ~/Public and 
~/Downloads). Therefore I'm not sure if it should be in the set of 
profiles that are enabled by default.

I'm thinking about introducing an apparmor-profiles-paranoid package 
(with a big warning that it _will_ break what a typical user often does) 
since some time - maybe this profile would be a reason to finally do it 
;-)

See below for more details.

 Index: b/profiles/apparmor.d/usr.bin.chromium-browser
 ===
 --- /dev/null
 +++ b/profiles/apparmor.d/usr.bin.chromium-browser
 @@ -0,0 +1,221 @@
 +# Author: Jamie Strandboge ja...@canonical.com
 +#include tunables/global
 +
 +# We need 'flags=(attach_disconnected)' in newer chromium versions
 +/usr/lib/chromium-browser/chromium-browser
 flags=(attach_disconnected) {
 +  #include abstractions/audio
 +  #include abstractions/cups-client
 +  #include abstractions/dbus-session

just curious - would dbus-session-strict be enough?

 +  #include abstractions/gnome
 +  #include abstractions/ibus
 +  #include abstractions/nameservice
 +  #include abstractions/user-tmp
 +
 +  # This include specifies which ubuntu-browsers.d abstractions to
 use. Eg, if +  # you want access to productivity applications, adjust
 the following file +  # accordingly.
 +  #include abstractions/ubuntu-browsers.d/chromium-browser

Users of other distributions will *love* ubuntu-browsers.d ;-)

I know that it's only a name, nevertheless it would be a good idea to 
rename it (not the most urgent problem, but... ;-)

[...]
 +  # Default profile allows downloads to ~/Downloads and uploads from
 ~/Public 

This comment is wrong - uploads are allowed from ~/Public/ and 
~/Downloads/ ;-)

That said: yes, I know this setup is very secure, but I'm also sure it 
will cause some ;-) bugreports like I can't download files to 
~/coolstuff

The perfect solution would be to wait for the content helper - what's 
the current status there?

 +  owner @{HOME}/ r,
 +  owner @{HOME}/Public/ r,
 +  owner @{HOME}/Public/* r,
 +  owner @{HOME}/Downloads/ r,
 +  owner @{HOME}/Downloads/* rw,
 +
 +  # Helpers
 +  /usr/bin/xdg-open ixr,
 +  /usr/bin/gnome-open ixr,
 +  /usr/bin/gvfs-open ixr,
 +  # TODO: kde, xfce

Oh nice - this TODO will result in the next flood of bugreports 
(according to a survey  70% of the openSUSE users use KDE as their 
desktop - guess how many annoyed users and bugreports that means...)

Hint: For KDE, it is probably /usr/bin/kde-open

 +  profile xdgsettings {
[...]
 +# Setting the default browser
[...]
 +owner @{HOME}/.local/share/applications/ w,

Hmm, why write permissions for the directory?

 +owner @{HOME}/.local/share/applications/mimeapps.list* rw,

Personally, I'd say a browser should never be allowed to change the 
default browser (and I'd even forbid to check if it is the current 
default browser - I'm not the biggest fan of the hey, I'm not your 
default browser warnings ;-)

Additionally, there's a chance that malicious code changes the default 
application for a file the user just downloaded, which could in theory 
cause some delayed remote code execution (somewhat similar to stored 
XSS)

 +  }
 +
 +  # Site-specific additions and overrides. See local/README for
 details. 
 +  #include local/usr.bin.chromium-browser

Hiding this #include between two child profiles is, hmm, interesting ;-)
Can you move it to a more visible place, please? (like the end of the 
main profile, above the child profiles)

 +profile chromium_browser_sandbox {
[...]
 +# *Sigh*
 +capability sys_ptrace,

Nice comment, but not too useful for the average user...


Regards,

Christian Boltz
-- 
Graphisch??? Wie meinen? Hast du zuviel Fleisch von zu gluecklichen
Rindern gefuttert? *scnr*  Wozu zum Henker sollte man sowas brauchen?
Logo ginge auch per ASCII :)  (Logo?  welches Logo? Wozu ueberhaupt?)
[David Haller in suse-linux]


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


[apparmor] [patch] update abstractions/winbind

2014-02-14 Thread Christian Boltz
Hello,

abstractions/winbind needs an update:
- some *.dat files live in a different directory nowadays (at least in 
  openSUSE)
- the openSUSE smb.conf includes the (autogenerated) dhcp.conf, so this
  file also needs to be readable.

References: https://bugzilla.novell.com/show_bug.cgi?id=863226

I also propose this patch for 2.8


=== modified file 'profiles/apparmor.d/abstractions/winbind'
--- profiles/apparmor.d/abstractions/winbind2011-11-01 17:35:29
+++ profiles/apparmor.d/abstractions/winbind2014-02-14 18:34:15
@@ -13,7 +13,9 @@
   /tmp/.winbindd/pipe  rw,
   /var/{lib,run}/samba/winbindd_privileged/pipe rw,
   /etc/samba/smb.conf r,
+  /etc/samba/dhcp.confr,
   /usr/lib*/samba/valid.dat   r,
   /usr/lib*/samba/upcase.dat  r,
   /usr/lib*/samba/lowcase.dat r,
+  /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r,
 



Regards,

Christian Boltz
-- 
Planung ist der Ersatz des Zufalls durch den Irrtum.


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


Re: [apparmor] [PATCH 0/2] Preliminary XDG user dir support

2014-02-14 Thread Christian Boltz
Hello,

Am Freitag, 14. Februar 2014 schrieb Jamie Strandboge:
 Add basic support for XDG user dirs:
 
 1. Update profiles/apparmor.d/tunables/global to include
 xdg-user-dirs. 2. Create the xdg-user-dirs tunable using the default
 'C' xdg-user-dirs values: $ cat
 profiles/apparmor.d/tunables/xdg-user-dirs
 # Define the common set of XDG user directories (usually defined in
 # /etc/xdg/user-dirs.defaults)
 @{XDG_DESKTOP_DIR}=Desktop
 @{XDG_DOWNLOAD_DIR}=Downloads
 @{XDG_TEMPLATES_DIR}=Templates
 @{XDG_PUBLICSHARE_DIR}=Public
 @{XDG_DOCUMENTS_DIR}=Documents
 @{XDG_MUSIC_DIR}=Music
 @{XDG_PICTURES_DIR}=Pictures
 @{XDG_VIDEOS_DIR}=Videos
 
 # Also, include files in tunables/xdg-user-dirs.d for site-specific
 adjustments # to the various XDG directories
 #include tunables/xdg-user-dirs.d
 
 3. Add profiles/apparmor.d/tunables/xdg-user-dirs.d/site.local with
 commented out examples on how to use the directory.

Acked-By: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
Weil es sehr weit verbreitet ist, eingespielt und überall drauf.
Die weite Verbreitung ist allenfalls geeignet, die kaputte Syntax
auszugleichen, ein Erfordernis also, kein Pluspunkt.
[ Ratti und Thorsten Haude in suse-linux zur Frage Warum procmail?]


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


[apparmor] [patch] dnsmasq profile - NetworkManager integration

2014-02-17 Thread Christian Boltz
Hello,

the dnsmasq profile needs some more permissions for NetworkManager 
integration.

This is an updated version of the previous dnsmasq profile patch, again 
from develop7 [at] develop7.info

I propose this patch for trunk and the 2.8 branch.


=== modified file 'profiles/apparmor.d/usr.sbin.dnsmasq'
--- profiles/apparmor.d/usr.sbin.dnsmasq2014-01-17 19:58:21
+++ profiles/apparmor.d/usr.sbin.dnsmasq2014-02-17 21:31:53
@@ -29,6 +29,8 @@
   /etc/dnsmasq.d/ r,
   /etc/dnsmasq.d/* r,
   /etc/ethers r,
+  /etc/NetworkManager/dnsmasq.d/ r,
+  /etc/NetworkManager/dnsmasq.d/* r,
 
   /usr/sbin/dnsmasq mr,
 
@@ -56,6 +58,7 @@
   /{,var/}run/nm-dns-dnsmasq.conf r,
   /{,var/}run/sendsigs.omit.d/*dnsmasq.pid w,
   /{,var/}run/NetworkManager/dnsmasq.conf r,
+  /{,var/}run/NetworkManager/dnsmasq.pid w,
 
   # Site-specific additions and overrides. See local/README for 
details.
   #include local/usr.sbin.dnsmasq



Regards,

Christian Boltz
-- 
|#|Die drei wichtigsten Tugenden eines Programmierers:
|#| Faulheit, Ungeduld und Selbstüberschätzung


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


Re: [apparmor] new profile tools - review of merging branch

2014-02-17 Thread Christian Boltz
Hello,

Am Sonntag, 16. Februar 2014 schrieb Kshitij Gupta:
 Sorry for the delayed response. I wasn't well and have been barely
 crawling in and out of bed.

That's why laptops were invented - you can move them next to your 
bed ;-))

I hope you get well soon!

 I'm starting to catch up all the mails and patches. Please bear with
 me.
 
 Btw @cboltz, you have the commit rights I suppose. Always feel free to
 commit patches. (They can always be reverted ;) )

AFAIK, I don't have write access to the apparmor-profile-tools repo - 
but that doesn't matter too much.

In the meantime, Steve merged your code into the official repo, with 
some changes and fixes applied [1]. This means the latest code is in the 
official repo (in other words: the apparmor-profile-tools repo is 
outdated ;-) and the development should continue in the apparmor repo.
(I'd propose that you add a notice about this to the apparmor-profile-
tools repo's README.md and then switch over to the apparmor repo.)

This also means that everybody with commit access can commit (after 
following the usual review policy - send the patch to the mailinglist 
and wait for someone to send an ACK before you commit it).

Speaking about commit access - it would be a good idea to give you 
commit access to the apparmor repo ;-)
@Steve or John: can you do that, please?


Regards,

Christian Boltz

[1] some of Steve's changes were quite big, like several whitespace 
changes to make PEP8 happy. This also means you'll get an unreadable 
diff ;-)

bzr log -r 2383 -n0 | less
will tell you what Steve did (commit messages), and something like
bzr diff -r2368.1.21..2368.1.22
will show you what exactly he changed between the two specified 
revisions.

-- 
 ich habe mir gerade eine DVD mit meinen Daten geschrieben, scheint
 aber nicht lesbar zu sein.
Wie genau hast du sie denn geschrieben?
- Mit einem Stift: Lösung: Hm. In ca 30 cm vor die Nase halten und
  lesen.  [ Anca Tibor Attila und Martin Ereth in suse-linux]


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


Re: [apparmor] new profile tools - review of merging branch

2014-02-17 Thread Christian Boltz
Hello,

Am Freitag, 14. Februar 2014 schrieb Steve Beattie:
 On Sat, Feb 15, 2014 at 12:36:03AM +0100, Christian Boltz wrote:

  I also noticed my patches
  - new profile tools: preserve full initial comment
  - new profile tools - handling of (F)inish
  are not included yet. Can you please review and (hopefully) merge
  them?
 I'm hoping to, yes, but it requires a bit more deep understanding of
 the issues and code then I've got at the moment. Sorry.

Hmm, I thought you read all the code you merge, didn't you? ;-)

You can check both issues by testing (with and without the patches 
applied) - and my patch descriptions should explain what to test ;-)

  +++ utils/apparmor/translations.py
 
 [snip]
 
  +
  +__apparmor_gettext__ = None
  +
  +def init_translation():
  +global __apparmor_gettext__
  +if __apparmor_gettext__ is None:
  +t = gettext.translation(TRANSLATION_DOMAIN, fallback=True)
  +__apparmor_gettext__ = t.gettext
  +return __apparmor_gettext__
  
  # what's the reason to make __apparmor_gettext__ global?
 
 In python, globals are a bit different than other languages. Globals
 are local to the module (i.e. have the scope of the file/module,
 translations.py in this case). Also, you can read their value freely,
 if there's no variable with the same name declared in a scope more
 local than the module (i.e. declared in the current function).
 But in order to modify the value and have that change be reflected
 outside of the scope of the function, it needs the global declaration;
 otherwise the change stays local to the function and is lost when the
 function ends.

Thanks for explaining the details. Only needing the global declaration 
for writing sounds like an interesting[tm] design decision (even PHP is 
better here - it requires global for read and write access to global 
variables) - but I'm afraid we won't get that changed in python ;-)

This still doesn't explain why you didn't make __apparmor_gettext__ 
local to the function.

The only reason I can imagine is a bit of performance tuning if more 
than one script or module needs translations (the second one will get a 
cached instance of t.gettext).

  +++
  /home/cb/apparmor/sbeattie-gsoc-pre-merge/pre-merger-cleanups/Testi
  ng/easyprof.conf 2014-02-11 18:48:23.035073000 +0100 @@ -1,5 +1,5 @@
  
   # Location of system policygroups
  
  -POLICYGROUPS_DIR=./policygroups
  +POLICYGROUPS_DIR=/usr/share/apparmor/easyprof/policygroups
  
   # Location of system templates
  
  -TEMPLATES_DIR=./templates
  +TEMPLATES_DIR=/usr/share/apparmor/easyprof/templates
  
  
  # I'd understand this change for the system-wide config, but in
  /test/ ?
 I think you're a bit confused because of the wonky way
 merging has happened, but that's how it was on kshitij's branch:
 http://bazaar.launchpad.net/~kgupta8592/apparmor-profile-tools/trunk/v
 iew/head:/Testing/easyprof.conf Please note that I reverted that
 particular change when I
 merged in Kshitij's branch into my to-be-merged-into-trunk
 branch, which from the VCS point of view ended up being a NOP:

Right, in the merged result it is ./ so everything is fine :-)


  === modified file 'utils/apparmor/aa.py'

snipped - I'll let Kshitij answer the technical details ;-)


Regards,

Christian Boltz
-- 
wie jeder weiß ist Debian auf ISDN die langsamste bekannte Methode
Selbstmord zu begehen (Selbstmord durch Erosion)
[http://blog.koehntopp.de/archives/113-Debian-ist-doch-schlecht..html]


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


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

2014-02-24 Thread Christian Boltz
Hello,

[patch v2, see below]

Am Montag, 27. Januar 2014 schrieb Christian Boltz:
 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 
 copypaste)

Ignore the except ... part - that's fixed in the v2 patch ;-)

 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?


Here's an updated version of the patch that applies to current bzr trunk.

=== modified file 'utils/apparmor/aa.py'
--- utils/apparmor/aa.py2014-02-24 18:20:11 +
+++ utils/apparmor/aa.py2014-02-24 18:42:03 +
@@ -498,6 +498,10 @@
 
 ans = ''
 while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in ans:
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 ans, arg = aaui.UI_PromptUser(q)
 p = profile_hash[options[arg]]
 q['selected'] = options.index(options[arg])
@@ -990,6 +994,10 @@
 
 ans = aaui.UI_PromptUser(q)
 
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 transitions[context] = ans
 
 if ans == 'CMD_ADDHAT':
@@ -1251,6 +1259,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'
@@ -1550,6 +1563,11 @@
 done = False
 while not done:
 ans, selected = aaui.UI_PromptUser(q)
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 # Ignore the log entry
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
@@ -1794,6 +1812,10 @@
 
 ans, selected = aaui.UI_PromptUser(q)
 
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
 break
@@ -1945,6 +1967,11 @@
 done = False
 while not done:
 ans, selected = aaui.UI_PromptUser(q)
+
+if ans == 'CMD_FINISHED':
+save_profiles()
+return;
+
 if ans == 'CMD_IGNORE_ENTRY':
 done = True
 break

=== modified file 'utils/apparmor/ui.py'
--- utils/apparmor/ui.py2014-02-13 18:01:03 +
+++ utils/apparmor/ui.py2014-02-24 18:32:40 +
@@ -288,10 +288,6 @@
 if cmd == 'CMD_ABORT':
 confirm_and_abort()
 cmd = 'XXXINVALIDXXX'
-elif cmd == 'CMD_FINISHED':
-if not params:
-confirm_and_finish()
-cmd = 'XXXINVALIDXXX'
 return (cmd, arg)
 
 def confirm_and_abort():
@@ -317,9 +313,6 @@
 })
 ypath, yarg = GetDataFromYast()
 
-def confirm_and_finish():
-sys.stdout.write(_('FINISHING...\n'))
-sys.exit(0)
 
 def Text_PromptUser(question):
 title = question['title']



Regards,

Christian Boltz
-- 
Wenn ich mir ein System installiere, welches dieses oder jenes oder
welches für mich tut (gegen mich?), dann kann ich gleich Windows
nehmen. Das fährt sich so gut, wie ein elektronischer Chauffeur das
eben kann: Nämlich bis die Elektronik ein Dixi-Klo mit 'ner Garage
verwechselt: Eckig und die Tür war offen.   [Ratti in suse-linux

[apparmor] [patch] common.py: add debugging, py2 compat fix

2014-02-24 Thread Christian Boltz
Hello,

this patch fixes two (unrelated) issues in common.py:
- it adds some debug logging in valid_path()
- it fixes a py2 incompability in DebugLogger.__init__


=== modified file 'utils/apparmor/common.py'
--- utils/apparmor/common.py2014-02-12 23:54:00 +
+++ utils/apparmor/common.py2014-02-24 18:55:21 +
@@ -112,6 +112,7 @@
 return False
 
 if '' in path:  # We double quote elsewhere
+debug(%s (contains quote) % (m))
 return False
 
 try:
@@ -228,7 +229,7 @@
 try:
 logging.basicConfig(filename=self.logfile, 
level=self.debug_level,
 format='%(asctime)s - %(name)s - 
%(message)s\n')
-except OSError:
+except IOError:
 # Unable to open the default logfile, so create a temporary 
logfile and tell use about it
 import tempfile
 templog = tempfile.NamedTemporaryFile('w', prefix='apparmor', 
suffix='.log', delete=False)


Regards,

Christian Boltz
-- 
*pieps* Die Verkehrshinweise: Im Netzwerkkabel von Marc 100 MB Stau
wegen einer Vollsperrung der Ausfahrt Festplatte. Bitte warten Sie auf
dem Rasthof FTP-Server. *pieps*
[Christian Boltz, gefunden von D. Haller]


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


[apparmor] [patch] libapparmor README

2014-02-24 Thread Christian Boltz
Hello,

this patch updates the bugtracker link in the libapparmor README.

=== modified file 'libraries/libapparmor/README'
--- libraries/libapparmor/README2008-05-19 22:48:31 +
+++ libraries/libapparmor/README2014-02-24 20:45:19 +
@@ -1,1 +1,3 @@
-What little documentation exists is in src/aalogparse.h.  Please file bugs 
using http://bugzilla.novell.com under the AppArmor product.
+What little documentation exists is in src/aalogparse.h.
+
+Please file bugs using https://bugs.launchpad.net/apparmor/+filebug



Regards,

Christian Boltz
-- 
By the way, it's a sign of how good the distribution is that we're
arguing about the name and not dealing with problems in the essence of
the thing. Even the overloaded servers are what an old boss of mine
would have called a success problem. [Randall Schulz in opensuse]


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


Re: [apparmor] [patch] utils: fix cmd reference in apparmor/tools.py

2014-02-24 Thread Christian Boltz
Hello,

Am Montag, 24. Februar 2014 schrieb Steve Beattie:
 This patch fixes up the parser command invocation via
 apparmor/common.py:cmd(), as it handles stdout/stderr redirection,
 and the redirection that was being attempted were being handed as
 arguments to the parser.

Nice, good catch!

 (As an aside, we generally try to avoid invoking the shell when
 running external commands, to avoid shell quoting issues.)
 
 Signed-off-by: Steve Beattie st...@nxnw.org
 ---
  utils/apparmor/tools.py |5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
 
 Index: b/utils/apparmor/tools.py
 ===
 --- a/utils/apparmor/tools.py
 +++ b/utils/apparmor/tools.py
 @@ -16,7 +16,7 @@ import sys
 
  import apparmor.aa as apparmor
  import apparmor.ui as aaui
 -from apparmor.common import user_perm
 +from apparmor.common import user_perm, cmd
 
  # setup module translations
  from apparmor.translations import init_translation
 @@ -118,8 +118,7 @@ class aa_tools:
  # One simply does not walk in here!
  raise apparmor.AppArmorException('Unknown
 tool: %s' % self.name)
 
 -cmd_info = apparmor.cmd([apparmor.parser,
 filename, '-I%s' % apparmor.profile_dir, '-R 21', '1/dev/null']) -
#cmd_info = apparmor.cmd(['cat', filename, '|',
 apparmor.parser, '-I%s'%apparmor.profile_dir, '-R 21',
 '1/dev/null']) +cmd_info = cmd([apparmor.parser,
 '-I%s' % apparmor.profile_dir, '-R', filename])
 
  if cmd_info[0] != 0:
  raise apparmor.AppArmorException(cmd_info[1])

Acked-by: Christian Boltz appar...@cboltz.de


Regards,

Christian Boltz
-- 
Früher habe ich auch gerne CDs gekauft [...] Aber ich habe gelernt, daß
ich damit nicht Musiker fördere, sondern nur koksende Sony-Spacken, die
mir zum Dank für meine Investition [...] ein Rootkit auf meine Karre
installieren, gleich neben den Staatstrojaner.
[http://blog.koehntopp.de/archives/3154-Nicht-Urheberrecht-ist-das-Kernthema.html]


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


[apparmor] [patch] complain flag is enough, no symlink needed

2014-02-24 Thread Christian Boltz
Hello,

let me compile 20 minutes of discussions into the addition of a   #   ;-)


Change aa-complain / set_complain() to (only) add the complain flag. 
We don't need to additionally create a force-complain symlink.

=== modified file 'utils/apparmor/aa.py'
--- utils/apparmor/aa.py2014-02-24 19:56:28 +
+++ utils/apparmor/aa.py2014-02-24 23:11:32 +
@@ -257,7 +257,8 @@
 def set_complain(filename, program):
 Sets the profile to complain mode
 aaui.UI_Info(_('Setting %s to complain mode.') % program)
-create_symlink('force-complain', filename)
+# a force-complain symlink is more packaging-friendly, but breaks caching
+# create_symlink('force-complain', filename)
 change_profile_flags(filename, program, 'complain', True)
 
 def set_enforce(filename, program):



Regards,

Christian Boltz
-- 
Ich habe ein update für 2.0.1 released, welches die Änderung im Makefile
auf die alte Version zurückportiert (Was für ein großes Wort für zwei
Anführungsstriche).  [Ratti in fontlinge-devel - nach Änderung von zwei 
»'« zu »« in einem Makefile]


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


[apparmor] [Bug 1180230] Re: glob in aa-genprof repeats same option

2014-02-24 Thread Christian Boltz
This patch was commited to 2.8 branch and trunk, and later changed to
use grep instead of ~~~.

AppArmor 2.8.3 contains the fix.

** Changed in: apparmor
   Status: Fix Committed = Fix Released

-- 
You received this bug notification because you are a member of AppArmor
Developers, which is a bug assignee.
https://bugs.launchpad.net/bugs/1180230

Title:
  glob in aa-genprof repeats same option

Status in AppArmor Linux application security framework:
  Fix Released

Bug description:
  When using glob, the glob does not check if the entries mentioned
  previously is repeated or not. Using a simple check to match against
  the previous entry will solve this and prevent such long pointless
  lists.

  | [(A)llow] / (D)eny / (G)lob / Glob w/(E)xt / (N)ew / Abo(r)t / (F)inish / 
(O)pts
  |
  | Profile:  /usr/sbin/mtr
  | Path: /etc/gai.conf
  | Mode: r
  | Severity: unknown
  |
  |
  | 1 - #include abstractions/apache2-common 
  |  2 - #include abstractions/nameservice 
  |  3 - /etc/gai.conf 
  |  4 - /etc/* 
  |  5 - /** 
  |  6 - /** 
  |  7 - /** 
  |  8 - /** 
  |  9 - /** 
  |  10 - /** 
  |  11 - /** 
  |  12 - /** 
  |  13 - /** 
  |  14 - /** 
  | [15 - /**]
  |
  | [(A)llow] / (D)eny / (G)lob / Glob w/(E)xt / (N)ew / Abo(r)t / (F)inish / 
(O)pts

  I'm assuming this is a problem with the AppArmor library based in Perl. I'm 
trying to work a way to fix it.
  (BTW, my first bug report ever.)

To manage notifications about this bug go to:
https://bugs.launchpad.net/apparmor/+bug/1180230/+subscriptions

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


<    1   2   3   4   5   6   7   8   9   10   >