checkpolicy,libsepol: add prefix/suffix matching to filename type transitions #1

JurajMarcin · 2022-10-12T12:31:44Z

Currently, filename transitions are stored separately from other type
enforcement rules and only support exact name matching. However, in
practice, the names contain variable parts. This leads to many
duplicated rules in the policy that differ only in the part of the name,
or it is even impossible to cover all possible combinations.

This series implements equivalent changes made by this kernel patch
series 1.

First, this series of patches moves the filename transitions to be part
of the avtab and avrule structures. This not only makes the
implementation of prefix/suffix matching and future enhancements easier,
but also reduces the technical debt regarding the filename transitions.
Next, the last three patches implement the support for prefix/suffix
name matching itself by extending the structures added in previous
patches in this series and adding the support to CIL in the last of the
triple.

Even though, moving everything to avtab increases the memory usage and
the size of the binary policy itself and thus the loading time, the
ability to match the prefix or suffix of the name will reduce the
overall number of rules in the policy which should mitigate this issue.

Changelog:

v2:

rebased to the latest upstream main
added syntax examples to commit messages (suggested by Jim)
removed the "match_" prefix from keywords and dropped the
"match_exact" keyword (suggested by Jim, discussed with Ondrej)
fixed the bug Jim found 2 where the output to conf and CIL did not
work
reworked avtab entry skipping when writing older versions of the
policy
fixed bug where expand_terule_helper() skipped next entries after
filename transition entry
fixed memory leak in filenametr_destroy() found by Christian in the
kernel version of the code 3

libsepol/src/avtab.c

libsepol/src/expand.c

libsepol/src/kernel_to_cil.c

libsepol/include/sepol/policydb/policydb.h

libsepol/src/write.c

libsepol/src/policydb_validate.c

libsepol/src/expand.c

libsepol/src/write.c

WOnder93

Just two minor suggestions regarding the newly added comment, but otherwise LGTM (including the commit messages + cover text). 👍

libsepol/src/expand.c

libsepol/src/avtab.c

checkpolicy/test/dispol.c

libsepol/src/expand.c

libsepol/src/kernel_to_cil.c

libsepol/src/kernel_to_conf.c

libsepol/src/avtab.c

libsepol/src/write.c

libsepol/src/avtab.c

libsepol/src/policydb_validate.c

checkpolicy/policy_define.c

Add return check for regex_data_create() to avoid NULL reference of regex_data (gdb) bt #0 0x00007fbde5caec14 in pthread_mutex_init () from /usr/lib64/libc.so.6 #1 0x00007fbde5e3a489 in regex_data_create () at regex.c:260 #2 0x00007fbde5e3a4af in regex_prepare_data (regex=regex@entry=0x7fbde4613770, pattern_string=pattern_string@entry=0x563c6799a820 "^/home$", errordata=errordata@entry=0x7ffeb83fa950) at regex.c:76 SELinuxProject#3 0x00007fbde5e32fe6 in compile_regex (errbuf=0x0, spec=0x7fbde4613748) at label_file.h:407 SELinuxProject#4 lookup_all (key=0x563c679974e5 "/var/log/kadmind.log", type=<optimized out>, partial=partial@entry=false, match_count=match_count@entry=0x0, rec=<optimized out>, rec=<optimized out>) at label_file.c:949 SELinuxProject#5 0x00007fbde5e33350 in lookup (rec=<optimized out>, key=<optimized out>, type=<optimized out>) at label_file.c:1092 SELinuxProject#6 0x00007fbde5e31878 in selabel_lookup_common (rec=0x563c67998cc0, translating=1, key=<optimized out>, type=<optimized out>) at label.c:167 Signed-off-by: Jie Lu <[email protected]> Acked-by: James Carter <[email protected]>

The output parameter `role_arr` of semanage_user_get_roles() is an array of non-owned role names. Since the array is never used again, as its contents have been copied into the return value `roles`, free it. Example leak report from useradd(8): Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x5597624284a8 in __interceptor_calloc (./shadow/src/useradd+0xee4a8) #1 0x7f53aefcbbf9 in sepol_user_get_roles src/user_record.c:270:21

To move filename transitions to be part of avtab, we need to create space for it in the avtab_datum structure which holds the rule for a certain combination of stype, ttype and tclass. As only type transitions have a special variant that uses a filename, it would be suboptimal to add a (mostly empty) pointer to some structure to all avtab rules. Therefore, this patch adds a new structure to the avtab_datum and moves the otype of the transition to this structure. In the next patch, this structure will also hold filename transitions for the combination of stype, ttype and tclass. Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Juraj Marcin <[email protected]>

Currently, filename transitions are stored separately from other type enforcement rules. This leads to possibly sub-optimal performance and makes further improvements cumbersome. This patch adds a symbol table with filename transitions to the transition structure added to avtab in the previous patch. It also implements functions required for reading and writing filename transitions (either binary or source formats) and updates the code for expanding attributes. Last but not least, it updates the conflict check in the conditional avtab to account for empty transitions in the non-conditional avtab. These changes are expected to cause higher memory usage, as now there needs to be a filename transition structure for every stype. This patch effectively undoes most of the commit 42ae834 ("libsepol,checkpolicy: optimize storage of filename transitions"), but this will be mitigated by providing support for matching prefix/suffix of the filename for filename transitions in future patches which will reduce to need to have so many of them. Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Juraj Marcin <[email protected]>

Similarly to the previous patch, filename transition rules are stored and parsed separately from other type enforcement rules. Moving them to avrule makes it consistent with the filename transitions in avtab and makes future improvements easier to implement. This patch adds an optional object name attribute to the avrule structure and uses this new attribute to move filename transition rules to avrule. It also updates functions for parsing type enforcement rules to accept rules with a filename as their last argument (filename transition rules), separate functions for parsing filename transitions are therefore no longer needed. Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Juraj Marcin <[email protected]>

Implement a new binary policy format that closely matches the new internal representation introduced in the previous patch. This patch bumps the maximum kernel policy version and implements reading/writing functions such that kernel binary policy structure matches internal representation. These changes can cause the binary policy to grow in size due to effectively undoing the benefits of the commit 8206b8c ("libsepol: implement POLICYDB_VERSION_COMP_FTRANS "), but this will be mitigated by adding the prefix/suffix support as described in the previous patch. Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Juraj Marcin <[email protected]>

Implement a new module policy format that closely matches the new internal representation of avrule introduced in the previous patch. This patch bumps the maximum module policy version and implements reading/writing functions such that the module binary policy structure matches its internal representation, namely, the object name attribute used for the filename transition rules. These changes have no significant effect on the size of the module policy file (tested with Fedora policy). Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Juraj Marcin <[email protected]>

Currently, filename type transitions support only exact name matching. However, in practice, the names contain variable parts. This leads to many duplicated rules in the policy that differ only in the part of the name, or it is even impossible to cover all possible combinations. This patch extends the filename type transitions structures to include new types of filename transitions - prefix and suffix filename transitions. It also implements the reading and writing of those rules in the kernel binary policy format together with increasing its version. Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Juraj Marcin <[email protected]>

This patch extends the structures for module and base policy (avrule_t) to support prefix/suffix transitions. In addition to this, it implements the necessary changes to functions for reading and writing the binary policy as well as parsing the policy conf. Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Juraj Marcin <[email protected]>

This patch implements the support for prefix/suffix filename transitions in CIL structures as well as to CIL policy parser. Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Juraj Marcin <[email protected]>

In case the init function for a selabel backend fails, free the possible already allocated data: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x5e7e2bf001e3 in malloc (/tmp/destdir/usr/sbin/selabel_digest+0xc71e3) #1 0x7233764baa65 in selabel_media_init /home/christian/Coding/workspaces/selinux/libselinux/src/label_media.c:226:30 #2 0x7233764ac1fe in selabel_open /home/christian/Coding/workspaces/selinux/libselinux/src/label.c:227:6 SELinuxProject#3 0x5e7e2bf3ebfc in main /home/christian/Coding/workspaces/selinux/libselinux/utils/selabel_digest.c:125:8 SELinuxProject#4 0x7233761856c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s). Signed-off-by: Christian Göttsche <[email protected]> Acked-by: James Carter <[email protected]>

JurajMarcin force-pushed the selinux-2935-filename-transitions branch 2 times, most recently from f0e8994 to 2165676 Compare October 12, 2022 13:50

WOnder93 suggested changes Oct 18, 2022

View reviewed changes

JurajMarcin force-pushed the selinux-2935-filename-transitions branch 4 times, most recently from 7b5d5cc to 21a4058 Compare October 26, 2022 13:10

WOnder93 suggested changes Oct 27, 2022

View reviewed changes

libsepol/src/expand.c Outdated Show resolved Hide resolved

JurajMarcin force-pushed the selinux-2935-filename-transitions branch from 21a4058 to 0c45d54 Compare October 27, 2022 18:09

JurajMarcin force-pushed the selinux-2935-filename-transitions branch 4 times, most recently from 49639ca to c141f72 Compare November 23, 2022 13:26

WOnder93 suggested changes Nov 23, 2022

View reviewed changes

JurajMarcin force-pushed the selinux-2935-filename-transitions branch 2 times, most recently from fcedfd7 to 1fccdae Compare November 23, 2022 17:22

JurajMarcin commented Nov 30, 2022

View reviewed changes

checkpolicy/policy_define.c Outdated Show resolved Hide resolved

JurajMarcin force-pushed the selinux-2935-filename-transitions branch from 1fccdae to 5880890 Compare December 14, 2022 13:03

JurajMarcin force-pushed the selinux-2935-filename-transitions branch 2 times, most recently from 1fe50d4 to a662e7a Compare March 30, 2023 13:16

JurajMarcin force-pushed the selinux-2935-filename-transitions branch from a662e7a to 45c36a6 Compare May 15, 2023 15:13

JurajMarcin added 5 commits May 31, 2023 13:04

JurajMarcin force-pushed the selinux-2935-filename-transitions branch from 2fdad7c to 9d4425a Compare May 31, 2023 11:09

JurajMarcin changed the title ~~Move filename transitions to be part of avtab~~ checkpolicy, libsepol: add prefix/suffix matching to filename type transitions May 31, 2023

JurajMarcin added 3 commits June 1, 2023 16:35

JurajMarcin force-pushed the selinux-2935-filename-transitions branch from 9d4425a to 4fbe3fa Compare June 1, 2023 14:36

JurajMarcin changed the base branch from master to main June 13, 2023 11:40

JurajMarcin changed the title ~~checkpolicy, libsepol: add prefix/suffix matching to filename type transitions~~ checkpolicy,libsepol: add prefix/suffix matching to filename type transitions Jun 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

checkpolicy,libsepol: add prefix/suffix matching to filename type transitions #1

checkpolicy,libsepol: add prefix/suffix matching to filename type transitions #1

JurajMarcin commented Oct 12, 2022 •

edited

Loading

WOnder93 left a comment

checkpolicy,libsepol: add prefix/suffix matching to filename type transitions #1

Are you sure you want to change the base?

checkpolicy,libsepol: add prefix/suffix matching to filename type transitions #1

Conversation

JurajMarcin commented Oct 12, 2022 • edited Loading

WOnder93 left a comment

Choose a reason for hiding this comment

JurajMarcin commented Oct 12, 2022 •

edited

Loading