| Message ID | 72d6aaee-29cf-cf76-421c-ad140ae49140@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,xenial] UBUNTU: SAUCE: apparmor: fix nnp subset test for unconfined | expand |
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 <john.johansen@canonical.com> > --- > 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 > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
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 <john.johansen@canonical.com> >> --- >> 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 >> kernel-team@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
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 <john.johansen@canonical.com> > >> --- > >> 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 > >> kernel-team@lists.ubuntu.com > >> https://lists.ubuntu.com/mailman/listinfo/kernel-team >
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; +} /**
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 <john.johansen@canonical.com> --- security/apparmor/domain.c | 4 ++-- security/apparmor/include/label.h | 1 + security/apparmor/label.c | 33 +++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-)