| Message ID | bf25962f-85d3-1290-80d3-ccd5cffb9d2a@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,disco] UBUNTU: SAUCE: apparmor: fix nnp subset test for unconfined | expand |
On 2019-10-03 12:14:46, 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 4.17 kernel caused by the > reworking of domain transitions. > > Fixes: 9fcf78cca1986 ("apparmor: update domain transitions that are subsets of confinement at nnp") > BugLink: https://bugs.launchpad.net/bugs/1844186 > Signed-off-by: John Johansen <john.johansen@canonical.com> Acked-by: Tyler Hicks <tyhicks@canonical.com> Thanks! Tyler > --- > security/apparmor/domain.c | 8 ++++---- > security/apparmor/include/label.h | 1 + > security/apparmor/label.c | 33 +++++++++++++++++++++++++++++++ > 3 files changed, 38 insertions(+), 4 deletions(-) > > diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c > index 3b266a438776..1c3cd1228e97 100644 > --- a/security/apparmor/domain.c > +++ b/security/apparmor/domain.c > @@ -935,7 +935,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, ctx->nnp)) { > + !unconfined(label) && !aa_label_is_unconfined_subset(new, ctx->nnp)) { > error = -EPERM; > info = "no new privs"; > goto audit; > @@ -1213,7 +1213,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(new, ctx->nnp)) { > + !aa_label_is_unconfined_subset(new, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > @@ -1234,7 +1234,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(previous, ctx->nnp)) { > + !aa_label_is_unconfined_subset(previous, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > @@ -1430,7 +1430,7 @@ int aa_change_profile(const char *fqname, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(new, ctx->nnp)) { > + !aa_label_is_unconfined_subset(new, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h > index d871e7ff0952..71af9f6f58ed 100644 > --- a/security/apparmor/include/label.h > +++ b/security/apparmor/include/label.h > @@ -285,6 +285,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 83e4f6d62c66..8de6f1655bb1 100644 > --- a/security/apparmor/label.c > +++ b/security/apparmor/label.c > @@ -555,6 +555,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 03.10.19 21:14, 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 4.17 kernel caused by the > reworking of domain transitions. > > Fixes: 9fcf78cca1986 ("apparmor: update domain transitions that are subsets of confinement at nnp") > BugLink: https://bugs.launchpad.net/bugs/1844186 > Signed-off-by: John Johansen <john.johansen@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- Any "complaints" already in the Eoan reply. > security/apparmor/domain.c | 8 ++++---- > security/apparmor/include/label.h | 1 + > security/apparmor/label.c | 33 +++++++++++++++++++++++++++++++ > 3 files changed, 38 insertions(+), 4 deletions(-) > > diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c > index 3b266a438776..1c3cd1228e97 100644 > --- a/security/apparmor/domain.c > +++ b/security/apparmor/domain.c > @@ -935,7 +935,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, ctx->nnp)) { > + !unconfined(label) && !aa_label_is_unconfined_subset(new, ctx->nnp)) { > error = -EPERM; > info = "no new privs"; > goto audit; > @@ -1213,7 +1213,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(new, ctx->nnp)) { > + !aa_label_is_unconfined_subset(new, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > @@ -1234,7 +1234,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(previous, ctx->nnp)) { > + !aa_label_is_unconfined_subset(previous, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > @@ -1430,7 +1430,7 @@ int aa_change_profile(const char *fqname, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(new, ctx->nnp)) { > + !aa_label_is_unconfined_subset(new, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h > index d871e7ff0952..71af9f6f58ed 100644 > --- a/security/apparmor/include/label.h > +++ b/security/apparmor/include/label.h > @@ -285,6 +285,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 83e4f6d62c66..8de6f1655bb1 100644 > --- a/security/apparmor/label.c > +++ b/security/apparmor/label.c > @@ -555,6 +555,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; > +} > > > /** >
On 03.10.19 21:14, 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 4.17 kernel caused by the > reworking of domain transitions. > > Fixes: 9fcf78cca1986 ("apparmor: update domain transitions that are subsets of confinement at nnp") > BugLink: https://bugs.launchpad.net/bugs/1844186 > Signed-off-by: John Johansen <john.johansen@canonical.com> Applied to disco/master-next branch. Thanks, Kleber > --- > security/apparmor/domain.c | 8 ++++---- > security/apparmor/include/label.h | 1 + > security/apparmor/label.c | 33 +++++++++++++++++++++++++++++++ > 3 files changed, 38 insertions(+), 4 deletions(-) > > diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c > index 3b266a438776..1c3cd1228e97 100644 > --- a/security/apparmor/domain.c > +++ b/security/apparmor/domain.c > @@ -935,7 +935,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, ctx->nnp)) { > + !unconfined(label) && !aa_label_is_unconfined_subset(new, ctx->nnp)) { > error = -EPERM; > info = "no new privs"; > goto audit; > @@ -1213,7 +1213,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(new, ctx->nnp)) { > + !aa_label_is_unconfined_subset(new, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > @@ -1234,7 +1234,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(previous, ctx->nnp)) { > + !aa_label_is_unconfined_subset(previous, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > @@ -1430,7 +1430,7 @@ int aa_change_profile(const char *fqname, int flags) > * reduce restrictions. > */ > if (task_no_new_privs(current) && !unconfined(label) && > - !aa_label_is_subset(new, ctx->nnp)) { > + !aa_label_is_unconfined_subset(new, ctx->nnp)) { > /* not an apparmor denial per se, so don't log it */ > AA_DEBUG("no_new_privs - change_hat denied"); > error = -EPERM; > diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h > index d871e7ff0952..71af9f6f58ed 100644 > --- a/security/apparmor/include/label.h > +++ b/security/apparmor/include/label.h > @@ -285,6 +285,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 83e4f6d62c66..8de6f1655bb1 100644 > --- a/security/apparmor/label.c > +++ b/security/apparmor/label.c > @@ -555,6 +555,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; > +} > > > /** >
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 3b266a438776..1c3cd1228e97 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -935,7 +935,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, ctx->nnp)) { + !unconfined(label) && !aa_label_is_unconfined_subset(new, ctx->nnp)) { error = -EPERM; info = "no new privs"; goto audit; @@ -1213,7 +1213,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) * reduce restrictions. */ if (task_no_new_privs(current) && !unconfined(label) && - !aa_label_is_subset(new, ctx->nnp)) { + !aa_label_is_unconfined_subset(new, ctx->nnp)) { /* not an apparmor denial per se, so don't log it */ AA_DEBUG("no_new_privs - change_hat denied"); error = -EPERM; @@ -1234,7 +1234,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) * reduce restrictions. */ if (task_no_new_privs(current) && !unconfined(label) && - !aa_label_is_subset(previous, ctx->nnp)) { + !aa_label_is_unconfined_subset(previous, ctx->nnp)) { /* not an apparmor denial per se, so don't log it */ AA_DEBUG("no_new_privs - change_hat denied"); error = -EPERM; @@ -1430,7 +1430,7 @@ int aa_change_profile(const char *fqname, int flags) * reduce restrictions. */ if (task_no_new_privs(current) && !unconfined(label) && - !aa_label_is_subset(new, ctx->nnp)) { + !aa_label_is_unconfined_subset(new, ctx->nnp)) { /* not an apparmor denial per se, so don't log it */ AA_DEBUG("no_new_privs - change_hat denied"); error = -EPERM; diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h index d871e7ff0952..71af9f6f58ed 100644 --- a/security/apparmor/include/label.h +++ b/security/apparmor/include/label.h @@ -285,6 +285,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 83e4f6d62c66..8de6f1655bb1 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -555,6 +555,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 4.17 kernel caused by the reworking of domain transitions. Fixes: 9fcf78cca1986 ("apparmor: update domain transitions that are subsets of confinement at nnp") BugLink: https://bugs.launchpad.net/bugs/1844186 Signed-off-by: John Johansen <john.johansen@canonical.com> --- security/apparmor/domain.c | 8 ++++---- security/apparmor/include/label.h | 1 + security/apparmor/label.c | 33 +++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-)