Merge ProfileStorage: store correct name

Instead of always storing the name of the main profile, store the child
profile/hat name if we are in a child profile or hat.

As a result, we always get the correct "profile xy" header even for
child profiles when dumping the ProfileStorage object.

Also extend the tests to check that the name gets stored correctly.

.

Add aa-complain tests for profile with hats and subprofiles

So far, change_profile_flags() in aa.py is the only user of
ProfileStorage's 'name'.

Rewrite minitools test_cleanprof() so that most of its code can be
reused, and add a test that runs 'aa-complain
/usr/bin/a/simple/cleanprof/test/profile' on cleanprof.in to ensure
aa-complain still works as expected on subprofiles and hats.

Note: aa-complain $profilename will change the flags of hats, but not
child profiles. This is a known issue, and doesn't change with this MR.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1359
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>

utils/apparmor/profile_storage.py

@@ -222,10 +222,12 @@ class ProfileStorage:
                     % {'profile': profile, 'file': file, 'line': lineno + 1})
 
             hat = matches['profile']
+            prof_or_hat_name = hat
             pps_set_hat_external = False
 
         else:  # stand-alone profile
             profile = matches['profile']
+            prof_or_hat_name = profile
             if len(profile.split('//')) > 2:
                 raise AppArmorException(
                     "Nested child profiles ('%(profile)s', found in %(file)s) are not supported by the AppArmor tools yet."
@@ -239,7 +241,7 @@ class ProfileStorage:
 
         prof_storage = cls(profile, hat, cls.__name__ + '.parse()')
 
-        prof_storage['name'] = profile
+        prof_storage['name'] = prof_or_hat_name
         prof_storage['filename'] = file
         prof_storage['external'] = pps_set_hat_external
         prof_storage['flags'] = matches['flags']

utils/test/cleanprof_test.complain

@@ -0,0 +1,98 @@
+# A simple test comment which will persist
+#include <tunables/global>
+
+#include  if  exists <tunables/nothing>
+
+  #include if exists <tunables/global>
+  include if exists <tunables/global>
+
+		alias /foo    ->    /bar	,
+
+@{xy}    =   y   x
+
+  abi  <abi/4.19>    ,
+
+  @{asdf}       =   foo           ""
+
+$foo = false
+
+   $bar    =    true
+
+/usr/bin/a/simple/cleanprof/test/profile flags=(complain) {
+	# Just for the heck of it, this comment won't see the day of light
+	#include <abstractions/base>
+
+#include  if  exists <foo>
+	#include if exists <abstractions/base>
+	include <abstractions/base>
+
+    capability sys_admin,
+    audit capability,
+
+    change_profile -> /bin/foo,
+    change_profile,
+
+    network inet stream,
+               abi  "abi/4.20"    ,
+    network stream,
+
+	#Below rule comes from abstractions/base
+	allow /usr/share/X11/locale/**  r,
+	allow /home/*/** r,
+
+    ptrace tracedby peer=/bin/strace,
+    ptrace tracedby,
+    unix (receive) type=dgram,
+
+    dbus send bus=session,
+    dbus send bus=session peer=(label=foo),
+
+    profile test_child /foobar {
+            /etc/child rw,
+            }
+
+    set rlimit nofile <= 256,
+    set rlimit nofile <= 64,
+
+    signal set=(hup int quit ill trap abrt)
+             set=(bus,fpe,,,kill,usr1)
+                      set=segv set=usr2 set=pipe set=alrm set=term set=stkflt set=chld,
+    signal set=(hup int quit),
+
+  ^foo flags=(complain) {
+            /etc/fstab r,
+        capability dac_override,
+        }
+
+    ^foo, # hat declarations are obsolete and will be removed when aa-cleanprof or aa-logprof writes the profile
+
+	mount options=(rw,suid) /c -> /3,
+
+  hat bar flags=(complain) {
+        /etc/passwd r,
+        capability sys_admin,
+        }
+
+	pivot_root oldroot=/mnt/root/old/,
+
+	deny    owner       link       /some/thing    ->     /foo/bar    ,
+	unix shutdown addr=@HypotheticalServiceDaemon, # covered in abstractions/base, will be removed
+
+	link subset /alpha/beta -> /tmp/**,
+
+	allow /home/foo/bar r,
+	allow /home/foo/** w,
+}
+
+/usr/bin/other/cleanprof/test/profile {
+	# This one shouldn't be affected by the processing
+	# However this comment will be wiped, need to change that
+	allow /home/*/** rw,
+	allow /home/foo/bar r,
+}
+
+/what/ever/xattr   xattrs=(   foo=bar      )
+       flags=(  complain
+	   ) {
+ /what/ever r,
+ }

utils/test/test-minitools.py

@@ -226,26 +226,44 @@ class MinitoolsTest(AATest):
 
         self.assertIsNot(output_force, '', 'Failed to run aa-unconfined in paranoid mode')
 
-    def test_cleanprof(self):
+    def _test_with_cleanprof_profile(self, command, output_file, errormsg, delete_first_line):
         input_file = 'cleanprof_test.in'
-        output_file = 'cleanprof_test.out'
+        profile = '/usr/bin/a/simple/cleanprof/test/profile'
         # We position the local testfile
         shutil.copy('./' + input_file, self.profile_dir)
-        # Our silly test program whose profile we wish to clean
-        cleanprof_test = '/usr/bin/a/simple/cleanprof/test/profile'
 
         subprocess.check_output(
-            '{} ./../aa-cleanprof  --no-reload -d {} -s {} --configdir ./'.format(
+            '{} ./../{}  --no-reload -d {} {} --configdir ./'.format(
-                python_interpreter, self.profile_dir, cleanprof_test),
+                python_interpreter, command, self.profile_dir, profile),
             shell=True)
 
         # Strip off the first line (#modified line)
+        if delete_first_line:
             subprocess.check_output('sed -i 1d {}/{}'.format(self.profile_dir, input_file), shell=True)
 
         exp_content = read_file('./' + output_file)
         real_content = read_file('{}/{}'.format(self.profile_dir, input_file))
         self.maxDiff = None
-        self.assertEqual(exp_content, real_content, 'Failed to cleanup profile properly')
+        self.assertEqual(exp_content, real_content, errormsg)
+
+    def test_cleanprof(self):
+        ''' run aa-cleanprof on cleanprof.in and check if it matches cleanprof.out '''
+
+        command = 'aa-cleanprof -s'
+        output_file = 'cleanprof_test.out'
+        errormsg = 'Failed to cleanup profile properly'
+
+        self._test_with_cleanprof_profile(command, output_file, errormsg, True)
+
+    def test_complain_cleanprof(self):
+        ''' test if all child profiles in cleanprof_test.in get the complain flag added when switching the profile to complain mode '''
+        # TODO: works for hats, but not for child profiles
+
+        command = 'aa-complain'
+        output_file = 'cleanprof_test.complain'
+        errormsg = 'Failed to switch profile to complain mode'
+
+        self._test_with_cleanprof_profile(command, output_file, errormsg, False)
 
 
 setup_aa(apparmor)

utils/test/test-profile-storage.py

@@ -141,28 +141,29 @@ class AaTest_repr(AATest):
 
 class AaTest_parse_profile_start(AATest):
     tests = (
-        # profile start line                                    profile  hat     profile                 hat                attachment   xattrs                     flags       pps_set_hat_external
+        # profile start line                                    profile  hat     name                       profile                 hat                attachment   xattrs                     flags       pps_set_hat_external
-        (('/foo {',                                             None,    None), ('/foo',                 '/foo',                 '',     '',                        None,       False)),
+        (('/foo {',                                             None,    None), ('/foo',                    '/foo',                 '/foo',                 '',     '',                        None,       False)),
-        (('/foo (complain) {',                                  None,    None), ('/foo',                 '/foo',                 '',     '',                        'complain', False)),
+        (('/foo (complain) {',                                  None,    None), ('/foo',                    '/foo',                 '/foo',                 '',     '',                        'complain', False)),
-        (('profile foo /foo {',                                 None,    None), ('foo',                  'foo',                  '/foo', '',                        None,       False)),  # named profile
+        (('profile foo /foo {',                                 None,    None), ('foo',                     'foo',                  'foo',                  '/foo', '',                        None,       False)),  # named profile
-        (('profile /foo {',                                     '/bar',  None), ('/bar',                 '/foo',                 '',     '',                        None,       False)),  # child profile
+        (('profile /foo {',                                     '/bar',  None), ('/foo',                    '/bar',                 '/foo',                 '',     '',                        None,       False)),  # child profile
-        (('/foo//bar {',                                        None,    None), ('/foo',                 'bar',                  '',     '',                        None,       True)),   # external hat
+        (('/foo//bar {',                                        None,    None), ('/foo//bar',               '/foo',                 'bar',                  '',     '',                        None,       True)),   # external hat
-        (('profile "/foo" (complain) {',                        None,    None), ('/foo',                 '/foo',                 '',     '',                        'complain', False)),
+        (('profile "/foo" (complain) {',                        None,    None), ('/foo',                    '/foo',                 '/foo',                 '',     '',                        'complain', False)),
-        (('profile "/foo" xattrs=(user.bar=bar) {',             None,    None), ('/foo',                 '/foo',                 '',     'user.bar=bar',            None,       False)),
+        (('profile "/foo" xattrs=(user.bar=bar) {',             None,    None), ('/foo',                    '/foo',                 '/foo',                 '',     'user.bar=bar',            None,       False)),
-        (('profile "/foo" xattrs=(user.bar=bar user.foo=*) {',  None,    None), ('/foo',                 '/foo',                 '',     'user.bar=bar user.foo=*', None,       False)),
+        (('profile "/foo" xattrs=(user.bar=bar user.foo=*) {',  None,    None), ('/foo',                    '/foo',                 '/foo',                 '',     'user.bar=bar user.foo=*', None,       False)),
-        (('/usr/bin/xattrs-test xattrs=(myvalue="foo.bar") {',  None,    None), ('/usr/bin/xattrs-test', '/usr/bin/xattrs-test', '',     'myvalue="foo.bar"',       None,       False)),
+        (('/usr/bin/xattrs-test xattrs=(myvalue="foo.bar") {',  None,    None), ('/usr/bin/xattrs-test',    '/usr/bin/xattrs-test', '/usr/bin/xattrs-test', '',     'myvalue="foo.bar"',       None,       False)),
     )
 
     def _run_test(self, params, expected):
         (profile, hat, prof_storage) = ProfileStorage.parse(params[0], 'somefile', 1, params[1], params[2])
 
-        self.assertEqual(profile,                    expected[0])
+        self.assertEqual(prof_storage['name'],       expected[0])
-        self.assertEqual(hat,                        expected[1])
+        self.assertEqual(profile,                    expected[1])
-        self.assertEqual(prof_storage['attachment'], expected[2])
+        self.assertEqual(hat,                        expected[2])
-        self.assertEqual(prof_storage['xattrs'],     expected[3])
+        self.assertEqual(prof_storage['attachment'], expected[3])
-        self.assertEqual(prof_storage['flags'],      expected[4])
+        self.assertEqual(prof_storage['xattrs'],     expected[4])
+        self.assertEqual(prof_storage['flags'],      expected[5])
         self.assertEqual(prof_storage['is_hat'],     False)
-        self.assertEqual(prof_storage['external'],   expected[5])
+        self.assertEqual(prof_storage['external'],   expected[6])
 
 
 class AaTest_parse_profile_start_errors(AATest):