All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] LTP IMA fix bundle
@ 2019-01-07  2:26 ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia

Hi Mimi & Petr,

This fix bundle fixes and cleans up IMA-related testcases in LTP.

My test result is attached here.

#cat LTP_RUN_ON-2019_01_07-10h_05m_16s.log 
Test Start Time: Mon Jan  7 10:05:18 2019
-----------------------------------------
Testcase                                           Result     Exit Value
--------                                           ------     ----------
ima_measurements                                   PASS       0    
ima_policy                                         PASS       0    
ima_tpm                                            CONF       32   
ima_violations                                     PASS       0    

-----------------------------------------------
Total Tests: 4
Total Skipped Tests: 1
Total Failures: 0
Kernel Version: 4.20.0+
Machine Architecture: x86_64
Hostname: test-machine

Note:
- The original PR is available at https://github.com/linux-test-project/ltp/pull/459
- The test2() in ima_tpm requires /sys/class/tpm/tpm0/device/pcrs but this interface
  is not available if TPM2 device used. So the test result showed above is expected.

Jia

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 0/6] LTP IMA fix bundle
@ 2019-01-07  2:26 ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: ltp

Hi Mimi & Petr,

This fix bundle fixes and cleans up IMA-related testcases in LTP.

My test result is attached here.

#cat LTP_RUN_ON-2019_01_07-10h_05m_16s.log 
Test Start Time: Mon Jan  7 10:05:18 2019
-----------------------------------------
Testcase                                           Result     Exit Value
--------                                           ------     ----------
ima_measurements                                   PASS       0    
ima_policy                                         PASS       0    
ima_tpm                                            CONF       32   
ima_violations                                     PASS       0    

-----------------------------------------------
Total Tests: 4
Total Skipped Tests: 1
Total Failures: 0
Kernel Version: 4.20.0+
Machine Architecture: x86_64
Hostname: test-machine

Note:
- The original PR is available at https://github.com/linux-test-project/ltp/pull/459
- The test2() in ima_tpm requires /sys/class/tpm/tpm0/device/pcrs but this interface
  is not available if TPM2 device used. So the test result showed above is expected.

Jia

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/6] ima/ima_boot_aggregate: Fix the definition of event log
  2019-01-07  2:26 ` [LTP] " Jia Zhang
@ 2019-01-07  2:26   ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia

According to [1], the structure of event log should be packed,
and certain fields should be 32-bit unsigned integer. Fortunately,
keeping natural alignment seems to make everything working as
expected all the time.

[1] page 17,18 @https://trustedcomputinggroup.org/wp-content/uploads/TCG_EFI_Protocol_1_22_Final-v05.pdf

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index f6e7be0..d85d222 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -53,10 +53,10 @@ int main(int argc, char *argv[])
 	struct {
 		struct {
 			u_int32_t pcr;
-			int type;
-			unsigned char digest[SHA_DIGEST_LENGTH];
-			u_int16_t len;
-		} header;
+			u_int32_t type;
+			u_int8_t digest[SHA_DIGEST_LENGTH];
+			u_int32_t len;
+		} header __attribute__ ((packed));
 		char *data;
 	} event;
 	struct {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 1/6] ima/ima_boot_aggregate: Fix the definition of event log
@ 2019-01-07  2:26   ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: ltp

According to [1], the structure of event log should be packed,
and certain fields should be 32-bit unsigned integer. Fortunately,
keeping natural alignment seems to make everything working as
expected all the time.

[1] page 17,18 @https://trustedcomputinggroup.org/wp-content/uploads/TCG_EFI_Protocol_1_22_Final-v05.pdf

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index f6e7be0..d85d222 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -53,10 +53,10 @@ int main(int argc, char *argv[])
 	struct {
 		struct {
 			u_int32_t pcr;
-			int type;
-			unsigned char digest[SHA_DIGEST_LENGTH];
-			u_int16_t len;
-		} header;
+			u_int32_t type;
+			u_int8_t digest[SHA_DIGEST_LENGTH];
+			u_int32_t len;
+		} header __attribute__ ((packed));
 		char *data;
 	} event;
 	struct {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash
  2019-01-07  2:26 ` [LTP] " Jia Zhang
@ 2019-01-07  2:26   ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia

Instead, use SHA_DIGEST_LENGTH.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index d85d222..67be6a7 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -42,7 +42,7 @@ static void display_sha1_digest(unsigned char *pcr)
 {
 	int i;
 
-	for (i = 0; i < 20; i++)
+	for (i = 0; i < SHA_DIGEST_LENGTH; i++)
 		printf("%02x", *(pcr + i) & 0xff);
 	printf("\n");
 }
@@ -94,8 +94,9 @@ int main(int argc, char *argv[])
 			display_sha1_digest(event.header.digest);
 		}
 		SHA1_Init(&c);
-		SHA1_Update(&c, pcr[event.header.pcr].digest, 20);
-		SHA1_Update(&c, event.header.digest, 20);
+		SHA1_Update(&c, pcr[event.header.pcr].digest,
+			    SHA_DIGEST_LENGTH);
+		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
 		SHA1_Final(pcr[event.header.pcr].digest, &c);
 #if MAX_EVENT_DATA_SIZE < USHRT_MAX
 		if (event.header.len > MAX_EVENT_DATA_SIZE) {
@@ -116,7 +117,7 @@ int main(int argc, char *argv[])
 			printf("PCR-%2.2x: ", i);
 			display_sha1_digest(pcr[i].digest);
 		}
-		SHA1_Update(&c, pcr[i].digest, 20);
+		SHA1_Update(&c, pcr[i].digest, SHA_DIGEST_LENGTH);
 	}
 	SHA1_Final(boot_aggregate, &c);
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash
@ 2019-01-07  2:26   ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: ltp

Instead, use SHA_DIGEST_LENGTH.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index d85d222..67be6a7 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -42,7 +42,7 @@ static void display_sha1_digest(unsigned char *pcr)
 {
 	int i;
 
-	for (i = 0; i < 20; i++)
+	for (i = 0; i < SHA_DIGEST_LENGTH; i++)
 		printf("%02x", *(pcr + i) & 0xff);
 	printf("\n");
 }
@@ -94,8 +94,9 @@ int main(int argc, char *argv[])
 			display_sha1_digest(event.header.digest);
 		}
 		SHA1_Init(&c);
-		SHA1_Update(&c, pcr[event.header.pcr].digest, 20);
-		SHA1_Update(&c, event.header.digest, 20);
+		SHA1_Update(&c, pcr[event.header.pcr].digest,
+			    SHA_DIGEST_LENGTH);
+		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
 		SHA1_Final(pcr[event.header.pcr].digest, &c);
 #if MAX_EVENT_DATA_SIZE < USHRT_MAX
 		if (event.header.len > MAX_EVENT_DATA_SIZE) {
@@ -116,7 +117,7 @@ int main(int argc, char *argv[])
 			printf("PCR-%2.2x: ", i);
 			display_sha1_digest(pcr[i].digest);
 		}
-		SHA1_Update(&c, pcr[i].digest, 20);
+		SHA1_Update(&c, pcr[i].digest, SHA_DIGEST_LENGTH);
 	}
 	SHA1_Final(boot_aggregate, &c);
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7
  2019-01-07  2:26 ` [LTP] " Jia Zhang
@ 2019-01-07  2:26   ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia

The boot aggragate calculation should never touch PCRs beyond PCR 0-7,
even a PCR extension really manipulates out-of-domain PCRs.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 .../security/integrity/ima/src/ima_boot_aggregate.c       | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index 67be6a7..98893b9 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -93,11 +93,16 @@ int main(int argc, char *argv[])
 			printf("%03u ", event.header.pcr);
 			display_sha1_digest(event.header.digest);
 		}
-		SHA1_Init(&c);
-		SHA1_Update(&c, pcr[event.header.pcr].digest,
-			    SHA_DIGEST_LENGTH);
-		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
-		SHA1_Final(pcr[event.header.pcr].digest, &c);
+
+		if (event.header.pcr < NUM_PCRS) {
+			SHA1_Init(&c);
+			SHA1_Update(&c, pcr[event.header.pcr].digest,
+				    SHA_DIGEST_LENGTH);
+			SHA1_Update(&c, event.header.digest,
+				    SHA_DIGEST_LENGTH);
+			SHA1_Final(pcr[event.header.pcr].digest, &c);
+		}
+
 #if MAX_EVENT_DATA_SIZE < USHRT_MAX
 		if (event.header.len > MAX_EVENT_DATA_SIZE) {
 			printf("Error event too long\n");
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7
@ 2019-01-07  2:26   ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: ltp

The boot aggragate calculation should never touch PCRs beyond PCR 0-7,
even a PCR extension really manipulates out-of-domain PCRs.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 .../security/integrity/ima/src/ima_boot_aggregate.c       | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index 67be6a7..98893b9 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -93,11 +93,16 @@ int main(int argc, char *argv[])
 			printf("%03u ", event.header.pcr);
 			display_sha1_digest(event.header.digest);
 		}
-		SHA1_Init(&c);
-		SHA1_Update(&c, pcr[event.header.pcr].digest,
-			    SHA_DIGEST_LENGTH);
-		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
-		SHA1_Final(pcr[event.header.pcr].digest, &c);
+
+		if (event.header.pcr < NUM_PCRS) {
+			SHA1_Init(&c);
+			SHA1_Update(&c, pcr[event.header.pcr].digest,
+				    SHA_DIGEST_LENGTH);
+			SHA1_Update(&c, event.header.digest,
+				    SHA_DIGEST_LENGTH);
+			SHA1_Final(pcr[event.header.pcr].digest, &c);
+		}
+
 #if MAX_EVENT_DATA_SIZE < USHRT_MAX
 		if (event.header.len > MAX_EVENT_DATA_SIZE) {
 			printf("Error event too long\n");
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 4/6] ima: Code cleanup
  2019-01-07  2:26 ` [LTP] " Jia Zhang
@ 2019-01-07  2:26   ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia

- Don't use the legacy policy function name in policy files.
- Use the variable IMA_POLICY instead of hard code path.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 testcases/kernel/security/integrity/ima/policy/measure.policy         | 2 +-
 testcases/kernel/security/integrity/ima/policy/measure.policy-invalid | 2 +-
 testcases/kernel/security/integrity/ima/tests/ima_policy.sh           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy b/testcases/kernel/security/integrity/ima/policy/measure.policy
index c68e722..9976ddf 100644
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy
+++ b/testcases/kernel/security/integrity/ima/policy/measure.policy
@@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
 dont_measure fsmagic=0x73636673
 measure func=FILE_MMAP mask=MAY_EXEC
 measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=PATH_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
index e406757..04dff89 100644
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
+++ b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
@@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
 dnt_measure fsmagic=0x73636673
 measure func=FILE_MMAP mask=MAY_EXEC
 measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=PATH_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index 64aa8cb..a0c7869 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -28,7 +28,7 @@ check_policy_writable()
 {
 	local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
 
-	[ -f /sys/kernel/security/ima/policy ] || tst_brk TCONF "$err"
+	[ -f $IMA_POLICY ] || tst_brk TCONF "$err"
 	# CONFIG_IMA_READ_POLICY
 	echo "" 2> log > $IMA_POLICY
 	grep -q "Device or resource busy" log && tst_brk TCONF "$err"
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 4/6] ima: Code cleanup
@ 2019-01-07  2:26   ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: ltp

- Don't use the legacy policy function name in policy files.
- Use the variable IMA_POLICY instead of hard code path.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 testcases/kernel/security/integrity/ima/policy/measure.policy         | 2 +-
 testcases/kernel/security/integrity/ima/policy/measure.policy-invalid | 2 +-
 testcases/kernel/security/integrity/ima/tests/ima_policy.sh           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy b/testcases/kernel/security/integrity/ima/policy/measure.policy
index c68e722..9976ddf 100644
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy
+++ b/testcases/kernel/security/integrity/ima/policy/measure.policy
@@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
 dont_measure fsmagic=0x73636673
 measure func=FILE_MMAP mask=MAY_EXEC
 measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=PATH_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
index e406757..04dff89 100644
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
+++ b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
@@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
 dnt_measure fsmagic=0x73636673
 measure func=FILE_MMAP mask=MAY_EXEC
 measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=PATH_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index 64aa8cb..a0c7869 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -28,7 +28,7 @@ check_policy_writable()
 {
 	local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
 
-	[ -f /sys/kernel/security/ima/policy ] || tst_brk TCONF "$err"
+	[ -f $IMA_POLICY ] || tst_brk TCONF "$err"
 	# CONFIG_IMA_READ_POLICY
 	echo "" 2> log > $IMA_POLICY
 	grep -q "Device or resource busy" log && tst_brk TCONF "$err"
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 5/6] ima: Rename the folder name for policy files to datafiles
  2019-01-07  2:26 ` [LTP] " Jia Zhang
@ 2019-01-07  2:26   ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia

If we choose to run ima_policy.sh locally without installation,
a failure message is reported as following:

ima_policy 1 TCONF: missing <path>/ltp/testcases/kernel/security/integrity/ima/datafiles/measure.policy

TST_DATAROOT would be extended to datafiles but the policy files
are actually placed under policy.

In order to make it easier, just rename the folder name to datafiles.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 testcases/kernel/security/integrity/ima/Makefile   |  2 +-
 .../security/integrity/ima/datafiles/Makefile      | 31 ++++++++++++++++++++++
 .../integrity/ima/datafiles/measure.policy         | 16 +++++++++++
 .../integrity/ima/datafiles/measure.policy-invalid | 16 +++++++++++
 .../kernel/security/integrity/ima/policy/Makefile  | 31 ----------------------
 .../security/integrity/ima/policy/measure.policy   | 16 -----------
 .../integrity/ima/policy/measure.policy-invalid    | 16 -----------
 7 files changed, 64 insertions(+), 64 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/Makefile
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/measure.policy
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
 delete mode 100644 testcases/kernel/security/integrity/ima/policy/Makefile
 delete mode 100644 testcases/kernel/security/integrity/ima/policy/measure.policy
 delete mode 100644 testcases/kernel/security/integrity/ima/policy/measure.policy-invalid

diff --git a/testcases/kernel/security/integrity/ima/Makefile b/testcases/kernel/security/integrity/ima/Makefile
index 1290e6f..19b10ff 100644
--- a/testcases/kernel/security/integrity/ima/Makefile
+++ b/testcases/kernel/security/integrity/ima/Makefile
@@ -24,6 +24,6 @@ top_srcdir		?= ../../../../..
 
 include $(top_srcdir)/include/mk/env_pre.mk
 
-SUBDIRS			:= policy src tests
+SUBDIRS			:= datafiles src tests
 
 include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/Makefile b/testcases/kernel/security/integrity/ima/datafiles/Makefile
new file mode 100644
index 0000000..a960f9d
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/Makefile
@@ -0,0 +1,31 @@
+#
+#    testcases/kernel/security/integrity/ima/policy testcases Makefile.
+#
+#    Copyright (C) 2009, Cisco Systems Inc.
+#
+#    This program is free software; you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License as published by
+#    the Free Software Foundation; either version 2 of the License, or
+#    (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+#    You should have received a copy of the GNU General Public License along
+#    with this program; if not, write to the Free Software Foundation, Inc.,
+#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Ngie Cooper, July 2009
+#
+
+top_srcdir		?= ../../../../../..
+
+include	$(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_DIR		:= testcases/data/ima_policy
+
+INSTALL_TARGETS		:= measure*
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
new file mode 100644
index 0000000..9976ddf
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
@@ -0,0 +1,16 @@
+#
+# Integrity measure policy
+#
+# PROC_SUPER_MAGIC
+dont_measure fsmagic=0x9fa0
+# SYSFS_MAGIC
+dont_measure fsmagic=0x62656572
+# DEBUGFS_MAGIC
+dont_measure fsmagic=0x64626720
+# TMPFS_MAGIC
+dont_measure fsmagic=0x01021994
+# SECURITYFS_MAGIC
+dont_measure fsmagic=0x73636673
+measure func=FILE_MMAP mask=MAY_EXEC
+measure func=BPRM_CHECK mask=MAY_EXEC
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
new file mode 100644
index 0000000..04dff89
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
@@ -0,0 +1,16 @@
+#
+# Integrity measure policy
+#
+# PROC_SUPER_MAGIC
+dont_measure fsmagic=0x9fa0
+# SYSFS_MAGIC
+dont_measure fsmagic=0x62656572
+# DEBUGFS_MAGIC
+dont_measure fsmagic=0x64626720
+# TMPFS_MAGIC
+dont_measure fsmagic=0x01021994
+# SECURITYFS_MAGIC
+dnt_measure fsmagic=0x73636673
+measure func=FILE_MMAP mask=MAY_EXEC
+measure func=BPRM_CHECK mask=MAY_EXEC
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/Makefile b/testcases/kernel/security/integrity/ima/policy/Makefile
deleted file mode 100644
index a960f9d..0000000
--- a/testcases/kernel/security/integrity/ima/policy/Makefile
+++ /dev/null
@@ -1,31 +0,0 @@
-#
-#    testcases/kernel/security/integrity/ima/policy testcases Makefile.
-#
-#    Copyright (C) 2009, Cisco Systems Inc.
-#
-#    This program is free software; you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License as published by
-#    the Free Software Foundation; either version 2 of the License, or
-#    (at your option) any later version.
-#
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
-#
-#    You should have received a copy of the GNU General Public License along
-#    with this program; if not, write to the Free Software Foundation, Inc.,
-#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-# Ngie Cooper, July 2009
-#
-
-top_srcdir		?= ../../../../../..
-
-include	$(top_srcdir)/include/mk/env_pre.mk
-
-INSTALL_DIR		:= testcases/data/ima_policy
-
-INSTALL_TARGETS		:= measure*
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy b/testcases/kernel/security/integrity/ima/policy/measure.policy
deleted file mode 100644
index 9976ddf..0000000
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy
+++ /dev/null
@@ -1,16 +0,0 @@
-#
-# Integrity measure policy
-#
-# PROC_SUPER_MAGIC
-dont_measure fsmagic=0x9fa0
-# SYSFS_MAGIC
-dont_measure fsmagic=0x62656572
-# DEBUGFS_MAGIC
-dont_measure fsmagic=0x64626720
-# TMPFS_MAGIC
-dont_measure fsmagic=0x01021994
-# SECURITYFS_MAGIC
-dont_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
-measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
deleted file mode 100644
index 04dff89..0000000
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
+++ /dev/null
@@ -1,16 +0,0 @@
-#
-# Integrity measure policy
-#
-# PROC_SUPER_MAGIC
-dont_measure fsmagic=0x9fa0
-# SYSFS_MAGIC
-dont_measure fsmagic=0x62656572
-# DEBUGFS_MAGIC
-dont_measure fsmagic=0x64626720
-# TMPFS_MAGIC
-dont_measure fsmagic=0x01021994
-# SECURITYFS_MAGIC
-dnt_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
-measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 5/6] ima: Rename the folder name for policy files to datafiles
@ 2019-01-07  2:26   ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: ltp

If we choose to run ima_policy.sh locally without installation,
a failure message is reported as following:

ima_policy 1 TCONF: missing <path>/ltp/testcases/kernel/security/integrity/ima/datafiles/measure.policy

TST_DATAROOT would be extended to datafiles but the policy files
are actually placed under policy.

In order to make it easier, just rename the folder name to datafiles.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 testcases/kernel/security/integrity/ima/Makefile   |  2 +-
 .../security/integrity/ima/datafiles/Makefile      | 31 ++++++++++++++++++++++
 .../integrity/ima/datafiles/measure.policy         | 16 +++++++++++
 .../integrity/ima/datafiles/measure.policy-invalid | 16 +++++++++++
 .../kernel/security/integrity/ima/policy/Makefile  | 31 ----------------------
 .../security/integrity/ima/policy/measure.policy   | 16 -----------
 .../integrity/ima/policy/measure.policy-invalid    | 16 -----------
 7 files changed, 64 insertions(+), 64 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/Makefile
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/measure.policy
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
 delete mode 100644 testcases/kernel/security/integrity/ima/policy/Makefile
 delete mode 100644 testcases/kernel/security/integrity/ima/policy/measure.policy
 delete mode 100644 testcases/kernel/security/integrity/ima/policy/measure.policy-invalid

diff --git a/testcases/kernel/security/integrity/ima/Makefile b/testcases/kernel/security/integrity/ima/Makefile
index 1290e6f..19b10ff 100644
--- a/testcases/kernel/security/integrity/ima/Makefile
+++ b/testcases/kernel/security/integrity/ima/Makefile
@@ -24,6 +24,6 @@ top_srcdir		?= ../../../../..
 
 include $(top_srcdir)/include/mk/env_pre.mk
 
-SUBDIRS			:= policy src tests
+SUBDIRS			:= datafiles src tests
 
 include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/Makefile b/testcases/kernel/security/integrity/ima/datafiles/Makefile
new file mode 100644
index 0000000..a960f9d
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/Makefile
@@ -0,0 +1,31 @@
+#
+#    testcases/kernel/security/integrity/ima/policy testcases Makefile.
+#
+#    Copyright (C) 2009, Cisco Systems Inc.
+#
+#    This program is free software; you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License as published by
+#    the Free Software Foundation; either version 2 of the License, or
+#    (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+#    You should have received a copy of the GNU General Public License along
+#    with this program; if not, write to the Free Software Foundation, Inc.,
+#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Ngie Cooper, July 2009
+#
+
+top_srcdir		?= ../../../../../..
+
+include	$(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_DIR		:= testcases/data/ima_policy
+
+INSTALL_TARGETS		:= measure*
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
new file mode 100644
index 0000000..9976ddf
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
@@ -0,0 +1,16 @@
+#
+# Integrity measure policy
+#
+# PROC_SUPER_MAGIC
+dont_measure fsmagic=0x9fa0
+# SYSFS_MAGIC
+dont_measure fsmagic=0x62656572
+# DEBUGFS_MAGIC
+dont_measure fsmagic=0x64626720
+# TMPFS_MAGIC
+dont_measure fsmagic=0x01021994
+# SECURITYFS_MAGIC
+dont_measure fsmagic=0x73636673
+measure func=FILE_MMAP mask=MAY_EXEC
+measure func=BPRM_CHECK mask=MAY_EXEC
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
new file mode 100644
index 0000000..04dff89
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
@@ -0,0 +1,16 @@
+#
+# Integrity measure policy
+#
+# PROC_SUPER_MAGIC
+dont_measure fsmagic=0x9fa0
+# SYSFS_MAGIC
+dont_measure fsmagic=0x62656572
+# DEBUGFS_MAGIC
+dont_measure fsmagic=0x64626720
+# TMPFS_MAGIC
+dont_measure fsmagic=0x01021994
+# SECURITYFS_MAGIC
+dnt_measure fsmagic=0x73636673
+measure func=FILE_MMAP mask=MAY_EXEC
+measure func=BPRM_CHECK mask=MAY_EXEC
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/Makefile b/testcases/kernel/security/integrity/ima/policy/Makefile
deleted file mode 100644
index a960f9d..0000000
--- a/testcases/kernel/security/integrity/ima/policy/Makefile
+++ /dev/null
@@ -1,31 +0,0 @@
-#
-#    testcases/kernel/security/integrity/ima/policy testcases Makefile.
-#
-#    Copyright (C) 2009, Cisco Systems Inc.
-#
-#    This program is free software; you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License as published by
-#    the Free Software Foundation; either version 2 of the License, or
-#    (at your option) any later version.
-#
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
-#
-#    You should have received a copy of the GNU General Public License along
-#    with this program; if not, write to the Free Software Foundation, Inc.,
-#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-# Ngie Cooper, July 2009
-#
-
-top_srcdir		?= ../../../../../..
-
-include	$(top_srcdir)/include/mk/env_pre.mk
-
-INSTALL_DIR		:= testcases/data/ima_policy
-
-INSTALL_TARGETS		:= measure*
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy b/testcases/kernel/security/integrity/ima/policy/measure.policy
deleted file mode 100644
index 9976ddf..0000000
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy
+++ /dev/null
@@ -1,16 +0,0 @@
-#
-# Integrity measure policy
-#
-# PROC_SUPER_MAGIC
-dont_measure fsmagic=0x9fa0
-# SYSFS_MAGIC
-dont_measure fsmagic=0x62656572
-# DEBUGFS_MAGIC
-dont_measure fsmagic=0x64626720
-# TMPFS_MAGIC
-dont_measure fsmagic=0x01021994
-# SECURITYFS_MAGIC
-dont_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
-measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
deleted file mode 100644
index 04dff89..0000000
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
+++ /dev/null
@@ -1,16 +0,0 @@
-#
-# Integrity measure policy
-#
-# PROC_SUPER_MAGIC
-dont_measure fsmagic=0x9fa0
-# SYSFS_MAGIC
-dont_measure fsmagic=0x62656572
-# DEBUGFS_MAGIC
-dont_measure fsmagic=0x64626720
-# TMPFS_MAGIC
-dont_measure fsmagic=0x01021994
-# SECURITYFS_MAGIC
-dnt_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
-measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 6/6] ima: Use ima tcb policy files for test
  2019-01-07  2:26 ` [LTP] " Jia Zhang
@ 2019-01-07  2:26   ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia

In order to make all tests running smoothly, the policy files should
keep up with the default ima tcb policy. Especially ima_violations.sh
expects to have a func=FILE_CHECK with mask=MAY_WRITE to trigger open
writer and ToMtoU violations. Unfortunately, if ima_policy.sh
which would change the system IMA policy ran before ima_violations.sh,
ima_violations.sh would fail for sure because its prerequisite is broken.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 .../security/integrity/ima/datafiles/measure.policy     | 17 +++++++++++++++--
 .../integrity/ima/datafiles/measure.policy-invalid      | 17 +++++++++++++++--
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
index 9976ddf..546267c 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
@@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
 dont_measure fsmagic=0x01021994
 # SECURITYFS_MAGIC
 dont_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
+# DEVPTS_SUPER_MAGIC
+dont_measure fsmagic=0x1cd1
+# BINFMTFS_MAGIC
+dont_measure fsmagic=0x42494e4d
+# SELINUX_MAGIC
+dont_measure fsmagic=0xf97cff8c
+# CGROUP_SUPER_MAGIC
+dont_measure fsmagic=0x27e0eb
+# NSFS_MAGIC
+dont_measure fsmagic=0x6e736673
+measure func=MMAP_CHECK mask=MAY_EXEC
 measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK euid=0
+measure func=FILE_CHECK uid=0
+measure func=MODULE_CHECK
+measure func=FIRMWARE_CHECK
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
index 04dff89..bc72d0c 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
@@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
 dont_measure fsmagic=0x01021994
 # SECURITYFS_MAGIC
 dnt_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
+# DEVPTS_SUPER_MAGIC
+dont_measure fsmagic=0x1cd1
+# BINFMTFS_MAGIC
+dont_measure fsmagic=0x42494e4d
+# SELINUX_MAGIC
+dont_measure fsmagic=0xf97cff8c
+# CGROUP_SUPER_MAGIC
+dont_measure fsmagic=0x27e0eb
+# NSFS_MAGIC
+dont_measure fsmagic=0x6e736673
+measure func=MMAP_CHECK mask=MAY_EXEC
 measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK euid=0
+measure func=FILE_CHECK uid=0
+measure func=MODULE_CHECK
+measure func=FIRMWARE_CHECK
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 6/6] ima: Use ima tcb policy files for test
@ 2019-01-07  2:26   ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07  2:26 UTC (permalink / raw)
  To: ltp

In order to make all tests running smoothly, the policy files should
keep up with the default ima tcb policy. Especially ima_violations.sh
expects to have a func=FILE_CHECK with mask=MAY_WRITE to trigger open
writer and ToMtoU violations. Unfortunately, if ima_policy.sh
which would change the system IMA policy ran before ima_violations.sh,
ima_violations.sh would fail for sure because its prerequisite is broken.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 .../security/integrity/ima/datafiles/measure.policy     | 17 +++++++++++++++--
 .../integrity/ima/datafiles/measure.policy-invalid      | 17 +++++++++++++++--
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
index 9976ddf..546267c 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
@@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
 dont_measure fsmagic=0x01021994
 # SECURITYFS_MAGIC
 dont_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
+# DEVPTS_SUPER_MAGIC
+dont_measure fsmagic=0x1cd1
+# BINFMTFS_MAGIC
+dont_measure fsmagic=0x42494e4d
+# SELINUX_MAGIC
+dont_measure fsmagic=0xf97cff8c
+# CGROUP_SUPER_MAGIC
+dont_measure fsmagic=0x27e0eb
+# NSFS_MAGIC
+dont_measure fsmagic=0x6e736673
+measure func=MMAP_CHECK mask=MAY_EXEC
 measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK euid=0
+measure func=FILE_CHECK uid=0
+measure func=MODULE_CHECK
+measure func=FIRMWARE_CHECK
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
index 04dff89..bc72d0c 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
@@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
 dont_measure fsmagic=0x01021994
 # SECURITYFS_MAGIC
 dnt_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
+# DEVPTS_SUPER_MAGIC
+dont_measure fsmagic=0x1cd1
+# BINFMTFS_MAGIC
+dont_measure fsmagic=0x42494e4d
+# SELINUX_MAGIC
+dont_measure fsmagic=0xf97cff8c
+# CGROUP_SUPER_MAGIC
+dont_measure fsmagic=0x27e0eb
+# NSFS_MAGIC
+dont_measure fsmagic=0x6e736673
+measure func=MMAP_CHECK mask=MAY_EXEC
 measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK euid=0
+measure func=FILE_CHECK uid=0
+measure func=MODULE_CHECK
+measure func=FIRMWARE_CHECK
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] ima: Use ima tcb policy files for test
  2019-01-07  2:26   ` [LTP] " Jia Zhang
@ 2019-01-07 15:59     ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07 15:59 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp

On 2019/1/7 上午10:26, Jia Zhang wrote:
> In order to make all tests running smoothly, the policy files should
> keep up ith the default ima tcb policy. Especially ima_violations.sh
> expects to have a func=FILE_CHECK with mask=MAY_WRITE to trigger open

Oops ... This is not true. mask=MAY_READ is already working well for
open writer and ToMToU violations. So please drop this patch.

Jia

> writer and ToMtoU violations. Unfortunately, if ima_policy.sh
> which would change the system IMA policy ran before ima_violations.sh,
> ima_violations.sh would fail for sure because its prerequisite is broken.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> ---
>  .../security/integrity/ima/datafiles/measure.policy     | 17 +++++++++++++++--
>  .../integrity/ima/datafiles/measure.policy-invalid      | 17 +++++++++++++++--
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> index 9976ddf..546267c 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>  dont_measure fsmagic=0x01021994
>  # SECURITYFS_MAGIC
>  dont_measure fsmagic=0x73636673
> -measure func=FILE_MMAP mask=MAY_EXEC
> +# DEVPTS_SUPER_MAGIC
> +dont_measure fsmagic=0x1cd1
> +# BINFMTFS_MAGIC
> +dont_measure fsmagic=0x42494e4d
> +# SELINUX_MAGIC
> +dont_measure fsmagic=0xf97cff8c
> +# CGROUP_SUPER_MAGIC
> +dont_measure fsmagic=0x27e0eb
> +# NSFS_MAGIC
> +dont_measure fsmagic=0x6e736673
> +measure func=MMAP_CHECK mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=FILE_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK euid=0
> +measure func=FILE_CHECK uid=0
> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> index 04dff89..bc72d0c 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>  dont_measure fsmagic=0x01021994
>  # SECURITYFS_MAGIC
>  dnt_measure fsmagic=0x73636673
> -measure func=FILE_MMAP mask=MAY_EXEC
> +# DEVPTS_SUPER_MAGIC
> +dont_measure fsmagic=0x1cd1
> +# BINFMTFS_MAGIC
> +dont_measure fsmagic=0x42494e4d
> +# SELINUX_MAGIC
> +dont_measure fsmagic=0xf97cff8c
> +# CGROUP_SUPER_MAGIC
> +dont_measure fsmagic=0x27e0eb
> +# NSFS_MAGIC
> +dont_measure fsmagic=0x6e736673
> +measure func=MMAP_CHECK mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=FILE_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK euid=0
> +measure func=FILE_CHECK uid=0
> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 6/6] ima: Use ima tcb policy files for test
@ 2019-01-07 15:59     ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-07 15:59 UTC (permalink / raw)
  To: ltp

On 2019/1/7 上午10:26, Jia Zhang wrote:
> In order to make all tests running smoothly, the policy files should
> keep up ith the default ima tcb policy. Especially ima_violations.sh
> expects to have a func=FILE_CHECK with mask=MAY_WRITE to trigger open

Oops ... This is not true. mask=MAY_READ is already working well for
open writer and ToMToU violations. So please drop this patch.

Jia

> writer and ToMtoU violations. Unfortunately, if ima_policy.sh
> which would change the system IMA policy ran before ima_violations.sh,
> ima_violations.sh would fail for sure because its prerequisite is broken.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> ---
>  .../security/integrity/ima/datafiles/measure.policy     | 17 +++++++++++++++--
>  .../integrity/ima/datafiles/measure.policy-invalid      | 17 +++++++++++++++--
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> index 9976ddf..546267c 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>  dont_measure fsmagic=0x01021994
>  # SECURITYFS_MAGIC
>  dont_measure fsmagic=0x73636673
> -measure func=FILE_MMAP mask=MAY_EXEC
> +# DEVPTS_SUPER_MAGIC
> +dont_measure fsmagic=0x1cd1
> +# BINFMTFS_MAGIC
> +dont_measure fsmagic=0x42494e4d
> +# SELINUX_MAGIC
> +dont_measure fsmagic=0xf97cff8c
> +# CGROUP_SUPER_MAGIC
> +dont_measure fsmagic=0x27e0eb
> +# NSFS_MAGIC
> +dont_measure fsmagic=0x6e736673
> +measure func=MMAP_CHECK mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=FILE_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK euid=0
> +measure func=FILE_CHECK uid=0
> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> index 04dff89..bc72d0c 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>  dont_measure fsmagic=0x01021994
>  # SECURITYFS_MAGIC
>  dnt_measure fsmagic=0x73636673
> -measure func=FILE_MMAP mask=MAY_EXEC
> +# DEVPTS_SUPER_MAGIC
> +dont_measure fsmagic=0x1cd1
> +# BINFMTFS_MAGIC
> +dont_measure fsmagic=0x42494e4d
> +# SELINUX_MAGIC
> +dont_measure fsmagic=0xf97cff8c
> +# CGROUP_SUPER_MAGIC
> +dont_measure fsmagic=0x27e0eb
> +# NSFS_MAGIC
> +dont_measure fsmagic=0x6e736673
> +measure func=MMAP_CHECK mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=FILE_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK euid=0
> +measure func=FILE_CHECK uid=0
> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 0/6] LTP IMA fix bundle
  2019-01-07  2:26 ` [LTP] " Jia Zhang
@ 2019-01-13 12:14   ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-13 12:14 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp

Hi Mimi,

Could you review this patchset for LTP?

Thanks,
Jia

On 2019/1/7 上午10:26, Jia Zhang wrote:
> Hi Mimi & Petr,
> 
> This fix bundle fixes and cleans up IMA-related testcases in LTP.
> 
> My test result is attached here.
> 
> #cat LTP_RUN_ON-2019_01_07-10h_05m_16s.log 
> Test Start Time: Mon Jan  7 10:05:18 2019
> -----------------------------------------
> Testcase                                           Result     Exit Value
> --------                                           ------     ----------
> ima_measurements                                   PASS       0    
> ima_policy                                         PASS       0    
> ima_tpm                                            CONF       32   
> ima_violations                                     PASS       0    
> 
> -----------------------------------------------
> Total Tests: 4
> Total Skipped Tests: 1
> Total Failures: 0
> Kernel Version: 4.20.0+
> Machine Architecture: x86_64
> Hostname: test-machine
> 
> Note:
> - The original PR is available at https://github.com/linux-test-project/ltp/pull/459
> - The test2() in ima_tpm requires /sys/class/tpm/tpm0/device/pcrs but this interface
>   is not available if TPM2 device used. So the test result showed above is expected.
> 
> Jia
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 0/6] LTP IMA fix bundle
@ 2019-01-13 12:14   ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-13 12:14 UTC (permalink / raw)
  To: ltp

Hi Mimi,

Could you review this patchset for LTP?

Thanks,
Jia

On 2019/1/7 上午10:26, Jia Zhang wrote:
> Hi Mimi & Petr,
> 
> This fix bundle fixes and cleans up IMA-related testcases in LTP.
> 
> My test result is attached here.
> 
> #cat LTP_RUN_ON-2019_01_07-10h_05m_16s.log 
> Test Start Time: Mon Jan  7 10:05:18 2019
> -----------------------------------------
> Testcase                                           Result     Exit Value
> --------                                           ------     ----------
> ima_measurements                                   PASS       0    
> ima_policy                                         PASS       0    
> ima_tpm                                            CONF       32   
> ima_violations                                     PASS       0    
> 
> -----------------------------------------------
> Total Tests: 4
> Total Skipped Tests: 1
> Total Failures: 0
> Kernel Version: 4.20.0+
> Machine Architecture: x86_64
> Hostname: test-machine
> 
> Note:
> - The original PR is available at https://github.com/linux-test-project/ltp/pull/459
> - The test2() in ima_tpm requires /sys/class/tpm/tpm0/device/pcrs but this interface
>   is not available if TPM2 device used. So the test result showed above is expected.
> 
> Jia
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] ima: Use ima tcb policy files for test
  2019-01-07  2:26   ` [LTP] " Jia Zhang
@ 2019-01-14 19:32     ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2019-01-14 19:32 UTC (permalink / raw)
  To: Jia Zhang, zohar, pvorel; +Cc: linux-integrity, ltp

On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> In order to make all tests running smoothly, the policy files should
> keep up with the default ima tcb policy.

Keeping the policy rules in sync is a good idea, but some of the rules
might cause a regression with older kernels (eg. NSFS magic).  Not
including the rule, also poses a problem.

The kernel headers package includes magic.h.  One solution would be to check whether a magic name is included in magic.h.

> Especially ima_violations.sh
> expects to have a func=FILE_CHECK with mask=MAY_WRITE to trigger open
> writer and ToMtoU violations. Unfortunately, if ima_policy.sh
> which would change the system IMA policy ran before ima_violations.sh,
> ima_violations.sh would fail for sure because its prerequisite is broken.

We're not really interested in measuring files that are opened for
write.  They're changing.  The violation checking is independent of
having a measurement write rule.  Look at the
kernel ima_rdwr_violation_check().

Mimi

> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> ---
>  .../security/integrity/ima/datafiles/measure.policy     | 17 +++++++++++++++--
>  .../integrity/ima/datafiles/measure.policy-invalid      | 17 +++++++++++++++--
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> index 9976ddf..546267c 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>  dont_measure fsmagic=0x01021994
>  # SECURITYFS_MAGIC
>  dont_measure fsmagic=0x73636673
> -measure func=FILE_MMAP mask=MAY_EXEC
> +# DEVPTS_SUPER_MAGIC
> +dont_measure fsmagic=0x1cd1
> +# BINFMTFS_MAGIC
> +dont_measure fsmagic=0x42494e4d
> +# SELINUX_MAGIC
> +dont_measure fsmagic=0xf97cff8c
> +# CGROUP_SUPER_MAGIC
> +dont_measure fsmagic=0x27e0eb
> +# NSFS_MAGIC
> +dont_measure fsmagic=0x6e736673
> +measure func=MMAP_CHECK mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=FILE_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK euid=0
> +measure func=FILE_CHECK uid=0
> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> index 04dff89..bc72d0c 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>  dont_measure fsmagic=0x01021994
>  # SECURITYFS_MAGIC
>  dnt_measure fsmagic=0x73636673
> -measure func=FILE_MMAP mask=MAY_EXEC
> +# DEVPTS_SUPER_MAGIC
> +dont_measure fsmagic=0x1cd1
> +# BINFMTFS_MAGIC
> +dont_measure fsmagic=0x42494e4d
> +# SELINUX_MAGIC
> +dont_measure fsmagic=0xf97cff8c
> +# CGROUP_SUPER_MAGIC
> +dont_measure fsmagic=0x27e0eb
> +# NSFS_MAGIC
> +dont_measure fsmagic=0x6e736673
> +measure func=MMAP_CHECK mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=FILE_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK euid=0
> +measure func=FILE_CHECK uid=0
> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 6/6] ima: Use ima tcb policy files for test
@ 2019-01-14 19:32     ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2019-01-14 19:32 UTC (permalink / raw)
  To: ltp

On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> In order to make all tests running smoothly, the policy files should
> keep up with the default ima tcb policy.

Keeping the policy rules in sync is a good idea, but some of the rules
might cause a regression with older kernels (eg. NSFS magic).  Not
including the rule, also poses a problem.

The kernel headers package includes magic.h.  One solution would be to check whether a magic name is included in magic.h.

> Especially ima_violations.sh
> expects to have a func=FILE_CHECK with mask=MAY_WRITE to trigger open
> writer and ToMtoU violations. Unfortunately, if ima_policy.sh
> which would change the system IMA policy ran before ima_violations.sh,
> ima_violations.sh would fail for sure because its prerequisite is broken.

We're not really interested in measuring files that are opened for
write.  They're changing.  The violation checking is independent of
having a measurement write rule.  Look at the
kernel ima_rdwr_violation_check().

Mimi

> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> ---
>  .../security/integrity/ima/datafiles/measure.policy     | 17 +++++++++++++++--
>  .../integrity/ima/datafiles/measure.policy-invalid      | 17 +++++++++++++++--
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> index 9976ddf..546267c 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>  dont_measure fsmagic=0x01021994
>  # SECURITYFS_MAGIC
>  dont_measure fsmagic=0x73636673
> -measure func=FILE_MMAP mask=MAY_EXEC
> +# DEVPTS_SUPER_MAGIC
> +dont_measure fsmagic=0x1cd1
> +# BINFMTFS_MAGIC
> +dont_measure fsmagic=0x42494e4d
> +# SELINUX_MAGIC
> +dont_measure fsmagic=0xf97cff8c
> +# CGROUP_SUPER_MAGIC
> +dont_measure fsmagic=0x27e0eb
> +# NSFS_MAGIC
> +dont_measure fsmagic=0x6e736673
> +measure func=MMAP_CHECK mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=FILE_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK euid=0
> +measure func=FILE_CHECK uid=0
> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> index 04dff89..bc72d0c 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>  dont_measure fsmagic=0x01021994
>  # SECURITYFS_MAGIC
>  dnt_measure fsmagic=0x73636673
> -measure func=FILE_MMAP mask=MAY_EXEC
> +# DEVPTS_SUPER_MAGIC
> +dont_measure fsmagic=0x1cd1
> +# BINFMTFS_MAGIC
> +dont_measure fsmagic=0x42494e4d
> +# SELINUX_MAGIC
> +dont_measure fsmagic=0xf97cff8c
> +# CGROUP_SUPER_MAGIC
> +dont_measure fsmagic=0x27e0eb
> +# NSFS_MAGIC
> +dont_measure fsmagic=0x6e736673
> +measure func=MMAP_CHECK mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=FILE_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK euid=0
> +measure func=FILE_CHECK uid=0
> +measure func=MODULE_CHECK
> +measure func=FIRMWARE_CHECK


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7
  2019-01-07  2:26   ` [LTP] " Jia Zhang
@ 2019-01-14 20:32     ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2019-01-14 20:32 UTC (permalink / raw)
  To: Jia Zhang, zohar, pvorel; +Cc: linux-integrity, ltp

On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> The boot aggragate calculation should never touch PCRs beyond PCR 0-7,
> even a PCR extension really manipulates out-of-domain PCRs.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

Thanks!

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>


> ---
>  .../security/integrity/ima/src/ima_boot_aggregate.c       | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> index 67be6a7..98893b9 100644
> --- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> +++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> @@ -93,11 +93,16 @@ int main(int argc, char *argv[])
>  			printf("%03u ", event.header.pcr);
>  			display_sha1_digest(event.header.digest);
>  		}
> -		SHA1_Init(&c);
> -		SHA1_Update(&c, pcr[event.header.pcr].digest,
> -			    SHA_DIGEST_LENGTH);
> -		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
> -		SHA1_Final(pcr[event.header.pcr].digest, &c);
> +
> +		if (event.header.pcr < NUM_PCRS) {
> +			SHA1_Init(&c);
> +			SHA1_Update(&c, pcr[event.header.pcr].digest,
> +				    SHA_DIGEST_LENGTH);
> +			SHA1_Update(&c, event.header.digest,
> +				    SHA_DIGEST_LENGTH);
> +			SHA1_Final(pcr[event.header.pcr].digest, &c);
> +		}
> +
>  #if MAX_EVENT_DATA_SIZE < USHRT_MAX
>  		if (event.header.len > MAX_EVENT_DATA_SIZE) {
>  			printf("Error event too long\n");


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7
@ 2019-01-14 20:32     ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2019-01-14 20:32 UTC (permalink / raw)
  To: ltp

On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> The boot aggragate calculation should never touch PCRs beyond PCR 0-7,
> even a PCR extension really manipulates out-of-domain PCRs.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

Thanks!

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>


> ---
>  .../security/integrity/ima/src/ima_boot_aggregate.c       | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> index 67be6a7..98893b9 100644
> --- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> +++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> @@ -93,11 +93,16 @@ int main(int argc, char *argv[])
>  			printf("%03u ", event.header.pcr);
>  			display_sha1_digest(event.header.digest);
>  		}
> -		SHA1_Init(&c);
> -		SHA1_Update(&c, pcr[event.header.pcr].digest,
> -			    SHA_DIGEST_LENGTH);
> -		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
> -		SHA1_Final(pcr[event.header.pcr].digest, &c);
> +
> +		if (event.header.pcr < NUM_PCRS) {
> +			SHA1_Init(&c);
> +			SHA1_Update(&c, pcr[event.header.pcr].digest,
> +				    SHA_DIGEST_LENGTH);
> +			SHA1_Update(&c, event.header.digest,
> +				    SHA_DIGEST_LENGTH);
> +			SHA1_Final(pcr[event.header.pcr].digest, &c);
> +		}
> +
>  #if MAX_EVENT_DATA_SIZE < USHRT_MAX
>  		if (event.header.len > MAX_EVENT_DATA_SIZE) {
>  			printf("Error event too long\n");


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash
  2019-01-07  2:26   ` [LTP] " Jia Zhang
@ 2019-01-14 20:33     ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2019-01-14 20:33 UTC (permalink / raw)
  To: Jia Zhang, pvorel; +Cc: linux-integrity, ltp

On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> Instead, use SHA_DIGEST_LENGTH.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> index d85d222..67be6a7 100644
> --- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> +++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> @@ -42,7 +42,7 @@ static void display_sha1_digest(unsigned char *pcr)
>  {
>  	int i;
> 
> -	for (i = 0; i < 20; i++)
> +	for (i = 0; i < SHA_DIGEST_LENGTH; i++)
>  		printf("%02x", *(pcr + i) & 0xff);
>  	printf("\n");
>  }
> @@ -94,8 +94,9 @@ int main(int argc, char *argv[])
>  			display_sha1_digest(event.header.digest);
>  		}
>  		SHA1_Init(&c);
> -		SHA1_Update(&c, pcr[event.header.pcr].digest, 20);
> -		SHA1_Update(&c, event.header.digest, 20);
> +		SHA1_Update(&c, pcr[event.header.pcr].digest,
> +			    SHA_DIGEST_LENGTH);
> +		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
>  		SHA1_Final(pcr[event.header.pcr].digest, &c);
>  #if MAX_EVENT_DATA_SIZE < USHRT_MAX
>  		if (event.header.len > MAX_EVENT_DATA_SIZE) {
> @@ -116,7 +117,7 @@ int main(int argc, char *argv[])
>  			printf("PCR-%2.2x: ", i);
>  			display_sha1_digest(pcr[i].digest);
>  		}
> -		SHA1_Update(&c, pcr[i].digest, 20);
> +		SHA1_Update(&c, pcr[i].digest, SHA_DIGEST_LENGTH);
>  	}
>  	SHA1_Final(boot_aggregate, &c);
> 


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash
@ 2019-01-14 20:33     ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2019-01-14 20:33 UTC (permalink / raw)
  To: ltp

On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> Instead, use SHA_DIGEST_LENGTH.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> index d85d222..67be6a7 100644
> --- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> +++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
> @@ -42,7 +42,7 @@ static void display_sha1_digest(unsigned char *pcr)
>  {
>  	int i;
> 
> -	for (i = 0; i < 20; i++)
> +	for (i = 0; i < SHA_DIGEST_LENGTH; i++)
>  		printf("%02x", *(pcr + i) & 0xff);
>  	printf("\n");
>  }
> @@ -94,8 +94,9 @@ int main(int argc, char *argv[])
>  			display_sha1_digest(event.header.digest);
>  		}
>  		SHA1_Init(&c);
> -		SHA1_Update(&c, pcr[event.header.pcr].digest, 20);
> -		SHA1_Update(&c, event.header.digest, 20);
> +		SHA1_Update(&c, pcr[event.header.pcr].digest,
> +			    SHA_DIGEST_LENGTH);
> +		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
>  		SHA1_Final(pcr[event.header.pcr].digest, &c);
>  #if MAX_EVENT_DATA_SIZE < USHRT_MAX
>  		if (event.header.len > MAX_EVENT_DATA_SIZE) {
> @@ -116,7 +117,7 @@ int main(int argc, char *argv[])
>  			printf("PCR-%2.2x: ", i);
>  			display_sha1_digest(pcr[i].digest);
>  		}
> -		SHA1_Update(&c, pcr[i].digest, 20);
> +		SHA1_Update(&c, pcr[i].digest, SHA_DIGEST_LENGTH);
>  	}
>  	SHA1_Final(boot_aggregate, &c);
> 


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 4/6] ima: Code cleanup
  2019-01-07  2:26   ` [LTP] " Jia Zhang
@ 2019-01-14 21:09     ` Mimi Zohar
  -1 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2019-01-14 21:09 UTC (permalink / raw)
  To: Jia Zhang, zohar, pvorel; +Cc: linux-integrity, ltp

On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> - Don't use the legacy policy function name in policy files.

The name change is from PATH_CHECK to FILE_CHECK, nothing to do with
"policy".

> - Use the variable IMA_POLICY instead of hard code path.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

After removing the word "policy" above,

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
>  testcases/kernel/security/integrity/ima/policy/measure.policy         | 2 +-
>  testcases/kernel/security/integrity/ima/policy/measure.policy-invalid | 2 +-
>  testcases/kernel/security/integrity/ima/tests/ima_policy.sh           | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy b/testcases/kernel/security/integrity/ima/policy/measure.policy
> index c68e722..9976ddf 100644
> --- a/testcases/kernel/security/integrity/ima/policy/measure.policy
> +++ b/testcases/kernel/security/integrity/ima/policy/measure.policy
> @@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
>  dont_measure fsmagic=0x73636673
>  measure func=FILE_MMAP mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=PATH_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK mask=MAY_READ uid=0
> diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
> index e406757..04dff89 100644
> --- a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
> +++ b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
> @@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
>  dnt_measure fsmagic=0x73636673
>  measure func=FILE_MMAP mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=PATH_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK mask=MAY_READ uid=0
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> index 64aa8cb..a0c7869 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> @@ -28,7 +28,7 @@ check_policy_writable()
>  {
>  	local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
> 
> -	[ -f /sys/kernel/security/ima/policy ] || tst_brk TCONF "$err"
> +	[ -f $IMA_POLICY ] || tst_brk TCONF "$err"
>  	# CONFIG_IMA_READ_POLICY
>  	echo "" 2> log > $IMA_POLICY
>  	grep -q "Device or resource busy" log && tst_brk TCONF "$err"


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 4/6] ima: Code cleanup
@ 2019-01-14 21:09     ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2019-01-14 21:09 UTC (permalink / raw)
  To: ltp

On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> - Don't use the legacy policy function name in policy files.

The name change is from PATH_CHECK to FILE_CHECK, nothing to do with
"policy".

> - Use the variable IMA_POLICY instead of hard code path.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

After removing the word "policy" above,

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
>  testcases/kernel/security/integrity/ima/policy/measure.policy         | 2 +-
>  testcases/kernel/security/integrity/ima/policy/measure.policy-invalid | 2 +-
>  testcases/kernel/security/integrity/ima/tests/ima_policy.sh           | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy b/testcases/kernel/security/integrity/ima/policy/measure.policy
> index c68e722..9976ddf 100644
> --- a/testcases/kernel/security/integrity/ima/policy/measure.policy
> +++ b/testcases/kernel/security/integrity/ima/policy/measure.policy
> @@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
>  dont_measure fsmagic=0x73636673
>  measure func=FILE_MMAP mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=PATH_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK mask=MAY_READ uid=0
> diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
> index e406757..04dff89 100644
> --- a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
> +++ b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
> @@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
>  dnt_measure fsmagic=0x73636673
>  measure func=FILE_MMAP mask=MAY_EXEC
>  measure func=BPRM_CHECK mask=MAY_EXEC
> -measure func=PATH_CHECK mask=MAY_READ uid=0
> +measure func=FILE_CHECK mask=MAY_READ uid=0
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> index 64aa8cb..a0c7869 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
> @@ -28,7 +28,7 @@ check_policy_writable()
>  {
>  	local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
> 
> -	[ -f /sys/kernel/security/ima/policy ] || tst_brk TCONF "$err"
> +	[ -f $IMA_POLICY ] || tst_brk TCONF "$err"
>  	# CONFIG_IMA_READ_POLICY
>  	echo "" 2> log > $IMA_POLICY
>  	grep -q "Device or resource busy" log && tst_brk TCONF "$err"


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] ima: Use ima tcb policy files for test
  2019-01-14 19:32     ` [LTP] " Mimi Zohar
@ 2019-01-15  1:12       ` Jia Zhang
  -1 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-15  1:12 UTC (permalink / raw)
  To: Mimi Zohar, zohar, pvorel; +Cc: linux-integrity, ltp



On 2019/1/15 上午3:32, Mimi Zohar wrote:
> On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
>> In order to make all tests running smoothly, the policy files should
>> keep up with the default ima tcb policy.
> 
> Keeping the policy rules in sync is a good idea, but some of the rules
> might cause a regression with older kernels (eg. NSFS magic).  Not
> including the rule, also poses a problem.
> 
> The kernel headers package includes magic.h.  One solution would be to check whether a magic name is included in magic.h.
> 
>> Especially ima_violations.sh
>> expects to have a func=FILE_CHECK with mask=MAY_WRITE to trigger open
>> writer and ToMtoU violations. Unfortunately, if ima_policy.sh
>> which would change the system IMA policy ran before ima_violations.sh,
>> ima_violations.sh would fail for sure because its prerequisite is broken.
> 
> We're not really interested in measuring files that are opened for
> write.  They're changing.  The violation checking is independent of
> having a measurement write rule.  Look at the
> kernel ima_rdwr_violation_check().

Thanks for the commits. I will drop this patch in V2.

Jia

> 
> Mimi
> 
>>
>> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
>> ---
>>  .../security/integrity/ima/datafiles/measure.policy     | 17 +++++++++++++++--
>>  .../integrity/ima/datafiles/measure.policy-invalid      | 17 +++++++++++++++--
>>  2 files changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
>> index 9976ddf..546267c 100644
>> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy
>> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
>> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>>  dont_measure fsmagic=0x01021994
>>  # SECURITYFS_MAGIC
>>  dont_measure fsmagic=0x73636673
>> -measure func=FILE_MMAP mask=MAY_EXEC
>> +# DEVPTS_SUPER_MAGIC
>> +dont_measure fsmagic=0x1cd1
>> +# BINFMTFS_MAGIC
>> +dont_measure fsmagic=0x42494e4d
>> +# SELINUX_MAGIC
>> +dont_measure fsmagic=0xf97cff8c
>> +# CGROUP_SUPER_MAGIC
>> +dont_measure fsmagic=0x27e0eb
>> +# NSFS_MAGIC
>> +dont_measure fsmagic=0x6e736673
>> +measure func=MMAP_CHECK mask=MAY_EXEC
>>  measure func=BPRM_CHECK mask=MAY_EXEC
>> -measure func=FILE_CHECK mask=MAY_READ uid=0
>> +measure func=FILE_CHECK euid=0
>> +measure func=FILE_CHECK uid=0
>> +measure func=MODULE_CHECK
>> +measure func=FIRMWARE_CHECK
>> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
>> index 04dff89..bc72d0c 100644
>> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
>> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
>> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>>  dont_measure fsmagic=0x01021994
>>  # SECURITYFS_MAGIC
>>  dnt_measure fsmagic=0x73636673
>> -measure func=FILE_MMAP mask=MAY_EXEC
>> +# DEVPTS_SUPER_MAGIC
>> +dont_measure fsmagic=0x1cd1
>> +# BINFMTFS_MAGIC
>> +dont_measure fsmagic=0x42494e4d
>> +# SELINUX_MAGIC
>> +dont_measure fsmagic=0xf97cff8c
>> +# CGROUP_SUPER_MAGIC
>> +dont_measure fsmagic=0x27e0eb
>> +# NSFS_MAGIC
>> +dont_measure fsmagic=0x6e736673
>> +measure func=MMAP_CHECK mask=MAY_EXEC
>>  measure func=BPRM_CHECK mask=MAY_EXEC
>> -measure func=FILE_CHECK mask=MAY_READ uid=0
>> +measure func=FILE_CHECK euid=0
>> +measure func=FILE_CHECK uid=0
>> +measure func=MODULE_CHECK
>> +measure func=FIRMWARE_CHECK

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 6/6] ima: Use ima tcb policy files for test
@ 2019-01-15  1:12       ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-15  1:12 UTC (permalink / raw)
  To: ltp



On 2019/1/15 上午3:32, Mimi Zohar wrote:
> On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
>> In order to make all tests running smoothly, the policy files should
>> keep up with the default ima tcb policy.
> 
> Keeping the policy rules in sync is a good idea, but some of the rules
> might cause a regression with older kernels (eg. NSFS magic).  Not
> including the rule, also poses a problem.
> 
> The kernel headers package includes magic.h.  One solution would be to check whether a magic name is included in magic.h.
> 
>> Especially ima_violations.sh
>> expects to have a func=FILE_CHECK with mask=MAY_WRITE to trigger open
>> writer and ToMtoU violations. Unfortunately, if ima_policy.sh
>> which would change the system IMA policy ran before ima_violations.sh,
>> ima_violations.sh would fail for sure because its prerequisite is broken.
> 
> We're not really interested in measuring files that are opened for
> write.  They're changing.  The violation checking is independent of
> having a measurement write rule.  Look at the
> kernel ima_rdwr_violation_check().

Thanks for the commits. I will drop this patch in V2.

Jia

> 
> Mimi
> 
>>
>> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
>> ---
>>  .../security/integrity/ima/datafiles/measure.policy     | 17 +++++++++++++++--
>>  .../integrity/ima/datafiles/measure.policy-invalid      | 17 +++++++++++++++--
>>  2 files changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
>> index 9976ddf..546267c 100644
>> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy
>> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
>> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>>  dont_measure fsmagic=0x01021994
>>  # SECURITYFS_MAGIC
>>  dont_measure fsmagic=0x73636673
>> -measure func=FILE_MMAP mask=MAY_EXEC
>> +# DEVPTS_SUPER_MAGIC
>> +dont_measure fsmagic=0x1cd1
>> +# BINFMTFS_MAGIC
>> +dont_measure fsmagic=0x42494e4d
>> +# SELINUX_MAGIC
>> +dont_measure fsmagic=0xf97cff8c
>> +# CGROUP_SUPER_MAGIC
>> +dont_measure fsmagic=0x27e0eb
>> +# NSFS_MAGIC
>> +dont_measure fsmagic=0x6e736673
>> +measure func=MMAP_CHECK mask=MAY_EXEC
>>  measure func=BPRM_CHECK mask=MAY_EXEC
>> -measure func=FILE_CHECK mask=MAY_READ uid=0
>> +measure func=FILE_CHECK euid=0
>> +measure func=FILE_CHECK uid=0
>> +measure func=MODULE_CHECK
>> +measure func=FIRMWARE_CHECK
>> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
>> index 04dff89..bc72d0c 100644
>> --- a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
>> +++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
>> @@ -11,6 +11,19 @@ dont_measure fsmagic=0x64626720
>>  dont_measure fsmagic=0x01021994
>>  # SECURITYFS_MAGIC
>>  dnt_measure fsmagic=0x73636673
>> -measure func=FILE_MMAP mask=MAY_EXEC
>> +# DEVPTS_SUPER_MAGIC
>> +dont_measure fsmagic=0x1cd1
>> +# BINFMTFS_MAGIC
>> +dont_measure fsmagic=0x42494e4d
>> +# SELINUX_MAGIC
>> +dont_measure fsmagic=0xf97cff8c
>> +# CGROUP_SUPER_MAGIC
>> +dont_measure fsmagic=0x27e0eb
>> +# NSFS_MAGIC
>> +dont_measure fsmagic=0x6e736673
>> +measure func=MMAP_CHECK mask=MAY_EXEC
>>  measure func=BPRM_CHECK mask=MAY_EXEC
>> -measure func=FILE_CHECK mask=MAY_READ uid=0
>> +measure func=FILE_CHECK euid=0
>> +measure func=FILE_CHECK uid=0
>> +measure func=MODULE_CHECK
>> +measure func=FIRMWARE_CHECK

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash
  2019-01-15  2:14 [PATCH v2 " Jia Zhang
@ 2019-01-15  2:14 ` Jia Zhang
  0 siblings, 0 replies; 31+ messages in thread
From: Jia Zhang @ 2019-01-15  2:14 UTC (permalink / raw)
  To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia

Instead, use SHA_DIGEST_LENGTH.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index d85d222..67be6a7 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -42,7 +42,7 @@ static void display_sha1_digest(unsigned char *pcr)
 {
 	int i;
 
-	for (i = 0; i < 20; i++)
+	for (i = 0; i < SHA_DIGEST_LENGTH; i++)
 		printf("%02x", *(pcr + i) & 0xff);
 	printf("\n");
 }
@@ -94,8 +94,9 @@ int main(int argc, char *argv[])
 			display_sha1_digest(event.header.digest);
 		}
 		SHA1_Init(&c);
-		SHA1_Update(&c, pcr[event.header.pcr].digest, 20);
-		SHA1_Update(&c, event.header.digest, 20);
+		SHA1_Update(&c, pcr[event.header.pcr].digest,
+			    SHA_DIGEST_LENGTH);
+		SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
 		SHA1_Final(pcr[event.header.pcr].digest, &c);
 #if MAX_EVENT_DATA_SIZE < USHRT_MAX
 		if (event.header.len > MAX_EVENT_DATA_SIZE) {
@@ -116,7 +117,7 @@ int main(int argc, char *argv[])
 			printf("PCR-%2.2x: ", i);
 			display_sha1_digest(pcr[i].digest);
 		}
-		SHA1_Update(&c, pcr[i].digest, 20);
+		SHA1_Update(&c, pcr[i].digest, SHA_DIGEST_LENGTH);
 	}
 	SHA1_Final(boot_aggregate, &c);
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] ima: Use ima tcb policy files for test
  2019-01-14 19:32     ` [LTP] " Mimi Zohar
@ 2019-01-15 10:50       ` Petr Vorel
  -1 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2019-01-15 10:50 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: Jia Zhang, zohar, linux-integrity, ltp

Hi Mimi, Jia,

> On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> > In order to make all tests running smoothly, the policy files should
> > keep up with the default ima tcb policy.

> Keeping the policy rules in sync is a good idea, but some of the rules
> might cause a regression with older kernels (eg. NSFS magic).  Not
> including the rule, also poses a problem.

Mimi, you added NSFS_MAGIC into policy in v4.2 (cd025f7f9410 "ima: do not
measure or appraise the NSFS filesystem"), in the commit is Cc for 3.19, but
it's not in origin/linux-3.19.y stable tree (v3.19.8). So regression could be
from kernel <= 4.1.

> The kernel headers package includes magic.h.  One solution would be to check whether a magic name is included in magic.h.
Interesting approach, I like this approach. Policy would have to be generated on
the fly, but that shouldn't be a problem.


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH 6/6] ima: Use ima tcb policy files for test
@ 2019-01-15 10:50       ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2019-01-15 10:50 UTC (permalink / raw)
  To: ltp

Hi Mimi, Jia,

> On Mon, 2019-01-07 at 10:26 +0800, Jia Zhang wrote:
> > In order to make all tests running smoothly, the policy files should
> > keep up with the default ima tcb policy.

> Keeping the policy rules in sync is a good idea, but some of the rules
> might cause a regression with older kernels (eg. NSFS magic).  Not
> including the rule, also poses a problem.

Mimi, you added NSFS_MAGIC into policy in v4.2 (cd025f7f9410 "ima: do not
measure or appraise the NSFS filesystem"), in the commit is Cc for 3.19, but
it's not in origin/linux-3.19.y stable tree (v3.19.8). So regression could be
from kernel <= 4.1.

> The kernel headers package includes magic.h.  One solution would be to check whether a magic name is included in magic.h.
Interesting approach, I like this approach. Policy would have to be generated on
the fly, but that shouldn't be a problem.


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2019-01-15 10:50 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-07  2:26 [PATCH 0/6] LTP IMA fix bundle Jia Zhang
2019-01-07  2:26 ` [LTP] " Jia Zhang
2019-01-07  2:26 ` [PATCH 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
2019-01-07  2:26   ` [LTP] " Jia Zhang
2019-01-07  2:26 ` [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash Jia Zhang
2019-01-07  2:26   ` [LTP] " Jia Zhang
2019-01-14 20:33   ` Mimi Zohar
2019-01-14 20:33     ` [LTP] " Mimi Zohar
2019-01-07  2:26 ` [PATCH 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7 Jia Zhang
2019-01-07  2:26   ` [LTP] " Jia Zhang
2019-01-14 20:32   ` Mimi Zohar
2019-01-14 20:32     ` [LTP] " Mimi Zohar
2019-01-07  2:26 ` [PATCH 4/6] ima: Code cleanup Jia Zhang
2019-01-07  2:26   ` [LTP] " Jia Zhang
2019-01-14 21:09   ` Mimi Zohar
2019-01-14 21:09     ` [LTP] " Mimi Zohar
2019-01-07  2:26 ` [PATCH 5/6] ima: Rename the folder name for policy files to datafiles Jia Zhang
2019-01-07  2:26   ` [LTP] " Jia Zhang
2019-01-07  2:26 ` [PATCH 6/6] ima: Use ima tcb policy files for test Jia Zhang
2019-01-07  2:26   ` [LTP] " Jia Zhang
2019-01-07 15:59   ` Jia Zhang
2019-01-07 15:59     ` [LTP] " Jia Zhang
2019-01-14 19:32   ` Mimi Zohar
2019-01-14 19:32     ` [LTP] " Mimi Zohar
2019-01-15  1:12     ` Jia Zhang
2019-01-15  1:12       ` [LTP] " Jia Zhang
2019-01-15 10:50     ` Petr Vorel
2019-01-15 10:50       ` [LTP] " Petr Vorel
2019-01-13 12:14 ` [PATCH 0/6] LTP IMA fix bundle Jia Zhang
2019-01-13 12:14   ` [LTP] " Jia Zhang
  -- strict thread matches above, loose matches on Subject: below --
2019-01-15  2:14 [PATCH v2 " Jia Zhang
2019-01-15  2:14 ` [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash Jia Zhang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.