On Tue, 2015-05-12 at 11:50 +0200, Patrick Ohly wrote:
> On Mon, 2015-05-11 at 17:39 +0200, Zbigniew Jasiński wrote:
> > >      2. private key stored on the device and
> > >              A. booting with evm=fix or
> > >              B. manipulate files only through a process which has
> > >                 exclusive access to the private key
> 
> [...]
> 
> > > For option 2A you wanted to add instructions to the Wiki - any progress 
> > > for
> > > that?
> > > 
> >  
> > I think tizen.org Wiki 'Sign working device' covers this case.
> 
> Okay, so let's talk about that. Which part of that Wiki page describes
> how to teach the kernel about the private key for evm?
> 
> It mentions /etc/ima/x509_evm.der, but that is the public key, isn't it?
> 
> The suggested kernel config has:
> CONFIG_EVM_KEY_PATH="/etc/ima/evm-key"
> CONFIG_EVM_KMK_PATH="/etc/ima/evm-kmk"
> 
> But nothing in the Tizen Wiki explains how to create these files. There
> is https://wiki.tizen.org/wiki/Security:IntegrityMeasurement/Using_TPM
> but it describes completely different files.
> 
> I'll look at the source next.

Here's my analysis, please correct me if I got something wrong.

The x509_evm.der part in the Wiki is about checking checking files
signed with evmctl based on asymmetric keys. That features is
experimental and not yet in upstream Linux.

The traditional approach depends on a different set of keys, as
described in "Using TPM". These are the keys needed when writing to the
file system and evm is enabled, because they are used by the kernel to
calculate the evm HMAC. evmctl cannot pre-calculate these hashes on a
build host; the approach for signing the system is to run with evm=fix
and touching all files.

The problem that I am running into ("evm: init_desc failed") is caused
by only loading x509_evm.der without those other keys. evm_load_keys()
then enables EVM (evm_initialized |= EVM_STATE_X509_SET, evm_enabled =
evm_fixmode ? EVM_STATE_FIX : EVM_STATE_ENABLED).

Normally, when 
evm_inode_init_security() gets called without EVM enabled, it just
returns. But when EVM is enabled with only the X509 cert, it proceeds
and fails because it misses the keys to create the security.evm xattr.
The same also happens basically everywhere where there's an if check for
"evm_enabled".

Is just loading x509_evm.der a valid mode of operation? It seems very
reasonable to me (no need for any confidential keys on the device).

Attached my attempt to make this mode work. It works for me (= can write
files again), but I am concerned that I might have disabled checks too
aggressively (= haven't actually checked appraisal). For example, when a
new file gets created, it has no security.evm, causing
evm_inode_setattr() to reject further changes with EPERM because
evm_verify_current_integrity() fails. Is it really correct to circumvent
that check?

Either way, my original goal was (and is) to create a fully valid
filesystem in advance which (at least in theory) could be booted
read-only; I don't see how that can be done with EVM using the official
upstream source, so I think I'll have to postpone work on EVM and keep
it disabled.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.


diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 3103d64..664c22c 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -297,7 +297,10 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (!evm_enabled || !S_ISREG(inode->i_mode) || evm_enabled == EVM_STATE_FIX)
+	if (!evm_enabled ||
+            !(evm_initialized & EVM_STATE_KEY_SET) ||
+            !S_ISREG(inode->i_mode) ||
+            evm_enabled == EVM_STATE_FIX)
 		return 0;
 	return evm_verify_hmac(dentry, NULL, NULL, 0, NULL);
 }
@@ -405,7 +408,9 @@ int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name)
 void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
 			     const void *xattr_value, size_t xattr_value_len)
 {
-	if (!evm_enabled || (!evm_protected_xattr(xattr_name)
+	if (!evm_enabled ||
+            !(evm_initialized & EVM_STATE_KEY_SET) ||
+            (!evm_protected_xattr(xattr_name)
 				 && !posix_xattr_acl(xattr_name)))
 		return;
 
@@ -426,7 +431,9 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (!evm_enabled || !evm_protected_xattr(xattr_name))
+	if (!evm_enabled ||
+            !(evm_initialized & EVM_STATE_KEY_SET) ||
+            !evm_protected_xattr(xattr_name))
 		return;
 
 	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
@@ -468,7 +475,8 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr)
  */
 void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
 {
-	if (!evm_enabled)
+	if (!evm_enabled ||
+            !(evm_initialized & EVM_STATE_KEY_SET))
 		return;
 
 	if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))
@@ -485,7 +493,9 @@ int evm_inode_init_security(struct inode *inode,
 	struct evm_ima_xattr_data *xattr_data;
 	int rc;
 
-	if (!evm_enabled || !evm_protected_xattr(lsm_xattr->name))
+	if (!evm_enabled ||
+            !(evm_initialized & EVM_STATE_KEY_SET) ||
+            !evm_protected_xattr(lsm_xattr->name))
 		return 0;
 
 	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
_______________________________________________
Dev mailing list
[email protected]
https://lists.tizen.org/listinfo/dev

Reply via email to