[PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: flock mediation is not being, enforced on cache check

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

[PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: flock mediation is not being, enforced on cache check

John Johansen-2
When an open file with cached permissions is checked for the flock
permission. The cache check fails and falls through to no error instead
of auditing, and returning an error.

For the fall through to do a permission check, so it will audit the
failed flock permission check.

BugLink: http://bugs.launchpad.net/bugs/1838090
BugLink: http://bugs.launchpad.net/bugs/1658219
Signed-off-by: John Johansen <[hidden email]>
---
 security/apparmor/file.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 69eed1c3e0d0..646d3effd0f9 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -536,18 +536,23 @@ static int __file_path_perm(const char *op, struct aa_label *label,
  error = fn_for_each_not_in_set(flabel, label, profile,
  profile_path_perm(op, profile, &file->f_path, buffer,
   request, &cond, flags, &perms));
- if (denied) {
+ if (denied && !error) {
  /* check every profile in file label that was not tested
  * in the initial check above.
  */
  /* TODO: cache full perms so this only happens because of
  * conditionals */
  /* TODO: don't audit here */
- last_error(error,
- fn_for_each_not_in_set(label, flabel, profile,
+ if (label == flabel)
+ error = fn_for_each(label, profile,
+ profile_path_perm(op, profile, &file->f_path,
+  buffer, request, &cond, flags,
+  &perms));
+ else
+ error = fn_for_each_not_in_set(label, flabel, profile,
  profile_path_perm(op, profile, &file->f_path,
   buffer, request, &cond, flags,
-  &perms)));
+  &perms));
  }
  if (!error)
  update_file_ctx(file_ctx(file), label, request);
--
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
|

ACK: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: flock mediation is not being, enforced on cache check

Tyler Hicks-2
On 2019-08-05 16:26:38, John Johansen wrote:

> When an open file with cached permissions is checked for the flock
> permission. The cache check fails and falls through to no error instead
> of auditing, and returning an error.
>
> For the fall through to do a permission check, so it will audit the
> failed flock permission check.
>
> BugLink: http://bugs.launchpad.net/bugs/1838090
> BugLink: http://bugs.launchpad.net/bugs/1658219
> Signed-off-by: John Johansen <[hidden email]>

I tested this change with the upstream AppArmor regression tests with
positive results.

 Acked-by: Tyler Hicks <[hidden email]>

Thanks!

Tyler

> ---
>  security/apparmor/file.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index 69eed1c3e0d0..646d3effd0f9 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -536,18 +536,23 @@ static int __file_path_perm(const char *op, struct aa_label *label,
>   error = fn_for_each_not_in_set(flabel, label, profile,
>   profile_path_perm(op, profile, &file->f_path, buffer,
>    request, &cond, flags, &perms));
> - if (denied) {
> + if (denied && !error) {
>   /* check every profile in file label that was not tested
>   * in the initial check above.
>   */
>   /* TODO: cache full perms so this only happens because of
>   * conditionals */
>   /* TODO: don't audit here */
> - last_error(error,
> - fn_for_each_not_in_set(label, flabel, profile,
> + if (label == flabel)
> + error = fn_for_each(label, profile,
> + profile_path_perm(op, profile, &file->f_path,
> +  buffer, request, &cond, flags,
> +  &perms));
> + else
> + error = fn_for_each_not_in_set(label, flabel, profile,
>   profile_path_perm(op, profile, &file->f_path,
>    buffer, request, &cond, flags,
> -  &perms)));
> +  &perms));
>   }
>   if (!error)
>   update_file_ctx(file_ctx(file), label, request);
> --
> 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
|

ACK: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: flock mediation is not being, enforced on cache check

Stefan Bader-2
In reply to this post by John Johansen-2
On 06.08.19 01:26, John Johansen wrote:

> When an open file with cached permissions is checked for the flock
> permission. The cache check fails and falls through to no error instead
> of auditing, and returning an error.
>
> For the fall through to do a permission check, so it will audit the
> failed flock permission check.
>
> BugLink: http://bugs.launchpad.net/bugs/1838090
> BugLink: http://bugs.launchpad.net/bugs/1658219
> Signed-off-by: John Johansen <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

> ---
>  security/apparmor/file.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index 69eed1c3e0d0..646d3effd0f9 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -536,18 +536,23 @@ static int __file_path_perm(const char *op, struct aa_label *label,
>   error = fn_for_each_not_in_set(flabel, label, profile,
>   profile_path_perm(op, profile, &file->f_path, buffer,
>    request, &cond, flags, &perms));
> - if (denied) {
> + if (denied && !error) {
>   /* check every profile in file label that was not tested
>   * in the initial check above.
>   */
>   /* TODO: cache full perms so this only happens because of
>   * conditionals */
>   /* TODO: don't audit here */
> - last_error(error,
> - fn_for_each_not_in_set(label, flabel, profile,
> + if (label == flabel)
> + error = fn_for_each(label, profile,
> + profile_path_perm(op, profile, &file->f_path,
> +  buffer, request, &cond, flags,
> +  &perms));
> + else
> + error = fn_for_each_not_in_set(label, flabel, profile,
>   profile_path_perm(op, profile, &file->f_path,
>    buffer, request, &cond, flags,
> -  &perms)));
> +  &perms));
>   }
>   if (!error)
>   update_file_ctx(file_ctx(file), label, request);
>


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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: flock mediation is not being, enforced on cache check

Stefan Bader-2
In reply to this post by John Johansen-2
On 06.08.19 01:26, John Johansen wrote:
> When an open file with cached permissions is checked for the flock
> permission. The cache check fails and falls through to no error instead
> of auditing, and returning an error.
>
> For the fall through to do a permission check, so it will audit the
> failed flock permission check.
>
> BugLink: http://bugs.launchpad.net/bugs/1838090
  ^ that one leads nowhere
> BugLink: https://bugs.launchpad.net/bugs/1658219
  ^change to https when committing

> Signed-off-by: John Johansen <[hidden email]>
> ---
>  security/apparmor/file.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index 69eed1c3e0d0..646d3effd0f9 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -536,18 +536,23 @@ static int __file_path_perm(const char *op, struct aa_label *label,
>   error = fn_for_each_not_in_set(flabel, label, profile,
>   profile_path_perm(op, profile, &file->f_path, buffer,
>    request, &cond, flags, &perms));
> - if (denied) {
> + if (denied && !error) {
>   /* check every profile in file label that was not tested
>   * in the initial check above.
>   */
>   /* TODO: cache full perms so this only happens because of
>   * conditionals */
>   /* TODO: don't audit here */
> - last_error(error,
> - fn_for_each_not_in_set(label, flabel, profile,
> + if (label == flabel)
> + error = fn_for_each(label, profile,
> + profile_path_perm(op, profile, &file->f_path,
> +  buffer, request, &cond, flags,
> +  &perms));
> + else
> + error = fn_for_each_not_in_set(label, flabel, profile,
>   profile_path_perm(op, profile, &file->f_path,
>    buffer, request, &cond, flags,
> -  &perms)));
> +  &perms));
>   }
>   if (!error)
>   update_file_ctx(file_ctx(file), label, request);
>


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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: flock mediation is not being, enforced on cache check

Kleber Souza
In reply to this post by John Johansen-2
On 8/6/19 1:26 AM, John Johansen wrote:
> When an open file with cached permissions is checked for the flock
> permission. The cache check fails and falls through to no error instead
> of auditing, and returning an error.
>
> For the fall through to do a permission check, so it will audit the
> failed flock permission check.
>
> BugLink: http://bugs.launchpad.net/bugs/1838090

The above BugLink doesn't work. Is this bug report flagged as private?


Kleber

> BugLink: http://bugs.launchpad.net/bugs/1658219
> Signed-off-by: John Johansen <[hidden email]>
> ---
>  security/apparmor/file.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index 69eed1c3e0d0..646d3effd0f9 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -536,18 +536,23 @@ static int __file_path_perm(const char *op, struct aa_label *label,
>   error = fn_for_each_not_in_set(flabel, label, profile,
>   profile_path_perm(op, profile, &file->f_path, buffer,
>    request, &cond, flags, &perms));
> - if (denied) {
> + if (denied && !error) {
>   /* check every profile in file label that was not tested
>   * in the initial check above.
>   */
>   /* TODO: cache full perms so this only happens because of
>   * conditionals */
>   /* TODO: don't audit here */
> - last_error(error,
> - fn_for_each_not_in_set(label, flabel, profile,
> + if (label == flabel)
> + error = fn_for_each(label, profile,
> + profile_path_perm(op, profile, &file->f_path,
> +  buffer, request, &cond, flags,
> +  &perms));
> + else
> + error = fn_for_each_not_in_set(label, flabel, profile,
>   profile_path_perm(op, profile, &file->f_path,
>    buffer, request, &cond, flags,
> -  &perms)));
> +  &perms));
>   }
>   if (!error)
>   update_file_ctx(file_ctx(file), label, request);
>


--
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: flock mediation is not being, enforced on cache check

John Johansen-2
On 8/12/19 7:56 AM, Kleber Souza wrote:

> On 8/6/19 1:26 AM, John Johansen wrote:
>> When an open file with cached permissions is checked for the flock
>> permission. The cache check fails and falls through to no error instead
>> of auditing, and returning an error.
>>
>> For the fall through to do a permission check, so it will audit the
>> failed flock permission check.
>>
>> BugLink: http://bugs.launchpad.net/bugs/1838090
>
> The above BugLink doesn't work. Is this bug report flagged as private?
>
>
yes it was marked private security

it is also marked now as a duplicate of
https://bugs.launchpad.net/apparmor/+bug/1658219/

which is public so I have switched it to public as well


> Kleber
>
>> BugLink: http://bugs.launchpad.net/bugs/1658219
>> Signed-off-by: John Johansen <[hidden email]>
>> ---
>>  security/apparmor/file.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
>> index 69eed1c3e0d0..646d3effd0f9 100644
>> --- a/security/apparmor/file.c
>> +++ b/security/apparmor/file.c
>> @@ -536,18 +536,23 @@ static int __file_path_perm(const char *op, struct aa_label *label,
>>   error = fn_for_each_not_in_set(flabel, label, profile,
>>   profile_path_perm(op, profile, &file->f_path, buffer,
>>    request, &cond, flags, &perms));
>> - if (denied) {
>> + if (denied && !error) {
>>   /* check every profile in file label that was not tested
>>   * in the initial check above.
>>   */
>>   /* TODO: cache full perms so this only happens because of
>>   * conditionals */
>>   /* TODO: don't audit here */
>> - last_error(error,
>> - fn_for_each_not_in_set(label, flabel, profile,
>> + if (label == flabel)
>> + error = fn_for_each(label, profile,
>> + profile_path_perm(op, profile, &file->f_path,
>> +  buffer, request, &cond, flags,
>> +  &perms));
>> + else
>> + error = fn_for_each_not_in_set(label, flabel, profile,
>>   profile_path_perm(op, profile, &file->f_path,
>>    buffer, request, &cond, flags,
>> -  &perms)));
>> +  &perms));
>>   }
>>   if (!error)
>>   update_file_ctx(file_ctx(file), label, request);
>>
>


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

APPLIED: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: flock mediation is not being, enforced on cache check

Khalid Elmously
In reply to this post by John Johansen-2
On 2019-08-05 16:26:38 , John Johansen wrote:

> When an open file with cached permissions is checked for the flock
> permission. The cache check fails and falls through to no error instead
> of auditing, and returning an error.
>
> For the fall through to do a permission check, so it will audit the
> failed flock permission check.
>
> BugLink: http://bugs.launchpad.net/bugs/1838090
> BugLink: http://bugs.launchpad.net/bugs/1658219
> Signed-off-by: John Johansen <[hidden email]>
> ---
>  security/apparmor/file.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index 69eed1c3e0d0..646d3effd0f9 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -536,18 +536,23 @@ static int __file_path_perm(const char *op, struct aa_label *label,
>   error = fn_for_each_not_in_set(flabel, label, profile,
>   profile_path_perm(op, profile, &file->f_path, buffer,
>    request, &cond, flags, &perms));
> - if (denied) {
> + if (denied && !error) {
>   /* check every profile in file label that was not tested
>   * in the initial check above.
>   */
>   /* TODO: cache full perms so this only happens because of
>   * conditionals */
>   /* TODO: don't audit here */
> - last_error(error,
> - fn_for_each_not_in_set(label, flabel, profile,
> + if (label == flabel)
> + error = fn_for_each(label, profile,
> + profile_path_perm(op, profile, &file->f_path,
> +  buffer, request, &cond, flags,
> +  &perms));
> + else
> + error = fn_for_each_not_in_set(label, flabel, profile,
>   profile_path_perm(op, profile, &file->f_path,
>    buffer, request, &cond, flags,
> -  &perms)));
> +  &perms));
>   }
>   if (!error)
>   update_file_ctx(file_ctx(file), label, request);
> --
> 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: flock mediation is not being, enforced on cache check

Tyler Hicks-2
Unfortunately, this change is breaking the network-manager snap and,
therefore, would cause networking issues for devices in the field. It
needs to be reverted ASAP and the 4.4 kernels in -proposed need to be
respun with the revert.

John is going to prioritize SRU verification of the other AppArmor bugs
that are pending SRU verification.

More details about the regression are available here:

 https://bugs.launchpad.net/apparmor/+bug/1658219/comments/20

Tyler

On 2019-08-13 00:07:06, Khaled Elmously wrote:

> On 2019-08-05 16:26:38 , John Johansen wrote:
> > When an open file with cached permissions is checked for the flock
> > permission. The cache check fails and falls through to no error instead
> > of auditing, and returning an error.
> >
> > For the fall through to do a permission check, so it will audit the
> > failed flock permission check.
> >
> > BugLink: http://bugs.launchpad.net/bugs/1838090
> > BugLink: http://bugs.launchpad.net/bugs/1658219
> > Signed-off-by: John Johansen <[hidden email]>
> > ---
> >  security/apparmor/file.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> > index 69eed1c3e0d0..646d3effd0f9 100644
> > --- a/security/apparmor/file.c
> > +++ b/security/apparmor/file.c
> > @@ -536,18 +536,23 @@ static int __file_path_perm(const char *op, struct aa_label *label,
> >   error = fn_for_each_not_in_set(flabel, label, profile,
> >   profile_path_perm(op, profile, &file->f_path, buffer,
> >    request, &cond, flags, &perms));
> > - if (denied) {
> > + if (denied && !error) {
> >   /* check every profile in file label that was not tested
> >   * in the initial check above.
> >   */
> >   /* TODO: cache full perms so this only happens because of
> >   * conditionals */
> >   /* TODO: don't audit here */
> > - last_error(error,
> > - fn_for_each_not_in_set(label, flabel, profile,
> > + if (label == flabel)
> > + error = fn_for_each(label, profile,
> > + profile_path_perm(op, profile, &file->f_path,
> > +  buffer, request, &cond, flags,
> > +  &perms));
> > + else
> > + error = fn_for_each_not_in_set(label, flabel, profile,
> >   profile_path_perm(op, profile, &file->f_path,
> >    buffer, request, &cond, flags,
> > -  &perms)));
> > +  &perms));
> >   }
> >   if (!error)
> >   update_file_ctx(file_ctx(file), label, request);
> > --
> > 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

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