[PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: fix nnp subset test for unconfined

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: fix nnp subset test for unconfined

John Johansen-2
The subset test is not taking into account the unconfined exception
which will cause profile transitions in the stacked confinement
case to fail when no_new_privs is applied.

This fixes a regression introduced in the fix for
https://bugs.launchpad.net/bugs/1839037

Fixes: 0924738c7386d ("UBUNTU: SAUCE: apparmor: fix nnp subset check failure when, stacking")
BugLink: https://bugs.launchpad.net/bugs/1844186
Signed-off-by: John Johansen <[hidden email]>
---
 security/apparmor/domain.c        |  4 ++--
 security/apparmor/include/label.h |  1 +
 security/apparmor/label.c         | 33 +++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 38595ca7e642e..231535930f09c 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -765,7 +765,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
  * aways results in a further reduction of permissions.
  */
  if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
-    !unconfined(label) && !aa_label_is_subset(new, label)) {
+    !unconfined(label) && !aa_label_is_unconfined_subset(new, label)) {
  error = -EPERM;
  info = "no new privs";
  goto audit;
@@ -1117,7 +1117,7 @@ static int change_profile_perms_wrapper(const char *op, const char *name,
  */
  if (task_no_new_privs(current) && !stack &&
     !profile_unconfined(profile) &&
-    !aa_label_is_subset(target, &profile->label)) {
+    !aa_label_is_unconfined_subset(target, &profile->label)) {
  info = "no new privs";
  error = -EPERM;
  }
diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h
index 829d6e452cb86..10830078ba890 100644
--- a/security/apparmor/include/label.h
+++ b/security/apparmor/include/label.h
@@ -356,6 +356,7 @@ bool aa_label_init(struct aa_label *label, int size);
 struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp);
 
 bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub);
+bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub);
 struct aa_profile *__aa_label_next_not_in_set(struct label_it *I,
      struct aa_label *set,
      struct aa_label *sub);
diff --git a/security/apparmor/label.c b/security/apparmor/label.c
index 69a600b4c1a15..a942c14505ce2 100644
--- a/security/apparmor/label.c
+++ b/security/apparmor/label.c
@@ -551,6 +551,39 @@ bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub)
  return __aa_label_next_not_in_set(&i, set, sub) == NULL;
 }
 
+/**
+ * aa_label_is_unconfined_subset - test if @sub is a subset of @set
+ * @set: label to test against
+ * @sub: label to test if is subset of @set
+ *
+ * This checks for subset but taking into account unconfined. IF
+ * @sub contains an unconfined profile that does not have a matching
+ * unconfined in @set then this will not cause the test to fail.
+ * Conversely we don't care about an unconfined in @set that is not in
+ * @sub
+ *
+ * Returns: true if @sub is special_subset of @set
+ *     else false
+ */
+bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub)
+{
+ struct label_it i = { };
+ struct aa_profile *p;
+
+ AA_BUG(!set);
+ AA_BUG(!sub);
+
+ if (sub == set)
+ return true;
+
+ do {
+ p = __aa_label_next_not_in_set(&i, set, sub);
+ if (p && !profile_unconfined(p))
+ break;
+ } while (p);
+
+ return p == NULL;
+}
 
 
 /**
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: fix nnp subset test for unconfined

Tyler Hicks-2
On 2019-10-03 12:15:03, John Johansen wrote:

> The subset test is not taking into account the unconfined exception
> which will cause profile transitions in the stacked confinement
> case to fail when no_new_privs is applied.
>
> This fixes a regression introduced in the fix for
> https://bugs.launchpad.net/bugs/1839037
>
> Fixes: 0924738c7386d ("UBUNTU: SAUCE: apparmor: fix nnp subset check failure when, stacking")
> BugLink: https://bugs.launchpad.net/bugs/1844186
> Signed-off-by: John Johansen <[hidden email]>
> ---
>  security/apparmor/domain.c        |  4 ++--
>  security/apparmor/include/label.h |  1 +
>  security/apparmor/label.c         | 33 +++++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index 38595ca7e642e..231535930f09c 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -765,7 +765,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>   * aways results in a further reduction of permissions.
>   */
>   if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
> -    !unconfined(label) && !aa_label_is_subset(new, label)) {
> +    !unconfined(label) && !aa_label_is_unconfined_subset(new, label)) {
>   error = -EPERM;
>   info = "no new privs";
>   goto audit;
> @@ -1117,7 +1117,7 @@ static int change_profile_perms_wrapper(const char *op, const char *name,
>   */
>   if (task_no_new_privs(current) && !stack &&
>      !profile_unconfined(profile) &&
> -    !aa_label_is_subset(target, &profile->label)) {
> +    !aa_label_is_unconfined_subset(target, &profile->label)) {
>   info = "no new privs";
>   error = -EPERM;
>   }

Same question in Xenial as Bionic in regards to fixing aa_change_hat().

Tyler

> diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h
> index 829d6e452cb86..10830078ba890 100644
> --- a/security/apparmor/include/label.h
> +++ b/security/apparmor/include/label.h
> @@ -356,6 +356,7 @@ bool aa_label_init(struct aa_label *label, int size);
>  struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp);
>  
>  bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub);
> +bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub);
>  struct aa_profile *__aa_label_next_not_in_set(struct label_it *I,
>       struct aa_label *set,
>       struct aa_label *sub);
> diff --git a/security/apparmor/label.c b/security/apparmor/label.c
> index 69a600b4c1a15..a942c14505ce2 100644
> --- a/security/apparmor/label.c
> +++ b/security/apparmor/label.c
> @@ -551,6 +551,39 @@ bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub)
>   return __aa_label_next_not_in_set(&i, set, sub) == NULL;
>  }
>  
> +/**
> + * aa_label_is_unconfined_subset - test if @sub is a subset of @set
> + * @set: label to test against
> + * @sub: label to test if is subset of @set
> + *
> + * This checks for subset but taking into account unconfined. IF
> + * @sub contains an unconfined profile that does not have a matching
> + * unconfined in @set then this will not cause the test to fail.
> + * Conversely we don't care about an unconfined in @set that is not in
> + * @sub
> + *
> + * Returns: true if @sub is special_subset of @set
> + *     else false
> + */
> +bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub)
> +{
> + struct label_it i = { };
> + struct aa_profile *p;
> +
> + AA_BUG(!set);
> + AA_BUG(!sub);
> +
> + if (sub == set)
> + return true;
> +
> + do {
> + p = __aa_label_next_not_in_set(&i, set, sub);
> + if (p && !profile_unconfined(p))
> + break;
> + } while (p);
> +
> + return p == NULL;
> +}
>  
>  
>  /**
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: fix nnp subset test for unconfined

John Johansen-2
On 10/3/19 2:14 PM, Tyler Hicks wrote:

> On 2019-10-03 12:15:03, John Johansen wrote:
>> The subset test is not taking into account the unconfined exception
>> which will cause profile transitions in the stacked confinement
>> case to fail when no_new_privs is applied.
>>
>> This fixes a regression introduced in the fix for
>> https://bugs.launchpad.net/bugs/1839037
>>
>> Fixes: 0924738c7386d ("UBUNTU: SAUCE: apparmor: fix nnp subset check failure when, stacking")
>> BugLink: https://bugs.launchpad.net/bugs/1844186
>> Signed-off-by: John Johansen <[hidden email]>
>> ---
>>  security/apparmor/domain.c        |  4 ++--
>>  security/apparmor/include/label.h |  1 +
>>  security/apparmor/label.c         | 33 +++++++++++++++++++++++++++++++
>>  3 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
>> index 38595ca7e642e..231535930f09c 100644
>> --- a/security/apparmor/domain.c
>> +++ b/security/apparmor/domain.c
>> @@ -765,7 +765,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>>   * aways results in a further reduction of permissions.
>>   */
>>   if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
>> -    !unconfined(label) && !aa_label_is_subset(new, label)) {
>> +    !unconfined(label) && !aa_label_is_unconfined_subset(new, label)) {
>>   error = -EPERM;
>>   info = "no new privs";
>>   goto audit;
>> @@ -1117,7 +1117,7 @@ static int change_profile_perms_wrapper(const char *op, const char *name,
>>   */
>>   if (task_no_new_privs(current) && !stack &&
>>      !profile_unconfined(profile) &&
>> -    !aa_label_is_subset(target, &profile->label)) {
>> +    !aa_label_is_unconfined_subset(target, &profile->label)) {
>>   info = "no new privs";
>>   error = -EPERM;
>>   }
>
> Same question in Xenial as Bionic in regards to fixing aa_change_hat().
>
yep, I send a v2 with both fixes

> Tyler
>
>> diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h
>> index 829d6e452cb86..10830078ba890 100644
>> --- a/security/apparmor/include/label.h
>> +++ b/security/apparmor/include/label.h
>> @@ -356,6 +356,7 @@ bool aa_label_init(struct aa_label *label, int size);
>>  struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp);
>>  
>>  bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub);
>> +bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub);
>>  struct aa_profile *__aa_label_next_not_in_set(struct label_it *I,
>>       struct aa_label *set,
>>       struct aa_label *sub);
>> diff --git a/security/apparmor/label.c b/security/apparmor/label.c
>> index 69a600b4c1a15..a942c14505ce2 100644
>> --- a/security/apparmor/label.c
>> +++ b/security/apparmor/label.c
>> @@ -551,6 +551,39 @@ bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub)
>>   return __aa_label_next_not_in_set(&i, set, sub) == NULL;
>>  }
>>  
>> +/**
>> + * aa_label_is_unconfined_subset - test if @sub is a subset of @set
>> + * @set: label to test against
>> + * @sub: label to test if is subset of @set
>> + *
>> + * This checks for subset but taking into account unconfined. IF
>> + * @sub contains an unconfined profile that does not have a matching
>> + * unconfined in @set then this will not cause the test to fail.
>> + * Conversely we don't care about an unconfined in @set that is not in
>> + * @sub
>> + *
>> + * Returns: true if @sub is special_subset of @set
>> + *     else false
>> + */
>> +bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub)
>> +{
>> + struct label_it i = { };
>> + struct aa_profile *p;
>> +
>> + AA_BUG(!set);
>> + AA_BUG(!sub);
>> +
>> + if (sub == set)
>> + return true;
>> +
>> + do {
>> + p = __aa_label_next_not_in_set(&i, set, sub);
>> + if (p && !profile_unconfined(p))
>> + break;
>> + } while (p);
>> +
>> + return p == NULL;
>> +}
>>  
>>  
>>  /**
>> --
>> 2.17.1
>>
>>
>> --
>> kernel-team mailing list
>> [hidden email]
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NACK: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: fix nnp subset test for unconfined

Tyler Hicks-2
On 2019-10-03 16:26:42, John Johansen wrote:

> On 10/3/19 2:14 PM, Tyler Hicks wrote:
> > On 2019-10-03 12:15:03, John Johansen wrote:
> >> The subset test is not taking into account the unconfined exception
> >> which will cause profile transitions in the stacked confinement
> >> case to fail when no_new_privs is applied.
> >>
> >> This fixes a regression introduced in the fix for
> >> https://bugs.launchpad.net/bugs/1839037
> >>
> >> Fixes: 0924738c7386d ("UBUNTU: SAUCE: apparmor: fix nnp subset check failure when, stacking")
> >> BugLink: https://bugs.launchpad.net/bugs/1844186
> >> Signed-off-by: John Johansen <[hidden email]>
> >> ---
> >>  security/apparmor/domain.c        |  4 ++--
> >>  security/apparmor/include/label.h |  1 +
> >>  security/apparmor/label.c         | 33 +++++++++++++++++++++++++++++++
> >>  3 files changed, 36 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> >> index 38595ca7e642e..231535930f09c 100644
> >> --- a/security/apparmor/domain.c
> >> +++ b/security/apparmor/domain.c
> >> @@ -765,7 +765,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
> >>   * aways results in a further reduction of permissions.
> >>   */
> >>   if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
> >> -    !unconfined(label) && !aa_label_is_subset(new, label)) {
> >> +    !unconfined(label) && !aa_label_is_unconfined_subset(new, label)) {
> >>   error = -EPERM;
> >>   info = "no new privs";
> >>   goto audit;
> >> @@ -1117,7 +1117,7 @@ static int change_profile_perms_wrapper(const char *op, const char *name,
> >>   */
> >>   if (task_no_new_privs(current) && !stack &&
> >>      !profile_unconfined(profile) &&
> >> -    !aa_label_is_subset(target, &profile->label)) {
> >> +    !aa_label_is_unconfined_subset(target, &profile->label)) {
> >>   info = "no new privs";
> >>   error = -EPERM;
> >>   }
> >
> > Same question in Xenial as Bionic in regards to fixing aa_change_hat().
> >
> yep, I send a v2 with both fixes

Thanks!

Tyler

>
> > Tyler
> >
> >> diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h
> >> index 829d6e452cb86..10830078ba890 100644
> >> --- a/security/apparmor/include/label.h
> >> +++ b/security/apparmor/include/label.h
> >> @@ -356,6 +356,7 @@ bool aa_label_init(struct aa_label *label, int size);
> >>  struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp);
> >>  
> >>  bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub);
> >> +bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub);
> >>  struct aa_profile *__aa_label_next_not_in_set(struct label_it *I,
> >>       struct aa_label *set,
> >>       struct aa_label *sub);
> >> diff --git a/security/apparmor/label.c b/security/apparmor/label.c
> >> index 69a600b4c1a15..a942c14505ce2 100644
> >> --- a/security/apparmor/label.c
> >> +++ b/security/apparmor/label.c
> >> @@ -551,6 +551,39 @@ bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub)
> >>   return __aa_label_next_not_in_set(&i, set, sub) == NULL;
> >>  }
> >>  
> >> +/**
> >> + * aa_label_is_unconfined_subset - test if @sub is a subset of @set
> >> + * @set: label to test against
> >> + * @sub: label to test if is subset of @set
> >> + *
> >> + * This checks for subset but taking into account unconfined. IF
> >> + * @sub contains an unconfined profile that does not have a matching
> >> + * unconfined in @set then this will not cause the test to fail.
> >> + * Conversely we don't care about an unconfined in @set that is not in
> >> + * @sub
> >> + *
> >> + * Returns: true if @sub is special_subset of @set
> >> + *     else false
> >> + */
> >> +bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub)
> >> +{
> >> + struct label_it i = { };
> >> + struct aa_profile *p;
> >> +
> >> + AA_BUG(!set);
> >> + AA_BUG(!sub);
> >> +
> >> + if (sub == set)
> >> + return true;
> >> +
> >> + do {
> >> + p = __aa_label_next_not_in_set(&i, set, sub);
> >> + if (p && !profile_unconfined(p))
> >> + break;
> >> + } while (p);
> >> +
> >> + return p == NULL;
> >> +}
> >>  
> >>  
> >>  /**
> >> --
> >> 2.17.1
> >>
> >>
> >> --
> >> kernel-team mailing list
> >> [hidden email]
> >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team