mir/apparmor
2
0
Fork 0
mirror of https://gitlab.com/apparmor/apparmor synced 2025-08-28 12:58:07 +00:00
Code Issues Releases Wiki Activity

Merge MountRule: check for unknown fstype and options keywords, and fix issues uncovered by that

Browse Source 
* **MountRule: sync flags_keywords with parser code**

    ... based on /mount.cc mnt_opts_table

    Several keywords and aliases were missing in flags_keywords:
    - B
    - M
    - make-private
    - make-rprivate
    - make-rshared
    - make-rslave
    - make-runbindable
    - make-shared
    - make-slave
    - make-unbindable
    - r
    - R
    - read-only
    - w

    Also sort the keywords in the same order as in mount.cc.

    Note: AARE handling is still a TODO.

    After that, update the list of known parsing failures:
    - several valid profiles are now correctly parsed
    - some `"make-*" mount opt and an invalid src` bad profiles are no
      longer detected as being invalid

* **test-mount.py: fix MountRule instance creation**

    If fstype or options is a str, it has to be exactly one keyword, because
    \__init__() / check_and_split_list() won't parse a str.

    Our "normal" code already honors this, and only hands over fstype and
    options as sets or a single-keyword str.

    However, a few tests (wrongly) handed over a str that would need further
    parsing. Adjust the tests to no longer do this.

* **MountRule: check for unknown fstype and options**

    ... now that the previous commits fixed issues that ended up as unknown
    keywords.

    Also add mount/ok_12.sd as known-failing test. It uses fstype=AARE which
    MountRule doesn't support (yet?).

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1169
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
This commit is contained in:
Georgia Garcia 2024-03-04 11:52:53 +00:00
commit dfb02cbd93
4 changed files with 39 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
parser/mount.cc
View File

@ -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},

17
utils/apparmor/rule/mount.py
View File

@ -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():

18
utils/test/test-mount.py
View File

@ -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" ),

33
utils/test/test-parser-simple-tests.py
View File

@ -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
)