[PATCH] EVM: Add support for portable signature format

[PATCH] EVM: Add support for portable signature format

[Matthew Garrett]

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

RE: [PATCH] EVM: Add support for portable signature format

[Dmitry Kasatkin]

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

Re: [PATCH] EVM: Add support for portable signature format

[Mikhail Kurinnoi]

? 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

Re: [PATCH] EVM: Add support for portable signature format

[Mimi Zohar]

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

Re: [PATCH] EVM: Add support for portable signature format

[Mimi Zohar]

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
>  };
> 

RE: [PATCH] EVM: Add support for portable signature format

[Dmitry Kasatkin]

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

Re: [PATCH] EVM: Add support for portable signature format

[Matthew Garrett]

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.

Re: [PATCH] EVM: Add support for portable signature format

[Matthew Garrett]

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.

Re: [PATCH] EVM: Add support for portable signature format

[Mimi Zohar]

> >> 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.

Re: [PATCH] EVM: Add support for portable signature format

[Dmitry Kasatkin]

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

Re: [PATCH] EVM: Add support for portable signature format

[Dmitry Kasatkin]

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

Re: [PATCH] EVM: Add support for portable signature format

[Matthew Garrett]

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.

Re: [PATCH] EVM: Add support for portable signature format

[Dmitry Kasatkin]

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.