| Message ID | 20160901132250.GB145647@ubuntu-hedt (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2016-09-01 at 08:22 -0500, Seth Forshee wrote: > I've been reading back through all of this, and I'm not sure any > conclusion was reached. > For my part, I'm uneasy about writing out hmacs to mounts from a > non-root user. So I'll make a proposal - let's not read or write EVM or > IMA xattrs for filesystems from non-init user namespace, essentially > behaving as though they don't support xattrs. Something like the > (untested) patch below. This really doesn't sound like the right long term solution. The kernel, as well as root in the namespace, should write the EVM and IMA security xattrs, as long as their usage is limited to the uid/gid associated with that user_ns namespace. In terms of the USB stick scenario, instead of security.evm containing HMACs, they would need to be replaced with signatures, before using it on another system. Refer to the ima-evm-utils package for writing out security.evm signatures. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 06, 2016 at 04:00:32PM -0400, Mimi Zohar wrote: > On Thu, 2016-09-01 at 08:22 -0500, Seth Forshee wrote: > > > I've been reading back through all of this, and I'm not sure any > > conclusion was reached. > > > For my part, I'm uneasy about writing out hmacs to mounts from a > > non-root user. So I'll make a proposal - let's not read or write EVM or > > IMA xattrs for filesystems from non-init user namespace, essentially > > behaving as though they don't support xattrs. Something like the > > (untested) patch below. > > This really doesn't sound like the right long term solution. I'm not proposing this for a long term solution. I just haven't seen that anyone has a real use case in mind yet for using the integrity subsystem with these mounts and that it might be better to wait and decide exactly how it should behave until someone does. In the mean time I'm just trying to ensure there won't be vulnerabilities to exploit. > The kernel, as well as root in the namespace, should write the EVM and > IMA security xattrs, as long as their usage is limited to the uid/gid > associated with that user_ns namespace. But keep in mind that the uid mapping doesn't get written out to disk. If I create a file owned by uid 0 in my namespace (let's say that maps to uid 1000 in init_user_ns) then uid 0 is what will be written to the inode on disk. Based on my understanding of what you said previously, this means that I could create a file in my filesystem as root in my user ns and write out a security.capability xattr, and the kernel would then write out an EVM xattr with a valid signature. If I can then manage to get that file mounted in init_user_ns the kernel will be owned by real root and the kernel will see a valid signature for the capability xattr. Granted, this scenario is problematic even without EVM, but it seems like specifically the kind of thing someone might use EVM to protect against. Thanks, Seth > In terms of the USB stick scenario, instead of security.evm containing > HMACs, they would need to be replaced with signatures, before using it > on another system. Refer to the ima-evm-utils package for writing out > security.evm signatures. > > Mimi > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2016-09-06 at 15:52 -0500, Seth Forshee wrote: > On Tue, Sep 06, 2016 at 04:00:32PM -0400, Mimi Zohar wrote: > > On Thu, 2016-09-01 at 08:22 -0500, Seth Forshee wrote: > > > > > I've been reading back through all of this, and I'm not sure any > > > conclusion was reached. > > > > > For my part, I'm uneasy about writing out hmacs to mounts from a > > > non-root user. So I'll make a proposal - let's not read or write EVM or > > > IMA xattrs for filesystems from non-init user namespace, essentially > > > behaving as though they don't support xattrs. Something like the > > > (untested) patch below. > > > > This really doesn't sound like the right long term solution. > > I'm not proposing this for a long term solution. I just haven't seen > that anyone has a real use case in mind yet for using the integrity > subsystem with these mounts and that it might be better to wait and > decide exactly how it should behave until someone does. In the mean time > I'm just trying to ensure there won't be vulnerabilities to exploit. The main use case will be installing/updating packages within a container and writing the corresponding file signatures. Unfortunately, we're not there yet. > > The kernel, as well as root in the namespace, should write the EVM and > > IMA security xattrs, as long as their usage is limited to the uid/gid > > associated with that user_ns namespace. > > But keep in mind that the uid mapping doesn't get written out to disk. > If I create a file owned by uid 0 in my namespace (let's say that maps > to uid 1000 in init_user_ns) then uid 0 is what will be written to the > inode on disk. oh! > Based on my understanding of what you said previously, this means that I > could create a file in my filesystem as root in my user ns and write out > a security.capability xattr, and the kernel would then write out an EVM > xattr with a valid signature. If I can then manage to get that file > mounted in init_user_ns the kernel will be owned by real root and the > kernel will see a valid signature for the capability xattr. I see. > Granted, this scenario is problematic even without EVM, but it seems > like specifically the kind of thing someone might use EVM to protect > against. Agreed. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 06, 2016 at 05:28:49PM -0400, Mimi Zohar wrote: > On Tue, 2016-09-06 at 15:52 -0500, Seth Forshee wrote: > > On Tue, Sep 06, 2016 at 04:00:32PM -0400, Mimi Zohar wrote: > > > On Thu, 2016-09-01 at 08:22 -0500, Seth Forshee wrote: > > > > > > > I've been reading back through all of this, and I'm not sure any > > > > conclusion was reached. > > > > > > > For my part, I'm uneasy about writing out hmacs to mounts from a > > > > non-root user. So I'll make a proposal - let's not read or write EVM or > > > > IMA xattrs for filesystems from non-init user namespace, essentially > > > > behaving as though they don't support xattrs. Something like the > > > > (untested) patch below. > > > > > > This really doesn't sound like the right long term solution. > > > > I'm not proposing this for a long term solution. I just haven't seen > > that anyone has a real use case in mind yet for using the integrity > > subsystem with these mounts and that it might be better to wait and > > decide exactly how it should behave until someone does. In the mean time > > I'm just trying to ensure there won't be vulnerabilities to exploit. > > The main use case will be installing/updating packages within a > container and writing the corresponding file signatures. Unfortunately, > we're not there yet. Okay. If the distro is providing those signatures then I don't see a problem with allowing them to be written. What concerns me is if there's a scenario where the kernel would calculate and write out a signature or hmac that would make the kernel trust the file in other contexts. So when s_user_ns != &init_user_ns it sounds like we could allow the kernel to read and verify signatures and write out signatures provided by userspace, but not calculate new/updated signatures for files. Does that sound reasonable? Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2016-09-07 at 07:27 -0500, Seth Forshee wrote: > On Tue, Sep 06, 2016 at 05:28:49PM -0400, Mimi Zohar wrote: > > On Tue, 2016-09-06 at 15:52 -0500, Seth Forshee wrote: > > > On Tue, Sep 06, 2016 at 04:00:32PM -0400, Mimi Zohar wrote: > > > > On Thu, 2016-09-01 at 08:22 -0500, Seth Forshee wrote: > > > > > > > > > I've been reading back through all of this, and I'm not sure any > > > > > conclusion was reached. > > > > > > > > > For my part, I'm uneasy about writing out hmacs to mounts from a > > > > > non-root user. So I'll make a proposal - let's not read or write EVM or > > > > > IMA xattrs for filesystems from non-init user namespace, essentially > > > > > behaving as though they don't support xattrs. Something like the > > > > > (untested) patch below. > > > > > > > > This really doesn't sound like the right long term solution. > > > > > > I'm not proposing this for a long term solution. I just haven't seen > > > that anyone has a real use case in mind yet for using the integrity > > > subsystem with these mounts and that it might be better to wait and > > > decide exactly how it should behave until someone does. In the mean time > > > I'm just trying to ensure there won't be vulnerabilities to exploit. > > > > The main use case will be installing/updating packages within a > > container and writing the corresponding file signatures. Unfortunately, > > we're not there yet. > > Okay. If the distro is providing those signatures then I don't see a > problem with allowing them to be written. What concerns me is if there's > a scenario where the kernel would calculate and write out a signature or > hmac that would make the kernel trust the file in other contexts. There's really no way of differentiating the source of the file signatures. We either allow root in the namespace to write them or not. > So when s_user_ns != &init_user_ns it sounds like we could allow the > kernel to read and verify signatures and write out signatures provided > by userspace, but not calculate new/updated signatures for files. Does > that sound reasonable? Not really. At least initially, perhaps we should be differentiating between EVM and IMA xattrs, allowing only IMA xattrs to be read/written and returning EOPNOTSUPP, as you suggested, for EVM. Having just IMA xattrs, without EVM xattrs, would then work in the namespace, but not outside of it (assuming EVM is enabled). Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 07, 2016 at 11:20:19AM -0400, Mimi Zohar wrote: > On Wed, 2016-09-07 at 07:27 -0500, Seth Forshee wrote: > > On Tue, Sep 06, 2016 at 05:28:49PM -0400, Mimi Zohar wrote: > > > On Tue, 2016-09-06 at 15:52 -0500, Seth Forshee wrote: > > > > On Tue, Sep 06, 2016 at 04:00:32PM -0400, Mimi Zohar wrote: > > > > > On Thu, 2016-09-01 at 08:22 -0500, Seth Forshee wrote: > > > > > > > > > > > I've been reading back through all of this, and I'm not sure any > > > > > > conclusion was reached. > > > > > > > > > > > For my part, I'm uneasy about writing out hmacs to mounts from a > > > > > > non-root user. So I'll make a proposal - let's not read or write EVM or > > > > > > IMA xattrs for filesystems from non-init user namespace, essentially > > > > > > behaving as though they don't support xattrs. Something like the > > > > > > (untested) patch below. > > > > > > > > > > This really doesn't sound like the right long term solution. > > > > > > > > I'm not proposing this for a long term solution. I just haven't seen > > > > that anyone has a real use case in mind yet for using the integrity > > > > subsystem with these mounts and that it might be better to wait and > > > > decide exactly how it should behave until someone does. In the mean time > > > > I'm just trying to ensure there won't be vulnerabilities to exploit. > > > > > > The main use case will be installing/updating packages within a > > > container and writing the corresponding file signatures. Unfortunately, > > > we're not there yet. > > > > Okay. If the distro is providing those signatures then I don't see a > > problem with allowing them to be written. What concerns me is if there's > > a scenario where the kernel would calculate and write out a signature or > > hmac that would make the kernel trust the file in other contexts. > > There's really no way of differentiating the source of the file > signatures. We either allow root in the namespace to write them or not. I must be misunderstanding something then. My impression was that for EVM we'd be dealing with some kind of keyed signatures, not just hashes. In that case a user without the correct key could write the EVM xattrs, then if the signature was correct the kernel would permit access to the file, otherwise it would not. I.e. if the user was installing distro packages which shipped with correct signatures, the user could write those and the kernel would validate them as correct. But if the user tried to write out their own files with security xattrs they could not generate valid signatures, and the kernel would deny access to those files. Is something about that incorrect? > > So when s_user_ns != &init_user_ns it sounds like we could allow the > > kernel to read and verify signatures and write out signatures provided > > by userspace, but not calculate new/updated signatures for files. Does > > that sound reasonable? > > Not really. At least initially, perhaps we should be differentiating > between EVM and IMA xattrs, allowing only IMA xattrs to be read/written > and returning EOPNOTSUPP, as you suggested, for EVM. Having just IMA > xattrs, without EVM xattrs, would then work in the namespace, but not > outside of it (assuming EVM is enabled). That's okay with me assuming it doesn't weaken security for systems with EVM enabled. We're already effectively ignoring LSM security xattrs and scoping capability xattrs to s_user_ns, so I think we should be okay. Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2016-09-07 at 11:01 -0500, Seth Forshee wrote: > On Wed, Sep 07, 2016 at 11:20:19AM -0400, Mimi Zohar wrote: > > On Wed, 2016-09-07 at 07:27 -0500, Seth Forshee wrote: > > > On Tue, Sep 06, 2016 at 05:28:49PM -0400, Mimi Zohar wrote: > > > > On Tue, 2016-09-06 at 15:52 -0500, Seth Forshee wrote: > > > > > On Tue, Sep 06, 2016 at 04:00:32PM -0400, Mimi Zohar wrote: > > > > > > On Thu, 2016-09-01 at 08:22 -0500, Seth Forshee wrote: > > > > > > > > > > > > > I've been reading back through all of this, and I'm not sure any > > > > > > > conclusion was reached. > > > > > > > > > > > > > For my part, I'm uneasy about writing out hmacs to mounts from a > > > > > > > non-root user. So I'll make a proposal - let's not read or write EVM or > > > > > > > IMA xattrs for filesystems from non-init user namespace, essentially > > > > > > > behaving as though they don't support xattrs. Something like the > > > > > > > (untested) patch below. > > > > > > > > > > > > This really doesn't sound like the right long term solution. > > > > > > > > > > I'm not proposing this for a long term solution. I just haven't seen > > > > > that anyone has a real use case in mind yet for using the integrity > > > > > subsystem with these mounts and that it might be better to wait and > > > > > decide exactly how it should behave until someone does. In the mean time > > > > > I'm just trying to ensure there won't be vulnerabilities to exploit. > > > > > > > > The main use case will be installing/updating packages within a > > > > container and writing the corresponding file signatures. Unfortunately, > > > > we're not there yet. > > > > > > Okay. If the distro is providing those signatures then I don't see a > > > problem with allowing them to be written. What concerns me is if there's > > > a scenario where the kernel would calculate and write out a signature or > > > hmac that would make the kernel trust the file in other contexts. > > > > There's really no way of differentiating the source of the file > > signatures. We either allow root in the namespace to write them or not. > > I must be misunderstanding something then. By file signature, I meant file data signature, which refers to IMA (not EVM). > My impression was that for EVM we'd be dealing with some kind of keyed > signatures, not just hashes. In that case a user without the correct key > could write the EVM xattrs, then if the signature was correct the kernel > would permit access to the file, otherwise it would not. Right, except the EVM signature is not portable, as it includes the inode in the signature. Our work for including file signatures in packages is limited to the IMA file data signatures. A new portable EVM format will be need for including EVM signatures in packages. > I.e. if the > user was installing distro packages which shipped with correct > signatures, the user could write those and the kernel would validate > them as correct. But if the user tried to write out their own files with > security xattrs they could not generate valid signatures, and the kernel > would deny access to those files. > Is something about that incorrect? That sounds right. Userspace can write EVM signatures, using the ima-evm-utils package. The asymmetric public key used for verifying the signature, would need to be loaded onto the EVM keyring. > > > So when s_user_ns != &init_user_ns it sounds like we could allow the > > > kernel to read and verify signatures and write out signatures provided > > > by userspace, but not calculate new/updated signatures for files. Does > > > that sound reasonable? > > > > Not really. At least initially, perhaps we should be differentiating > > between EVM and IMA xattrs, allowing only IMA xattrs to be read/written > > and returning EOPNOTSUPP, as you suggested, for EVM. Having just IMA > > xattrs, without EVM xattrs, would then work in the namespace, but not > > outside of it (assuming EVM is enabled). > > That's okay with me assuming it doesn't weaken security for systems with > EVM enabled. We're already effectively ignoring LSM security xattrs and > scoping capability xattrs to s_user_ns, so I think we should be okay. The worst, which would be bad, is preventing access to a file, but it wouldn't make an invalid EVM signature/hmac valid. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 07, 2016 at 01:06:10PM -0400, Mimi Zohar wrote: > On Wed, 2016-09-07 at 11:01 -0500, Seth Forshee wrote: > > On Wed, Sep 07, 2016 at 11:20:19AM -0400, Mimi Zohar wrote: > > > On Wed, 2016-09-07 at 07:27 -0500, Seth Forshee wrote: > > > > On Tue, Sep 06, 2016 at 05:28:49PM -0400, Mimi Zohar wrote: > > > > > On Tue, 2016-09-06 at 15:52 -0500, Seth Forshee wrote: > > > > > > On Tue, Sep 06, 2016 at 04:00:32PM -0400, Mimi Zohar wrote: > > > > > > > On Thu, 2016-09-01 at 08:22 -0500, Seth Forshee wrote: > > > > > > > > > > > > > > > I've been reading back through all of this, and I'm not sure any > > > > > > > > conclusion was reached. > > > > > > > > > > > > > > > For my part, I'm uneasy about writing out hmacs to mounts from a > > > > > > > > non-root user. So I'll make a proposal - let's not read or write EVM or > > > > > > > > IMA xattrs for filesystems from non-init user namespace, essentially > > > > > > > > behaving as though they don't support xattrs. Something like the > > > > > > > > (untested) patch below. > > > > > > > > > > > > > > This really doesn't sound like the right long term solution. > > > > > > > > > > > > I'm not proposing this for a long term solution. I just haven't seen > > > > > > that anyone has a real use case in mind yet for using the integrity > > > > > > subsystem with these mounts and that it might be better to wait and > > > > > > decide exactly how it should behave until someone does. In the mean time > > > > > > I'm just trying to ensure there won't be vulnerabilities to exploit. > > > > > > > > > > The main use case will be installing/updating packages within a > > > > > container and writing the corresponding file signatures. Unfortunately, > > > > > we're not there yet. > > > > > > > > Okay. If the distro is providing those signatures then I don't see a > > > > problem with allowing them to be written. What concerns me is if there's > > > > a scenario where the kernel would calculate and write out a signature or > > > > hmac that would make the kernel trust the file in other contexts. > > > > > > There's really no way of differentiating the source of the file > > > signatures. We either allow root in the namespace to write them or not. > > > > I must be misunderstanding something then. > > By file signature, I meant file data signature, which refers to IMA (not > EVM). Okay. It does get a little confusing. > > My impression was that for EVM we'd be dealing with some kind of keyed > > signatures, not just hashes. In that case a user without the correct key > > could write the EVM xattrs, then if the signature was correct the kernel > > would permit access to the file, otherwise it would not. > > Right, except the EVM signature is not portable, as it includes the > inode in the signature. Our work for including file signatures in > packages is limited to the IMA file data signatures. A new portable EVM > format will be need for including EVM signatures in packages. > > > I.e. if the > > user was installing distro packages which shipped with correct > > signatures, the user could write those and the kernel would validate > > them as correct. But if the user tried to write out their own files with > > security xattrs they could not generate valid signatures, and the kernel > > would deny access to those files. > > > Is something about that incorrect? > > That sounds right. Userspace can write EVM signatures, using the > ima-evm-utils package. The asymmetric public key used for verifying the > signature, would need to be loaded onto the EVM keyring. Got it. Sounds like there currently wouldn't be any benefit to letting ns-root write out EVM xattrs in that case. Of course we can't prevent mounting a filesystem which already contains the xattrs, but we don't have to make use of them. > > > > So when s_user_ns != &init_user_ns it sounds like we could allow the > > > > kernel to read and verify signatures and write out signatures provided > > > > by userspace, but not calculate new/updated signatures for files. Does > > > > that sound reasonable? > > > > > > Not really. At least initially, perhaps we should be differentiating > > > between EVM and IMA xattrs, allowing only IMA xattrs to be read/written > > > and returning EOPNOTSUPP, as you suggested, for EVM. Having just IMA > > > xattrs, without EVM xattrs, would then work in the namespace, but not > > > outside of it (assuming EVM is enabled). > > > > That's okay with me assuming it doesn't weaken security for systems with > > EVM enabled. We're already effectively ignoring LSM security xattrs and > > scoping capability xattrs to s_user_ns, so I think we should be okay. > > The worst, which would be bad, is preventing access to a file, but it > wouldn't make an invalid EVM signature/hmac valid. Yeah, I've been taking another look at the code today and saw that was the case. The only time I see the kernel writing out a new signature is for IMA xattrs which are only digests and not signatures. So that seems okay, and some of my concerns were unfounded. I'll work up a patch then to deny reading and writing of EVM xattrs when s_user_ns != &init_user_ns. Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 11c1d30bd705..5a1738524fbb 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -182,7 +182,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, int error; int size; - if (!inode->i_op->getxattr) + if (inode->i_sb->s_user_ns != &init_user_ns || !inode->i_op->getxattr) return -EOPNOTSUPP; desc = init_desc(type); if (IS_ERR(desc)) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 35ab453ce861..3d16e4c523c4 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -78,7 +78,7 @@ static int evm_find_protected_xattrs(struct dentry *dentry) int error; int count = 0; - if (!inode->i_op->getxattr) + if (inode->i_sb->s_user_ns != &init_user_ns || !inode->i_op->getxattr) return -EOPNOTSUPP; for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { @@ -118,6 +118,9 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, enum integrity_status evm_status = INTEGRITY_PASS; int rc, xattr_len; + if (d_backing_inode(dentry)->i_sb->s_user_ns != &init_user_ns) + return INTEGRITY_UNKNOWN; + if (iint && iint->evm_status == INTEGRITY_PASS) return iint->evm_status; diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index a13fc6809554..7f6915a4f660 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -50,6 +50,9 @@ static int ima_fix_xattr(struct dentry *dentry, int rc, offset; u8 algo = iint->ima_hash->algo; + if (d_backing_inode(dentry)->i_sb->s_user_ns != &init_user_ns) + return -EOPNOTSUPP; + if (algo <= HASH_ALGO_SHA1) { offset = 1; iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST; @@ -170,7 +173,7 @@ int ima_read_xattr(struct dentry *dentry, { struct inode *inode = d_backing_inode(dentry); - if (!inode->i_op->getxattr) + if (inode->i_sb->s_user_ns != &init_user_ns || !inode->i_op->getxattr) return 0; return vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value, @@ -198,7 +201,7 @@ int ima_appraise_measurement(enum ima_hooks func, enum integrity_status status = INTEGRITY_UNKNOWN; int rc = xattr_len, hash_start = 0; - if (!inode->i_op->getxattr) + if (inode->i_sb->s_user_ns != &init_user_ns || !inode->i_op->getxattr) return INTEGRITY_UNKNOWN; if (rc <= 0) {