linux-ima-user — Q&A for users of IMA

Re: [Linux-ima-user] ima apprise and mkstemp()

[Mikhail K. <vie...@vi...>]

> > I faced with issue, when created by some programs files don't have
> > IMA/EVM sign (that should be), for example - git, a lot of gtk2/3
> > programs, etc.  
> 
> For a file to be labeled properly, the file must be defined in the
> policy.   Normally, the builtin policy ima_appraise_tcb is defined on
> the boot command and then replaced with a custom policy in the
> initramfs.        

Yes, I do exactly in this way. Builtin policy replaced on early boot
with custom policy in the initramfs.


> > 2) FS mounted with iversion flag.
> > 3) kernel 4.7.10, IMA/EVM-related boot options: rootflags=i_version
> > ima_appraise=fix evm=fix  
> 
> The "boot command line options "ima_appraise=fix" and "evm=fix" are
> for fixing a file system missing these xattrs.   Try removing these
> options and re-testing. 

I remove "ima_appraise=fix" and "evm=fix for testing purposes. Here are the results:

1) I have same results for regular file:
# touch /test
# getfattr -m . -d -e hex /test
  getfattr: Removing leading '/' from absolute path names
  # file: test
  security.evm=0x02bf70fed1341366c0d088b1345f2c38c6d2bcae06
  security.ima=0x0406cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
# echo "123" >> /test
# getfattr -m . -d -e hex /test
  getfattr: Removing leading '/' from absolute path names
  # file: test
  security.evm=0x0295ecb4eee8f5bca2d60c6b1864b68c5c4e4988ea
  security.ima=0x0406ea2fe56bb8c1fb5ada84963b42ed71b764a74b092d75755173ade06f2f4aada9c00d6c302e185035cbe85fdff31698bca93e8661f0cbcef52cf2ff65864fd742

No messages in audit syslog.


2) For mkstemp() test results are different:
# /a.out
# getfattr -m . -d -e hex /test-ah2mFC
(!!! still no output here, file don't have any xattrs)
# cat /test-ah2mFC
  cat: /test-ah2mFC: Permission denied
# echo "123" >> /test-ah2mFC
  bash: /test-ah2mFC: Permission denied

Audit syslog messages:
pid=7372 uid=0 auid=1000 ses=3 op="appraise_data" cause="missing-hash" comm="cat" name="/test-ah2mFC" fowner=0 dev="dm-1" ino=18961 res=0
pid=6460 uid=0 auid=1000 ses=3 op="appraise_data" cause="missing-hash" comm="bash" name="/test-ah2mFC" fowner=0 dev="dm-1" ino=18961 res=0



> > First test (create and write into regular file):
> > # touch /test
> >   or
> > # echo "123" > /test
> > # getfattr -m . -d /test  
> 
> When displaying the xattrs, it help to display them in hex ("-e hex"
> option)

Thanks for advice, Mimi.


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] ima apprise and mkstemp()

[Mimi Z. <zo...@li...>]

On Tue, 2016-12-27 at 10:52 +0300, Mikhail Kurinnoi wrote:
> I faced with issue, when created by some programs files don't have
> IMA/EVM sign (that should be), for example - git, a lot of gtk2/3 programs, etc.

For a file to be labeled properly, the file must be defined in the
policy.   Normally, the builtin policy ima_appraise_tcb is defined on
the boot command and then replaced with a custom policy in the
initramfs.        

> Short investigation show me, that all this programs in order to prevent
> data loss, create temporary file first in the same FS with dest file, after that, 
> remove dest file and rename temporary file into dest file name.
> All this programs work with mkstemp().



> Condition:
> 1) IMA policy:
> measure func=FILE_CHECK mask=MAY_READ
> appraise func=FILE_CHECK mask=MAY_READ
> measure func=FILE_CHECK mask=MAY_WRITE
> appraise func=FILE_CHECK mask=MAY_WRITE
> measure func=FILE_CHECK mask=MAY_APPEND
> appraise func=FILE_CHECK mask=MAY_APPEND


> 2) FS mounted with iversion flag.
> 3) kernel 4.7.10, IMA/EVM-related boot options: rootflags=i_version ima_appraise=fix evm=fix

The "boot command line options "ima_appraise=fix" and "evm=fix" are for
fixing a file system missing these xattrs.   Try removing these options
and re-testing. 

> First test (create and write into regular file):
> # touch /test
>   or
> # echo "123" > /test
> # getfattr -m . -d /test

When displaying the xattrs, it help to display them in hex ("-e hex"
option)
.
Mimi

>   getfattr: Removing leading '/' from absolute path names
>   # file: test
>   security.evm=0sAqWCwYz6qoUosD2IAD7s8x3E+G5Q
>   security.ima=0sBAbPg+E1fu+4vfFUKFDWbYAH1iDkBQtXFdyD9Kkh02zpzkfQ0TxdhfKw/4MY0od+7C9juTG9R0F6gaU4Mnr5J9o+
> # echo "123" >> /test
> # getfattr -m . -d /test
>   getfattr: Removing leading '/' from absolute path names
>   # file: test
>   security.evm=0sAqB8MIEQSvsB+xs1MalIXkD2r3fq
>   security.ima=0sBAbqL+VruMH7WtqEljtC7XG3ZKdLCS11dVFzreBvL0qtqcANbDAuGFA1y+hf3/MWmLypPoZh8MvO9Szy/2WGT9dC
> 
> As you can see - everything working. No messages in audit syslog.

[Linux-ima-user] ima apprise and mkstemp()

[Mikhail K. <vie...@vi...>]

I faced with issue, when created by some programs files don't have
IMA/EVM sign (that should be), for example - git, a lot of gtk2/3 programs, etc.
Short investigation show me, that all this programs in order to prevent
data loss, create temporary file first in the same FS with dest file, after that, 
remove dest file and rename temporary file into dest file name.
All this programs work with mkstemp().

Condition:
1) IMA policy:
measure func=FILE_CHECK mask=MAY_READ
appraise func=FILE_CHECK mask=MAY_READ
measure func=FILE_CHECK mask=MAY_WRITE
appraise func=FILE_CHECK mask=MAY_WRITE
measure func=FILE_CHECK mask=MAY_APPEND
appraise func=FILE_CHECK mask=MAY_APPEND
2) FS mounted with iversion flag.
3) kernel 4.7.10, IMA/EVM-related boot options: rootflags=i_version ima_appraise=fix evm=fix

First test (create and write into regular file):
# touch /test
  or
# echo "123" > /test
# getfattr -m . -d /test
  getfattr: Removing leading '/' from absolute path names
  # file: test
  security.evm=0sAqWCwYz6qoUosD2IAD7s8x3E+G5Q
  security.ima=0sBAbPg+E1fu+4vfFUKFDWbYAH1iDkBQtXFdyD9Kkh02zpzkfQ0TxdhfKw/4MY0od+7C9juTG9R0F6gaU4Mnr5J9o+
# echo "123" >> /test
# getfattr -m . -d /test
  getfattr: Removing leading '/' from absolute path names
  # file: test
  security.evm=0sAqB8MIEQSvsB+xs1MalIXkD2r3fq
  security.ima=0sBAbqL+VruMH7WtqEljtC7XG3ZKdLCS11dVFzreBvL0qtqcANbDAuGFA1y+hf3/MWmLypPoZh8MvO9Szy/2WGT9dC

As you can see - everything working. No messages in audit syslog.

I write simple program to test this issue (/test.cpp):
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>

int main () {
	int test_fd = -1;
	char *tmpname;
	
	if (asprintf(&tmpname, "%s-XXXXXX", "test")<0) {
		printf("asprintf");
		exit(1);
	}
	
	test_fd = mkstemp(tmpname);
	
	if (test_fd == -1) {
		printf("mkstemp\n");
		exit(1);
	}
	
	write(test_fd, "test 123 test\n", sizeof("test 123 test\n"));
	
	return 0;
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# g++ /test.cpp
# /a.out
# ls
  a.out  dev   lib    lost+found  opt   run   test         tmp
  bin    etc   lib32  media       proc  sbin  test.cpp     usr
  boot   home  lib64  mnt         root  sys   test-yPVjX9  var
# getfattr -m . -d /test-yPVjX9
(!!!) no output here, file don't have any xattrs
# cat /test-yPVjX9
  test 123 test
# getfattr -m . -d /test-yPVjX9
  getfattr: Removing leading '/' from absolute path names
  # file: test-yPVjX9
  security.evm=0sAiPz9jIP/WWLMFYT5C/pG8pAl/8T
  security.ima=0sBAa/SpRDsU2c2pwqoIUyiVWqUZmpZ83lJzUHNwwPRD4/YnfwIRC3wlrmpzZLYs3l98Y+cI0mbJd/dngau8SQ6hX+

Audit syslog message:
pid=41020 uid=0 auid=1000 ses=3 op="appraise_data" cause="missing-hash" comm="cat" name="/test-yPVjX9" fowner=0 dev="dm-1" ino=18962 res=0

By some reason, with mkstemp, xattrs was added only on first read access.
Could you please help?

-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: provide portable EVM signature version.

[Mikhail K. <vie...@vi...>]

Hi, Mimi

> On Sat, 2016-12-24 at 01:59 +0300, Mikhail Kurinnoi wrote:
> > I am not sure, if portable EVM signature version is still in
> > discussion or not, but, in case of someone interested in this
> > feature too, I propose to discuss patch that I am using. This patch
> > are used for custom kernels in order to provide initial EVM signed
> > files in packages from package build server to desktop PCs.  
> 
> A portable EVM signature, which can be included in an archive, is
> important.   There were good reasons for including file system
> specific information in the HMAC calculation.  By removing these
> fields,  the new format does not provide the same security guarantees
> as the existing format.  
> 
> Instead of converting the EVM signature to an HMAC on first access,  I
> would prefer that the new format never be written out to the file
> system, but converted to an HMAC after verification in
> evm_inode_post_setxattr().  This would provide the benefits of a
> portable EVM format, without loosing the existing security guarantees.

Yes, I think, we can use additional verification in
evm_inode_post_setxattr() since we have xattr_value and could use it
directly in evm_calc_hash() and integrity_digsig_verify(). In the same
time, all work with EVM portable signature version must be prohibited in
evm_verify_hmac() in order to prevent any work with it for sure.

I will think more about this. I hope, Dmitry will take a part in this
thread discussion, probably, he have some ideas too.

-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evm: provide portable EVM signature version.

[Mimi Z. <zo...@li...>]

[cc'ing Dmitry Kasatkin]

Hi Mikhail,

On Sat, 2016-12-24 at 01:59 +0300, Mikhail Kurinnoi wrote:
> I am not sure, if portable EVM signature version is still in
> discussion or not, but, in case of someone interested in this feature
> too, I propose to discuss patch that I am using. This patch are used
> for custom kernels in order to provide initial EVM signed files in
> packages from package build server to desktop PCs.

A portable EVM signature, which can be included in an archive, is
important.   There were good reasons for including file system specific
information in the HMAC calculation.  By removing these fields,  the new
format does not provide the same security guarantees as the existing
format.  

Instead of converting the EVM signature to an HMAC on first access,  I
would prefer that the new format never be written out to the file
system, but converted to an HMAC after verification in
evm_inode_post_setxattr().  This would provide the benefits of a
portable EVM format, without loosing the existing security guarantees.

Mimi

[Linux-ima-user] [PATCH] evm: provide portable EVM signature version.

[Mikhail K. <vie...@vi...>]

I am not sure, if portable EVM signature version is still in discussion or not, but, in case of someone interested in this feature too, I propose to discuss patch that I am using. This patch are used for custom kernels in order to provide initial EVM signed files in packages from package build server to desktop PCs.

This patch add portable EVM signature version support (can be tested on files signed by evmctl util with "-i" flag). This patch based on previous Dmitry's patch (https://sourceforge.net/p/linux-ima/mailman/message/32987311/) cleaned and revised in order to provide only portable EVM signature version support and nothing more.


Signed-off-by: Dmitry Kasatkin <d.k...@sa...>
Signed-off-by: Mikhail Kurinnoi <vie...@vi...>


--- a/security/integrity/digsig.c	2016-07-24 22:23:50.000000000 +0300
+++ b/security/integrity/digsig.c	2016-12-22 21:18:16.504929652 +0300
@@ -71,6 +71,7 @@ int integrity_digsig_verify(const unsign
 		return digsig_verify(keyring[id], sig + 1, siglen - 1,
 				     digest, digestlen);
 	case 2:
+	case 3:
 		return asymmetric_verify(keyring[id], sig, siglen,
 					 digest, digestlen);
 	}
--- a/security/integrity/evm/evm.h	2016-07-24 22:23:50.000000000 +0300
+++ b/security/integrity/evm/evm.h	2016-12-22 21:19:02.225835301 +0300
@@ -48,7 +48,7 @@ int evm_calc_hmac(struct dentry *dentry,
 		  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, int version, char *digest);
 int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
 		  char *hmac_val);
 int evm_init_secfs(void);
--- a/security/integrity/evm/evm_crypto.c	2016-07-24 22:23:50.000000000 +0300
+++ b/security/integrity/evm/evm_crypto.c	2016-12-22 21:20:36.667704195 +0300
@@ -161,6 +161,23 @@ static void hmac_add_misc(struct shash_d
 	crypto_shash_final(desc, digest);
 }
 
+static void hmac_add_misc_digsig(struct shash_desc *desc, struct inode *inode,
+				 char *digest)
+{
+	struct h_misc {
+		uid_t uid;
+		gid_t gid;
+		umode_t mode;
+	} hmac_misc;
+
+	memset(&hmac_misc, 0, sizeof(hmac_misc));
+	hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
+	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));
+	crypto_shash_final(desc, digest);
+}
+
 /*
  * Calculate the HMAC value across the set of protected security xattrs.
  *
@@ -210,7 +227,10 @@ static int evm_calc_hmac_or_hash(struct
 		xattr_size = size;
 		crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
 	}
-	hmac_add_misc(desc, inode, digest);
+	if (type == EVM_IMA_XATTR_DIGSIG)
+		hmac_add_misc_digsig(desc, inode, digest);
+	else
+		hmac_add_misc(desc, inode, digest);
 
 out:
 	kfree(xattr_value);
@@ -228,10 +248,12 @@ int evm_calc_hmac(struct dentry *dentry,
 
 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)
+		  int version, 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,
+				version == 3 ? EVM_IMA_XATTR_DIGSIG : IMA_XATTR_DIGEST,
+				digest);
 }
 
 /*
--- a/security/integrity/evm/evm_main.c	2016-12-22 21:16:15.159523000 +0300
+++ b/security/integrity/evm/evm_main.c	2016-12-22 21:21:42.445004576 +0300
@@ -120,7 +120,7 @@ static enum integrity_status evm_verify_
 	struct evm_ima_xattr_data *xattr_data = NULL;
 	struct evm_ima_xattr_data calc;
 	enum integrity_status evm_status = INTEGRITY_PASS;
-	int rc, xattr_len;
+	int rc, xattr_len, version;
 
 	if (iint && iint->evm_status == INTEGRITY_PASS)
 		return iint->evm_status;
@@ -159,8 +159,9 @@ static enum integrity_status evm_verify_
 			rc = -EINVAL;
 		break;
 	case EVM_IMA_XATTR_DIGSIG:
+		version = ((const char *)xattr_data)[1];
 		rc = evm_calc_hash(dentry, xattr_name, xattr_value,
-				xattr_value_len, calc.digest);
+				xattr_value_len, version, calc.digest);
 		if (rc)
 			break;
 		rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM,

Re: [Linux-ima-user] [PATCH] evmctl verify: immutable EVM sign verification support, ignore -i flag.

[Mikhail K. <vie...@vi...>]

В Wed, 21 Dec 2016 20:37:02 -0500
Mimi Zohar <zo...@li...> пишет:

> On Tue, 2016-12-20 at 01:58 +0300, Mikhail Kurinnoi wrote:
> > This patch add verification support for immutable EVM sign, ignore
> > -i flag during EVM sign verification.
> > 
> >  - verify_hash function (/src/libimaevm.c) parse DIGSIG_VERSION_3 in
> > same way as DIGSIG_VERSION_2, since version 3 on this stage should
> > use same code as version 2.
> > 
> >  - verify_evm function (/src/evmctl.c) care about "evm_immutable"
> > internal flag to be sure, that "-i" flag is ignored and hash will be
> > generated accordingly to the file EVM sign version. Please note, I
> > don't use in this source file DIGSIG_VERSION_3 from "enum
> > digsig_version", since Dmitry Kasatkin used "3" for some reason in
> > his patch (see
> > https://sourceforge.net/p/linux-ima/ima-evm-utils/ci/92033dc4042668aaec2df45aa864edc705bd607b/).
> > 
> >  - this patch fix issue in EVM sign verification, when flag "-i" is
> > provided by mistake during EVM sign version 1 or version 2
> > verification (in this case hash will be generated for version 3
> > accordingly to provided flags, but not to the file EVM sign version
> > as it should be, so, verification will be failed for sure).  
> 
> Dmitry started working on a portable EVM signature version, that could
> be included in archives. Kernel support for the "new" format has not
> been upstreamed.
> 
> Mimi

I use revised Dmitry's patch for kernel in order to use IMA/EVM
feature with stand alone package build server and desktop PCs.

Do you plan remove this feature from emctl util, since this feature was
not been upstreamed into kernel, or this work still in progress?

-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] [PATCH] evmctl verify: immutable EVM sign verification support, ignore -i flag.

[Mimi Z. <zo...@li...>]

On Tue, 2016-12-20 at 01:58 +0300, Mikhail Kurinnoi wrote:
> This patch add verification support for immutable EVM sign, ignore -i
> flag during EVM sign verification.
> 
>  - verify_hash function (/src/libimaevm.c) parse DIGSIG_VERSION_3 in
> same way as DIGSIG_VERSION_2, since version 3 on this stage should use
> same code as version 2.
> 
>  - verify_evm function (/src/evmctl.c) care about "evm_immutable"
> internal flag to be sure, that "-i" flag is ignored and hash will be
> generated accordingly to the file EVM sign version. Please note, I
> don't use in this source file DIGSIG_VERSION_3 from "enum
> digsig_version", since Dmitry Kasatkin used "3" for some reason in his
> patch (see
> https://sourceforge.net/p/linux-ima/ima-evm-utils/ci/92033dc4042668aaec2df45aa864edc705bd607b/).
> 
>  - this patch fix issue in EVM sign verification, when flag "-i" is
> provided by mistake during EVM sign version 1 or version 2
> verification (in this case hash will be generated for version 3
> accordingly to provided flags, but not to the file EVM sign version as
> it should be, so, verification will be failed for sure).

Dmitry started working on a portable EVM signature version, that could
be included in archives.  Kernel support for the "new" format has not
been upstreamed.

Mimi
> 
> Signed-off-by: Mikhail Kurinnoi <vie...@vi...>
> 
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -730,24 +730,29 @@ static int verify_evm(const char *file)
>  {
>  	unsigned char hash[20];
>  	unsigned char sig[1024];
> -	int len;
> -
> -	len = calc_evm_hash(file, hash);
> -	if (len <= 1)
> -		return len;
> -
> -	len = lgetxattr(file, "security.evm", sig, sizeof(sig));
> -	if (len < 0) {
> +	int sig_len, hash_len;
> +
> +	sig_len = lgetxattr(file, "security.evm", sig, sizeof(sig));
> +	if (sig_len < 0) {
>  		log_err("getxattr failed: %s\n", file);
> -		return len;
> +		return sig_len;
>  	}
> 
>  	if (sig[0] != 0x03) {
>  		log_err("security.evm has no signature\n");
>  		return -1;
>  	}
> -
> -	return verify_hash(hash, sizeof(hash), sig + 1, len - 1);
> +	
> +	if (sig[1] == 3)
> +		evm_immutable = true;
> +	else
> +		evm_immutable = false;
> +	
> +	hash_len = calc_evm_hash(file, hash);
> +	if (hash_len <= 1)
> +		return hash_len;
> +
> +	return verify_hash(hash, sizeof(hash), sig + 1, sig_len - 1);
>  }
> 
>  static int cmd_verify_evm(struct command *cmd)
> --- a/src/imaevm.h
> +++ b/src/imaevm.h
> @@ -127,7 +127,8 @@
> 
>  enum digsig_version {
>  	DIGSIG_VERSION_1 = 1,
> -	DIGSIG_VERSION_2
> +	DIGSIG_VERSION_2,
> +	DIGSIG_VERSION_3
>  };
> 
>  struct pubkey_hdr {
> 
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -502,7 +502,7 @@ int verify_hash(const unsigned char *hash, int size, unsigned char *sig, int siglen)
>  		verify_hash = verify_hash_v1;
>  		/* Read pubkey from RSA key */
>  		x509 = 0;
> -	} else if (sig[0] == DIGSIG_VERSION_2) {
> +	} else if ((sig[0] == DIGSIG_VERSION_2) || (sig[0] == DIGSIG_VERSION_3)) {
>  		verify_hash = verify_hash_v2;
>  		/* Read pubkey from x509 cert */
>  		x509 = 1;
> 
> ------------------------------------------------------------------------------
> Developer Access Program for Intel Xeon Phi Processors
> Access to Intel Xeon Phi processor-based developer platforms.
> With one year of Intel Parallel Studio XE.
> Training and support from Colfax.
> Order your platform today.http://sdm.link/intel
> _______________________________________________
> Linux-ima-user mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-user
> 

[Linux-ima-user] [PATCH] evmctl verify: immutable EVM sign verification support, ignore -i flag.

[Mikhail K. <vie...@vi...>]

This patch add verification support for immutable EVM sign, ignore -i flag during EVM sign verification.

 - verify_hash function (/src/libimaevm.c) parse DIGSIG_VERSION_3 in same way as DIGSIG_VERSION_2, since version 3 on this stage should use same code as version 2.

 - verify_evm function (/src/evmctl.c) care about "evm_immutable" internal flag to be sure, that "-i" flag is ignored and hash will be generated accordingly to the file EVM sign version. Please note, I don't use in this source file DIGSIG_VERSION_3 from "enum digsig_version", since Dmitry Kasatkin used "3" for some reason in his patch (see https://sourceforge.net/p/linux-ima/ima-evm-utils/ci/92033dc4042668aaec2df45aa864edc705bd607b/).

 - this patch fix issue in EVM sign verification, when flag "-i" is provided by mistake during EVM sign version 1 or version 2 verification (in this case hash will be generated for version 3 accordingly to provided flags, but not to the file EVM sign version as it should be, so, verification will be failed for sure).


Signed-off-by: Mikhail Kurinnoi <vie...@vi...>

--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -730,24 +730,29 @@ static int verify_evm(const char *file)
 {
 	unsigned char hash[20];
 	unsigned char sig[1024];
-	int len;
-
-	len = calc_evm_hash(file, hash);
-	if (len <= 1)
-		return len;
-
-	len = lgetxattr(file, "security.evm", sig, sizeof(sig));
-	if (len < 0) {
+	int sig_len, hash_len;
+
+	sig_len = lgetxattr(file, "security.evm", sig, sizeof(sig));
+	if (sig_len < 0) {
 		log_err("getxattr failed: %s\n", file);
-		return len;
+		return sig_len;
 	}
 
 	if (sig[0] != 0x03) {
 		log_err("security.evm has no signature\n");
 		return -1;
 	}
-
-	return verify_hash(hash, sizeof(hash), sig + 1, len - 1);
+	
+	if (sig[1] == 3)
+		evm_immutable = true;
+	else
+		evm_immutable = false;
+	
+	hash_len = calc_evm_hash(file, hash);
+	if (hash_len <= 1)
+		return hash_len;
+
+	return verify_hash(hash, sizeof(hash), sig + 1, sig_len - 1);
 }
 
 static int cmd_verify_evm(struct command *cmd)
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -127,7 +127,8 @@
 
 enum digsig_version {
 	DIGSIG_VERSION_1 = 1,
-	DIGSIG_VERSION_2
+	DIGSIG_VERSION_2,
+	DIGSIG_VERSION_3
 };
 
 struct pubkey_hdr {

--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -502,7 +502,7 @@ int verify_hash(const unsigned char *hash, int size, unsigned char *sig, int siglen)
 		verify_hash = verify_hash_v1;
 		/* Read pubkey from RSA key */
 		x509 = 0;
-	} else if (sig[0] == DIGSIG_VERSION_2) {
+	} else if ((sig[0] == DIGSIG_VERSION_2) || (sig[0] == DIGSIG_VERSION_3)) {
 		verify_hash = verify_hash_v2;
 		/* Read pubkey from x509 cert */
 		x509 = 1;

Re: [Linux-ima-user] [Acl-devel] [PATCH 2/2] Suppress error messages when copying security.ima fails

[Mimi Z. <zo...@li...>]

cc'ing Patrick Ohly, linux-ima-user

On Fri, 2016-12-09 at 17:31 -0500, Stefan Berger wrote:
> On 12/09/2016 04:58 PM, Mike Frysinger wrote:
> > On 09 Dec 2016 16:14, Stefan Berger wrote:
> >> On 12/09/2016 04:02 PM, Mike Frysinger wrote:
> >>> On 09 Dec 2016 15:18, Stefan Berger wrote:
> >>>> On 12/09/2016 02:40 PM, Mike Frysinger wrote:
> >>>>> On 25 Oct 2016 13:36, Stefan Berger wrote:
> >>>>>> The security.ima extended attribute may be copied when it contains
> >>>>>> a digital signature. In case it is a hash, the copying will fail
> >>>>>> and we suppress the error message in that case.
> >>>>> i'm not sure hardcoding specific attributes in the C code like this
> >>>>> is a good idea.  can't we leverage the existing conf file ?
> >>>> Should we add an option to not display an error? Like 'quiet' ?
> >>> that's already possible by not passing in an error context.
> >>> but that's not what i meant.  we already have xattr.conf that
> >>> explicitly lists attributes and whether we should skip them.
> >>> can't we leverage that database in these files and have it
> >>> (silently) skip attributes when they're listed as "skip" ?
> >> The security.ima extended attribute can either be a hash or a signature.
> >> In case of a signature, we want it to be copied, in case of a hash we
> >> don't want to show the error messages appearing when the copying failed.
> > i haven't been following the ima work closely.  but if the xattr is just
> > a hash of the content, why would copying it be rejected by the kernel ?
> 
> I believe it was a recently extension that prevents userspace from 
> writing the hash value. Mimi can probably say more about this.

As part of Dmity Kasatkin's work on locking down IMA, we disabled
userspace from writing a file hash (c68ed80c97d9 "ima: limit file hash
setting by user to fix and log modes"), since the kernel automatically
calculates and writes the file hash as an extended attribute for files
in policy.  Only writing file signatures is permitted.

Mainly because of these unnecessary error messages, which make it
difficult to notice real errors,  Patrick Ohly requested we revert
Dmitry's patch.  Reluctantly I agreed to revert it (f5acb3dcba1f), which
is being upstreamed in this open window.  The better solution would be
to suppress these messages (and fix the other issues).

Mimi

Re: [Linux-ima-user] integrity: Add fowner field into integrity_audit_msg output.

[Mimi Z. <zo...@li...>]

On Fri, 2016-12-16 at 19:36 +0300, Mikhail Kurinnoi wrote:
> Hi Mimi,
> 
> > As long as we're making changes, are there any other
> > changes needed?   Perhaps an indication as to whether the audit info
> > is from the init_user_ns?
> 
> I was need only "fowner" in my work as additional info, since I found
> all I needed in audit message was already implemented.

That's great!  Before making this change,  I was hoping others would
join this discussion as well.

Mimi

> > Samples of format changes need to be sent to the audit mailing as
> > well.
> 
> audit output will look like this (output from my /var/log/audit.log):
> 
> Dec 15 15:10:27 totoro kernel: [12912.070841] audit: type=1800
> audit(1481803827.825:805): pid=42788 uid=0 auid=1000 ses=3
> op="appraise_data" cause="invalid-HMAC" comm="thunar"
> name="/usr/lib64/qt5/mkspecs/qconfig.pri" fowner=0 dev="dm-1"
> ino=925675 res=0
> Dec 16 17:16:41 totoro kernel: [14605.321665] audit: type=1800
> audit(1481897801.780:19459): pid=53087 uid=0 auid=1000 ses=3
> op="appraise_data" cause="invalid-signature" comm="evmctl"
> name="/etc/logcheck/ignore.d.workstation/local_rules" fowner=106
> dev="dm-1" ino=264181 res=0

Re: [Linux-ima-user] Single file measurement

[Mimi Z. <zo...@li...>]

On Tue, 2016-11-29 at 09:50 +0100, Svart Kanin wrote:
> Thank you,
> I enabled i_version on all mounted filesystems, nevertheless when changing
> measured files, these changes do not seem to be detected!?
>   - Open the file once -> it's added to the measurement list
>   - Open the file again and make some changes, but the same measurement is
> still there
> Shouldn't this be recognized now by IMA since i_version is enabled?

Unfortunately the "fstab" and "mount" options are different.  For
"fstab" it is iversion, while for "mount" it is i_version.  Please make
sure that you used the correct version.

Mimi

Re: [Linux-ima-user] integrity: Add fowner field into integrity_audit_msg output.

[Mikhail K. <vie...@vi...>]

Hi Mimi,

> As long as we're making changes, are there any other
> changes needed?   Perhaps an indication as to whether the audit info
> is from the init_user_ns?

I was need only "fowner" in my work as additional info, since I found
all I needed in audit message was already implemented.

> Samples of format changes need to be sent to the audit mailing as
> well.

audit output will look like this (output from my /var/log/audit.log):

Dec 15 15:10:27 totoro kernel: [12912.070841] audit: type=1800 audit(1481803827.825:805): pid=42788 uid=0 auid=1000 ses=3 op="appraise_data" cause="invalid-HMAC" comm="thunar" name="/usr/lib64/qt5/mkspecs/qconfig.pri" fowner=0 dev="dm-1" ino=925675 res=0
Dec 16 17:16:41 totoro kernel: [14605.321665] audit: type=1800 audit(1481897801.780:19459): pid=53087 uid=0 auid=1000 ses=3 op="appraise_data" cause="invalid-signature" comm="evmctl" name="/etc/logcheck/ignore.d.workstation/local_rules" fowner=106 dev="dm-1" ino=264181 res=0


-- 
Best regards,
Mikhail Kurinnoi

Re: [Linux-ima-user] integrity: Add fowner field into integrity_audit_msg output.

[Mimi Z. <zo...@li...>]

Hi Mikhail,

On Mon, 2016-12-12 at 19:50 +0300, Mikhail Kurinnoi wrote:
> since we have "fowner=" option, is it possible add fowner into
> integrity_audit_msg output too? I found this useful in my case.

I don't have a problem with including additional audit information in
the logs.  As long as we're making changes, are there any other changes
needed?   Perhaps an indication as to whether the audit info is from the
init_user_ns?

Samples of format changes need to be sent to the audit mailing as well.

Mimi

> I mean something like this:
> 
> 
> --- a/security/integrity/integrity_audit.c.orig	2016-12-12 19:41:45.885938794 +0300
> +++ b/security/integrity/integrity_audit.c	2016-12-03 17:29:47.104503180 +0300
> @@ -56,6 +56,7 @@ void integrity_audit_msg(int audit_msgno
>  		audit_log_untrustedstring(ab, fname);
>  	}
>  	if (inode) {
> +		audit_log_format(ab, " fowner=%u", __kuid_val(inode->i_uid));
>  		audit_log_format(ab, " dev=");
>  		audit_log_untrustedstring(ab, inode->i_sb->s_id);
>  		audit_log_format(ab, " ino=%lu", inode->i_ino);
> 
> 

[Linux-ima-user] integrity: Add fowner field into integrity_audit_msg output.

[Mikhail K. <vie...@vi...>]

since we have "fowner=" option, is it possible add fowner into
integrity_audit_msg output too? I found this useful in my case.
I mean something like this:


--- a/security/integrity/integrity_audit.c.orig	2016-12-12 19:41:45.885938794 +0300
+++ b/security/integrity/integrity_audit.c	2016-12-03 17:29:47.104503180 +0300
@@ -56,6 +56,7 @@ void integrity_audit_msg(int audit_msgno
 		audit_log_untrustedstring(ab, fname);
 	}
 	if (inode) {
+		audit_log_format(ab, " fowner=%u", __kuid_val(inode->i_uid));
 		audit_log_format(ab, " dev=");
 		audit_log_untrustedstring(ab, inode->i_sb->s_id);
 		audit_log_format(ab, " ino=%lu", inode->i_ino);


-- 
Best regards,
Mikhail Kurinnoi

[Linux-ima-user] docker container fails to execute in enforce mode with overlayfs

[Magalhaes, G. (B. R&D-CL) <gui...@hp...>]

Hi.
I am using kernel version 4.8.8 and I see the permission error below when using IMA in 'enforce' mode. I am trying to instantiate a new docker container (version 1.12.1) and every time the '.dockerenv' file is created without the security.ima xattr and then a openat syscall (whith O_CREAT) fails with 'permission denied'. The error is not reproducible if I turn IMA off. I am using the TCB policy.


# docker run alpine sh
docker : Error response from daemon: open /var/lib/docker/overlay/<id>-init/upper/.dockerenv permission denied.


This is the related dmesg line:
[  928.351976] audit: type=1800 audit(1480955917.279:5): pid=3661 uid=0 auid=4294967295 ses=4294967295 op="appraise_data" cause="missing-hash" comm="dockerd" name="/var/lib/docker/overlay/d69486a52aca472d5bc06ef47dec48460fbca4c2d4e07cde71e5e5759a7db251-init/upper/.dockerenv" dev="sda1" ino=26359 res=0


The 'dockerenv' hidden file is created by Docker during the container initialization along with other /etc and /dev files. All these files are created, but the required xattr is missing.

The Docker daemon is using overlayfs as the storage driver. If I use another storage driver the container is instantiated correctly. Actually I used AUFS as the alternative storage driver to test.

I could found a similar error with Docker containers in recent kernel versions. Although it is not related to IMA, it might be helpful:
https://github.com/docker/docker/issues/20950

Is it a known issue?
Thanks.

--
Guilherme

[Linux-ima-user] [PATCH v2 2/2] xattrs: Properly handle security.ima extended attribute

[Stefan B. <st...@li...>]

This patch addresses the proper handling of the security.ima
extended attribute in the following two cases:

- The security.ima extended attribute is not writeable if its value
represents a hash, since hash values are only writeable by the kernel.
We therefore ignore errors when security.ima could not be written.

- Similarly, when the kernel creates a security.ima extended
attribute with a hash value for a new file, we don't want to erase
the security.ima xattr (erasing is possible).
---
 xattrs.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/xattrs.c b/xattrs.c
index 3b72e61..64fc84a 100644
--- a/xattrs.c
+++ b/xattrs.c
@@ -1024,10 +1024,16 @@ static int rsync_xal_set(const char *fname, item_list *xalp,
 		}
 
 		if (sys_lsetxattr(fname, name, rxas[i].datum, rxas[i].datum_len) < 0) {
-			rsyserr(FERROR_XFER, errno,
-				"rsync_xal_set: lsetxattr(\"%s\",\"%s\") failed",
-				full_fname(fname), name);
-			ret = -1;
+			if (!strcmp(name, "security.ima")) {
+				/* security.ima may not be writeable
+				 * if it's a hash -- skip error output
+				 */
+			} else {
+				rsyserr(FERROR_XFER, errno,
+					"rsync_xal_set: lsetxattr(\"%s\",\"%s\") failed",
+					full_fname(fname), name);
+				ret = -1;
+			}
 		} else /* make sure caller sets mtime */
 			sxp->st.st_mtime = (time_t)-1;
 	}
@@ -1044,7 +1050,8 @@ static int rsync_xal_set(const char *fname, item_list *xalp,
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
 
-		if (!strcmp(name, "security.evm"))
+		if (!strcmp(name, "security.evm") ||
+		    !strcmp(name, "security.ima"))
 			continue;
 #endif
 		if (am_root < 0 && name_len > RPRE_LEN
-- 
2.7.4

[Linux-ima-user] [PATCH v2 1/2] xattrs: Skip security.evm extended attribute

[Stefan B. <st...@li...>]

The security.evm extended attribute is fully owned by the Linux kernel
and cannot be directly written from userspace. Therefore, we can always
skip it.
---
 xattrs.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/xattrs.c b/xattrs.c
index b105392..3b72e61 100644
--- a/xattrs.c
+++ b/xattrs.c
@@ -255,6 +255,9 @@ static int rsync_xal_get(const char *fname, item_list *xalp)
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 
 		/* No rsync.%FOO attributes are copied w/o 2 -X options. */
@@ -358,6 +361,9 @@ int copy_xattrs(const char *source, const char *dest)
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 
 		datum_len = 0;
@@ -828,7 +834,9 @@ void receive_xattr(int f, struct file_struct *file)
 		}
 #ifdef HAVE_LINUX_XATTRS
 		/* Non-root can only save the user namespace. */
-		if (am_root <= 0 && !HAS_PREFIX(name, USER_PREFIX)) {
+		if (am_root <= 0 &&
+		    (!HAS_PREFIX(name, USER_PREFIX) ||
+		     !strcmp(name, "security.evm"))) {
 			if (!am_root) {
 				free(ptr);
 				continue;
@@ -962,6 +970,11 @@ static int rsync_xal_set(const char *fname, item_list *xalp,
 	for (i = 0; i < xalp->count; i++) {
 		name = rxas[i].name;
 
+#ifdef HAVE_LINUX_XATTRS
+		if (!strcmp(name, "security.evm"))
+			continue;
+#endif
+
 		if (XATTR_ABBREV(rxas[i])) {
 			/* See if the fnamecmp version is identical. */
 			len = name_len = rxas[i].name_len;
@@ -1030,6 +1043,9 @@ static int rsync_xal_set(const char *fname, item_list *xalp,
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 		if (am_root < 0 && name_len > RPRE_LEN
 		 && name[RPRE_LEN] == '%' && strcmp(name, XSTAT_ATTR) == 0)
-- 
2.7.4

[Linux-ima-user] [PATCH 2/2] xattrs: Properly handle security.ima extended attribute

[Stefan B. <st...@li...>]

This patch addresses the proper handling of the security.ima
extended attribute in the following two cases:

- The security.ima extended attribute is not writeable if its value
represents a hash, since hash values are only writeable by the kernel.
We therefore ignore errors when security.ima could not be written.

- Similarly, when the kernel creates a security.ima extended
attribute with a hash value for a new file, we don't want to erase
the security.ima xattr (erasing is possible).
---
 xattrs.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/xattrs.c b/xattrs.c
index 3b72e61..64fc84a 100644
--- a/xattrs.c
+++ b/xattrs.c
@@ -1024,10 +1024,16 @@ static int rsync_xal_set(const char *fname, item_list *xalp,
 		}
 
 		if (sys_lsetxattr(fname, name, rxas[i].datum, rxas[i].datum_len) < 0) {
-			rsyserr(FERROR_XFER, errno,
-				"rsync_xal_set: lsetxattr(\"%s\",\"%s\") failed",
-				full_fname(fname), name);
-			ret = -1;
+			if (!strcmp(name, "security.ima")) {
+				/* security.ima may not be writeable
+				 * if it's a hash -- skip error output
+				 */
+			} else {
+				rsyserr(FERROR_XFER, errno,
+					"rsync_xal_set: lsetxattr(\"%s\",\"%s\") failed",
+					full_fname(fname), name);
+				ret = -1;
+			}
 		} else /* make sure caller sets mtime */
 			sxp->st.st_mtime = (time_t)-1;
 	}
@@ -1044,7 +1050,8 @@ static int rsync_xal_set(const char *fname, item_list *xalp,
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
 
-		if (!strcmp(name, "security.evm"))
+		if (!strcmp(name, "security.evm") ||
+		    !strcmp(name, "security.ima"))
 			continue;
 #endif
 		if (am_root < 0 && name_len > RPRE_LEN
-- 
2.7.4

[Linux-ima-user] [PATCH 1/2] xattrs: Skip security.evm extended attribute

[Stefan B. <st...@li...>]

The security.evm extende attribute is fully owned by the Linux kernel
and cannot be written directly written from userspace. Therefore,
we can always skip it.
---
 xattrs.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/xattrs.c b/xattrs.c
index b105392..3b72e61 100644
--- a/xattrs.c
+++ b/xattrs.c
@@ -255,6 +255,9 @@ static int rsync_xal_get(const char *fname, item_list *xalp)
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 
 		/* No rsync.%FOO attributes are copied w/o 2 -X options. */
@@ -358,6 +361,9 @@ int copy_xattrs(const char *source, const char *dest)
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 
 		datum_len = 0;
@@ -828,7 +834,9 @@ void receive_xattr(int f, struct file_struct *file)
 		}
 #ifdef HAVE_LINUX_XATTRS
 		/* Non-root can only save the user namespace. */
-		if (am_root <= 0 && !HAS_PREFIX(name, USER_PREFIX)) {
+		if (am_root <= 0 &&
+		    (!HAS_PREFIX(name, USER_PREFIX) ||
+		     !strcmp(name, "security.evm"))) {
 			if (!am_root) {
 				free(ptr);
 				continue;
@@ -962,6 +970,11 @@ static int rsync_xal_set(const char *fname, item_list *xalp,
 	for (i = 0; i < xalp->count; i++) {
 		name = rxas[i].name;
 
+#ifdef HAVE_LINUX_XATTRS
+		if (!strcmp(name, "security.evm"))
+			continue;
+#endif
+
 		if (XATTR_ABBREV(rxas[i])) {
 			/* See if the fnamecmp version is identical. */
 			len = name_len = rxas[i].name_len;
@@ -1030,6 +1043,9 @@ static int rsync_xal_set(const char *fname, item_list *xalp,
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 		if (am_root < 0 && name_len > RPRE_LEN
 		 && name[RPRE_LEN] == '%' && strcmp(name, XSTAT_ATTR) == 0)
-- 
2.7.4

Re: [Linux-ima-user] [PATCH v3] 98integrity: Use /etc/ima as dir for IMA policy and config file

[Stefan B. <st...@li...>]

Harald,

   please do not apply this patch.

     Stefan
>
> >
> > From: Stefan Berger <st...@us...>
> >
> > To sync with systemd, use the filepath /etc/ima/ima-policy as
> > the default file location for the IMA policy. At the same time we
> > move the ima config file location to /etc/ima/ima. Adapt the
> > documentation to the new path. Maintain backwards compatibility
> > by still reading the old files in case the new ones do not exist.
> >
> > Signed-off-by: Stefan Berger <st...@li...>
> > ---
> >  modules.d/98integrity/README   |  8 ++++----
> >  modules.d/98integrity/ima-keys-load.sh   |  7 ++++++-
> >  modules.d/98integrity/ima-policy-load.sh | 20 +++++++++++++++++---
> >  3 files changed, 27 insertions(+), 8 deletions(-)
> >
> > diff --git a/modules.d/98integrity/README b/modules.d/98integrity/README
> > index 64de0ae..c8ccee5 100644
> > --- a/modules.d/98integrity/README
> > +++ b/modules.d/98integrity/README
> > @@ -33,10 +33,10 @@ line.
> >
> >  # Save the policy in a file.
> >
> > -# Create the configuration file '/etc/sysconfig/ima' to override
> > the path name of
> > +# Create the configuration file '/etc/ima/ima' to override the path 
> name of
> >  # the IMA custom policy.
> > -------------- '/etc/sysconfig/ima' (with the default value) 
> -------------
> > -IMAPOLICY="/etc/sysconfig/ima-policy"
> > +------------- '/etc/ima/ima' (with the default value) -------------
> > +IMAPOLICY="/etc/ima/ima-policy"
> > 
>  -------------------------------------------------------------------------
> >
> >
> > @@ -64,5 +64,5 @@ IMAPOLICY="/etc/sysconfig/ima-policy"
> >
> >  # 98integrity/ima-keys-load.sh script loads the signed certificates 
> stored
> >  # in the $IMAKEYSDIR onto the trusted IMA keyring.  The default 
> $IMAKEYSDIR
> > -# directory is /etc/keys/ima, but can be specified in the 
> /etc/sysconfig/ima
> > +# directory is /etc/keys/ima, but can be specified in the /etc/ima/ima
> >  # policy.
> > diff --git a/modules.d/98integrity/ima-keys-load.sh b/modules.d/
> > 98integrity/ima-keys-load.sh
> > index 659b722..922af1e 100755
> > --- a/modules.d/98integrity/ima-keys-load.sh
> > +++ b/modules.d/98integrity/ima-keys-load.sh
> > @@ -2,7 +2,10 @@
> >
> >  SECURITYFSDIR="/sys/kernel/security"
> >  IMASECDIR="${SECURITYFSDIR}/ima"
> > -IMACONFIG="${NEWROOT}/etc/sysconfig/ima"
> > +IMACONFIG="${NEWROOT}/etc/ima/ima"
> > +
> > +# for backwards compatibility
> > +IMACONFIG_OLD="${NEWROOT}/etc/sysconfig/ima"
> >
> >  load_x509_keys()
> >  {
> > @@ -11,6 +14,8 @@ load_x509_keys()
> >      # override the default configuration
> >      if [ -f "${IMACONFIG}" ]; then
> >          . ${IMACONFIG}
> > +    elif [ -f "${IMACONFIG_OLD}" ]; then
> > +        . ${IMACONFIG_OLD}
> >      fi
> >
> >      if [ -z "${IMAKEYDIR}" ]; then
> > diff --git a/modules.d/98integrity/ima-policy-load.sh b/modules.d/
> > 98integrity/ima-policy-load.sh
> > index 85cd3b9..a92b1bb 100755
> > --- a/modules.d/98integrity/ima-policy-load.sh
> > +++ b/modules.d/98integrity/ima-policy-load.sh
> > @@ -5,10 +5,19 @@
> >  # Copyright (C) 2011 Politecnico di Torino, Italy
> >  #  TORSEC group -- http://security.polito.it 
> <http://security.polito.it/>
> >  # Roberto Sassu <rob...@po...>
> > +#
> > +# Copyright (C) 2016 IBM Corporation
> > +#
> > +# Stefan Berger <st...@li...>
> > +#
> >
> >  IMASECDIR="${SECURITYFSDIR}/ima"
> > -IMACONFIG="${NEWROOT}/etc/sysconfig/ima"
> > -IMAPOLICY="/etc/sysconfig/ima-policy"
> > +IMACONFIG="${NEWROOT}/etc/ima/ima"
> > +IMAPOLICY="/etc/ima/ima-policy"
> > +
> > +# for backwards compatibility
> > +IMACONFIG_OLD="${NEWROOT}/etc/sysconfig/ima"
> > +IMAPOLICY_OLD="/etc/sysconfig/ima-policy"
> >
> >  load_ima_policy()
> >  {
> > @@ -21,11 +30,16 @@ load_ima_policy()
> >      fi
> >
> >      # override the default configuration
> > -    [ -f "${IMACONFIG}" ] && \
> > +    if [ -f "${IMACONFIG}" ]; then
> >          . ${IMACONFIG}
> > +    elif [ -f "${IMACONFIG_OLD}" ]; then
> > +        . ${IMACONFIG_OLD}
> > +    fi
> >
> >      # set the IMA policy path name
> >      IMAPOLICYPATH="${NEWROOT}${IMAPOLICY}"
> > +    [ ! -f "${IMAPOLICYPATH}" ] && \
> > +        IMAPOLICYPATH="${NEWROOT}${IMAPOLICY_OLD}"
> >
> >      # check the existence of the IMA policy file
> >      [ -f "${IMAPOLICYPATH}" ] && {
> > --
> > 2.8.3
> >
>

Re: [Linux-ima-user] [PATCH v3] 98integrity: Use /etc/ima as dir for IMA policy and config file

[Stefan B. <st...@us...>]

Stefan Berger <st...@li...> wrote on 11/30/2016 11:18:54 
AM:

Harald,

  please do not apply this patch.

   Stefan


> 
> From: Stefan Berger <st...@us...>
> 
> To sync with systemd, use the filepath /etc/ima/ima-policy as
> the default file location for the IMA policy. At the same time we
> move the ima config file location to /etc/ima/ima. Adapt the
> documentation to the new path. Maintain backwards compatibility
> by still reading the old files in case the new ones do not exist.
> 
> Signed-off-by: Stefan Berger <st...@li...>
> ---
>  modules.d/98integrity/README             |  8 ++++----
>  modules.d/98integrity/ima-keys-load.sh   |  7 ++++++-
>  modules.d/98integrity/ima-policy-load.sh | 20 +++++++++++++++++---
>  3 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/modules.d/98integrity/README b/modules.d/98integrity/README
> index 64de0ae..c8ccee5 100644
> --- a/modules.d/98integrity/README
> +++ b/modules.d/98integrity/README
> @@ -33,10 +33,10 @@ line.
> 
>  # Save the policy in a file.
> 
> -# Create the configuration file '/etc/sysconfig/ima' to override 
> the path name of
> +# Create the configuration file '/etc/ima/ima' to override the path 
name of
>  # the IMA custom policy.
> -------------- '/etc/sysconfig/ima' (with the default value) 
-------------
> -IMAPOLICY="/etc/sysconfig/ima-policy"
> +------------- '/etc/ima/ima' (with the default value) -------------
> +IMAPOLICY="/etc/ima/ima-policy"
> 
-------------------------------------------------------------------------
> 
> 
> @@ -64,5 +64,5 @@ IMAPOLICY="/etc/sysconfig/ima-policy"
> 
>  # 98integrity/ima-keys-load.sh script loads the signed certificates 
stored 
>  # in the $IMAKEYSDIR onto the trusted IMA keyring.  The default 
$IMAKEYSDIR
> -# directory is /etc/keys/ima, but can be specified in the 
/etc/sysconfig/ima
> +# directory is /etc/keys/ima, but can be specified in the /etc/ima/ima
>  # policy.
> diff --git a/modules.d/98integrity/ima-keys-load.sh b/modules.d/
> 98integrity/ima-keys-load.sh
> index 659b722..922af1e 100755
> --- a/modules.d/98integrity/ima-keys-load.sh
> +++ b/modules.d/98integrity/ima-keys-load.sh
> @@ -2,7 +2,10 @@
> 
>  SECURITYFSDIR="/sys/kernel/security"
>  IMASECDIR="${SECURITYFSDIR}/ima"
> -IMACONFIG="${NEWROOT}/etc/sysconfig/ima"
> +IMACONFIG="${NEWROOT}/etc/ima/ima"
> +
> +# for backwards compatibility
> +IMACONFIG_OLD="${NEWROOT}/etc/sysconfig/ima"
> 
>  load_x509_keys()
>  {
> @@ -11,6 +14,8 @@ load_x509_keys()
>      # override the default configuration
>      if [ -f "${IMACONFIG}" ]; then
>          . ${IMACONFIG}
> +    elif [ -f "${IMACONFIG_OLD}" ]; then
> +        . ${IMACONFIG_OLD}
>      fi
> 
>      if [ -z "${IMAKEYDIR}" ]; then
> diff --git a/modules.d/98integrity/ima-policy-load.sh b/modules.d/
> 98integrity/ima-policy-load.sh
> index 85cd3b9..a92b1bb 100755
> --- a/modules.d/98integrity/ima-policy-load.sh
> +++ b/modules.d/98integrity/ima-policy-load.sh
> @@ -5,10 +5,19 @@
>  # Copyright (C) 2011 Politecnico di Torino, Italy
>  #                    TORSEC group -- http://security.polito.it
>  # Roberto Sassu <rob...@po...>
> +#
> +# Copyright (C) 2016 IBM Corporation
> +#
> +# Stefan Berger <st...@li...>
> +#
> 
>  IMASECDIR="${SECURITYFSDIR}/ima"
> -IMACONFIG="${NEWROOT}/etc/sysconfig/ima"
> -IMAPOLICY="/etc/sysconfig/ima-policy"
> +IMACONFIG="${NEWROOT}/etc/ima/ima"
> +IMAPOLICY="/etc/ima/ima-policy"
> +
> +# for backwards compatibility
> +IMACONFIG_OLD="${NEWROOT}/etc/sysconfig/ima"
> +IMAPOLICY_OLD="/etc/sysconfig/ima-policy"
> 
>  load_ima_policy()
>  {
> @@ -21,11 +30,16 @@ load_ima_policy()
>      fi
> 
>      # override the default configuration
> -    [ -f "${IMACONFIG}" ] && \
> +    if [ -f "${IMACONFIG}" ]; then
>          . ${IMACONFIG}
> +    elif [ -f "${IMACONFIG_OLD}" ]; then
> +        . ${IMACONFIG_OLD}
> +    fi
> 
>      # set the IMA policy path name
>      IMAPOLICYPATH="${NEWROOT}${IMAPOLICY}"
> +    [ ! -f "${IMAPOLICYPATH}" ] && \
> +        IMAPOLICYPATH="${NEWROOT}${IMAPOLICY_OLD}"
> 
>      # check the existence of the IMA policy file
>      [ -f "${IMAPOLICYPATH}" ] && {
> -- 
> 2.8.3
> 

[Linux-ima-user] [PATCH v3] 98integrity: Use /etc/ima as dir for IMA policy and config file

[Stefan B. <st...@li...>]

From: Stefan Berger <st...@us...>

To sync with systemd, use the filepath /etc/ima/ima-policy as
the default file location for the IMA policy. At the same time we
move the ima config file location to /etc/ima/ima. Adapt the
documentation to the new path. Maintain backwards compatibility
by still reading the old files in case the new ones do not exist.

Signed-off-by: Stefan Berger <st...@li...>
---
 modules.d/98integrity/README             |  8 ++++----
 modules.d/98integrity/ima-keys-load.sh   |  7 ++++++-
 modules.d/98integrity/ima-policy-load.sh | 20 +++++++++++++++++---
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/modules.d/98integrity/README b/modules.d/98integrity/README
index 64de0ae..c8ccee5 100644
--- a/modules.d/98integrity/README
+++ b/modules.d/98integrity/README
@@ -33,10 +33,10 @@ line.
 
 # Save the policy in a file.
 
-# Create the configuration file '/etc/sysconfig/ima' to override the path name of
+# Create the configuration file '/etc/ima/ima' to override the path name of
 # the IMA custom policy.
-------------- '/etc/sysconfig/ima' (with the default value) -------------
-IMAPOLICY="/etc/sysconfig/ima-policy"
+------------- '/etc/ima/ima' (with the default value) -------------
+IMAPOLICY="/etc/ima/ima-policy"
 -------------------------------------------------------------------------
 
 
@@ -64,5 +64,5 @@ IMAPOLICY="/etc/sysconfig/ima-policy"
 
 # 98integrity/ima-keys-load.sh script loads the signed certificates stored 
 # in the $IMAKEYSDIR onto the trusted IMA keyring.  The default $IMAKEYSDIR
-# directory is /etc/keys/ima, but can be specified in the /etc/sysconfig/ima
+# directory is /etc/keys/ima, but can be specified in the /etc/ima/ima
 # policy.
diff --git a/modules.d/98integrity/ima-keys-load.sh b/modules.d/98integrity/ima-keys-load.sh
index 659b722..922af1e 100755
--- a/modules.d/98integrity/ima-keys-load.sh
+++ b/modules.d/98integrity/ima-keys-load.sh
@@ -2,7 +2,10 @@
 
 SECURITYFSDIR="/sys/kernel/security"
 IMASECDIR="${SECURITYFSDIR}/ima"
-IMACONFIG="${NEWROOT}/etc/sysconfig/ima"
+IMACONFIG="${NEWROOT}/etc/ima/ima"
+
+# for backwards compatibility
+IMACONFIG_OLD="${NEWROOT}/etc/sysconfig/ima"
 
 load_x509_keys()
 {
@@ -11,6 +14,8 @@ load_x509_keys()
     # override the default configuration
     if [ -f "${IMACONFIG}" ]; then
         . ${IMACONFIG}
+    elif [ -f "${IMACONFIG_OLD}" ]; then
+        . ${IMACONFIG_OLD}
     fi
 
     if [ -z "${IMAKEYDIR}" ]; then
diff --git a/modules.d/98integrity/ima-policy-load.sh b/modules.d/98integrity/ima-policy-load.sh
index 85cd3b9..a92b1bb 100755
--- a/modules.d/98integrity/ima-policy-load.sh
+++ b/modules.d/98integrity/ima-policy-load.sh
@@ -5,10 +5,19 @@
 # Copyright (C) 2011 Politecnico di Torino, Italy
 #                    TORSEC group -- http://security.polito.it
 # Roberto Sassu <rob...@po...>
+#
+# Copyright (C) 2016 IBM Corporation
+#
+# Stefan Berger <st...@li...>
+#
 
 IMASECDIR="${SECURITYFSDIR}/ima"
-IMACONFIG="${NEWROOT}/etc/sysconfig/ima"
-IMAPOLICY="/etc/sysconfig/ima-policy"
+IMACONFIG="${NEWROOT}/etc/ima/ima"
+IMAPOLICY="/etc/ima/ima-policy"
+
+# for backwards compatibility
+IMACONFIG_OLD="${NEWROOT}/etc/sysconfig/ima"
+IMAPOLICY_OLD="/etc/sysconfig/ima-policy"
 
 load_ima_policy()
 {
@@ -21,11 +30,16 @@ load_ima_policy()
     fi
 
     # override the default configuration
-    [ -f "${IMACONFIG}" ] && \
+    if [ -f "${IMACONFIG}" ]; then
         . ${IMACONFIG}
+    elif [ -f "${IMACONFIG_OLD}" ]; then
+        . ${IMACONFIG_OLD}
+    fi
 
     # set the IMA policy path name
     IMAPOLICYPATH="${NEWROOT}${IMAPOLICY}"
+    [ ! -f "${IMAPOLICYPATH}" ] && \
+        IMAPOLICYPATH="${NEWROOT}${IMAPOLICY_OLD}"
 
     # check the existence of the IMA policy file
     [ -f "${IMAPOLICYPATH}" ] && {
-- 
2.8.3

Re: [Linux-ima-user] [PATCH v2] 98integrity: Use /etc/ima as dir for IMA policy and config file

[Stefan B. <st...@li...>]

On 11/30/2016 10:52 AM, Harald Hoyer wrote:
> On 30.11.2016 16:24, Stefan Berger wrote:
>> On 11/30/2016 10:16 AM, Harald Hoyer wrote:
>>> On 30.11.2016 16:10, Stefan Berger wrote:
>>>> From: Stefan Berger <st...@us...>
>>>>
>>>> To sync with systemd, use the filepath /etc/ima/ima-policy as
>>>> the file location for the IMA policy. At the same time we
>>>> move the ima config file location to /etc/ima/ima. Adapt the
>>>> documentation to the new path.
>>>>
>>>> Signed-off-by: Stefan Berger <st...@li...>
>>> One more thing: Do you want to be backwards compatible and also read the old files, if they exist?
>> I had thought about that and can certainly add it.  Neither Fedora, RHEL, nor SUSE are packaging these files so far. So likely
>> there aren't many users out there. Considering that, what would you suggest?
>>
> Hmm, I'll add it to the dracut NEWS file

Let me send a v3 of the patch with backwards compatibility. I'll have it 
look for the new location first, then fall back to the old files.

Re: [Linux-ima-user] [PATCH v2] 98integrity: Use /etc/ima as dir for IMA policy and config file

[Harald H. <ha...@re...>]

On 30.11.2016 16:24, Stefan Berger wrote:
> On 11/30/2016 10:16 AM, Harald Hoyer wrote:
>> On 30.11.2016 16:10, Stefan Berger wrote:
>>> From: Stefan Berger <st...@us...>
>>>
>>> To sync with systemd, use the filepath /etc/ima/ima-policy as
>>> the file location for the IMA policy. At the same time we
>>> move the ima config file location to /etc/ima/ima. Adapt the
>>> documentation to the new path.
>>>
>>> Signed-off-by: Stefan Berger <st...@li...>
>>
>> One more thing: Do you want to be backwards compatible and also read the old files, if they exist?
> 
> I had thought about that and can certainly add it.  Neither Fedora, RHEL, nor SUSE are packaging these files so far. So likely
> there aren't many users out there. Considering that, what would you suggest?
> 

Hmm, I'll add it to the dracut NEWS file