diff --git a/parser/mount.cc b/parser/mount.cc index 5d9c6c974..c3af39a81 100644 --- a/parser/mount.cc +++ b/parser/mount.cc @@ -234,6 +234,7 @@ struct mnt_keyword_table { unsigned int clear; }; +// keep in sync with utils/apparmor/rule/mount.py flags_keywords static struct mnt_keyword_table mnt_opts_table[] = { {"ro", MS_RDONLY, 0}, {"r", MS_RDONLY, 0}, diff --git a/utils/apparmor/rule/mount.py b/utils/apparmor/rule/mount.py index 800ffb0d9..64e9cbdd5 100644 --- a/utils/apparmor/rule/mount.py +++ b/utils/apparmor/rule/mount.py @@ -36,11 +36,14 @@ valid_fs = [ 'none', 'bdev', 'proc', 'pipefs', 'pstore', 'btrfs', 'xfs', '9p', ] flags_keywords = [ - 'ro', 'rw', 'nosuid', 'suid', 'nodev', 'dev', 'noexec', 'exec', 'sync', 'async', 'remount', 'mand', 'nomand', - 'dirsync', 'noatime', 'atime', 'nodiratime', 'diratime', 'bind', 'rbind', 'move', 'verbose', 'silent', 'loud', - 'acl', 'noacl', 'unbindable', 'runbindable', 'private', 'rprivate', 'slave', 'rslave', 'shared', 'rshared', - 'relatime', 'norelatime', 'iversion', 'noiversion', 'strictatime', 'nostrictatime', 'lazytime', 'nolazytime', - 'nouser', 'user', 'symfollow', 'nosymfollow', '([A-Za-z0-9]|AARE)', # TODO: handle AARE + # keep in sync with parser/mount.cc mnt_opts_table! + 'ro', 'r', 'read-only', 'rw', 'w', 'suid', 'nosuid', 'dev', 'nodev', 'exec', 'noexec', 'sync', 'async', 'remount', + 'mand', 'nomand', 'dirsync', 'symfollow', 'nosymfollow', 'atime', 'noatime', 'diratime', 'nodiratime', 'bind', 'B', + 'move', 'M', 'rbind', 'R', 'verbose', 'silent', 'loud', 'acl', 'noacl', 'unbindable', 'make-unbindable', 'runbindable', + 'make-runbindable', 'private', 'make-private', 'rprivate', 'make-rprivate', 'slave', 'make-slave', 'rslave', 'make-rslave', + 'shared', 'make-shared', 'rshared', 'make-rshared', 'relatime', 'norelatime', 'iversion', 'noiversion', 'strictatime', + 'nostrictatime', 'lazytime', 'nolazytime', 'user', 'nouser', + '([A-Za-z0-9]|AARE)', # TODO: handle AARE ] join_valid_flags = '|'.join(flags_keywords) join_valid_fs = '|'.join(valid_fs) @@ -91,9 +94,13 @@ class MountRule(BaseRule): self.operation = operation self.fstype, self.all_fstype, unknown_items = check_and_split_list(fstype[1] if fstype != self.ALL else fstype, valid_fs, self.ALL, type(self).__name__, 'fstype') + if unknown_items: + raise AppArmorException(_('Passed unknown fstype keyword to %s: %s') % (type(self).__name__, ' '.join(unknown_items))) self.is_fstype_equal = fstype[0] if not self.all_fstype else None self.options, self.all_options, unknown_items = check_and_split_list(options[1] if options != self.ALL else options, flags_keywords, self.ALL, type(self).__name__, 'options') + if unknown_items: + raise AppArmorException(_('Passed unknown options keyword to %s: %s') % (type(self).__name__, ' '.join(unknown_items))) self.is_options_equal = options[0] if not self.all_options else None if source != self.ALL and source[0].isalpha(): diff --git a/utils/test/test-mount.py b/utils/test/test-mount.py index 7d9b0317d..a3505cf5b 100644 --- a/utils/test/test-mount.py +++ b/utils/test/test-mount.py @@ -88,20 +88,28 @@ class MountTestParseInvalid(AATest): def test_diff_non_mountrule(self): exp = namedtuple('exp', ('audit', 'deny')) - obj = MountRule('mount',("=", '(ext4)'), MountRule.ALL, MountRule.ALL, MountRule.ALL) + obj = MountRule('mount',("=", 'ext4'), MountRule.ALL, MountRule.ALL, MountRule.ALL) with self.assertRaises(AppArmorBug): obj.is_equal(exp(False, False), False) def test_diff_invalid_fstype_equals_or_in(self): with self.assertRaises(AppArmorBug): - MountRule('mount', ('ext3', '(ext4)'), MountRule.ALL, MountRule.ALL, MountRule.ALL) # fstype[0] should be '=' or 'in' + MountRule('mount', ('ext3', 'ext4'), MountRule.ALL, MountRule.ALL, MountRule.ALL) # fstype[0] should be '=' or 'in' + + def test_diff_invalid_fstype_keyword(self): + with self.assertRaises(AppArmorException): + MountRule('mount', ('=', 'invalidfs'), MountRule.ALL, MountRule.ALL, MountRule.ALL) # fstype[0] should be '=' or 'in' def test_diff_invalid_options_equals_or_in(self): with self.assertRaises(AppArmorBug): - MountRule('mount', MountRule.ALL, ('rbind', '(rw)'), MountRule.ALL, MountRule.ALL) # fstype[0] should be '=' or 'in' + MountRule('mount', MountRule.ALL, ('rbind', 'rw'), MountRule.ALL, MountRule.ALL) # fstype[0] should be '=' or 'in' + + def test_diff_invalid_options_keyword(self): + with self.assertRaises(AppArmorException): + MountRule('mount', MountRule.ALL, ('=', 'invalid'), MountRule.ALL, MountRule.ALL) # fstype[0] should be '=' or 'in' def test_diff_fstype(self): - obj1 = MountRule('mount',("=", '(ext4)'), MountRule.ALL, MountRule.ALL, MountRule.ALL) + obj1 = MountRule('mount',("=", 'ext4'), MountRule.ALL, MountRule.ALL, MountRule.ALL) obj2 = MountRule('mount',MountRule.ALL, MountRule.ALL, MountRule.ALL, MountRule.ALL) self.assertFalse(obj1.is_equal(obj2, False)) @@ -204,7 +212,7 @@ class MountIsCoveredTest(AATest): obj = MountRule("mount", ("=", ('ext3', 'ext4')), ("=", ('ro')), "/foo/b*", "/b*") tests = [ ("mount", ("in", ('ext3', 'ext4')), ("=", ('ro')), "/foo/bar", "/bar" ), - ("mount", ("=", ('procfs, ext4')), ("=", ('ro')), "/foo/bar", "/bar" ), + ("mount", ("=", ('procfs', 'ext4')), ("=", ('ro')), "/foo/bar", "/bar" ), ("mount", ("=", ('ext3')), ("=", ('rw')), "/foo/bar", "/bar" ), ("mount", ("=", ('ext3', 'ext4')), MountRule.ALL, "/foo/b*", "/bar" ), ("mount", MountRule.ALL, ("=", ('ro')), "/foo/b*", "/bar" ), diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py index 0b8618437..f76273cd2 100644 --- a/utils/test/test-parser-simple-tests.py +++ b/utils/test/test-parser-simple-tests.py @@ -85,6 +85,16 @@ exception_not_raised = ( 'mount/bad_1.sd', 'mount/bad_2.sd', + # not checked/detected: "make-*" mount opt and an invalid src + 'mount/bad_opt_17.sd', + 'mount/bad_opt_18.sd', + 'mount/bad_opt_19.sd', + 'mount/bad_opt_20.sd', + 'mount/bad_opt_21.sd', + 'mount/bad_opt_22.sd', + 'mount/bad_opt_23.sd', + 'mount/bad_opt_24.sd', + 'profile/flags/flags_bad10.sd', 'profile/flags/flags_bad11.sd', 'profile/flags/flags_bad12.sd', @@ -314,15 +324,8 @@ unknown_line = ( 'bare_include_tests/ok_85.sd', 'bare_include_tests/ok_86.sd', - # option = make-${valid-option} (e.g. make-private) is not supported - 'mount/ok_opt_48.sd', - 'mount/ok_opt_49.sd', - 'mount/ok_opt_50.sd', - 'mount/ok_opt_51.sd', - 'mount/ok_opt_52.sd', - 'mount/ok_opt_53.sd', - 'mount/ok_opt_54.sd', - 'mount/ok_opt_55.sd', + # mount with fstype using AARE + 'mount/ok_12.sd', # Mount with flags in {remount, [r]unbindable, [r]shared, [r]private, [r]slave} does not support a source 'mount/ok_opt_68.sd', @@ -334,15 +337,7 @@ unknown_line = ( 'mount/ok_opt_74.sd', 'mount/ok_opt_75.sd', - # option = make-${valid-option} (e.g. make-private) is not supported - 'mount/ok_opt_76.sd', - 'mount/ok_opt_77.sd', - 'mount/ok_opt_78.sd', - 'mount/ok_opt_79.sd', - 'mount/ok_opt_80.sd', - 'mount/ok_opt_81.sd', - 'mount/ok_opt_82.sd', - 'mount/ok_opt_83.sd', + # options=slave with /** src (first rule in the test causes exception) 'mount/ok_opt_84.sd', # According to spec mount should be in the form fstype=... options=... and NOT in the form options=... fstype=... @@ -351,8 +346,6 @@ unknown_line = ( 'mount/ok_opt_combo_1.sd', 'mount/ok_opt_combo_4.sd', - # Invalid keyword: read-only --> Should be ro - 'mount/ok_opt_3.sd', # Options should be comma separated 'mount/in_4.sd', # also order option then fstype is invalid )