Re: [RFC 4/8] lsm: smack: smack callbacks for kdbus security hooks

[Casey Schaufler <casey@schaufler-ca.com>]

On 7/8/2015 6:42 AM, Stephen Smalley wrote:
> On 07/08/2015 06:25 AM, Paul Osmialowski wrote:
>> This adds implementation of three smack callbacks sitting behind kdbus
>> security hooks as proposed by Karol Lewandowski.
>>
>> Originates from:
>>
>> git://git.infradead.org/users/pcmoore/selinux (branch: working-kdbus)
>> commit: fc3505d058c001fe72a6f66b833e0be5b2d118f3
>>
>> https://github.com/lmctl/linux.git (branch: kdbus-lsm-v4.for-systemd-v212)
>> commit: 103c26fd27d1ec8c32d85dd3d85681f936ac66fb
>>
>> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
>> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
>> ---
>>  security/smack/smack_lsm.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index a143328..033b756 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -41,6 +41,7 @@
>>  #include <linux/msg.h>
>>  #include <linux/shm.h>
>>  #include <linux/binfmts.h>
>> +#include <kdbus/connection.h>
>>  #include "smack.h"
>>  
>>  #define TRANS_TRUE	"TRUE"
>> @@ -3336,6 +3337,69 @@ static int smack_setprocattr(struct task_struct *p, char *name,
>>  }
>>  
>>  /**
>> + * smack_kdbus_connect - Set the security blob for a KDBus connection
>> + * @conn: the connection
>> + * @secctx: smack label
>> + * @seclen: smack label length
>> + *
>> + * Returns 0
>> + */
>> +static int smack_kdbus_connect(struct kdbus_conn *conn,
>> +			       const char *secctx, u32 seclen)
>> +{
>> +	struct smack_known *skp;
>> +
>> +	if (secctx && seclen > 0)
>> +		skp = smk_import_entry(secctx, seclen);
>> +	else
>> +		skp = smk_of_current();
>> +	conn->security = skp;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * smack_kdbus_conn_free - Clear the security blob for a KDBus connection
>> + * @conn: the connection
>> + *
>> + * Clears the blob pointer
>> + */
>> +static void smack_kdbus_conn_free(struct kdbus_conn *conn)
>> +{
>> +	conn->security = NULL;
>> +}
>> +
>> +/**
>> + * smack_kdbus_talk - Smack access on KDBus
>> + * @src: source kdbus connection
>> + * @dst: destination kdbus connection
>> + *
>> + * Return 0 if a subject with the smack of sock could access
>> + * an object with the smack of other, otherwise an error code
>> + */
>> +static int smack_kdbus_talk(const struct kdbus_conn *src,
>> +			    const struct kdbus_conn *dst)
>> +{
>> +	struct smk_audit_info ad;
>> +	struct smack_known *sskp = src->security;
>> +	struct smack_known *dskp = dst->security;
>> +	int ret;
>> +
>> +	BUG_ON(sskp == NULL);
>> +	BUG_ON(dskp == NULL);
>> +
>> +	if (smack_privileged(CAP_MAC_OVERRIDE))
>> +		return 0;
>> +
>> +	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_NONE);
>> +
>> +	ret = smk_access(sskp, dskp, MAY_WRITE, &ad);
>> +	if (ret)
>> +		return ret;
>> +	return 0;
>> +}
>> +
>> +/**
>>   * smack_unix_stream_connect - Smack access on UDS
>>   * @sock: one sock
>>   * @other: the other sock
>> @@ -4393,6 +4457,10 @@ struct security_hook_list smack_hooks[] = {
>>  	LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
>>  	LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
>>  	LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
>> +
>> +	LSM_HOOK_INIT(kdbus_connect, smack_kdbus_connect),
>> +	LSM_HOOK_INIT(kdbus_conn_free, smack_kdbus_conn_free),
>> +	LSM_HOOK_INIT(kdbus_talk, smack_kdbus_talk),
>>  };
> If Smack only truly needs 3 hooks, then it begs the question of why
> there are so many other hooks defined.  Are the other hooks just to
> support finer-grained distinctions, or is Smack's coverage incomplete?

I haven't been following kdbus closely for a while, but the original
intent for Smack and kdbus was that it Smack controls would be on the
objects involved, and that to accomplish that only a small number of
hooks would be necessary. After all, Smack uses fewer hooks than SELinux
on other things. I do agree that without a user there is no point in
having hooks. If SELinux requires the other hooks we might want to
hold off on asking for the hooks until the SELinux implementation is
exposed. I also think that AppArmor should be examined as a potential
user of the hooks, just to make sure the hooks aren't excessively
oriented toward subject/object based security modules.

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>