SELinuxProject / selinux-kernel

GitHub mirror of the SELinux kernel repository
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
Other
149 stars 60 forks source link

RFE: dac_override false positives and inadequate audit info #6

Closed stephensmalley closed 4 years ago

stephensmalley commented 7 years ago

At present, CAP_DAC_OVERRIDE is checked by the kernel first even if only read/search access is requested, and then CAP_DAC_READ_SEARCH is checked if CAP_DAC_OVERRIDE is not allowed. This causes SELinux to audit dac_override denials in many cases where dac_override is not truly required, which leads to overly liberal policy. Also, since capable() does not provide the inode information, dac_override and dac_read_search denials do not provide information about the relevant path unless system call auditing is enabled and at least one syscall audit filter is defined. However, the kernel now calls capable_wrt_inode_uidgid() for these checks, so we could pass down the inode to the security hook in those cases and allow auditing of the file with the avc denial itself.

stephensmalley commented 7 years ago

https://www.mail-archive.com/selinux@tycho.nsa.gov/msg03970.html will address the check ordering issue, if accepted by the vfs folks. With that change, if dac_read_search is allowed in policy and only read/search access is requested, we will no longer generate a dac_override avc denial. We will still however end up auditing both dac_override and dac_read_search if both are denied, but I can live with that. Auditing the specific file involved without depending on syscall audit would be a separate logical change.

pcmoore commented 5 years ago

For reference, this was merged upstream with commit 2a4c22426955d4fc04069811997b7390c0fb858e. Can we close this out @stephensmalley?

stephensmalley commented 5 years ago

No, we still haven't addressed the inadequate audit info part of the issue. Also we still audit both denials if both are denied.

stephensmalley commented 5 years ago

https://patchwork.kernel.org/patch/11096447/ was an RFC to pass object information to capable calls that would partly alleviate this issue.

stephensmalley commented 5 years ago

https://patchwork.kernel.org/patch/11096201/ was an RFC to allow LSMs to selectively enable audit collection that could partly alleviate this issue.

stephensmalley commented 4 years ago

Going to close this issue because we fixed the order of checks, are unlikely to change the "audit both denials if both are denied" behavior, and RFC patches were posted for enabling the auditing of additional information but tabled pending a clearer use case / pull from someone.