Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JurajMarcin
Copy link
Owner

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 patch implements the equivalent changes made by this kernel patch [1].

This patch updates the policydb structure to contain prefix and suffix filename transition tables along normal filename transitions table and updates the code that accesses those tables. Furthermore, it adds match_type attribute to module and CIL structures that store filename transitions and updates functions that parse conf and CIL policy files.

This patch does not significantly change the binary policy size when prefix/suffix rules are not used. On the other hand, with prefix/suffix rules, the number of filename transitions can be reduced, and therefore also binary policy size can be reduced.

Syntax of the new prefix/suffix filename transition rule:

type_transition source_type target_type : class default_type object_name match_type;

(typetransition source_type_id target_type_id class_id object_name match_type default_type_id)

where match_type is either keyword "prefix" or "suffix"

Examples:

type_transition ta tb:CLASS01 tc "file01" prefix;
type_transition td te:CLASS01 tf "file02" suffix;

(typetransition ta tb CLASS01 "file01" prefix td)
(typetransition td te CLASS01 "file02" suffix tf)

[1]: TODO: PASTE LINK

Copy link

@WOnder93 WOnder93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just minor issues found.

libsepol/cil/src/cil_resolve_ast.c Outdated Show resolved Hide resolved
libsepol/include/sepol/policydb/policydb.h Outdated Show resolved Hide resolved
libsepol/src/module_to_cil.c Outdated Show resolved Hide resolved
libsepol/src/module_to_cil.c Outdated Show resolved Hide resolved
libsepol/src/policydb_validate.c Outdated Show resolved Hide resolved
libsepol/src/write.c Outdated Show resolved Hide resolved
@JurajMarcin JurajMarcin force-pushed the selinux-prefix2-submit branch from 8fa67c7 to e07c00c Compare November 7, 2023 13:41
@@ -1,4 +1,3 @@

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit still does an unrelated whitespace change, which is often considered to be a bad practice. It's better to leave it as it is and submit the empty line removal as a separate patch.

Copy link
Owner Author

@JurajMarcin JurajMarcin Nov 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought that the spaces and also the line was added by me by accident. Will remove it from the patch.

@JurajMarcin JurajMarcin force-pushed the selinux-prefix2-submit branch 2 times, most recently from c6deff4 to eb036e5 Compare November 9, 2023 08:36
@JurajMarcin JurajMarcin force-pushed the selinux-prefix2-submit branch 3 times, most recently from 65526e3 to 80574e3 Compare November 24, 2023 10:16
JurajMarcin pushed a commit that referenced this pull request Apr 5, 2024
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]>
…nsitions

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 patch implements the equivalent changes made by this kernel
patch [1].

This patch updates the policydb structure to contain prefix and suffix
filename transition tables along normal filename transitions table and
updates the code that accesses those tables. Furthermore, it adds
match_type attribute to module and CIL structures that store filename
transitions and updates functions that parse conf and CIL policy files.

This patch does not significantly change the binary policy size when
prefix/suffix rules are not used. In addition, with prefix/suffix rules,
the number of filename transitions can be reduced, and therefore also
binary policy size can be reduced.

Syntax of the new prefix/suffix filename transition rule:

    type_transition source_type target_type : class default_type object_name match_type;

    (typetransition source_type_id target_type_id class_id object_name match_type default_type_id)

where match_type is either keyword "prefix" or "suffix"

Examples:

    type_transition ta tb:CLASS01 tc "file01" prefix;
    type_transition td te:CLASS01 tf "file02" suffix;

    (typetransition ta tb CLASS01 "file01" prefix td)
    (typetransition td te CLASS01 "file02" suffix tf)

In the kernel, the rules have the following order of priority, if no
matching rule is found, the code moves on to the next category:
- exact filename transitions,
- prefix filename transitions in the order of the longest prefix match,
- suffix filename transitions in the order of the longest suffix match.
This ensures the compatibility with older policies.

[1]: https://lore.kernel.org/selinux/[email protected]/

Reviewed-by: Ondrej Mosnacek <[email protected]>
Signed-off-by: Juraj Marcin <[email protected]>
---
v8:
- rebased to the latest main, fixed conflicts in checkpolicy/policy_parse.y

v7:
- rebased to the latest main, fixed conflicts in libsepol/src/kernel_to_*

v6:
- removed match_type_str from cil structures and moved match type
  parsing to the build phase, as suggested by Jim
- add match string to map arg structs instead of match type, as
  suggested by Jim

v5:
- rebased to the latest main, fixed conflicts in libsepol/cil

v4:
- rebased to the latest main, fixed conflicts in policydb_validate.c
- added syntax tests to checkpolicy and secilc as proposed by Christian
- retested with more test cases

v3:
- reworked the solution from scratch, this time only adding the
  prefix/suffix matching feature without moving filename transition
  rules to the avtab
@JurajMarcin JurajMarcin force-pushed the selinux-prefix2-submit branch from 80574e3 to 55357e9 Compare April 23, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants