* [PATCH] EVM: Add support for portable signature format
@ 2017-10-18 18:01 Matthew Garrett
2017-10-19 11:02 ` Dmitry Kasatkin
2017-10-19 12:52 ` Mimi Zohar
0 siblings, 2 replies; 13+ messages in thread
From: Matthew Garrett @ 2017-10-18 18:01 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.
Admins should note that creating portable signatures that do not include
the security.ima xattr would allow these signatures to be applied to any
file with the same owners and security labels, which would allow
subversion of EVM's security guarantees. The kernel does not attempt to
enforce this.
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>
---
security/integrity/evm/evm.h | 2 +-
security/integrity/evm/evm_crypto.c | 24 +++++++++++++++---------
security/integrity/evm/evm_main.c | 7 +++++--
security/integrity/integrity.h | 1 +
4 files changed, 22 insertions(+), 12 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..6435f12b0067 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_IMA_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_IMA_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);
@@ -219,7 +225,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
xattr_size = size;
crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
}
- hmac_add_misc(desc, inode, digest);
+ hmac_add_misc(desc, inode, type, digest);
out:
kfree(xattr_value);
@@ -232,15 +238,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 +286,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..ff3c35a7058c 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -161,8 +161,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
rc = -EINVAL;
break;
case EVM_IMA_XATTR_DIGSIG:
+ case EVM_IMA_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,
@@ -345,7 +347,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_IMA_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..0a721c110e92 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_IMA_XATTR_PORTABLE_DIGSIG,
IMA_XATTR_LAST
};
--
2.15.0.rc1.287.g2b38de12cc-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: [PATCH] EVM: Add support for portable signature format
2017-10-18 18:01 [PATCH] EVM: Add support for portable signature format Matthew Garrett
@ 2017-10-19 11:02 ` Dmitry Kasatkin
2017-10-19 11:55 ` Mikhail Kurinnoi
2017-10-19 12:00 ` Mimi Zohar
2017-10-19 12:52 ` Mimi Zohar
1 sibling, 2 replies; 13+ messages in thread
From: Dmitry Kasatkin @ 2017-10-19 11:02 UTC (permalink / raw)
To: Matthew Garrett, linux-integrity@vger.kernel.org
Cc: zohar@linux.vnet.ibm.com, Mikhail Kurinnoi
BTW.
Just to refresh my mind. What would be the correct order for setting this signature from package?
On any attr/xattr change, EVM will set HMAC.
What is the value of setting signature after that unless there is a policy to require signature (immutable)?
In my original patchset portable was also immutable and also included policy support to require EVM signatures.
Dmitry
________________________________________
From: Matthew Garrett [mjg59@google.com]
Sent: Wednesday, October 18, 2017 9:01 PM
To: linux-integrity@vger.kernel.org
Cc: zohar@linux.vnet.ibm.com; Matthew Garrett; Dmitry Kasatkin; Mikhail Kurinnoi
Subject: [PATCH] EVM: Add support for portable signature format
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.
Admins should note that creating portable signatures that do not include
the security.ima xattr would allow these signatures to be applied to any
file with the same owners and security labels, which would allow
subversion of EVM's security guarantees. The kernel does not attempt to
enforce this.
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>
---
security/integrity/evm/evm.h | 2 +-
security/integrity/evm/evm_crypto.c | 24 +++++++++++++++---------
security/integrity/evm/evm_main.c | 7 +++++--
security/integrity/integrity.h | 1 +
4 files changed, 22 insertions(+), 12 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..6435f12b0067 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_IMA_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_IMA_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);
@@ -219,7 +225,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
xattr_size = size;
crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
}
- hmac_add_misc(desc, inode, digest);
+ hmac_add_misc(desc, inode, type, digest);
out:
kfree(xattr_value);
@@ -232,15 +238,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 +286,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..ff3c35a7058c 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -161,8 +161,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
rc = -EINVAL;
break;
case EVM_IMA_XATTR_DIGSIG:
+ case EVM_IMA_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,
@@ -345,7 +347,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_IMA_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..0a721c110e92 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_IMA_XATTR_PORTABLE_DIGSIG,
IMA_XATTR_LAST
};
--
2.15.0.rc1.287.g2b38de12cc-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 11:02 ` Dmitry Kasatkin
@ 2017-10-19 11:55 ` Mikhail Kurinnoi
2017-10-19 12:00 ` Mimi Zohar
1 sibling, 0 replies; 13+ messages in thread
From: Mikhail Kurinnoi @ 2017-10-19 11:55 UTC (permalink / raw)
To: Dmitry Kasatkin
Cc: Matthew Garrett, linux-integrity@vger.kernel.org,
zohar@linux.vnet.ibm.com
? Thu, 19 Oct 2017 11:02:51 +0000
Dmitry Kasatkin <dmitry.kasatkin@huawei.com> ?????:
> BTW.
>
> Just to refresh my mind. What would be the correct order for setting
> this signature from package? On any attr/xattr change, EVM will set
> HMAC.
from tar's code:
- uid/git/mode/data/etc...
- all xattrs
- caps
- selinux
- EVM xattr
EVM xattr should be restored the last one, when all xattrs/metadata
already restored, but... as soon, as first protected xattr will be
restored from package, EVM HMAC will be generated.
> What is the value of setting signature after that unless there is a
> policy to require signature (immutable)? In my original patchset
> portable was also immutable and also included policy support to
> require EVM signatures.
--
Best regards,
Mikhail Kurinnoi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 11:02 ` Dmitry Kasatkin
2017-10-19 11:55 ` Mikhail Kurinnoi
@ 2017-10-19 12:00 ` Mimi Zohar
2017-10-19 15:11 ` Dmitry Kasatkin
1 sibling, 1 reply; 13+ messages in thread
From: Mimi Zohar @ 2017-10-19 12:00 UTC (permalink / raw)
To: Dmitry Kasatkin, Matthew Garrett, linux-integrity@vger.kernel.org
Cc: Mikhail Kurinnoi
On Thu, 2017-10-19 at 11:02 +0000, Dmitry Kasatkin wrote:
> BTW.
>
> Just to refresh my mind. What would be the correct order for setting
> this signature from package?
> On any attr/xattr change, EVM will set HMAC.
The system is running without an EVM symmetric key, just an asymmetric
key.
> What is the value of setting signature after that unless there is a
> policy to require signature (immutable)?
> In my original patchset portable was also immutable and also
> included policy support to require EVM signatures.
In Matthew's usecase scenario, only immutable and portable signatures
exist. The EVM verification for unsigned files will be
INTEGRITY_UNKNOWN.
Mimi
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] EVM: Add support for portable signature format
2017-10-19 12:00 ` Mimi Zohar
@ 2017-10-19 15:11 ` Dmitry Kasatkin
2017-10-19 17:11 ` Matthew Garrett
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Kasatkin @ 2017-10-19 15:11 UTC (permalink / raw)
To: Mimi Zohar, Matthew Garrett, linux-integrity@vger.kernel.org
Cc: Mikhail Kurinnoi
Hi,
1. I do not get the idea...
> ship EVM signatures in packages
System up and running EVM without hmac?
How it creates new files without hmac?
2. Mathew's patch is about portable signatures... not immutable
And I do not get idea why they are needed if they replaced by HMAC.
Practically file gets HMAC, then signature is set, and then replaced with HMAC
Dmitry
________________________________________
From: Mimi Zohar [zohar@linux.vnet.ibm.com]
Sent: Thursday, October 19, 2017 3:00 PM
To: Dmitry Kasatkin; Matthew Garrett; linux-integrity@vger.kernel.org
Cc: Mikhail Kurinnoi
Subject: Re: [PATCH] EVM: Add support for portable signature format
On Thu, 2017-10-19 at 11:02 +0000, Dmitry Kasatkin wrote:
> BTW.
>
> Just to refresh my mind. What would be the correct order for setting
> this signature from package?
> On any attr/xattr change, EVM will set HMAC.
The system is running without an EVM symmetric key, just an asymmetric
key.
> What is the value of setting signature after that unless there is a
> policy to require signature (immutable)?
> In my original patchset portable was also immutable and also
> included policy support to require EVM signatures.
In Matthew's usecase scenario, only immutable and portable signatures
exist. The EVM verification for unsigned files will be
INTEGRITY_UNKNOWN.
Mimi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 15:11 ` Dmitry Kasatkin
@ 2017-10-19 17:11 ` Matthew Garrett
2017-10-19 18:02 ` Dmitry Kasatkin
0 siblings, 1 reply; 13+ messages in thread
From: Matthew Garrett @ 2017-10-19 17:11 UTC (permalink / raw)
To: Dmitry Kasatkin
Cc: Mimi Zohar, linux-integrity@vger.kernel.org, Mikhail Kurinnoi
On Thu, Oct 19, 2017 at 8:11 AM, Dmitry Kasatkin
<dmitry.kasatkin@huawei.com> wrote:
> Hi,
>
> 1. I do not get the idea...
>
>> ship EVM signatures in packages
>
> System up and running EVM without hmac?
Correct.
> How it creates new files without hmac?
New files won't have EVM signatures. Appraisal will only be performed
on executables that are running in a privileged security context.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 17:11 ` Matthew Garrett
@ 2017-10-19 18:02 ` Dmitry Kasatkin
2017-10-19 18:13 ` Dmitry Kasatkin
2017-10-19 18:15 ` Matthew Garrett
0 siblings, 2 replies; 13+ messages in thread
From: Dmitry Kasatkin @ 2017-10-19 18:02 UTC (permalink / raw)
To: Matthew Garrett
Cc: Dmitry Kasatkin, Mimi Zohar, linux-integrity@vger.kernel.org,
Mikhail Kurinnoi
On Thu, Oct 19, 2017 at 8:11 PM, Matthew Garrett <mjg59@google.com> wrote:
> On Thu, Oct 19, 2017 at 8:11 AM, Dmitry Kasatkin
> <dmitry.kasatkin@huawei.com> wrote:
>> Hi,
>>
>> 1. I do not get the idea...
>>
>>> ship EVM signatures in packages
>>
>> System up and running EVM without hmac?
>
> Correct.
>
>> How it creates new files without hmac?
>
> New files won't have EVM signatures. Appraisal will only be performed
> on executables that are running in a privileged security context.
This patch was there for 3 years to enable policy to require evm
digital signatures.
https://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig.git/commit/?h=evm-next&id=580e1ad19dd9917ce8ca5edbdf823c30397ccd47
I was running a system where certain (privileged) components were
required to use evm signatures.
Before initramfs supported xattrs, we were running from rootfs /init
and some binaries with EVM signature required. HMAC key was unsealed
and initalized during this process.
Now it is also possible to use external initramfs with xattrs and
require evm digsigs.
you are basically doing the same.
Dmitry
--
Thanks,
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 18:02 ` Dmitry Kasatkin
@ 2017-10-19 18:13 ` Dmitry Kasatkin
2017-10-19 18:15 ` Matthew Garrett
1 sibling, 0 replies; 13+ messages in thread
From: Dmitry Kasatkin @ 2017-10-19 18:13 UTC (permalink / raw)
To: Matthew Garrett
Cc: Dmitry Kasatkin, Mimi Zohar, linux-integrity@vger.kernel.org,
Mikhail Kurinnoi
On Thu, Oct 19, 2017 at 9:02 PM, Dmitry Kasatkin
<dmitry.kasatkin@gmail.com> wrote:
> On Thu, Oct 19, 2017 at 8:11 PM, Matthew Garrett <mjg59@google.com> wrote:
>> On Thu, Oct 19, 2017 at 8:11 AM, Dmitry Kasatkin
>> <dmitry.kasatkin@huawei.com> wrote:
>>> Hi,
>>>
>>> 1. I do not get the idea...
>>>
>>>> ship EVM signatures in packages
>>>
>>> System up and running EVM without hmac?
>>
>> Correct.
>>
>>> How it creates new files without hmac?
>>
>> New files won't have EVM signatures. Appraisal will only be performed
>> on executables that are running in a privileged security context.
>
> This patch was there for 3 years to enable policy to require evm
> digital signatures.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig.git/commit/?h=evm-next&id=580e1ad19dd9917ce8ca5edbdf823c30397ccd47
>
> I was running a system where certain (privileged) components were
> required to use evm signatures.
>
> Before initramfs supported xattrs, we were running from rootfs /init
> and some binaries with EVM signature required. HMAC key was unsealed
> and initalized during this process.
> Now it is also possible to use external initramfs with xattrs and
> require evm digsigs.
>
> you are basically doing the same.
>
Ok. i got the use case. No hmac at all.
Currently EVM is enabled if any of the key (symmetric or public) is loaded.
Currently with only public key, it works fine when rootfs is mounted
read-only, because no need to generate hmac.
But when fully running and remounted rw, then it would require HMAC.
So we need a way to have only evm signatures..
--
Thanks,
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 18:02 ` Dmitry Kasatkin
2017-10-19 18:13 ` Dmitry Kasatkin
@ 2017-10-19 18:15 ` Matthew Garrett
2017-10-19 18:50 ` Dmitry Kasatkin
1 sibling, 1 reply; 13+ messages in thread
From: Matthew Garrett @ 2017-10-19 18:15 UTC (permalink / raw)
To: Dmitry Kasatkin
Cc: Dmitry Kasatkin, Mimi Zohar, linux-integrity@vger.kernel.org,
Mikhail Kurinnoi
On Thu, Oct 19, 2017 at 11:02 AM, Dmitry Kasatkin
<dmitry.kasatkin@gmail.com> wrote:
> On Thu, Oct 19, 2017 at 8:11 PM, Matthew Garrett <mjg59@google.com> wrote:
>> New files won't have EVM signatures. Appraisal will only be performed
>> on executables that are running in a privileged security context.
>
> This patch was there for 3 years to enable policy to require evm
> digital signatures.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig.git/commit/?h=evm-next&id=580e1ad19dd9917ce8ca5edbdf823c30397ccd47
>
> I was running a system where certain (privileged) components were
> required to use evm signatures.
>
> Before initramfs supported xattrs, we were running from rootfs /init
> and some binaries with EVM signature required. HMAC key was unsealed
> and initalized during this process.
> Now it is also possible to use external initramfs with xattrs and
> require evm digsigs.
>
> you are basically doing the same.
Broadly, but for our case we can't permit the local system to possess
a key that can create valid signatures, which means enhancing support
for portable asymmetric signatures.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 18:15 ` Matthew Garrett
@ 2017-10-19 18:50 ` Dmitry Kasatkin
0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Kasatkin @ 2017-10-19 18:50 UTC (permalink / raw)
To: Matthew Garrett
Cc: Dmitry Kasatkin, Mimi Zohar, linux-integrity@vger.kernel.org,
Mikhail Kurinnoi
On Thu, Oct 19, 2017 at 9:15 PM, Matthew Garrett <mjg59@google.com> wrote:
> On Thu, Oct 19, 2017 at 11:02 AM, Dmitry Kasatkin
> <dmitry.kasatkin@gmail.com> wrote:
>> On Thu, Oct 19, 2017 at 8:11 PM, Matthew Garrett <mjg59@google.com> wrote:
>>> New files won't have EVM signatures. Appraisal will only be performed
>>> on executables that are running in a privileged security context.
>>
>> This patch was there for 3 years to enable policy to require evm
>> digital signatures.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig.git/commit/?h=evm-next&id=580e1ad19dd9917ce8ca5edbdf823c30397ccd47
>>
>> I was running a system where certain (privileged) components were
>> required to use evm signatures.
>>
>> Before initramfs supported xattrs, we were running from rootfs /init
>> and some binaries with EVM signature required. HMAC key was unsealed
>> and initalized during this process.
>> Now it is also possible to use external initramfs with xattrs and
>> require evm digsigs.
>>
>> you are basically doing the same.
>
> Broadly, but for our case we can't permit the local system to possess
> a key that can create valid signatures, which means enhancing support
> for portable asymmetric signatures.
I was not there as well. Binaries were EVM signed on the build system.
Runtime did not have private key.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] EVM: Add support for portable signature format
2017-10-18 18:01 [PATCH] EVM: Add support for portable signature format Matthew Garrett
2017-10-19 11:02 ` Dmitry Kasatkin
@ 2017-10-19 12:52 ` Mimi Zohar
2017-10-19 17:09 ` Matthew Garrett
1 sibling, 1 reply; 13+ messages in thread
From: Mimi Zohar @ 2017-10-19 12:52 UTC (permalink / raw)
To: Matthew Garrett, linux-integrity; +Cc: Dmitry Kasatkin, Mikhail Kurinnoi
On Wed, 2017-10-18 at 11:01 -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.
The existing IMA signature format is already portable and immutable.
The new portable and immutable signature type is only needed for EVM.
I don't see a need to refer to it as EVM_IMA_XATTR_PORTABLE_DIGSIG.
> Admins should note that creating portable signatures that do not include
> the security.ima xattr would allow these signatures to be applied to any
> file with the same owners and security labels, which would allow
> subversion of EVM's security guarantees. The kernel does not attempt to
> enforce this.
As much as possible IMA and EVM should work independently of each
other. But in this case, I think we need to blur the lines a bit.
Currently, before writing a new security.evm value, the existing
security.evm value is verified. To do this it has to read the
security xattrs to calculate the hash/hmac. How hard would it really
be to verify that a security.ima xattr exists, before writing this new
EVM signature? How hard would it be to make sure that security.ima is
included in the calculation on verification?
Mimi
> 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>
> ---
> security/integrity/evm/evm.h | 2 +-
> security/integrity/evm/evm_crypto.c | 24 +++++++++++++++---------
> security/integrity/evm/evm_main.c | 7 +++++--
> security/integrity/integrity.h | 1 +
> 4 files changed, 22 insertions(+), 12 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..6435f12b0067 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_IMA_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_IMA_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);
> @@ -219,7 +225,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> xattr_size = size;
> crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
> }
> - hmac_add_misc(desc, inode, digest);
> + hmac_add_misc(desc, inode, type, digest);
>
> out:
> kfree(xattr_value);
> @@ -232,15 +238,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 +286,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..ff3c35a7058c 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -161,8 +161,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
> rc = -EINVAL;
> break;
> case EVM_IMA_XATTR_DIGSIG:
> + case EVM_IMA_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,
> @@ -345,7 +347,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_IMA_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..0a721c110e92 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_IMA_XATTR_PORTABLE_DIGSIG,
> IMA_XATTR_LAST
> };
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 12:52 ` Mimi Zohar
@ 2017-10-19 17:09 ` Matthew Garrett
2017-10-19 17:44 ` Mimi Zohar
0 siblings, 1 reply; 13+ messages in thread
From: Matthew Garrett @ 2017-10-19 17:09 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-integrity, Dmitry Kasatkin, Mikhail Kurinnoi
On Thu, Oct 19, 2017 at 5:52 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2017-10-18 at 11:01 -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.
>
> The existing IMA signature format is already portable and immutable.
> The new portable and immutable signature type is only needed for EVM.
> I don't see a need to refer to it as EVM_IMA_XATTR_PORTABLE_DIGSIG.
Ok, I can change the name.
>> Admins should note that creating portable signatures that do not include
>> the security.ima xattr would allow these signatures to be applied to any
>> file with the same owners and security labels, which would allow
>> subversion of EVM's security guarantees. The kernel does not attempt to
>> enforce this.
>
> As much as possible IMA and EVM should work independently of each
> other. But in this case, I think we need to blur the lines a bit.
>
> Currently, before writing a new security.evm value, the existing
> security.evm value is verified. To do this it has to read the
> security xattrs to calculate the hash/hmac. How hard would it really
> be to verify that a security.ima xattr exists, before writing this new
> EVM signature? How hard would it be to make sure that security.ima is
> included in the calculation on verification?
I don't think it would be especially hard to ensure that security.ima
is present if the portable digsig format is used, but as you say it
would blur the lines a little.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] EVM: Add support for portable signature format
2017-10-19 17:09 ` Matthew Garrett
@ 2017-10-19 17:44 ` Mimi Zohar
0 siblings, 0 replies; 13+ messages in thread
From: Mimi Zohar @ 2017-10-19 17:44 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-integrity, Dmitry Kasatkin, Mikhail Kurinnoi
> >> Admins should note that creating portable signatures that do not include
> >> the security.ima xattr would allow these signatures to be applied to any
> >> file with the same owners and security labels, which would allow
> >> subversion of EVM's security guarantees. The kernel does not attempt to
> >> enforce this.
> >
> > As much as possible IMA and EVM should work independently of each
> > other. But in this case, I think we need to blur the lines a bit.
> >
> > Currently, before writing a new security.evm value, the existing
> > security.evm value is verified. To do this it has to read the
> > security xattrs to calculate the hash/hmac. How hard would it really
> > be to verify that a security.ima xattr exists, before writing this new
> > EVM signature? How hard would it be to make sure that security.ima is
> > included in the calculation on verification?
>
> I don't think it would be especially hard to ensure that security.ima
> is present if the portable digsig format is used, but as you say it
> would blur the lines a little.
I'd rather err on the side of caution, preventing an unnecessary
possible attack. In this case, I think it is warranted.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-10-19 18:50 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-18 18:01 [PATCH] EVM: Add support for portable signature format Matthew Garrett
2017-10-19 11:02 ` Dmitry Kasatkin
2017-10-19 11:55 ` Mikhail Kurinnoi
2017-10-19 12:00 ` Mimi Zohar
2017-10-19 15:11 ` Dmitry Kasatkin
2017-10-19 17:11 ` Matthew Garrett
2017-10-19 18:02 ` Dmitry Kasatkin
2017-10-19 18:13 ` Dmitry Kasatkin
2017-10-19 18:15 ` Matthew Garrett
2017-10-19 18:50 ` Dmitry Kasatkin
2017-10-19 12:52 ` Mimi Zohar
2017-10-19 17:09 ` Matthew Garrett
2017-10-19 17:44 ` 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).