2011-06-29 01:44:44

by John Johansen

[permalink] [raw]
Subject: [Patch v2 0/2] Fix for masking of complain mode, and rcu protected ref

A couple of outstanding fixes for apparmor
v2 - added Cc: to stable
- updated commit message wording

The following changes since commit b0af8dfdd67699e25083478c63eedef2e72ebd85:

Linux 3.0-rc5 (2011-06-27 19:12:22 -0700)

are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/apparmor-dev.git for-security

John Johansen (2):
AppArmor: Fix reference to rcu protected pointer outside of rcu_read_lock
AppArmor: Fix masking of capabilities in complain mode

security/apparmor/domain.c | 2 +-
security/apparmor/lsm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)


2011-06-29 01:44:57

by John Johansen

[permalink] [raw]
Subject: [PATCH 1/2] AppArmor: Fix reference to rcu protected pointer outside of rcu_read_lock

The pointer returned from tracehook_tracer_task() is only valid inside
the rcu_read_lock. However the tracer pointer obtained is being passed
to aa_may_ptrace outside of the rcu_read_lock critical section.

Mover the aa_may_ptrace test into the rcu_read_lock critical section, to
fix this.

Kernels affected: 2.6.36 - 3.0

Reported-by: Oleg Nesterov <[email protected]>
Cc: [email protected]
Signed-off-by: John Johansen <[email protected]>
---
security/apparmor/domain.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index c825c6e..78adc43 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -73,7 +73,6 @@ static int may_change_ptraced_domain(struct task_struct *task,
cred = get_task_cred(tracer);
tracerp = aa_cred_profile(cred);
}
- rcu_read_unlock();

/* not ptraced */
if (!tracer || unconfined(tracerp))
@@ -82,6 +81,7 @@ static int may_change_ptraced_domain(struct task_struct *task,
error = aa_may_ptrace(tracer, tracerp, to_profile, PTRACE_MODE_ATTACH);

out:
+ rcu_read_unlock();
if (cred)
put_cred(cred);

--
1.7.4.1

2011-06-29 01:45:06

by John Johansen

[permalink] [raw]
Subject: [PATCH 2/2] AppArmor: Fix masking of capabilities in complain mode

AppArmor is masking the capabilities returned by capget against the
capabilities mask in the profile. This is wrong, in complain mode the
profile has effectively all capabilities, as the profile restrictions are
not being enforced, merely tested against to determine if an access is
known by the profile.

This can result in the wrong behavior of security conscience applications
like sshd which examine their capability set, and change their behavior
accordingly. In this case because of the masked capability set being
returned sshd fails due to DAC checks, even when the profile is in complain
mode.

Kernels affected: 2.6.36 - 3.0.

Signed-off-by: John Johansen <[email protected]>
---
security/apparmor/lsm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 3d2fd14..3783202 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -127,7 +127,7 @@ static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
*inheritable = cred->cap_inheritable;
*permitted = cred->cap_permitted;

- if (!unconfined(profile)) {
+ if (!unconfined(profile) && !COMPLAIN_MODE(profile)) {
*effective = cap_intersect(*effective, profile->caps.allow);
*permitted = cap_intersect(*permitted, profile->caps.allow);
}
--
1.7.4.1

2011-06-29 03:05:21

by James Morris

[permalink] [raw]
Subject: Re: [Patch v2 0/2] Fix for masking of complain mode, and rcu protected ref

On Wed, 29 Jun 2011, John Johansen wrote:

> A couple of outstanding fixes for apparmor
> v2 - added Cc: to stable
> - updated commit message wording
>

Applied.

--
James Morris
<[email protected]>

2011-06-29 04:27:56

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2/2] AppArmor: Fix masking of capabilities in complain mode

On Wed, Jun 29, 2011 at 02:44:34AM +0100, John Johansen wrote:
> AppArmor is masking the capabilities returned by capget against the
> capabilities mask in the profile. This is wrong, in complain mode the
> profile has effectively all capabilities, as the profile restrictions are
> not being enforced, merely tested against to determine if an access is
> known by the profile.
>
> This can result in the wrong behavior of security conscience applications
> like sshd which examine their capability set, and change their behavior
> accordingly. In this case because of the masked capability set being
> returned sshd fails due to DAC checks, even when the profile is in complain
> mode.
>
> Kernels affected: 2.6.36 - 3.0.
>
> Signed-off-by: John Johansen <[email protected]>
> ---

You say that multiple kernels are affected, then why not also include
[email protected] here as well?

confused,

greg k-h

2011-06-29 07:55:26

by John Johansen

[permalink] [raw]
Subject: Re: [PATCH 2/2] AppArmor: Fix masking of capabilities in complain mode

On 06/29/2011 05:24 AM, Greg KH wrote:
> On Wed, Jun 29, 2011 at 02:44:34AM +0100, John Johansen wrote:
>> AppArmor is masking the capabilities returned by capget against the
>> capabilities mask in the profile. This is wrong, in complain mode the
>> profile has effectively all capabilities, as the profile restrictions are
>> not being enforced, merely tested against to determine if an access is
>> known by the profile.
>>
>> This can result in the wrong behavior of security conscience applications
>> like sshd which examine their capability set, and change their behavior
>> accordingly. In this case because of the masked capability set being
>> returned sshd fails due to DAC checks, even when the profile is in complain
>> mode.
>>
>> Kernels affected: 2.6.36 - 3.0.
>>
>> Signed-off-by: John Johansen <[email protected]>
>> ---
>
> You say that multiple kernels are affected, then why not also include
> [email protected] here as well?
>
> confused,
>
Sorry I should have elaborated, I think its a borderline case for stable
release, and was as much a note to my self as anything.

The bug doesn't affect the enforcement of policy, only learning mode used
for generating policy, and it is something we can work around in userspace.
for already released kernels.

I can resend with a Cc: stable if you would like


2011-06-29 15:16:09

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2/2] AppArmor: Fix masking of capabilities in complain mode

On Wed, Jun 29, 2011 at 08:55:18AM +0100, John Johansen wrote:
> On 06/29/2011 05:24 AM, Greg KH wrote:
> > On Wed, Jun 29, 2011 at 02:44:34AM +0100, John Johansen wrote:
> >> AppArmor is masking the capabilities returned by capget against the
> >> capabilities mask in the profile. This is wrong, in complain mode the
> >> profile has effectively all capabilities, as the profile restrictions are
> >> not being enforced, merely tested against to determine if an access is
> >> known by the profile.
> >>
> >> This can result in the wrong behavior of security conscience applications
> >> like sshd which examine their capability set, and change their behavior
> >> accordingly. In this case because of the masked capability set being
> >> returned sshd fails due to DAC checks, even when the profile is in complain
> >> mode.
> >>
> >> Kernels affected: 2.6.36 - 3.0.
> >>
> >> Signed-off-by: John Johansen <[email protected]>
> >> ---
> >
> > You say that multiple kernels are affected, then why not also include
> > [email protected] here as well?
> >
> > confused,
> >
> Sorry I should have elaborated, I think its a borderline case for stable
> release, and was as much a note to my self as anything.

Notes to self probably shouldn't be in kernel changelog entries :)

> The bug doesn't affect the enforcement of policy, only learning mode used
> for generating policy, and it is something we can work around in userspace.
> for already released kernels.
>
> I can resend with a Cc: stable if you would like

If you don't think it should be in the stable kernel releases, that's
fine, you are the maintainer of the code. I just found it odd that you
said it affected older kernels and yet didn't want it applied there as
well.

greg k-h