[SRU][Xenial][PATCH 0/1] Revert "ima: limit file hash setting by user to fix and log modes"

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

[SRU][Xenial][PATCH 0/1] Revert "ima: limit file hash setting by user to fix and log modes"

Joseph Salisbury-3
BugLink: http://bugs.launchpad.net/bugs/1771826

== SRU Justification ==
On a system that has IMA appraisal enabled it is impossible to create
security.ima extended attribute files that contain IMA hash.  This is
due to mainline commit c68ed80c97d, which prevents writing file hashes as
security.ima xattrs.

This bug is fixed by reverting commit c68ed80c97d, which is done by
mainline commit f5acb3dcba1f as of v4.10-rc1.

== Fix ==
f5acb3dcba1f ("Revert "ima: limit file hash setting by user to fix and log modes"")

== Regression Potential ==
Low.  This revert happend in v4.10-rc1.  It has been in Artful and
Bionic for a while without any reported issues.

== Test Case ==
A test kernel was built with this patch and tested by the original bug reporter.
The bug reporter states the test kernel resolved the bug.

Mimi Zohar (1):
  Revert "ima: limit file hash setting by user to fix and log modes"

 security/integrity/ima/ima_appraise.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--
2.7.4


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

[SRU][Xenial][PATCH 1/1] Revert "ima: limit file hash setting by user to fix and log modes"

Joseph Salisbury-3
From: Mimi Zohar <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1771826

Userspace applications have been modified to write security xattrs,
but they are not context aware.  In the case of security.ima, the
security xattr can be either a file hash or a file signature.
Permitting writing one, but not the other requires the application to
be context aware.

In addition, userspace applications might write files to a staging
area, which might not be in policy, and then change some file metadata
(eg. owner) making it in policy.  As a result, these files are not
labeled properly.

This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533, which
prevents writing file hashes as security.ima xattrs.

Requested-by: Patrick Ohly <[hidden email]>
Cc: Dmitry Kasatkin <[hidden email]>
Signed-off-by: Mimi Zohar <[hidden email]>
(cherry picked from commit f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 security/integrity/ima/ima_appraise.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index d62ae31..80f2c6d 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -384,14 +384,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
  result = ima_protect_xattr(dentry, xattr_name, xattr_value,
    xattr_value_len);
  if (result == 1) {
- bool digsig;
-
  if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
  return -EINVAL;
- digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
- if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE))
- return -EPERM;
- ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
+ ima_reset_appraise_flags(d_backing_inode(dentry),
+ (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);
  result = 0;
  }
  return result;
--
2.7.4


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

ACK: [SRU][Xenial][PATCH 1/1] Revert "ima: limit file hash setting by user to fix and log modes"

Stefan Bader-2
On 23.05.2018 11:00, Joseph Salisbury wrote:

> From: Mimi Zohar <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1771826
>
> Userspace applications have been modified to write security xattrs,
> but they are not context aware.  In the case of security.ima, the
> security xattr can be either a file hash or a file signature.
> Permitting writing one, but not the other requires the application to
> be context aware.
>
> In addition, userspace applications might write files to a staging
> area, which might not be in policy, and then change some file metadata
> (eg. owner) making it in policy.  As a result, these files are not
> labeled properly.
>
> This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533, which
> prevents writing file hashes as security.ima xattrs.
>
> Requested-by: Patrick Ohly <[hidden email]>
> Cc: Dmitry Kasatkin <[hidden email]>
> Signed-off-by: Mimi Zohar <[hidden email]>
> (cherry picked from commit f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b)
> Signed-off-by: Joseph Salisbury <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

> ---
>  security/integrity/ima/ima_appraise.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index d62ae31..80f2c6d 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -384,14 +384,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
>   result = ima_protect_xattr(dentry, xattr_name, xattr_value,
>     xattr_value_len);
>   if (result == 1) {
> - bool digsig;
> -
>   if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
>   return -EINVAL;
> - digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
> - if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE))
> - return -EPERM;
> - ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
> + ima_reset_appraise_flags(d_backing_inode(dentry),
> + (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);
>   result = 0;
>   }
>   return result;
>


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

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

ACK/APPLIED: [SRU][Xenial][PATCH 1/1] Revert "ima: limit file hash setting by user to fix and log modes"

Juerg Haefliger
In reply to this post by Joseph Salisbury-3
On 05/23/2018 08:00 PM, Joseph Salisbury wrote:

> From: Mimi Zohar <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1771826
>
> Userspace applications have been modified to write security xattrs,
> but they are not context aware.  In the case of security.ima, the
> security xattr can be either a file hash or a file signature.
> Permitting writing one, but not the other requires the application to
> be context aware.
>
> In addition, userspace applications might write files to a staging
> area, which might not be in policy, and then change some file metadata
> (eg. owner) making it in policy.  As a result, these files are not
> labeled properly.
>
> This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533, which
> prevents writing file hashes as security.ima xattrs.
>
> Requested-by: Patrick Ohly <[hidden email]>
> Cc: Dmitry Kasatkin <[hidden email]>
> Signed-off-by: Mimi Zohar <[hidden email]>
> (cherry picked from commit f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b)
> Signed-off-by: Joseph Salisbury <[hidden email]>
Acked-by: Juerg Haefliger <[hidden email]>

Applied to xenial/master-next.

...Juerg

> ---
>  security/integrity/ima/ima_appraise.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index d62ae31..80f2c6d 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -384,14 +384,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
>   result = ima_protect_xattr(dentry, xattr_name, xattr_value,
>     xattr_value_len);
>   if (result == 1) {
> - bool digsig;
> -
>   if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
>   return -EINVAL;
> - digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
> - if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE))
> - return -EPERM;
> - ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
> + ima_reset_appraise_flags(d_backing_inode(dentry),
> + (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);
>   result = 0;
>   }
>   return result;
>


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

signature.asc (849 bytes) Download Attachment