* Size mismatch between vfs_getxattr_alloc() and vfs_getxattr() @ 2021-06-11 9:44 Roberto Sassu 2021-06-16 13:22 ` [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs Roberto Sassu 0 siblings, 1 reply; 11+ messages in thread From: Roberto Sassu @ 2021-06-11 9:44 UTC (permalink / raw) To: viro, Mimi Zohar, paul, Stephen Smalley, casey, Stefan Berger Cc: linux-integrity, linux-security-module, linux-kernel, selinux Hello the ima-evm-utils tool discovered an issue doing signature verification of xattrs. On kernel side, EVM reads the xattr value with vfs_getxattr_alloc(), which gets the value directly from the xattr handler. On user side, ima-evm-utils reads the value with the lgetxattr() system call, which gets the value from LSMs. There is a corner case, where security.selinux is set directly with setfattr without adding \0 at the end. In this case, the kernel and the user see different values due to the fact that the former gets the raw value from the xattr handler, and the latter gets the value normalized by SELinux (which adds \0). I found that originally also lgetxattr() was getting the value from the xattr handler. This changed with: commit 4bea58053f206be9a89ca35850f9ad295dac2042 Author: David P. Quigley <dpquigl@tycho.nsa.gov> Date: Mon Feb 4 22:29:40 2008 -0800 VFS: Reorder vfs_getxattr to avoid unnecessary calls to the LSM which directly calls LSMs for security.* xattrs. Given that this patch is there for a long time, I would ask if it makes sense to fix this issue. The way I would do it is to check if the size returned by the xattr handler is the same of the size returned by LSMs. If not, I would get the value from the xattr handler. Although this change does not check the xattr content, it is sufficient to fix the issue. Any opinion? Thanks Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-11 9:44 Size mismatch between vfs_getxattr_alloc() and vfs_getxattr() Roberto Sassu @ 2021-06-16 13:22 ` Roberto Sassu 2021-06-16 14:40 ` Stefan Berger 0 siblings, 1 reply; 11+ messages in thread From: Roberto Sassu @ 2021-06-16 13:22 UTC (permalink / raw) To: viro, zohar, paul, stephen.smalley.work, casey, stefanb Cc: linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux, Roberto Sassu vfs_getxattr() differs from vfs_setxattr() in the way it obtains the xattr value. The former gives precedence to the LSMs, and if the LSMs don't provide a value, obtains it from the filesystem handler. The latter does the opposite, first invokes the filesystem handler, and if the filesystem does not support xattrs, passes the xattr value to the LSMs. The problem is that not necessarily the user gets the same xattr value that he set. For example, if he sets security.selinux with a value not terminated with '\0', he gets a value terminated with '\0' because SELinux adds it during the translation from xattr to internal representation (vfs_setxattr()) and from internal representation to xattr (vfs_getxattr()). Normally, this does not have an impact unless the integrity of xattrs is verified with EVM. The kernel and the user see different values due to the different functions used to obtain them: kernel (EVM): uses vfs_getxattr_alloc() which obtains the xattr value from the filesystem handler (raw value); user (ima-evm-utils): uses vfs_getxattr() which obtains the xattr value from the LSMs (normalized value). Given that the difference between the raw value and the normalized value should be just the additional '\0' not the rest of the content, this patch modifies vfs_getxattr() to compare the size of the xattr value obtained from the LSMs to the size of the raw xattr value. If there is a mismatch and the filesystem handler does not return an error, vfs_getxattr() returns the raw value. This patch should have a minimal impact on existing systems, because if the SELinux label is written with the appropriate tools such as setfiles or restorecon, there will not be a mismatch (because the raw value also has '\0'). In the case where the SELinux label is written directly with setfattr and without '\0', this patch helps to align EVM and ima-evm-utils in terms of result provided (due to the fact that they both verify the integrity of xattrs from raw values). Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Tested-by: Mimi Zohar <zohar@linux.ibm.com> --- fs/xattr.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/fs/xattr.c b/fs/xattr.c index 5c8c5175b385..412ec875aa07 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -420,12 +420,27 @@ vfs_getxattr(struct user_namespace *mnt_userns, struct dentry *dentry, const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; int ret = xattr_getsecurity(mnt_userns, inode, suffix, value, size); + int ret_raw; + /* * Only overwrite the return value if a security module * is actually active. */ if (ret == -EOPNOTSUPP) goto nolsm; + + if (ret < 0) + return ret; + + /* + * Read raw xattr if the size from the filesystem handler + * differs from that returned by xattr_getsecurity() and is + * equal or greater than zero. + */ + ret_raw = __vfs_getxattr(dentry, inode, name, NULL, 0); + if (ret_raw >= 0 && ret_raw != ret) + goto nolsm; + return ret; } nolsm: -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-16 13:22 ` [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs Roberto Sassu @ 2021-06-16 14:40 ` Stefan Berger 2021-06-17 7:09 ` Roberto Sassu 0 siblings, 1 reply; 11+ messages in thread From: Stefan Berger @ 2021-06-16 14:40 UTC (permalink / raw) To: Roberto Sassu, viro, zohar, paul, stephen.smalley.work, casey Cc: linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux On 6/16/21 9:22 AM, Roberto Sassu wrote: > vfs_getxattr() differs from vfs_setxattr() in the way it obtains the xattr > value. The former gives precedence to the LSMs, and if the LSMs don't > provide a value, obtains it from the filesystem handler. The latter does > the opposite, first invokes the filesystem handler, and if the filesystem > does not support xattrs, passes the xattr value to the LSMs. > > The problem is that not necessarily the user gets the same xattr value that > he set. For example, if he sets security.selinux with a value not > terminated with '\0', he gets a value terminated with '\0' because SELinux > adds it during the translation from xattr to internal representation > (vfs_setxattr()) and from internal representation to xattr > (vfs_getxattr()). > > Normally, this does not have an impact unless the integrity of xattrs is > verified with EVM. The kernel and the user see different values due to the > different functions used to obtain them: > > kernel (EVM): uses vfs_getxattr_alloc() which obtains the xattr value from > the filesystem handler (raw value); > > user (ima-evm-utils): uses vfs_getxattr() which obtains the xattr value > from the LSMs (normalized value). Maybe there should be another implementation similar to vfs_getxattr_alloc() (or modify it) to behave like vfs_getxattr() but do the memory allocation part so that the kernel sees what user space see rather than modifying it with your patch so that user space now sees something different than what it has been for years (previous NUL-terminated SELinux xattr may not be NUL-terminated anymore)? Stefan > > Given that the difference between the raw value and the normalized value > should be just the additional '\0' not the rest of the content, this patch > modifies vfs_getxattr() to compare the size of the xattr value obtained > from the LSMs to the size of the raw xattr value. If there is a mismatch > and the filesystem handler does not return an error, vfs_getxattr() returns > the raw value. > > This patch should have a minimal impact on existing systems, because if the > SELinux label is written with the appropriate tools such as setfiles or > restorecon, there will not be a mismatch (because the raw value also has > '\0'). > > In the case where the SELinux label is written directly with setfattr and > without '\0', this patch helps to align EVM and ima-evm-utils in terms of > result provided (due to the fact that they both verify the integrity of > xattrs from raw values). > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > Tested-by: Mimi Zohar <zohar@linux.ibm.com> > --- > fs/xattr.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/fs/xattr.c b/fs/xattr.c > index 5c8c5175b385..412ec875aa07 100644 > --- a/fs/xattr.c > +++ b/fs/xattr.c > @@ -420,12 +420,27 @@ vfs_getxattr(struct user_namespace *mnt_userns, struct dentry *dentry, > const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; > int ret = xattr_getsecurity(mnt_userns, inode, suffix, value, > size); > + int ret_raw; > + > /* > * Only overwrite the return value if a security module > * is actually active. > */ > if (ret == -EOPNOTSUPP) > goto nolsm; > + > + if (ret < 0) > + return ret; > + > + /* > + * Read raw xattr if the size from the filesystem handler > + * differs from that returned by xattr_getsecurity() and is > + * equal or greater than zero. > + */ > + ret_raw = __vfs_getxattr(dentry, inode, name, NULL, 0); > + if (ret_raw >= 0 && ret_raw != ret) > + goto nolsm; > + > return ret; > } > nolsm: ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-16 14:40 ` Stefan Berger @ 2021-06-17 7:09 ` Roberto Sassu 2021-06-17 15:27 ` Mimi Zohar 0 siblings, 1 reply; 11+ messages in thread From: Roberto Sassu @ 2021-06-17 7:09 UTC (permalink / raw) To: Stefan Berger, viro, zohar, paul, stephen.smalley.work, casey Cc: linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux > From: Stefan Berger [mailto:stefanb@linux.ibm.com] > Sent: Wednesday, June 16, 2021 4:40 PM > On 6/16/21 9:22 AM, Roberto Sassu wrote: > > vfs_getxattr() differs from vfs_setxattr() in the way it obtains the xattr > > value. The former gives precedence to the LSMs, and if the LSMs don't > > provide a value, obtains it from the filesystem handler. The latter does > > the opposite, first invokes the filesystem handler, and if the filesystem > > does not support xattrs, passes the xattr value to the LSMs. > > > > The problem is that not necessarily the user gets the same xattr value that > > he set. For example, if he sets security.selinux with a value not > > terminated with '\0', he gets a value terminated with '\0' because SELinux > > adds it during the translation from xattr to internal representation > > (vfs_setxattr()) and from internal representation to xattr > > (vfs_getxattr()). > > > > Normally, this does not have an impact unless the integrity of xattrs is > > verified with EVM. The kernel and the user see different values due to the > > different functions used to obtain them: > > > > kernel (EVM): uses vfs_getxattr_alloc() which obtains the xattr value from > > the filesystem handler (raw value); > > > > user (ima-evm-utils): uses vfs_getxattr() which obtains the xattr value > > from the LSMs (normalized value). > > Maybe there should be another implementation similar to > vfs_getxattr_alloc() (or modify it) to behave like vfs_getxattr() but do > the memory allocation part so that the kernel sees what user space see > rather than modifying it with your patch so that user space now sees > something different than what it has been for years (previous > NUL-terminated SELinux xattr may not be NUL-terminated anymore)? I'm concerned that this would break HMACs/digital signatures calculated with raw values. An alternative would be to do the EVM verification twice if the first time didn't succeed (with vfs_getxattr_alloc() and with the new function that behaves like vfs_getxattr()). Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > Stefan > > > > > > > > Given that the difference between the raw value and the normalized value > > should be just the additional '\0' not the rest of the content, this patch > > modifies vfs_getxattr() to compare the size of the xattr value obtained > > from the LSMs to the size of the raw xattr value. If there is a mismatch > > and the filesystem handler does not return an error, vfs_getxattr() returns > > the raw value. > > > > This patch should have a minimal impact on existing systems, because if the > > SELinux label is written with the appropriate tools such as setfiles or > > restorecon, there will not be a mismatch (because the raw value also has > > '\0'). > > > > In the case where the SELinux label is written directly with setfattr and > > without '\0', this patch helps to align EVM and ima-evm-utils in terms of > > result provided (due to the fact that they both verify the integrity of > > xattrs from raw values). > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > Tested-by: Mimi Zohar <zohar@linux.ibm.com> > > --- > > fs/xattr.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/fs/xattr.c b/fs/xattr.c > > index 5c8c5175b385..412ec875aa07 100644 > > --- a/fs/xattr.c > > +++ b/fs/xattr.c > > @@ -420,12 +420,27 @@ vfs_getxattr(struct user_namespace *mnt_userns, > struct dentry *dentry, > > const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; > > int ret = xattr_getsecurity(mnt_userns, inode, suffix, value, > > size); > > + int ret_raw; > > + > > /* > > * Only overwrite the return value if a security module > > * is actually active. > > */ > > if (ret == -EOPNOTSUPP) > > goto nolsm; > > + > > + if (ret < 0) > > + return ret; > > + > > + /* > > + * Read raw xattr if the size from the filesystem handler > > + * differs from that returned by xattr_getsecurity() and is > > + * equal or greater than zero. > > + */ > > + ret_raw = __vfs_getxattr(dentry, inode, name, NULL, 0); > > + if (ret_raw >= 0 && ret_raw != ret) > > + goto nolsm; > > + > > return ret; > > } > > nolsm: ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-17 7:09 ` Roberto Sassu @ 2021-06-17 15:27 ` Mimi Zohar 2021-06-17 16:05 ` Roberto Sassu 2021-06-18 3:18 ` Paul Moore 0 siblings, 2 replies; 11+ messages in thread From: Mimi Zohar @ 2021-06-17 15:27 UTC (permalink / raw) To: Roberto Sassu, Stefan Berger, viro, paul, stephen.smalley.work, casey Cc: linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux On Thu, 2021-06-17 at 07:09 +0000, Roberto Sassu wrote: > > From: Stefan Berger [mailto:stefanb@linux.ibm.com] > > Sent: Wednesday, June 16, 2021 4:40 PM > > On 6/16/21 9:22 AM, Roberto Sassu wrote: > > > vfs_getxattr() differs from vfs_setxattr() in the way it obtains the xattr > > > value. The former gives precedence to the LSMs, and if the LSMs don't > > > provide a value, obtains it from the filesystem handler. The latter does > > > the opposite, first invokes the filesystem handler, and if the filesystem > > > does not support xattrs, passes the xattr value to the LSMs. > > > > > > The problem is that not necessarily the user gets the same xattr value that > > > he set. For example, if he sets security.selinux with a value not > > > terminated with '\0', he gets a value terminated with '\0' because SELinux > > > adds it during the translation from xattr to internal representation > > > (vfs_setxattr()) and from internal representation to xattr > > > (vfs_getxattr()). > > > > > > Normally, this does not have an impact unless the integrity of xattrs is > > > verified with EVM. The kernel and the user see different values due to the > > > different functions used to obtain them: > > > > > > kernel (EVM): uses vfs_getxattr_alloc() which obtains the xattr value from > > > the filesystem handler (raw value); > > > > > > user (ima-evm-utils): uses vfs_getxattr() which obtains the xattr value > > > from the LSMs (normalized value). > > > > Maybe there should be another implementation similar to > > vfs_getxattr_alloc() (or modify it) to behave like vfs_getxattr() but do > > the memory allocation part so that the kernel sees what user space see > > rather than modifying it with your patch so that user space now sees > > something different than what it has been for years (previous > > NUL-terminated SELinux xattr may not be NUL-terminated anymore)? > > I'm concerned that this would break HMACs/digital signatures > calculated with raw values. Which would happen if the LSM is not enabled (e.g. "lsm=" boot command line option). > > An alternative would be to do the EVM verification twice if the > first time didn't succeed (with vfs_getxattr_alloc() and with the > new function that behaves like vfs_getxattr()). Unfortunately, I don't see an alternative. thanks, Mimi ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-17 15:27 ` Mimi Zohar @ 2021-06-17 16:05 ` Roberto Sassu 2021-06-18 3:18 ` Paul Moore 1 sibling, 0 replies; 11+ messages in thread From: Roberto Sassu @ 2021-06-17 16:05 UTC (permalink / raw) To: Mimi Zohar, Stefan Berger, viro, paul, stephen.smalley.work, casey Cc: linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux > From: Mimi Zohar [mailto:zohar@linux.ibm.com] > Sent: Thursday, June 17, 2021 5:28 PM > On Thu, 2021-06-17 at 07:09 +0000, Roberto Sassu wrote: > > > From: Stefan Berger [mailto:stefanb@linux.ibm.com] > > > Sent: Wednesday, June 16, 2021 4:40 PM > > > On 6/16/21 9:22 AM, Roberto Sassu wrote: > > > > vfs_getxattr() differs from vfs_setxattr() in the way it obtains the xattr > > > > value. The former gives precedence to the LSMs, and if the LSMs don't > > > > provide a value, obtains it from the filesystem handler. The latter does > > > > the opposite, first invokes the filesystem handler, and if the filesystem > > > > does not support xattrs, passes the xattr value to the LSMs. > > > > > > > > The problem is that not necessarily the user gets the same xattr value > that > > > > he set. For example, if he sets security.selinux with a value not > > > > terminated with '\0', he gets a value terminated with '\0' because > SELinux > > > > adds it during the translation from xattr to internal representation > > > > (vfs_setxattr()) and from internal representation to xattr > > > > (vfs_getxattr()). > > > > > > > > Normally, this does not have an impact unless the integrity of xattrs is > > > > verified with EVM. The kernel and the user see different values due to > the > > > > different functions used to obtain them: > > > > > > > > kernel (EVM): uses vfs_getxattr_alloc() which obtains the xattr value > from > > > > the filesystem handler (raw value); > > > > > > > > user (ima-evm-utils): uses vfs_getxattr() which obtains the xattr value > > > > from the LSMs (normalized value). > > > > > > Maybe there should be another implementation similar to > > > vfs_getxattr_alloc() (or modify it) to behave like vfs_getxattr() but do > > > the memory allocation part so that the kernel sees what user space see > > > rather than modifying it with your patch so that user space now sees > > > something different than what it has been for years (previous > > > NUL-terminated SELinux xattr may not be NUL-terminated anymore)? > > > > I'm concerned that this would break HMACs/digital signatures > > calculated with raw values. > > Which would happen if the LSM is not enabled (e.g. "lsm=" boot command > line option). For files created after switching to the new behavior, yes, because EVM could eventually get the label without '\0' from the filesystem handler. However, it would happen also for files created before switching to the new behavior, since the HMAC could have been calculated without '\0' and after switching it would be calculated with '\0'. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > > An alternative would be to do the EVM verification twice if the > > first time didn't succeed (with vfs_getxattr_alloc() and with the > > new function that behaves like vfs_getxattr()). > > Unfortunately, I don't see an alternative. > > thanks, > > Mimi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-17 15:27 ` Mimi Zohar 2021-06-17 16:05 ` Roberto Sassu @ 2021-06-18 3:18 ` Paul Moore 2021-06-18 16:04 ` Mimi Zohar 1 sibling, 1 reply; 11+ messages in thread From: Paul Moore @ 2021-06-18 3:18 UTC (permalink / raw) To: Mimi Zohar Cc: Roberto Sassu, Stefan Berger, viro, stephen.smalley.work, casey, linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux On Thu, Jun 17, 2021 at 11:28 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > On Thu, 2021-06-17 at 07:09 +0000, Roberto Sassu wrote: ... > > An alternative would be to do the EVM verification twice if the > > first time didn't succeed (with vfs_getxattr_alloc() and with the > > new function that behaves like vfs_getxattr()). > > Unfortunately, I don't see an alternative. ... and while unfortunate, the impact should be non-existant if you are using the right tools to label files or ensuring that you are formatting labels properly if doing it by hand. Handling a corner case is good, but I wouldn't add a lot of code complexity trying to optimize it. -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-18 3:18 ` Paul Moore @ 2021-06-18 16:04 ` Mimi Zohar 2021-06-18 16:10 ` Roberto Sassu 2021-06-18 16:35 ` Paul Moore 0 siblings, 2 replies; 11+ messages in thread From: Mimi Zohar @ 2021-06-18 16:04 UTC (permalink / raw) To: Paul Moore Cc: Roberto Sassu, Stefan Berger, viro, stephen.smalley.work, casey, linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux On Thu, 2021-06-17 at 23:18 -0400, Paul Moore wrote: > On Thu, Jun 17, 2021 at 11:28 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Thu, 2021-06-17 at 07:09 +0000, Roberto Sassu wrote: > > ... > > > > An alternative would be to do the EVM verification twice if the > > > first time didn't succeed (with vfs_getxattr_alloc() and with the > > > new function that behaves like vfs_getxattr()). > > > > Unfortunately, I don't see an alternative. > > ... and while unfortunate, the impact should be non-existant if you > are using the right tools to label files or ensuring that you are > formatting labels properly if doing it by hand. > > Handling a corner case is good, but I wouldn't add a lot of code > complexity trying to optimize it. From userspace it's really difficult to understand the EVM signature verification failure is due to the missing NULL. Roberto, I just pushed the "evm: output EVM digest calculation info" patch to the next-integrity-testing branch, which includes some debugging. Instead of this patch, which returns the raw xattr data, how about adding additional debugging info in evm_calc_hmac_or_hash() indicating the size discrepancy between the raw xattr and the LSM returned xattr? thanks, Mimi ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-18 16:04 ` Mimi Zohar @ 2021-06-18 16:10 ` Roberto Sassu 2021-06-18 16:35 ` Paul Moore 1 sibling, 0 replies; 11+ messages in thread From: Roberto Sassu @ 2021-06-18 16:10 UTC (permalink / raw) To: Mimi Zohar, Paul Moore Cc: Stefan Berger, viro, stephen.smalley.work, casey, linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux > From: Mimi Zohar [mailto:zohar@linux.ibm.com] > Sent: Friday, June 18, 2021 6:04 PM > On Thu, 2021-06-17 at 23:18 -0400, Paul Moore wrote: > > On Thu, Jun 17, 2021 at 11:28 AM Mimi Zohar <zohar@linux.ibm.com> > wrote: > > > On Thu, 2021-06-17 at 07:09 +0000, Roberto Sassu wrote: > > > > ... > > > > > > An alternative would be to do the EVM verification twice if the > > > > first time didn't succeed (with vfs_getxattr_alloc() and with the > > > > new function that behaves like vfs_getxattr()). > > > > > > Unfortunately, I don't see an alternative. > > > > ... and while unfortunate, the impact should be non-existant if you > > are using the right tools to label files or ensuring that you are > > formatting labels properly if doing it by hand. > > > > Handling a corner case is good, but I wouldn't add a lot of code > > complexity trying to optimize it. > > From userspace it's really difficult to understand the EVM signature > verification failure is due to the missing NULL. > > Roberto, I just pushed the "evm: output EVM digest calculation info" > patch to the next-integrity-testing branch, which includes some > debugging. Instead of this patch, which returns the raw xattr data, > how about adding additional debugging info in evm_calc_hmac_or_hash() > indicating the size discrepancy between the raw xattr and the LSM > returned xattr? Good idea. Will do it. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > thanks, > > Mimi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-18 16:04 ` Mimi Zohar 2021-06-18 16:10 ` Roberto Sassu @ 2021-06-18 16:35 ` Paul Moore 2021-06-18 17:22 ` Mimi Zohar 1 sibling, 1 reply; 11+ messages in thread From: Paul Moore @ 2021-06-18 16:35 UTC (permalink / raw) To: Mimi Zohar Cc: Roberto Sassu, Stefan Berger, viro, stephen.smalley.work, casey, linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux On Fri, Jun 18, 2021 at 12:04 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > On Thu, 2021-06-17 at 23:18 -0400, Paul Moore wrote: > > On Thu, Jun 17, 2021 at 11:28 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > On Thu, 2021-06-17 at 07:09 +0000, Roberto Sassu wrote: > > > > ... > > > > > > An alternative would be to do the EVM verification twice if the > > > > first time didn't succeed (with vfs_getxattr_alloc() and with the > > > > new function that behaves like vfs_getxattr()). > > > > > > Unfortunately, I don't see an alternative. > > > > ... and while unfortunate, the impact should be non-existant if you > > are using the right tools to label files or ensuring that you are > > formatting labels properly if doing it by hand. > > > > Handling a corner case is good, but I wouldn't add a lot of code > > complexity trying to optimize it. > > From userspace it's really difficult to understand the EVM signature > verification failure is due to the missing NULL. I would argue that any signature verification failure, regardless of the mechanism, is hard to understand. It either passes or it fails, and if it fails good luck trying to determine what exactly isn't matching up; especially if you really don't know the Right Value. What I mean by the corner case was the fact that the recommended tools should always do the right thing with respect to '\0' termination, this should really only be an issue if someone is winging it and doing it by hand or with their own tools. -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs 2021-06-18 16:35 ` Paul Moore @ 2021-06-18 17:22 ` Mimi Zohar 0 siblings, 0 replies; 11+ messages in thread From: Mimi Zohar @ 2021-06-18 17:22 UTC (permalink / raw) To: Paul Moore Cc: Roberto Sassu, Stefan Berger, viro, stephen.smalley.work, casey, linux-fsdevel, linux-integrity, linux-security-module, linux-kernel, selinux On Fri, 2021-06-18 at 12:35 -0400, Paul Moore wrote: > On Fri, Jun 18, 2021 at 12:04 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Thu, 2021-06-17 at 23:18 -0400, Paul Moore wrote: > > > On Thu, Jun 17, 2021 at 11:28 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > > > On Thu, 2021-06-17 at 07:09 +0000, Roberto Sassu wrote: > > > > > > ... > > > > > > > > An alternative would be to do the EVM verification twice if the > > > > > first time didn't succeed (with vfs_getxattr_alloc() and with the > > > > > new function that behaves like vfs_getxattr()). > > > > > > > > Unfortunately, I don't see an alternative. > > > > > > ... and while unfortunate, the impact should be non-existant if you > > > are using the right tools to label files or ensuring that you are > > > formatting labels properly if doing it by hand. > > > > > > Handling a corner case is good, but I wouldn't add a lot of code > > > complexity trying to optimize it. > > > > From userspace it's really difficult to understand the EVM signature > > verification failure is due to the missing NULL. > > I would argue that any signature verification failure, regardless of > the mechanism, is hard to understand. It either passes or it fails, > and if it fails good luck trying to determine what exactly isn't > matching up; especially if you really don't know the Right Value. In this case, the discussion is about signing and verifying file meta- data hashes. With EVM portable and immutable signatures, the file meta-data is known. The userspace tool evmct is able to verify the file meta-data signature, which the kernel rejects. > What I mean by the corner case was the fact that the recommended tools > should always do the right thing with respect to '\0' termination, > this should really only be an issue if someone is winging it and doing > it by hand or with their own tools. I'm not disagreeing with you. However, it's still annoying, confusing, and really frustrating. That's why we're at least including debugging information. In addtion, Roberto will provide the reason. thanks, Mimi ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-06-18 17:22 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-11 9:44 Size mismatch between vfs_getxattr_alloc() and vfs_getxattr() Roberto Sassu 2021-06-16 13:22 ` [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs Roberto Sassu 2021-06-16 14:40 ` Stefan Berger 2021-06-17 7:09 ` Roberto Sassu 2021-06-17 15:27 ` Mimi Zohar 2021-06-17 16:05 ` Roberto Sassu 2021-06-18 3:18 ` Paul Moore 2021-06-18 16:04 ` Mimi Zohar 2021-06-18 16:10 ` Roberto Sassu 2021-06-18 16:35 ` Paul Moore 2021-06-18 17:22 ` Mimi Zohar
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.