Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1191734 > unrolled thread

[PATCH] Smack: replace capable() with ns_capable()

Back to article view | Back to linux.kernel

Contents

  [PATCH] Smack: replace capable() with ns_capable() Sungbae Yoo <sungbae.yoo@samsung.com> - 2015-07-24 13:30 +0200
    Re: [PATCH] Smack: replace capable() with ns_capable() Lukasz Pawelczyk <l.pawelczyk@samsung.com> - 2015-07-24 13:50 +0200
      Re: [PATCH] Smack: replace capable() with ns_capable() Casey Schaufler <casey@schaufler-ca.com> - 2015-07-25 19:00 +0200

#1191734 — [PATCH] Smack: replace capable() with ns_capable()

If current task has capabilities, Smack operations (eg. Changing own smack
label) should be available even inside of namespace.

Signed-off-by: Sungbae Yoo <sungbae.yoo@samsung.com>

diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 00f6b38..f6b2c35 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -639,7 +639,7 @@ int smack_privileged(int cap)
 	struct smack_known *skp = smk_of_current();
 	struct smack_onlycap *sop;
 
-	if (!capable(cap))
+	if (!ns_capable(current_user_ns(), cap))
 		return 0;
 
 	rcu_read_lock();
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a143328..7fdc3dd 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -403,7 +403,8 @@ static int smk_ptrace_rule_check(struct task_struct *tracer,
 			rc = 0;
 		else if (smack_ptrace_rule == SMACK_PTRACE_DRACONIAN)
 			rc = -EACCES;
-		else if (capable(CAP_SYS_PTRACE))
+		else if (ns_capable(__task_cred(tracer)->user_ns,
+				    CAP_SYS_PTRACE))
 			rc = 0;
 		else
 			rc = -EACCES;
-- 
1.9.1

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

[toc] | [next] | [standalone]

#1191746

On pią, 2015-07-24 at 20:26 +0900, Sungbae Yoo wrote:
> If current task has capabilities, Smack operations (eg. Changing own 
> smack
> label) should be available even inside of namespace.
> 
> Signed-off-by: Sungbae Yoo <sungbae.yoo@samsung.com>
> 
> diff --git a/security/smack/smack_access.c 
> b/security/smack/smack_access.c
> index 00f6b38..f6b2c35 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -639,7 +639,7 @@ int smack_privileged(int cap)
>  	struct smack_known *skp = smk_of_current();
>  	struct smack_onlycap *sop;
>  
> -	if (!capable(cap))
> +	if (!ns_capable(current_user_ns(), cap))
>  		return 0;

It's not that easy.

With this change Smack becomes completely insecure. You can change
rules as an unprivileged user without any problems now.
What you want is Smack namespace that was made to remedy exactly this
issue (e.g. changing own labels inside a namespace).

>  
>  	rcu_read_lock();
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index a143328..7fdc3dd 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -403,7 +403,8 @@ static int smk_ptrace_rule_check(struct 
> task_struct *tracer,
>  			rc = 0;
>  		else if (smack_ptrace_rule == 
> SMACK_PTRACE_DRACONIAN)
>  			rc = -EACCES;
> -		else if (capable(CAP_SYS_PTRACE))
> +		else if (ns_capable(__task_cred(tracer)->user_ns,
> +				    CAP_SYS_PTRACE))
>  			rc = 0;
>  		else
>  			rc = -EACCES;
-- 
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics



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

[toc] | [prev] | [next] | [standalone]

#1192406

On 7/24/2015 4:40 AM, Lukasz Pawelczyk wrote:
> On pią, 2015-07-24 at 20:26 +0900, Sungbae Yoo wrote:
>> If current task has capabilities, Smack operations (eg. Changing own 
>> smack
>> label) should be available even inside of namespace.
>>
>> Signed-off-by: Sungbae Yoo <sungbae.yoo@samsung.com>

For the reasons Lukasz outlines below.

Nacked-by: Casey Schaufler <casey@schaufler-ca.com>

>>
>> diff --git a/security/smack/smack_access.c 
>> b/security/smack/smack_access.c
>> index 00f6b38..f6b2c35 100644
>> --- a/security/smack/smack_access.c
>> +++ b/security/smack/smack_access.c
>> @@ -639,7 +639,7 @@ int smack_privileged(int cap)
>>  	struct smack_known *skp = smk_of_current();
>>  	struct smack_onlycap *sop;
>>  
>> -	if (!capable(cap))
>> +	if (!ns_capable(current_user_ns(), cap))
>>  		return 0;
> It's not that easy.
>
> With this change Smack becomes completely insecure. You can change
> rules as an unprivileged user without any problems now.
> What you want is Smack namespace that was made to remedy exactly this
> issue (e.g. changing own labels inside a namespace).
>
>>  
>>  	rcu_read_lock();
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index a143328..7fdc3dd 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -403,7 +403,8 @@ static int smk_ptrace_rule_check(struct 
>> task_struct *tracer,
>>  			rc = 0;
>>  		else if (smack_ptrace_rule == 
>> SMACK_PTRACE_DRACONIAN)
>>  			rc = -EACCES;
>> -		else if (capable(CAP_SYS_PTRACE))
>> +		else if (ns_capable(__task_cred(tracer)->user_ns,
>> +				    CAP_SYS_PTRACE))
>>  			rc = 0;
>>  		else
>>  			rc = -EACCES;

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web