SELinuxProject / selinux

This is the upstream repository for the Security Enhanced Linux (SELinux) userland libraries and tools. The software provided by this project complements the SELinux features integrated into the Linux kernel and is used by Linux distributions. All bugs and patches should be submitted to selinux@vger.kernel.org
Other
1.32k stars 356 forks source link

Make audit2allow more intelligent #31

Open stephensmalley opened 7 years ago

stephensmalley commented 7 years ago

audit2allow is pretty dumb, and for better or worse many users and developers rely on it to produce policy. Enhance it to support automatic generation/suggestion of new domains/types rather than only producing allow rules within the current domain/type space, to provide better assistance with MLS or other constraint denials, to support other macros/interfaces besides refpolicy (e.g. Android), and to help guide the user in making sound choices (e.g. don't allow dac_override if you only need dac_read_search).

ghost commented 7 years ago

On 11/17/2016 08:55 PM, stephensmalley wrote:

audit2allow is pretty dumb, and for better or worse many users and developers rely on it to produce policy. Enhance it to support automatic generation/suggestion of new domains/types rather than only producing allow rules within the current domain/type space, to provide better assistance with MLS or other constraint denials, to support other macros/interfaces besides refpolicy (e.g. Android), and to help guide the user in making sound choices (e.g. don't allow dac_override if you only need dac_read_search).

AFAIK there is no instance where you don't need dac_override. As a matter of fact, i believe that we could get rid of dac_read_search altogether. Since dac_override is checked before dac_read_search and since dac_override is a superset of dac_read_search.

Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 Dominick Grift

stephensmalley commented 7 years ago

No, you don't want to allow dac_override unless the program truly requires write permission. That's the problem. See https://github.com/SELinuxProject/selinux-kernel/issues/6. Many programs only truly need dac_read_search. The problem is that people keep adding allow dac_override in policy because that is what they see in audit logs.

ghost commented 7 years ago

On 11/17/2016 09:01 PM, stephensmalley wrote:

No, you don't want to allow dac_override unless the program truly requires write permission. That's the problem. See https://github.com/SELinuxProject/selinux-kernel/issues/6. Many programs only truly need dac_read_search. The problem is that people keep adding allow dac_override in policy because that is what they see in audit logs.

If you block/deny the dac_override event, then it , AFAIK, never reaches the dac_read_search, since dac_override is a superset of dac_read_search.

In other words you in practice you end up having to use dac_override anyway

Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 Dominick Grift

stephensmalley commented 7 years ago

No, that isn't true. The kernel source code tells a different story, see linux/fs/namei.c generic_permission(). It checks CAP_DAC_OVERRIDE first. If that passes, it returns 0 (success). If it fails and the mask did not request MAY_WRITE (i.e. only read/search/execute access), then it checks CAP_DAC_READ_SEARCH. If that passes, then it returns 0 (success).

ghost commented 7 years ago

On 11/17/2016 09:06 PM, stephensmalley wrote:

No, that isn't true. The kernel source code tells a different story, see linux/fs/namei.c generic_permission(). It checks CAP_DAC_OVERRIDE first. If that passes, it returns 0 (success). If it fails and the mask did not request MAY_WRITE (i.e. only read/search/execute access), then it checks CAP_DAC_READ_SEARCH. If that passes, then it returns 0 (success).

Okay if the code say's so. I would be more confident if you could tell me that you actually were able to confirm the codes story because I never was able to get it to work that way.

In my experience, dac_override is checked first and if you deny that access then it never reached dac_read_search and this is way i always end up using dac_override anyway.

Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 Dominick Grift

stephensmalley commented 7 years ago

I've seen it in practice as well, where I allowed dac_read_search only and not dac_override, and the program worked.

ghost commented 7 years ago

On 11/17/2016 09:12 PM, stephensmalley wrote:

I've seen it in practice as well, where I allowed dac_read_search only and not dac_override, and the program worked.

Okay , thank you

Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 Dominick Grift

ghost commented 7 years ago

On 11/17/2016 09:13 PM, Dominick Grift wrote:

On 11/17/2016 09:12 PM, stephensmalley wrote:

I've seen it in practice as well, where I allowed dac_read_search only and not dac_override, and the program worked.

Okay , thank you

Yes you were right. I was able to confirm that it works.

In practice it might not always be able to determine whether it works though.

Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 Dominick Grift

ghost commented 7 years ago

On 11/17/2016 09:24 PM, Dominick Grift wrote:

On 11/17/2016 09:13 PM, Dominick Grift wrote:

On 11/17/2016 09:12 PM, stephensmalley wrote:

I've seen it in practice as well, where I allowed dac_read_search only and not dac_override, and the program worked.

Okay , thank you

Yes you were right. I was able to confirm that it works.

In practice it might not always be able to determine whether it works though.

s/able/easy/

Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 Dominick Grift

stephensmalley commented 7 years ago

Yes, that's why i opened the issue on the selinux-kernel project to fix it so that we do not audit dac_override unless we truly need it. But in the meantime, it is something to keep in mind when generating policy.