[prev in list] [next in list] [prev in thread] [next in thread]
List: apparmor-dev
Subject: Re: [apparmor] [patch] Remove support for writing change hat declarations ("^hat, ")
From: Kshitij Gupta <kgupta8592 () gmail ! com>
Date: 2015-06-19 18:27:34
Message-ID: CAMBXP524u4TQ9fowCUa6SeY8LZ-jhfC1seGmv_V6RDpmTvLtXw () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hello,
On Sun, Jun 7, 2015 at 6:21 PM, Christian Boltz <apparmor@cboltz.de> wrote:
> Hello,
>
> change hat declarations ("^hat,") are no longer supported (see patch 46
> for details). Therefore remove support for writing them.
>
> This also means to completely remove the 'declared' flag, which was only
> needed for hat declarations, and was (after applying patch 46) always
> set to False.
>
> Also add a hat to the cleanprof_test.{in,out} test profile to make sure
> aa-cleanprof doesn't break hats.
>
> (This is "just" a cleanup, so trunk only)
>
>
> [ 47-remove-support-for-writing-hat-declarations.diff ]
>
> === modified file utils/apparmor/aa.py
> --- utils/apparmor/aa.py 2015-06-07 14:09:36.000405129 +0200
> +++ utils/apparmor/aa.py 2015-06-07 14:32:03.483282738 +0200
> @@ -108,7 +108,7 @@
> # a) rules (as dict): alias, include, lvar
> # b) rules (as hasher): allow, deny
> # c) one for each rule class
> -# d) other: declared, external, flags, name, profile, attachment,
> initial_comment,
> +# d) other: external, flags, name, profile, attachment, initial_comment,
> # profile_keyword, header_comment (these two are currently only
> set by set_profile_flags())
> aa = hasher() # Profiles originally in sd, replace by aa
> original_aa = hasher()
> @@ -1442,7 +1442,6 @@
> ynans = aaui.UI_YesNo(_('A profile for %s
> does not exist.\nDo you want to create one?') % exec_target, 'n')
> if ynans == 'y':
> hat = exec_target
> - aa[profile][hat]['declared'] = False
> aa[profile][hat]['profile'] = True
>
> if profile != hat:
> @@ -3007,7 +3006,6 @@
> flags = matches.group('flags')
>
> profile_data[profile][hat]['flags'] = flags
> - profile_data[profile][hat]['declared'] = False
> #profile_data[profile][hat]['allow']['path'] = hasher()
> #profile_data[profile][hat]['allow']['netdomain'] = hasher()
>
> @@ -3473,15 +3471,11 @@
> data += write_rules(profile_data[name], depth + 1)
>
> pre2 = ' ' * (depth + 1)
> - # External hat declarations
> - for hat in list(filter(lambda x: x != name,
> sorted(profile_data.keys()))):
> - if profile_data[hat].get('declared', False):
> - data.append('%s^%s,' % (pre2, hat))
>
> if not inhat:
> # Embedded hats
> for hat in list(filter(lambda x: x != name,
> sorted(profile_data.keys()))):
> - if not profile_data[hat]['external'] and not
> profile_data[hat]['declared']:
> + if not profile_data[hat]['external']:
> data.append('')
> if profile_data[hat]['profile']:
> data += list(map(str, write_header(profile_data[hat],
> depth + 1, hat, True, write_flags)))
> @@ -3730,7 +3724,7 @@
> depth = int((len(line) - len(line.lstrip())) / 2)
> pre2 = ' ' * (depth + 1)
> for hat in list(filter(lambda x: x != name,
> sorted(profile_data.keys()))):
> - if not profile_data[hat]['external'] and not
> profile_data[hat]['declared']:
> + if not profile_data[hat]['external']:
> data.append('')
> if profile_data[hat]['profile']:
> data += list(map(str,
> write_header(profile_data[hat], depth + 1, hat, True, include_flags)))
> @@ -3990,16 +3984,9 @@
> data.append(line)
>
> elif RE_PROFILE_CHANGE_HAT.search(line):
> - matches = RE_PROFILE_CHANGE_HAT.search(line).groups()
> - hat = matches[0]
> - hat = strip_quotes(hat)
> - if not write_prof_data[hat]['declared']:
> - correct = False
> - if correct:
> - data.append(line)
> - else:
> - #To-Do
> - pass
> + # "^hat," declarations are no longer supported, ignore
> them and don't write out the line
> + # (parse_profile_data() already prints a warning about
> that)
> + pass
> elif RE_PROFILE_HAT_DEF.search(line):
> matches = RE_PROFILE_HAT_DEF.search(line)
> in_contained_hat = True
> @@ -4009,8 +3996,6 @@
>
> if not write_prof_data[hat]['flags'] == flags:
> correct = False
> - if not write_prof_data[hat]['declared'] is False:
> - correct = False
> if not write_filelist['profile'][profile][hat]:
> correct = False
> if correct:
> === modified file utils/test/cleanprof_test.in
> --- utils/test/cleanprof_test.in 2015-05-25 17:30:59.798783638
> +0200
> +++ utils/test/cleanprof_test.in 2015-06-07 14:45:06.807307887
> +0200
> @@ -7,6 +7,12 @@
> #Below rule comes from abstractions/base
> allow /usr/share/X11/locale/** r,
> allow /home/*/** r,
> +
> + ^foo {
> + /etc/fstab r,
> + capability dac_override,
> + }
> +
>
umm why not also add a test for the removed/incorrect case of hat
declaration? The output would remain untouched just a ^foo, would be added
(If I understand correctly).
> allow /home/foo/bar r,
> allow /home/foo/** w,
> }
> === modified file utils/test/cleanprof_test.out
> --- utils/test/cleanprof_test.out 2015-05-25 17:30:59.798783638 +0200
> +++ utils/test/cleanprof_test.out 2015-06-07 14:46:15.334296605 +0200
> @@ -9,6 +9,13 @@
> /home/*/** r,
> /home/foo/** w,
>
> +
> + ^foo {
> + capability dac_override,
> +
> + /etc/fstab r,
> +
> + }
> }
> /usr/bin/other/cleanprof/test/profile {
> /home/*/** rw,
>
>
Thanks for the patch. Looks fine to me.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>.
>
> Regards,
>
> Christian Boltz
> --
> IT is everything that is more complicated than pushing buttons in
> the elevator. [from http://www.orkpiraten.de/blog/ugly-kid-jeans]
>
>
> --
> AppArmor mailing list
> AppArmor@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
--
Regards,
Kshitij Gupta
[Attachment #5 (text/html)]
<div dir="ltr">Hello,<br><div class="gmail_extra"><br><div class="gmail_quote">On \
Sun, Jun 7, 2015 at 6:21 PM, Christian Boltz <span dir="ltr"><<a \
href="mailto:apparmor@cboltz.de" target="_blank">apparmor@cboltz.de</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br> <br>
change hat declarations ("^hat,") are no longer supported (see patch 46<br>
for details). Therefore remove support for writing them.<br>
<br>
This also means to completely remove the 'declared' flag, which was only<br>
needed for hat declarations, and was (after applying patch 46) always<br>
set to False.<br>
<br>
Also add a hat to the cleanprof_test.{in,out} test profile to make sure<br>
aa-cleanprof doesn't break hats.<br>
<br>
(This is "just" a cleanup, so trunk only)<br>
<br>
<br>
[ 47-remove-support-for-writing-hat-declarations.diff ]<br>
<br>
=== modified file utils/apparmor/aa.py<br>
--- utils/apparmor/aa.py 2015-06-07 14:09:36.000405129 +0200<br>
+++ utils/apparmor/aa.py 2015-06-07 14:32:03.483282738 +0200<br>
@@ -108,7 +108,7 @@<br>
# a) rules (as dict): alias, include, lvar<br>
# b) rules (as hasher): allow, deny<br>
# c) one for each rule class<br>
-# d) other: declared, external, flags, name, profile, attachment, \
initial_comment,<br> +# d) other: external, flags, name, profile, attachment, \
initial_comment,<br> # profile_keyword, header_comment (these two \
are currently only set by set_profile_flags())<br> aa = hasher() # Profiles \
originally in sd, replace by aa<br> original_aa = hasher()<br>
@@ -1442,7 +1442,6 @@<br>
ynans = aaui.UI_YesNo(_('A \
profile for %s does not exist.\nDo you want to create one?') % exec_target, \
'n')<br> if ynans == 'y':<br>
hat = exec_target<br>
- aa[profile][hat]['declared'] \
= False<br>
aa[profile][hat]['profile'] \
= True<br> <br>
if profile != hat:<br>
@@ -3007,7 +3006,6 @@<br>
flags = matches.group('flags')<br>
<br>
profile_data[profile][hat]['flags'] = flags<br>
- profile_data[profile][hat]['declared'] = False<br>
#profile_data[profile][hat]['allow']['path'] = \
hasher()<br>
#profile_data[profile][hat]['allow']['netdomain'] \
= hasher()<br> <br>
@@ -3473,15 +3471,11 @@<br>
data += write_rules(profile_data[name], depth + 1)<br>
<br>
pre2 = ' ' * (depth + 1)<br>
- # External hat declarations<br>
- for hat in list(filter(lambda x: x != name, sorted(profile_data.keys()))):<br>
- if profile_data[hat].get('declared', False):<br>
- data.append('%s^%s,' % (pre2, hat))<br>
<br>
if not inhat:<br>
# Embedded hats<br>
for hat in list(filter(lambda x: x != name, \
sorted(profile_data.keys()))):<br>
- if not profile_data[hat]['external'] and not \
profile_data[hat]['declared']:<br> + if not \
profile_data[hat]['external']:<br> data.append('')<br>
if profile_data[hat]['profile']:<br>
data += list(map(str, write_header(profile_data[hat], \
depth + 1, hat, True, write_flags)))<br> @@ -3730,7 +3724,7 @@<br>
depth = int((len(line) - len(line.lstrip())) / 2)<br>
pre2 = ' ' * (depth + 1)<br>
for hat in list(filter(lambda x: x != name, \
sorted(profile_data.keys()))):<br>
- if not profile_data[hat]['external'] and \
not profile_data[hat]['declared']:<br> + \
if not profile_data[hat]['external']:<br> data.append('')<br>
if \
profile_data[hat]['profile']:<br>
data += list(map(str, \
write_header(profile_data[hat], depth + 1, hat, True, include_flags)))<br> @@ \
-3990,16 +3984,9 @@<br> data.append(line)<br>
<br>
elif RE_PROFILE_CHANGE_HAT.search(line):<br>
- matches = RE_PROFILE_CHANGE_HAT.search(line).groups()<br>
- hat = matches[0]<br>
- hat = strip_quotes(hat)<br>
- if not write_prof_data[hat]['declared']:<br>
- correct = False<br>
- if correct:<br>
- data.append(line)<br>
- else:<br>
- #To-Do<br>
- pass<br>
+ # "^hat," declarations are no longer supported, \
ignore them and don't write out the line<br> + # \
(parse_profile_data() already prints a warning about that)<br> + \
pass<br> elif RE_PROFILE_HAT_DEF.search(line):<br>
matches = RE_PROFILE_HAT_DEF.search(line)<br>
in_contained_hat = True<br>
@@ -4009,8 +3996,6 @@<br>
<br>
if not write_prof_data[hat]['flags'] == flags:<br>
correct = False<br>
- if not write_prof_data[hat]['declared'] is \
False:<br>
- correct = False<br>
if not write_filelist['profile'][profile][hat]:<br>
correct = False<br>
if correct:<br>
=== modified file utils/test/<a href="http://cleanprof_test.in" rel="noreferrer" \
target="_blank">cleanprof_test.in</a><br>
--- utils/test/<a href="http://cleanprof_test.in" rel="noreferrer" \
target="_blank">cleanprof_test.in</a> 2015-05-25 17:30:59.798783638 \
+0200<br>
+++ utils/test/<a href="http://cleanprof_test.in" rel="noreferrer" \
target="_blank">cleanprof_test.in</a> 2015-06-07 14:45:06.807307887 \
+0200<br> @@ -7,6 +7,12 @@<br>
#Below rule comes from abstractions/base<br>
allow /usr/share/X11/locale/** r,<br>
allow /home/*/** r,<br>
+<br>
+ ^foo {<br>
+ /etc/fstab r,<br>
+ capability dac_override,<br>
+ }<br>
+<br></blockquote><div>umm why not also add a test for the removed/incorrect case of \
hat declaration? The output would remain untouched just a ^foo, would be added (If I \
understand correctly).<br> <br></div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> allow /home/foo/bar r,<br>
allow /home/foo/** w,<br>
}<br>
=== modified file utils/test/cleanprof_test.out<br>
--- utils/test/cleanprof_test.out 2015-05-25 17:30:59.798783638 +0200<br>
+++ utils/test/cleanprof_test.out 2015-06-07 14:46:15.334296605 +0200<br>
@@ -9,6 +9,13 @@<br>
/home/*/** r,<br>
/home/foo/** w,<br>
<br>
+<br>
+ ^foo {<br>
+ capability dac_override,<br>
+<br>
+ /etc/fstab r,<br>
+<br>
+ }<br>
}<br>
/usr/bin/other/cleanprof/test/profile {<br>
/home/*/** rw,<br>
<br></blockquote><div><br>Thanks for the patch. Looks fine to me.<br><br>Acked-by: \
Kshitij Gupta <<a href="mailto:kgupta8592@gmail.com" \
target="_blank">kgupta8592@gmail.com</a>>. <br><br><br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> <br>
<br>
Regards,<br>
<br>
Christian Boltz<br>
<span class=""><font color="#888888">--<br>
IT is everything that is more complicated than pushing buttons in<br>
the elevator. [from <a href="http://www.orkpiraten.de/blog/ugly-kid-jeans" \
rel="noreferrer" target="_blank">http://www.orkpiraten.de/blog/ugly-kid-jeans</a>]<br>
<br>
<br>
--<br>
AppArmor mailing list<br>
<a href="mailto:AppArmor@lists.ubuntu.com">AppArmor@lists.ubuntu.com</a><br>
Modify settings or unsubscribe at: <a \
href="https://lists.ubuntu.com/mailman/listinfo/apparmor" rel="noreferrer" \
target="_blank">https://lists.ubuntu.com/mailman/listinfo/apparmor</a><br> \
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div \
class="gmail_signature"><div dir="ltr"><div>Regards,<br><br></div>Kshitij \
Gupta<br></div></div> </div></div>
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic