-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 04/27/2010 09:47 AM, Stephen Smalley wrote:
> On Fri, 2010-04-09 at 18:16 -0400, Eric Paris wrote:
>> Currently there are a number of applications (nautilus being the main one) which
>> calls access() on files in order to determine how they should be displayed. It
>> is normal and expected that nautilus will want to see if files are executable
>> or if they are really read/write-able. access() should return the real
>> permission. SELinux policy checks are done in access() and can result in lots
>> of AVC denials as policy denies RWX on files which DAC allows. Currently
>> SELinux must dontaudit actual attempts to read/write/execute a file in
>> order to silence these messages (and not flood the logs.) But dontaudit rules
>> like that can hide real attacks. This patch addes a new common file
>> permission audit_access. This permission is special in that it is meaningless
>> and should never show up in an allow rule. Instead the only place this
>> permission has meaning is in a dontaudit rule like so:
>>
>> dontaudit nautilus_t sbin_t:file audit_access
>>
>> With such a rule if nautilus just checks access() we will still get denied and
>> thus userspace will still get the correct answer but we will not log the denial.
>> If nautilus attempted to actually perform one of the forbidden actions
>> (rather than just querying access(2) about it) we would still log a denial.
>> This type of dontaudit rule should be used sparingly, as it could be a
>> method for an attacker to probe the system permissions without detection.
>
> So let's think about how this will likely play out in practice.
> If you add this check, what rules will Dan add to the standard policy?
> nautilus doesn't run in a separate domain nor is it likely to do so
> (otherwise you have to clone all of the user's permissions to it). So
> we'll likely end up with something like:
> dontaudit userdomain file_type:file audit_access;
>
> Right?
>
>> Signed-off-by: Eric Paris <eparis@xxxxxxxxxx>
>> ---
>>
>> security/selinux/hooks.c | 46 ++++++++++++++++++++++++++++++-----
>> security/selinux/include/classmap.h | 2 +-
>> 2 files changed, 40 insertions(+), 8 deletions(-)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 344ba62..34e9d1b 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -2696,19 +2696,51 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *na
>> return dentry_has_perm(cred, NULL, dentry, FILE__READ);
>> }
>>
>> -static int selinux_inode_permission(struct inode *inode, int mask)
>> +static int selinux_inode_permission(struct inode *inode, int in_mask)
>> {
>> const struct cred *cred = current_cred();
>> + struct inode_security_struct *isec;
>> + struct common_audit_data ad;
>> + struct av_decision avd;
>> + u32 sid, perms;
>> + int rc, mask;
>>
>> - mask &= (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND);
>> + mask = in_mask & (MAY_READ|MAY_WRITE|MAY_EXEC|MAY_APPEND);
>>
>> - if (!mask) {
>> - /* No permission to check. Existence test. */
>> + /* No permission to check. Existence test. */
>> + if (!mask)
>> + return 0;
>> +
>> + validate_creds(cred);
>> +
>> + if (unlikely(IS_PRIVATE(inode)))
>> return 0;
>
> This is handled by security_inode_permission(). The check inside
> inode_has_perm() stems from other code paths.
>
>> - }
>>
>> - return inode_has_perm(cred, inode,
>> - file_mask_to_av(inode->i_mode, mask), NULL);
>> + sid = cred_sid(cred);
>> + isec = inode->i_security;
>> +
>> + COMMON_AUDIT_DATA_INIT(&ad, FS);
>> + ad.u.fs.inode = inode;
>> +
>> + perms = file_mask_to_av(inode->i_mode, mask);
>> +
>> + rc = avc_has_perm_noaudit(sid, isec->sid, isec->sclass, perms, 0, &avd);
>> + /*
>> + * We want to audit if this call was not from access(2).
>> + * We also want to audit if the call was from access(2)
>> + * but the magic FILE__AUDIT_ACCESS permission was in the auditdeny
>> + * vector.
>> + *
>> + * aka there is a not dontaudit rule for file__audit_access. This
>> + * might make more sense as a test inside avc_audit, but then we would
>> + * have to push the MAY_ACCESS flag down to avc_audit and I think we
>> + * already have enough stuff down there.
>> + */
>
> Why can't we just push it down through inode_has_perm -> avc_has_perm ->
> avc_audit() via a field in common_audit_data?
>
>> + if (!(in_mask & MAY_ACCESS) ||
>> + (avd.auditdeny & FILE__AUDIT_ACCESS))
>> + avc_audit(sid, isec->sid, isec->sclass, perms, &avd, rc, &ad);
>> +
>> + return rc;
>> }
>>
>
>
Yes. Although I can probably start to remove some dontaudit stuff also.
Like every app that uses kernberos has a dontaudit rule
sesearch --dontaudit -t krb5_conf_t -p write -c file | wc
266 1874 14081
Might be able to remove the pam code that checks write on /etc/shadow also.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkvW+RwACgkQrlYvE4MpobNPfACgpUdEf1Wt/FzxmDmqetqv7Csl
4zYAoJeRlIj2Cml4duUFA9hwQy9HAp3g
=BhbJ
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
