-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Stephen Smalley wrote:
> On Mon, 2008-03-10 at 18:35 -0400, Eric Paris wrote:
>> Introduce the concept of a permissive type. A new ebitmap is introduced
>> to the policy database which indicates if a given type has the
>> permissive bit set or not. This bit is tested for the scontext of any
>> denial. The bit is meaningless on types which only appear as the target
>> of a decision and never the source. A domain running with a permissive
>> type will be allowed to perform any action similarly to when the system
>> is globally set permissive.
>>
>> Signed-off-by: Eric Paris <eparis@xxxxxxxxxx>
>>
>> ---
>>
>> - Threw out all of the atomic stuff. Either the sid is going to get
>> mapped to unlabeled or to the right context in the new policydb and will
>> be the new type number and all will be well. As usual I was just making
>> things harder on myself.
>
> Technically, the following could still happen:
> - policy has domain marked as permissive and does not allow an access,
> - the AVC determines that the access is denied and is following the
> permissive code path but has not yet taken the policy rdlock,
> - policy is reloaded with domain marked as non-permissive with access
> allowed,
> - security_permissive_sid is called and takes the policy rdlock,
> - the domain is found to be non-permissive,
> - the AVC denies the access.
>
> Particularly if we do something like:
> - insert policy module that just makes a domain permissive,
> - run for a while collecting audit messages,
> - generate new policy module from audit messages,
> - replace the permissive policy module with the one that contains the
> allows.
>
> Don't know if you are concerned about the above, but it could create
> mysterious breakage.
I think this is highly unlikely. I believe the standard way to use
permissive domains, would be to ship a policy. Once you have gone a
considerable amount of time without any AVC messages, you would reload
the policy with the permissive domain removed. So I don't think this
race is likely to happen unless forced.
>
>> - moved !requested into its own handling
>
> Separate patch, put it first, and I think BUG_ON is cleaner there.
>
>> - dropped unlikely() around the permissive sid, because really, if we
>> get a denial and we are selinux_enforcing its actually not all that
>> unlikely that we might not have a permissive domain. Just let the
>> compiler do its thing.
>
> Ok.
>
>> - dropped the test for the existence of node. avc_update_node might
>> return -ENOENT, but really who cares? We are on a slow code path here
>> so it seems to be needless complexity. Isn't this code nice and clear
>> now? I can add it in, but its just a slight perf win and not much
>> else..
>
> Seems fine - we will rarely not have a node (requires that there not
> already be a node for the key, and that the allocation of a new node
> fail), and it is harmless to proceed.
>> security/selinux/Kconfig | 2 +-
>> security/selinux/avc.c | 18 +++++++++++++-----
>> security/selinux/include/security.h | 5 ++++-
>> security/selinux/ss/policydb.c | 11 +++++++++++
>> security/selinux/ss/policydb.h | 2 ++
>> security/selinux/ss/services.c | 21 +++++++++++++++++++++
>> 6 files changed, 52 insertions(+), 7 deletions(-)
>>
>> diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
>> index 2b517d6..a436d1c 100644
>> --- a/security/selinux/Kconfig
>> +++ b/security/selinux/Kconfig
>> @@ -145,7 +145,7 @@ config SECURITY_SELINUX_POLICYDB_VERSION_MAX
>> config SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
>> int "NSA SELinux maximum supported policy format version value"
>> depends on SECURITY_SELINUX_POLICYDB_VERSION_MAX
>> - range 15 22
>> + range 15 23
>> default 19
>> help
>> This option sets the value for the maximum policy format version
>> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
>> index 187964e..8d78b88 100644
>> --- a/security/selinux/avc.c
>> +++ b/security/selinux/avc.c
>> @@ -890,13 +890,21 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid,
>>
>> denied = requested & ~(p_ae->avd.allowed);
>>
>> - if (!requested || denied) {
>> - if (selinux_enforcing || (flags & AVC_STRICT))
>> + if (unlikely(!requested)) {
>> + printk(KERN_ERR "%s: found !requested, fix me\n", __func__);
>
> Likely want SELinux somewhere in that message for clarity.
>
>> + WARN_ON(1);
>> + if (selinux_enforcing)
>> rc = -EACCES;
>
> Simpler as a BUG_ON, and correct.
>
>> + }
>> +
>> + if (denied) {
>> + if (flags & AVC_STRICT)
>> + rc = -EACCES;
>> + else if (!selinux_enforcing || security_permissive_sid(ssid))
>> + avc_update_node(AVC_CALLBACK_GRANT, requested, ssid,
>> + tsid, tclass);
>> else
>> - if (node)
>> - avc_update_node(AVC_CALLBACK_GRANT,requested,
>> - ssid,tsid,tclass);
>> + rc = -EACCES;
>> }
>>
>> rcu_read_unlock();
>> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
>> index f7d2f03..fde8690 100644
>> --- a/security/selinux/include/security.h
>> +++ b/security/selinux/include/security.h
>> @@ -26,13 +26,14 @@
>> #define POLICYDB_VERSION_AVTAB 20
>> #define POLICYDB_VERSION_RANGETRANS 21
>> #define POLICYDB_VERSION_POLCAP 22
>> +#define POLICYDB_VERSION_PERMISSIVE 23
>>
>> /* Range of policy versions we understand*/
>> #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE
>> #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
>> #define POLICYDB_VERSION_MAX CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
>> #else
>> -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_POLCAP
>> +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_PERMISSIVE
>> #endif
>>
>> #define CONTEXT_MNT 0x01
>> @@ -67,6 +68,8 @@ struct av_decision {
>> u32 seqno;
>> };
>>
>> +int security_permissive_sid(u32 sid);
>> +
>> int security_compute_av(u32 ssid, u32 tsid,
>> u16 tclass, u32 requested,
>> struct av_decision *avd);
>> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
>> index bd7d6a0..84dbab9 100644
>> --- a/security/selinux/ss/policydb.c
>> +++ b/security/selinux/ss/policydb.c
>> @@ -111,6 +111,11 @@ static struct policydb_compat_info policydb_compat[] = {
>> .version = POLICYDB_VERSION_POLCAP,
>> .sym_num = SYM_NUM,
>> .ocon_num = OCON_NUM,
>> + },
>> + {
>> + .version = POLICYDB_VERSION_PERMISSIVE,
>> + .sym_num = SYM_NUM,
>> + .ocon_num = OCON_NUM,
>> }
>> };
>>
>> @@ -194,6 +199,7 @@ static int policydb_init(struct policydb *p)
>> goto out_free_symtab;
>>
>> ebitmap_init(&p->policycaps);
>> + ebitmap_init(&p->permissive_map);
>>
>> out:
>> return rc;
>> @@ -687,6 +693,7 @@ void policydb_destroy(struct policydb *p)
>> kfree(p->type_attr_map);
>> kfree(p->undefined_perms);
>> ebitmap_destroy(&p->policycaps);
>> + ebitmap_destroy(&p->permissive_map);
>>
>> return;
>> }
>> @@ -1570,6 +1577,10 @@ int policydb_read(struct policydb *p, void *fp)
>> ebitmap_read(&p->policycaps, fp) != 0)
>> goto bad;
>>
>> + if (p->policyvers >= POLICYDB_VERSION_PERMISSIVE &&
>> + ebitmap_read(&p->permissive_map, fp) != 0)
>> + goto bad;
>> +
>> info = policydb_lookup_compat(p->policyvers);
>> if (!info) {
>> printk(KERN_ERR "security: unable to find policy compat info "
>> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
>> index c4ce996..ba593a3 100644
>> --- a/security/selinux/ss/policydb.h
>> +++ b/security/selinux/ss/policydb.h
>> @@ -243,6 +243,8 @@ struct policydb {
>>
>> struct ebitmap policycaps;
>>
>> + struct ebitmap permissive_map;
>> +
>> unsigned int policyvers;
>>
>> unsigned int reject_unknown : 1;
>> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
>> index f374186..f5d1dcc 100644
>> --- a/security/selinux/ss/services.c
>> +++ b/security/selinux/ss/services.c
>> @@ -416,6 +416,27 @@ inval_class:
>> return -EINVAL;
>> }
>>
>> +/*
>> + * Given a sid find if the type has the permissive flag set
>> + */
>> +int security_permissive_sid(u32 sid)
>> +{
>> + struct context *context;
>> + u32 type;
>> + int rc;
>> +
>> + POLICY_RDLOCK;
>> +
>> + context = sidtab_search(&sidtab, sid);
>> + BUG_ON(!context);
>> +
>> + type = context->type;
>> + rc = ebitmap_get_bit(&policydb.permissive_map, type);
>
> Maybe a comment then to note that we are intentionally not using (type -
> 1) here due to possibly global use of that bit - otherwise it is a
> confusing inconsistency with other ebitmap_get_bit calls.
>
>> +
>> + POLICY_RDUNLOCK;
>> + return rc;
>> +}
>> +
>> static int security_validtrans_handle_fail(struct context *ocontext,
>> struct context *ncontext,
>> struct context *tcontext,
>>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iEYEARECAAYFAkfXQEIACgkQrlYvE4MpobNsBgCffHW2x1Pq3g8ArEcgSF3HfUBG
nEgAoOMQlwnm0+g4VwC3IAF5vPgwTCkF
=wSvv
-----END PGP SIGNATURE-----
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.
