Message-id: <561F5A65.8070502@samsung.com>
Date:	Thu, 15 Oct 2015 09:48:53 +0200
From:	Rafał Krypa <r.krypa@...sung.com>
To:	Casey Schaufler <casey@...aufler-ca.com>
Cc:	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Zbigniew Jasinski <z.jasinski@...sung.com>,
	Tomasz Swierczek <t.swierczek@...sung.com>
Subject: Re: [PATCH v4] Smack: limited capability for changing process label

On 2015-10-14 17:54, Rafal Krypa wrote:
> From: Zbigniew Jasinski <z.jasinski@...sung.com>
>
> This feature introduces new kernel interface:
>
> - <smack_fs>/relabel-self - for setting transition labels list
>
> This list is used to control smack label transition mechanism.
> List is set by, and per process. Process can transit to new label only if
> label is on the list. Only process with CAP_MAC_ADMIN capability can add
> labels to this list. With this list, process can change it's label without
> CAP_MAC_ADMIN but only once. After label changing, list is unset.
>
> Changes in v2:
> * use list_for_each_entry instead of _rcu during label write
> * added missing description in security/Smack.txt
>
> Changes in v3:
> * squashed into one commit
>
> Changes in v4:
> * switch from global list to per-task list
> * since the per-task list is accessed only by the task itself
>   there is no need to use synchronization mechanisms on it
>
> Signed-off-by: Zbigniew Jasinski <z.jasinski@...sung.com>
> Signed-off-by: Rafal Krypa <r.krypa@...sung.com>
> ---
>  Documentation/security/Smack.txt |  14 ++++
>  security/smack/smack.h           |   3 +-
>  security/smack/smack_access.c    |   6 +-
>  security/smack/smack_lsm.c       |  73 ++++++++++++++++-
>  security/smack/smackfs.c         | 167 ++++++++++++++++++++++++++++++++++++---
>  5 files changed, 246 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
> index 5e6d07f..d9ace08 100644
> --- a/Documentation/security/Smack.txt
> +++ b/Documentation/security/Smack.txt
> @@ -255,6 +255,20 @@ unconfined
>  	the access permitted if it wouldn't be otherwise. Note that this
>  	is dangerous and can ruin the proper labeling of your system.
>  	It should never be used in production.
> +relabel-self
> +	This interface contains a list of labels to which the process can
> +	transition to, by writing to /proc/self/attr/current.
> +	Normally a process can change its own label to any legal value, but only
> +	if it has CAP_MAC_ADMIN. This interface allows a process without
> +	CAP_MAC_ADMIN to relabel itself to one of labels from predefined list.
> +	A process without CAP_MAC_ADMIN can change its label only once. When it
> +	does, this list will be cleared.
> +
> +	The format accepted on write is:
> +		"%s"
> +	for adding label, and:
> +		"-%s"
> +	for removing label from list.

I have one concern here, let me make some self-criticism.
The interface described here for relabel-self is convenient and suiting actual needs of user space parts that are going to use it.
But it is inconsistent with other existing interfaces in smackfs. Recently I submitted a patch (merged into v4.2) that extended onlycap to allow multiple labels in it.
The smackfs interface for onlycap always takes the full list of labels that replaces the list that was previously set.
Now relabel-self is also going to contain a list of labels. But smackfs interface gets one label at a time and performs add/remove operations.

Are you OK. with such inconsistency?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists