* [PATCH V3] EVM: Add support for portable signature format
@ 2017-10-25 9:54 Matthew Garrett
2017-10-25 9:56 ` James Morris
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Matthew Garrett @ 2017-10-25 9:54 UTC (permalink / raw)
To: linux-integrity; +Cc: zohar, Matthew Garrett, Dmitry Kasatkin, Mikhail Kurinnoi
The EVM signature includes the inode number and (optionally) the
filesystem UUID, making it impractical to ship EVM signatures in
packages. This patch adds a new portable format intended to allow
distributions to include EVM signatures. It is identical to the existing
format but hardcodes the inode and generation numbers to 0 and does not
include the filesystem UUID even if the kernel is configured to do so.
Removing the inode means that the metadata and signature from one file
could be copied to another file without invalidating it. This is avoided
by ensuring that an IMA xattr is present during EVM validation.
Based on earlier work by Dmitry Kasatkin and Mikhail Kurinnoi.
Signed-off-by: Matthew Garrett <mjg59@google.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
Cc: Mikhail Kurinnoi <viewizard@viewizard.com>
---
V3: include feedback from Mimi.
security/integrity/evm/evm.h | 2 +-
security/integrity/evm/evm_crypto.c | 37 ++++++++++++++++++++++++++++---------
security/integrity/evm/evm_main.c | 14 +++++++++-----
security/integrity/integrity.h | 1 +
4 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index f5f12727771a..2ff02459fcfd 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -48,7 +48,7 @@ int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
size_t req_xattr_value_len, char *digest);
int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
const char *req_xattr_value,
- size_t req_xattr_value_len, char *digest);
+ size_t req_xattr_value_len, char type, char *digest);
int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
char *hmac_val);
int evm_init_secfs(void);
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 1d32cd20009a..8855722529d4 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -138,7 +138,7 @@ static struct shash_desc *init_desc(char type)
* protection.)
*/
static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
- char *digest)
+ char type, char *digest)
{
struct h_misc {
unsigned long ino;
@@ -149,8 +149,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
} hmac_misc;
memset(&hmac_misc, 0, sizeof(hmac_misc));
- hmac_misc.ino = inode->i_ino;
- hmac_misc.generation = inode->i_generation;
+ /* Don't include the inode or generation number in portable
+ * signatures
+ */
+ if (type != EVM_XATTR_PORTABLE_DIGSIG) {
+ hmac_misc.ino = inode->i_ino;
+ hmac_misc.generation = inode->i_generation;
+ }
/* The hmac uid and gid must be encoded in the initial user
* namespace (not the filesystems user namespace) as encoding
* them in the filesystems user namespace allows an attack
@@ -163,7 +168,8 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
hmac_misc.mode = inode->i_mode;
crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
- if (evm_hmac_attrs & EVM_ATTR_FSUUID)
+ if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
+ type != EVM_XATTR_PORTABLE_DIGSIG)
crypto_shash_update(desc, &inode->i_sb->s_uuid.b[0],
sizeof(inode->i_sb->s_uuid));
crypto_shash_final(desc, digest);
@@ -189,6 +195,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
char *xattr_value = NULL;
int error;
int size;
+ bool ima_present = false;
if (!(inode->i_opflags & IOP_XATTR))
return -EOPNOTSUPP;
@@ -199,11 +206,18 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
error = -ENODATA;
for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
+ bool is_ima = false;
+
+ if (strcmp(*xattrname, XATTR_NAME_IMA) == 0)
+ is_ima = true;
+
if ((req_xattr_name && req_xattr_value)
&& !strcmp(*xattrname, req_xattr_name)) {
error = 0;
crypto_shash_update(desc, (const u8 *)req_xattr_value,
req_xattr_value_len);
+ if (is_ima)
+ ima_present = true;
continue;
}
size = vfs_getxattr_alloc(dentry, *xattrname,
@@ -218,9 +232,14 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
error = 0;
xattr_size = size;
crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
+ if (is_ima)
+ ima_present = true;
}
- hmac_add_misc(desc, inode, digest);
+ hmac_add_misc(desc, inode, type, digest);
+ /* Portable EVM signatures must include an IMA hash */
+ if (type == EVM_XATTR_PORTABLE_DIGSIG && !ima_present)
+ return -EPERM;
out:
kfree(xattr_value);
kfree(desc);
@@ -232,15 +251,15 @@ int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
char *digest)
{
return evm_calc_hmac_or_hash(dentry, req_xattr_name, req_xattr_value,
- req_xattr_value_len, EVM_XATTR_HMAC, digest);
+ req_xattr_value_len, EVM_XATTR_HMAC, digest);
}
int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
const char *req_xattr_value, size_t req_xattr_value_len,
- char *digest)
+ char type, char *digest)
{
return evm_calc_hmac_or_hash(dentry, req_xattr_name, req_xattr_value,
- req_xattr_value_len, IMA_XATTR_DIGEST, digest);
+ req_xattr_value_len, type, digest);
}
/*
@@ -280,7 +299,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
}
crypto_shash_update(desc, lsm_xattr->value, lsm_xattr->value_len);
- hmac_add_misc(desc, inode, hmac_val);
+ hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
kfree(desc);
return 0;
}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 063d38aef64e..38efee382eb0 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -161,19 +161,22 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
rc = -EINVAL;
break;
case EVM_IMA_XATTR_DIGSIG:
+ case EVM_XATTR_PORTABLE_DIGSIG:
rc = evm_calc_hash(dentry, xattr_name, xattr_value,
- xattr_value_len, calc.digest);
+ xattr_value_len, xattr_data->type,
+ calc.digest);
if (rc)
break;
rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM,
(const char *)xattr_data, xattr_len,
calc.digest, sizeof(calc.digest));
if (!rc) {
- /* Replace RSA with HMAC if not mounted readonly and
- * not immutable
+ /* Replace non-portable RSA with HMAC if not
+ * mounted readonly and not immutable.
*/
if (!IS_RDONLY(d_backing_inode(dentry)) &&
- !IS_IMMUTABLE(d_backing_inode(dentry)))
+ !IS_IMMUTABLE(d_backing_inode(dentry)) &&
+ xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG)
evm_update_evmxattr(dentry, xattr_name,
xattr_value,
xattr_value_len);
@@ -345,7 +348,8 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name,
if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
if (!xattr_value_len)
return -EINVAL;
- if (xattr_data->type != EVM_IMA_XATTR_DIGSIG)
+ if (xattr_data->type != EVM_IMA_XATTR_DIGSIG &&
+ xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG)
return -EPERM;
}
return evm_protect_xattr(dentry, xattr_name, xattr_value,
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index a53e7e4ab06c..1e268ca9706f 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -58,6 +58,7 @@ enum evm_ima_xattr_type {
EVM_XATTR_HMAC,
EVM_IMA_XATTR_DIGSIG,
IMA_XATTR_DIGEST_NG,
+ EVM_XATTR_PORTABLE_DIGSIG,
IMA_XATTR_LAST
};
--
2.15.0.rc2.357.g7e34df9404-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V3] EVM: Add support for portable signature format
2017-10-25 9:54 [PATCH V3] EVM: Add support for portable signature format Matthew Garrett
@ 2017-10-25 9:56 ` James Morris
2017-10-25 10:13 ` Mikhail Kurinnoi
2017-10-25 11:56 ` Mimi Zohar
2 siblings, 0 replies; 7+ messages in thread
From: James Morris @ 2017-10-25 9:56 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-integrity, zohar, Dmitry Kasatkin, Mikhail Kurinnoi
On Wed, 25 Oct 2017, Matthew Garrett wrote:
> The EVM signature includes the inode number and (optionally) the
> filesystem UUID, making it impractical to ship EVM signatures in
> packages. This patch adds a new portable format intended to allow
> distributions to include EVM signatures. It is identical to the existing
> format but hardcodes the inode and generation numbers to 0 and does not
> include the filesystem UUID even if the kernel is configured to do so.
>
> Removing the inode means that the metadata and signature from one file
> could be copied to another file without invalidating it. This is avoided
> by ensuring that an IMA xattr is present during EVM validation.
>
> Based on earlier work by Dmitry Kasatkin and Mikhail Kurinnoi.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> Cc: Mikhail Kurinnoi <viewizard@viewizard.com>
Reviewed-by: James Morris <james.l.morris@oracle.com>
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] EVM: Add support for portable signature format
2017-10-25 9:54 [PATCH V3] EVM: Add support for portable signature format Matthew Garrett
2017-10-25 9:56 ` James Morris
@ 2017-10-25 10:13 ` Mikhail Kurinnoi
2017-10-25 10:43 ` Matthew Garrett
2017-10-25 11:56 ` Mimi Zohar
2 siblings, 1 reply; 7+ messages in thread
From: Mikhail Kurinnoi @ 2017-10-25 10:13 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-integrity, zohar, Dmitry Kasatkin
? Wed, 25 Oct 2017 02:54:13 -0700
Matthew Garrett <mjg59@google.com> ?????:
> The EVM signature includes the inode number and (optionally) the
> filesystem UUID, making it impractical to ship EVM signatures in
> packages. This patch adds a new portable format intended to allow
> distributions to include EVM signatures. It is identical to the
> existing format but hardcodes the inode and generation numbers to 0
> and does not include the filesystem UUID even if the kernel is
> configured to do so.
>
> Removing the inode means that the metadata and signature from one file
> could be copied to another file without invalidating it. This is
> avoided by ensuring that an IMA xattr is present during EVM
> validation.
>
> Based on earlier work by Dmitry Kasatkin and Mikhail Kurinnoi.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> Cc: Mikhail Kurinnoi <viewizard@viewizard.com>
> ---
> V3: include feedback from Mimi.
>
> security/integrity/evm/evm.h | 2 +-
> security/integrity/evm/evm_crypto.c | 37
> ++++++++++++++++++++++++++++---------
> security/integrity/evm/evm_main.c | 14 +++++++++-----
> security/integrity/integrity.h | 1 + 4 files changed, 39
> insertions(+), 15 deletions(-)
>
> diff --git a/security/integrity/evm/evm.h
> b/security/integrity/evm/evm.h index f5f12727771a..2ff02459fcfd 100644
> --- a/security/integrity/evm/evm.h
> +++ b/security/integrity/evm/evm.h
> @@ -48,7 +48,7 @@ int evm_calc_hmac(struct dentry *dentry, const char
> *req_xattr_name, size_t req_xattr_value_len, char *digest);
> int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
> const char *req_xattr_value,
> - size_t req_xattr_value_len, char *digest);
> + size_t req_xattr_value_len, char type, char
> *digest); int evm_init_hmac(struct inode *inode, const struct xattr
> *xattr, char *hmac_val);
> int evm_init_secfs(void);
> diff --git a/security/integrity/evm/evm_crypto.c
> b/security/integrity/evm/evm_crypto.c index
> 1d32cd20009a..8855722529d4 100644 ---
> a/security/integrity/evm/evm_crypto.c +++
> b/security/integrity/evm/evm_crypto.c @@ -138,7 +138,7 @@ static
> struct shash_desc *init_desc(char type)
> * protection.)
> */
> static void hmac_add_misc(struct shash_desc *desc, struct inode
> *inode,
> - char *digest)
> + char type, char *digest)
> {
> struct h_misc {
> unsigned long ino;
> @@ -149,8 +149,13 @@ static void hmac_add_misc(struct shash_desc
> *desc, struct inode *inode, } hmac_misc;
>
> memset(&hmac_misc, 0, sizeof(hmac_misc));
> - hmac_misc.ino = inode->i_ino;
> - hmac_misc.generation = inode->i_generation;
> + /* Don't include the inode or generation number in portable
> + * signatures
> + */
> + if (type != EVM_XATTR_PORTABLE_DIGSIG) {
> + hmac_misc.ino = inode->i_ino;
> + hmac_misc.generation = inode->i_generation;
> + }
> /* The hmac uid and gid must be encoded in the initial user
> * namespace (not the filesystems user namespace) as encoding
> * them in the filesystems user namespace allows an attack
> @@ -163,7 +168,8 @@ static void hmac_add_misc(struct shash_desc
> *desc, struct inode *inode, hmac_misc.gid = from_kgid(&init_user_ns,
> inode->i_gid); hmac_misc.mode = inode->i_mode;
> crypto_shash_update(desc, (const u8 *)&hmac_misc,
> sizeof(hmac_misc));
> - if (evm_hmac_attrs & EVM_ATTR_FSUUID)
> + if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
> + type != EVM_XATTR_PORTABLE_DIGSIG)
> crypto_shash_update(desc, &inode->i_sb->s_uuid.b[0],
> sizeof(inode->i_sb->s_uuid));
> crypto_shash_final(desc, digest);
> @@ -189,6 +195,7 @@ static int evm_calc_hmac_or_hash(struct dentry
> *dentry, char *xattr_value = NULL;
> int error;
> int size;
> + bool ima_present = false;
>
> if (!(inode->i_opflags & IOP_XATTR))
> return -EOPNOTSUPP;
> @@ -199,11 +206,18 @@ static int evm_calc_hmac_or_hash(struct dentry
> *dentry,
> error = -ENODATA;
> for (xattrname = evm_config_xattrnames; *xattrname != NULL;
> xattrname++) {
> + bool is_ima = false;
> +
> + if (strcmp(*xattrname, XATTR_NAME_IMA) == 0)
> + is_ima = true;
> +
> if ((req_xattr_name && req_xattr_value)
> && !strcmp(*xattrname, req_xattr_name)) {
> error = 0;
> crypto_shash_update(desc, (const u8
> *)req_xattr_value, req_xattr_value_len);
> + if (is_ima)
> + ima_present = true;
> continue;
> }
> size = vfs_getxattr_alloc(dentry, *xattrname,
> @@ -218,9 +232,14 @@ static int evm_calc_hmac_or_hash(struct dentry
> *dentry, error = 0;
> xattr_size = size;
> crypto_shash_update(desc, (const u8 *)xattr_value,
> xattr_size);
> + if (is_ima)
> + ima_present = true;
> }
> - hmac_add_misc(desc, inode, digest);
> + hmac_add_misc(desc, inode, type, digest);
>
> + /* Portable EVM signatures must include an IMA hash */
> + if (type == EVM_XATTR_PORTABLE_DIGSIG && !ima_present)
> + return -EPERM;
> out:
> kfree(xattr_value);
> kfree(desc);
> @@ -232,15 +251,15 @@ int evm_calc_hmac(struct dentry *dentry, const
> char *req_xattr_name, char *digest)
> {
> return evm_calc_hmac_or_hash(dentry, req_xattr_name,
> req_xattr_value,
> - req_xattr_value_len, EVM_XATTR_HMAC,
> digest);
> + req_xattr_value_len, EVM_XATTR_HMAC,
> digest); }
>
> int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
> const char *req_xattr_value, size_t
> req_xattr_value_len,
> - char *digest)
> + char type, char *digest)
> {
> return evm_calc_hmac_or_hash(dentry, req_xattr_name,
> req_xattr_value,
> - req_xattr_value_len,
> IMA_XATTR_DIGEST, digest);
> + req_xattr_value_len, type,
> digest); }
>
> /*
> @@ -280,7 +299,7 @@ int evm_init_hmac(struct inode *inode, const
> struct xattr *lsm_xattr, }
>
> crypto_shash_update(desc, lsm_xattr->value,
> lsm_xattr->value_len);
> - hmac_add_misc(desc, inode, hmac_val);
> + hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
> kfree(desc);
> return 0;
> }
> diff --git a/security/integrity/evm/evm_main.c
> b/security/integrity/evm/evm_main.c index 063d38aef64e..38efee382eb0
> 100644 --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -161,19 +161,22 @@ static enum integrity_status
> evm_verify_hmac(struct dentry *dentry, rc = -EINVAL;
> break;
> case EVM_IMA_XATTR_DIGSIG:
> + case EVM_XATTR_PORTABLE_DIGSIG:
> rc = evm_calc_hash(dentry, xattr_name, xattr_value,
> - xattr_value_len, calc.digest);
> + xattr_value_len, xattr_data->type,
> + calc.digest);
> if (rc)
> break;
> rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM,
> (const char *)xattr_data,
> xattr_len, calc.digest, sizeof(calc.digest));
> if (!rc) {
> - /* Replace RSA with HMAC if not mounted
> readonly and
> - * not immutable
> + /* Replace non-portable RSA with HMAC if not
> + * mounted readonly and not immutable.
> */
> if (!IS_RDONLY(d_backing_inode(dentry)) &&
> - !IS_IMMUTABLE(d_backing_inode(dentry)))
> + !IS_IMMUTABLE(d_backing_inode(dentry)) &&
> + xattr_data->type !=
> EVM_XATTR_PORTABLE_DIGSIG) evm_update_evmxattr(dentry, xattr_name,
> xattr_value,
> xattr_value_len);
> @@ -345,7 +348,8 @@ int evm_inode_setxattr(struct dentry *dentry,
> const char *xattr_name, if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
> if (!xattr_value_len)
> return -EINVAL;
> - if (xattr_data->type != EVM_IMA_XATTR_DIGSIG)
> + if (xattr_data->type != EVM_IMA_XATTR_DIGSIG &&
> + xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG)
> return -EPERM;
> }
> return evm_protect_xattr(dentry, xattr_name, xattr_value,
> diff --git a/security/integrity/integrity.h
> b/security/integrity/integrity.h index a53e7e4ab06c..1e268ca9706f
> 100644 --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -58,6 +58,7 @@ enum evm_ima_xattr_type {
> EVM_XATTR_HMAC,
> EVM_IMA_XATTR_DIGSIG,
> IMA_XATTR_DIGEST_NG,
> + EVM_XATTR_PORTABLE_DIGSIG,
> IMA_XATTR_LAST
> };
>
In case of IMA hash update we will forced to update EVM xattr from
ima_fix_xattr() with __vfs_setxattr_noperm(), this mean we will not call
evm_inode_setxattr(), but call evm_inode_post_setxattr().
Dmitry's patch
https://sourceforge.net/p/linux-ima/mailman/message/32987311/
have work around for this issue. Since, in case we have immutable EVM,
we should prevent any file data changes (IMA hash update).
--
Best regards,
Mikhail Kurinnoi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] EVM: Add support for portable signature format
2017-10-25 10:13 ` Mikhail Kurinnoi
@ 2017-10-25 10:43 ` Matthew Garrett
2017-10-25 11:24 ` Mikhail Kurinnoi
0 siblings, 1 reply; 7+ messages in thread
From: Matthew Garrett @ 2017-10-25 10:43 UTC (permalink / raw)
To: Mikhail Kurinnoi; +Cc: linux-integrity, Mimi Zohar, Dmitry Kasatkin
On Wed, Oct 25, 2017 at 3:13 AM, Mikhail Kurinnoi
<viewizard@viewizard.com> wrote:
> In case of IMA hash update we will forced to update EVM xattr from
> ima_fix_xattr() with __vfs_setxattr_noperm(), this mean we will not call
> evm_inode_setxattr(), but call evm_inode_post_setxattr().
>
> Dmitry's patch
> https://sourceforge.net/p/linux-ima/mailman/message/32987311/
> have work around for this issue. Since, in case we have immutable EVM,
> we should prevent any file data changes (IMA hash update).
Ah - does this need any more than adding EVM_XATTR_PORTABLE_DIGSIG to
the check in ima_appraise_measurement()? I can't see any other way
that we could get to ima_fix_xattr().
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] EVM: Add support for portable signature format
2017-10-25 10:43 ` Matthew Garrett
@ 2017-10-25 11:24 ` Mikhail Kurinnoi
2017-10-25 11:42 ` Dmitry Kasatkin
0 siblings, 1 reply; 7+ messages in thread
From: Mikhail Kurinnoi @ 2017-10-25 11:24 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-integrity, Mimi Zohar, Dmitry Kasatkin
? Wed, 25 Oct 2017 03:43:34 -0700
Matthew Garrett <mjg59@google.com> ?????:
> On Wed, Oct 25, 2017 at 3:13 AM, Mikhail Kurinnoi
> <viewizard@viewizard.com> wrote:
> > In case of IMA hash update we will forced to update EVM xattr from
> > ima_fix_xattr() with __vfs_setxattr_noperm(), this mean we will not
> > call evm_inode_setxattr(), but call evm_inode_post_setxattr().
> >
> > Dmitry's patch
> > https://sourceforge.net/p/linux-ima/mailman/message/32987311/
> > have work around for this issue. Since, in case we have immutable
> > EVM, we should prevent any file data changes (IMA hash update).
>
> Ah - does this need any more than adding EVM_XATTR_PORTABLE_DIGSIG to
> the check in ima_appraise_measurement()? I can't see any other way
> that we could get to ima_fix_xattr().
I think, Dmitry put code into process_measurement() just because we
already have "mask" here, so, we could check it in easy way.
In previous discussion, Mimi asked move all EVM-related stuff
connected to this check into EVM module, instead of IMA. Probably, this
mean we should use EVM API (evm_verifyxattr(), called from
ima_appraise_measurement()), that we already have in order to EVM<->IMA
communication.
--
Best regards,
Mikhail Kurinnoi
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH V3] EVM: Add support for portable signature format
2017-10-25 11:24 ` Mikhail Kurinnoi
@ 2017-10-25 11:42 ` Dmitry Kasatkin
0 siblings, 0 replies; 7+ messages in thread
From: Dmitry Kasatkin @ 2017-10-25 11:42 UTC (permalink / raw)
To: Mikhail Kurinnoi, Matthew Garrett; +Cc: linux-integrity, Mimi Zohar
> -----Original Message-----
> From: Mikhail Kurinnoi [mailto:viewizard@gmail.com] On Behalf Of Mikhail
> Kurinnoi
> Sent: 25 October 2017 14:25
> To: Matthew Garrett <mjg59@google.com>
> Cc: linux-integrity <linux-integrity@vger.kernel.org>; Mimi Zohar
> <zohar@linux.vnet.ibm.com>; Dmitry Kasatkin
> <dmitry.kasatkin@huawei.com>
> Subject: Re: [PATCH V3] EVM: Add support for portable signature format
>
> ? Wed, 25 Oct 2017 03:43:34 -0700
> Matthew Garrett <mjg59@google.com> ?????:
>
> > On Wed, Oct 25, 2017 at 3:13 AM, Mikhail Kurinnoi
> > <viewizard@viewizard.com> wrote:
> > > In case of IMA hash update we will forced to update EVM xattr from
> > > ima_fix_xattr() with __vfs_setxattr_noperm(), this mean we will not
> > > call evm_inode_setxattr(), but call evm_inode_post_setxattr().
> > >
> > > Dmitry's patch
> > > https://sourceforge.net/p/linux-ima/mailman/message/32987311/
> > > have work around for this issue. Since, in case we have immutable
> > > EVM, we should prevent any file data changes (IMA hash update).
> >
> > Ah - does this need any more than adding EVM_XATTR_PORTABLE_DIGSIG
> to
> > the check in ima_appraise_measurement()? I can't see any other way
> > that we could get to ima_fix_xattr().
>
> I think, Dmitry put code into process_measurement() just because we
> already have "mask" here, so, we could check it in easy way.
>
> In previous discussion, Mimi asked move all EVM-related stuff connected to
> this check into EVM module, instead of IMA. Probably, this mean we should
> use EVM API (evm_verifyxattr(), called from ima_appraise_measurement()),
> that we already have in order to EVM<->IMA communication.
>
>
[DK] let me check/think this carefully later today.
>
> --
> Best regards,
> Mikhail Kurinnoi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3] EVM: Add support for portable signature format
2017-10-25 9:54 [PATCH V3] EVM: Add support for portable signature format Matthew Garrett
2017-10-25 9:56 ` James Morris
2017-10-25 10:13 ` Mikhail Kurinnoi
@ 2017-10-25 11:56 ` Mimi Zohar
2 siblings, 0 replies; 7+ messages in thread
From: Mimi Zohar @ 2017-10-25 11:56 UTC (permalink / raw)
To: Matthew Garrett, linux-integrity; +Cc: Dmitry Kasatkin, Mikhail Kurinnoi
On Wed, 2017-10-25 at 02:54 -0700, Matthew Garrett wrote:
> The EVM signature includes the inode number and (optionally) the
> filesystem UUID, making it impractical to ship EVM signatures in
> packages. This patch adds a new portable format intended to allow
> distributions to include EVM signatures. It is identical to the existing
> format but hardcodes the inode and generation numbers to 0 and does not
> include the filesystem UUID even if the kernel is configured to do so.
>
> Removing the inode means that the metadata and signature from one file
> could be copied to another file without invalidating it. This is avoided
> by ensuring that an IMA xattr is present during EVM validation.
>
> Based on earlier work by Dmitry Kasatkin and Mikhail Kurinnoi.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> Cc: Mikhail Kurinnoi <viewizard@viewizard.com>
> ---
> V3: include feedback from Mimi.
Some checks are still missing to prevent the portable/immutable
signature from being replaced with an HMAC (eg. setattr, removexattr,
etc).
Mimi
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-25 11:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-25 9:54 [PATCH V3] EVM: Add support for portable signature format Matthew Garrett
2017-10-25 9:56 ` James Morris
2017-10-25 10:13 ` Mikhail Kurinnoi
2017-10-25 10:43 ` Matthew Garrett
2017-10-25 11:24 ` Mikhail Kurinnoi
2017-10-25 11:42 ` Dmitry Kasatkin
2017-10-25 11:56 ` Mimi Zohar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).