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

Align policy classes and access vectors to kernel definitions #57

Closed
stephensmalley opened this issue May 18, 2017 · 5 comments
Closed

Comments

@stephensmalley
Copy link
Member

stephensmalley commented May 18, 2017

This is really an issue for refpolicy and Android policy but putting it here so I remember to do it, since it is unlikely anyone else will.

There have been a number of changes to the kernel class/perm definitions (moving shared permissions to common definitions, removing obsolete classes/perms) since we switched to dynamic class/perm mappings that have not been mirrored to refpolicy or (to a lesser extent) Android policy due to concerns with backward compatibility. Since even RHEL6 has the dynamic class/perm mapping support back-ported, we ought to be able to do this even in refpolicy for many of the changes. Optimally, some day, we'd like to be able to completely synchronize them such that we have the identity mapping between the kernel and policy definitions, even though that's not necessary. Current caveats:

  • Userspace can be brittle in the face of changes to their class definitions. Some old userspace used hardcoded definitions and will break if they change at all. Modern userspace supports at least dynamic class/perm mappings at initialization (e.g. selinux_set_mapping), but not all yet support dynamic class/perm mapping upon policy reload (e.g. selinux_check_access or equivalent). dbusd is the biggest known remaining offender, and needs its SELinux support overhauled in multiple ways. Xorg/XSELinux also has the same issue but it isn't used by default so it is less critical. Until we fix userspace, we can't make any changes that disturb the userspace class indices (which could happen e.g. just by removing or inserting a kernel class that precedes them, unless we replace it with a dummy placeholder class).

  • Dropping of netlink_firewall and netilnk_ip6_fw didn't happen until Linux v3.5, so we can't drop unless we are willing to break compatibility for kernels < 3.5 (including RHEL6).

Android policy has already dropped unused permission definitions (only used in pre-mainline or compat_net < 2.6.30) and netlink_firewall and netlink_ip6_fw classes (< 3.5). Android policy doesn't include the Linux userspace object manager classes and all of its userspace object managers use selinux_check_access(), so they are already safe (plus Android does not officially support runtime policy reloads, although some devices do).

Work for this issue would include:

  • Applying the same changes already done for Android policy to refpolicy, if acceptable from a refpolicy compatibility POV.
  • Synchronizing common definitions between the kernel and both Android policy and refpolicy.
  • Checking for any other inconsistencies that may have arisen between the kernel definitions and either refpolicy or Android policy.
@pcmoore
Copy link
Member

pcmoore commented May 18, 2017

FYI @wrabcak

@stephensmalley
Copy link
Member Author

pebenito took care of some of this for refpolicy via:
SELinuxProject/refpolicy@e5dbe75
SELinuxProject/refpolicy@c656b97
SELinuxProject/refpolicy@3952ecb

I just opened a PR for removing unused permissions in:
SELinuxProject/refpolicy#159

If/when that gets merged, I have another commit to remove the netlink_firewall and netlink_ip6fw classes that were removed from the kernel a while back but that could break legacy userspace object managers if not using dynamic class/perm support.

Looks like the ipc class is no longer used and could be dropped from both kernel and policy. It was a fallback in case we couldn't determine the finer-grained shm/sem/msgq class in older code.

Could then also re-order security_classes to match classmap.h order if desired, putting all kernel classes together at the beginning in the same order as the kernel to make the mapping the identity mapping. That also could break legacy userspace. Not strictly necessary but kind of nice if all the kernel classes were contiguous in policy.

FYI @WOnder93 @pebenito @jeffvanderstoep

@stephensmalley
Copy link
Member Author

SELinuxProject/refpolicy#161 removes the two obsolete netlink classes. This change unlike the previous one is likely not compatible with RHEL6 kernels, and could possibly impact userspace on RHEL7 if it still has any userspace object managers that use the old flask.h/av_permissions.h definitions for classes/perms instead of dynamically mapping them.

@stephensmalley
Copy link
Member Author

Amended SELinuxProject/refpolicy#161 to rename instead of remove to preserve userspace compatibility.
Given userspace compatibility constraints impacting even RHEL8 for dbus-daemon, don't think we can ever re-order security_classes to match classmap order, so dropping that as a goal for this issue.
So I think I can close this issue once the above PR is merged (or rejected).

@stephensmalley
Copy link
Member Author

refpolicy is now aligned to the kernel classes and access vector definitions aside from some class ordering issues (unfixable without breaking userspace) and not yet having recently added classes (perf_event, lockdown). It already has the watch permissions, unlike Fedora policy.
Some of the changes were already in Android policy and I submitted the rest just now so it should be aligned shortly unless they decide they cannot remove the chr_file:execute_no_trans/entrypoint and packet:flow_in/flow_out definitions compatibly. I'm going to close this issue as complete to the extent feasible today.

gbertao pushed a commit to gbertao/aosp-mirror_platform_system_sepolicy that referenced this issue Mar 26, 2024
Patch Set 1:

Alternatively, the watch* permissions could have been added to the common file definition and inherited by all of the dir/*_file classes, as they are in the kernel classmap. Adding them to the end of the classes was only necessary for backward kernel compatibility for kernels <= 2.6.32 (before dynamic class/perm mapping support), which is presumably not a problem for Android these days.  Similarly, the open, audit_access, and execmod permissions could be moved to common file, e.g. see fedora-selinux/selinux-policy#274 and SELinuxProject/selinux#57

Patch-set: 1
CC: Gerrit User 1010111 <1010111@85c56323-6fa9-3386-8a01-6480fb634889>
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

No branches or pull requests

2 participants