linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [LTP][PATCH v3 0/6] LTP IMA fix bundle
@ 2019-01-16  2:57 Jia Zhang
  2019-01-16  2:57 ` [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Jia Zhang @ 2019-01-16  2:57 UTC (permalink / raw)
  To: pvorel, zohar; +Cc: linux-integrity, ltp, zhang.jia

Changelog since V2:
- Rework patch 6. Here is a quick diff against V2:

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/int
index a1360b8..74223c2 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -20,6 +20,7 @@
 # Test whether ToMToU and open_writer violations invalidatethe PCR and are logged.
 
 TST_SETUP="setup"
+TST_CLEANUP="cleanup"
 TST_CNT=3
 TST_NEEDS_DEVICE=1
 
@@ -46,10 +47,12 @@ setup()
        tst_res TINFO "using log $LOG"
 }
 
-reset_printk_ratelimit()
+cleanup()
 {
        [ "$PRINTK_RATE_LIMIT" != "0" ] && \
                sysctl -wq kernel.printk_ratelimit=$PRINTK_RATE_LIMIT
+
+       ima_cleanup
 }
 
 open_file_read()
@@ -163,8 +166,6 @@ test3()
 
        validate $num_violations $count $search
 
-       reset_printk_ratelimit
-
        # wait for ima_mmap to exit, so we can umount
        tst_sleep 2s
 }

The latest test results are attached:

Test Start Time: Wed Jan 16 10:44:10 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: 5.0.0-rc1+
Machine Architecture: x86_64
Hostname: test-machine

Jia


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log
  2019-01-16  2:57 [LTP][PATCH v3 0/6] LTP IMA fix bundle Jia Zhang
@ 2019-01-16  2:57 ` Jia Zhang
  2019-01-20 18:13   ` Mimi Zohar
  2019-01-20 18:37   ` Mimi Zohar
  2019-01-16  2:57 ` [PATCH v3 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash Jia Zhang
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 15+ messages in thread
From: Jia Zhang @ 2019-01-16  2:57 UTC (permalink / raw)
  To: pvorel, zohar; +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] 15+ messages in thread
* [PATCH v3 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash
  2019-01-16  2:57 [LTP][PATCH v3 0/6] LTP IMA fix bundle Jia Zhang
  2019-01-16  2:57 ` [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
@ 2019-01-16  2:57 ` Jia Zhang
  2019-01-16  2:57 ` [PATCH v3 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7 Jia Zhang
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Jia Zhang @ 2019-01-16  2:57 UTC (permalink / raw)
  To: pvorel, zohar; +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] 15+ messages in thread
* [PATCH v3 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7
  2019-01-16  2:57 [LTP][PATCH v3 0/6] LTP IMA fix bundle Jia Zhang
  2019-01-16  2:57 ` [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
  2019-01-16  2:57 ` [PATCH v3 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash Jia Zhang
@ 2019-01-16  2:57 ` Jia Zhang
  2019-01-16  2:57 ` [PATCH v3 4/6] ima: Code cleanup Jia Zhang
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Jia Zhang @ 2019-01-16  2:57 UTC (permalink / raw)
  To: pvorel, zohar; +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>
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");
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH v3 4/6] ima: Code cleanup
  2019-01-16  2:57 [LTP][PATCH v3 0/6] LTP IMA fix bundle Jia Zhang
                   ` (2 preceding siblings ...)
  2019-01-16  2:57 ` [PATCH v3 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7 Jia Zhang
@ 2019-01-16  2:57 ` Jia Zhang
  2019-01-16  2:57 ` [PATCH v3 5/6] ima: Rename the folder name for policy files to datafiles Jia Zhang
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Jia Zhang @ 2019-01-16  2:57 UTC (permalink / raw)
  To: pvorel, zohar; +Cc: linux-integrity, ltp, zhang.jia

- Change the legacy function name from PATH_CHECK to FILE_CHECK.
- Use the variable IMA_POLICY instead of hard code path.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
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"
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH v3 5/6] ima: Rename the folder name for policy files to datafiles
  2019-01-16  2:57 [LTP][PATCH v3 0/6] LTP IMA fix bundle Jia Zhang
                   ` (3 preceding siblings ...)
  2019-01-16  2:57 ` [PATCH v3 4/6] ima: Code cleanup Jia Zhang
@ 2019-01-16  2:57 ` Jia Zhang
  2019-01-23 17:04   ` Petr Vorel
  2019-01-16  2:57 ` [PATCH v3 6/6] ima/ima_violations: Temporarily remove the printk rate limit Jia Zhang
  2019-01-16  7:20 ` [LTP][PATCH v3 0/6] LTP IMA fix bundle Petr Vorel
  6 siblings, 1 reply; 15+ messages in thread
From: Jia Zhang @ 2019-01-16  2:57 UTC (permalink / raw)
  To: pvorel, zohar; +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] 15+ messages in thread
* [PATCH v3 6/6] ima/ima_violations: Temporarily remove the printk rate limit
  2019-01-16  2:57 [LTP][PATCH v3 0/6] LTP IMA fix bundle Jia Zhang
                   ` (4 preceding siblings ...)
  2019-01-16  2:57 ` [PATCH v3 5/6] ima: Rename the folder name for policy files to datafiles Jia Zhang
@ 2019-01-16  2:57 ` Jia Zhang
  2019-01-20 18:38   ` Mimi Zohar
  2019-01-16  7:20 ` [LTP][PATCH v3 0/6] LTP IMA fix bundle Petr Vorel
  6 siblings, 1 reply; 15+ messages in thread
From: Jia Zhang @ 2019-01-16  2:57 UTC (permalink / raw)
  To: pvorel, zohar; +Cc: linux-integrity, ltp, zhang.jia

The output frequency of audit log is limited by printk_ratelimit()
in kernel if auditd not used. Thus, the test cases heavily depending
on searching certain keywords in log file may fail if the matching
patterns are exactly suppressed by printk_ratelimit().

In order to fix such a sort of failure, just temporarily remove the
printk rate limit, and restore its original setting when doing
cleanup.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 .../kernel/security/integrity/ima/tests/ima_setup.sh      |  2 +-
 .../kernel/security/integrity/ima/tests/ima_violations.sh | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index 6dfb4d2..fe60981 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -20,7 +20,7 @@
 TST_TESTFUNC="test"
 TST_SETUP_CALLER="$TST_SETUP"
 TST_SETUP="ima_setup"
-TST_CLEANUP="ima_cleanup"
+TST_CLEANUP="${TST_CLEANUP:-ima_cleanup}"
 TST_NEEDS_TMPDIR=1
 TST_NEEDS_ROOT=1
 
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index f3f40d4..74223c2 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -20,6 +20,7 @@
 # Test whether ToMToU and open_writer violations invalidatethe PCR and are logged.
 
 TST_SETUP="setup"
+TST_CLEANUP="cleanup"
 TST_CNT=3
 TST_NEEDS_DEVICE=1
 
@@ -31,15 +32,29 @@ setup()
 	FILE="test.txt"
 	IMA_VIOLATIONS="$SECURITYFS/ima/violations"
 	LOG="/var/log/messages"
+	PRINTK_RATE_LIMIT="0"
 
 	if status_daemon auditd; then
 		LOG="/var/log/audit/audit.log"
+	else
+		tst_check_cmds sysctl
+
+		PRINTK_RATE_LIMIT=`sysctl -n kernel.printk_ratelimit`
+		sysctl -wq kernel.printk_ratelimit=0
 	fi
 	[ -f "$LOG" ] || \
 		tst_brk TBROK "log $LOG does not exist (bug in detection?)"
 	tst_res TINFO "using log $LOG"
 }
 
+cleanup()
+{
+	[ "$PRINTK_RATE_LIMIT" != "0" ] && \
+		sysctl -wq kernel.printk_ratelimit=$PRINTK_RATE_LIMIT
+
+	ima_cleanup
+}
+
 open_file_read()
 {
 	exec 3< $FILE || exit 1
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [LTP][PATCH v3 0/6] LTP IMA fix bundle
  2019-01-16  2:57 [LTP][PATCH v3 0/6] LTP IMA fix bundle Jia Zhang
                   ` (5 preceding siblings ...)
  2019-01-16  2:57 ` [PATCH v3 6/6] ima/ima_violations: Temporarily remove the printk rate limit Jia Zhang
@ 2019-01-16  7:20 ` Petr Vorel
  2019-01-20 14:59   ` Jia Zhang
  2019-01-24  2:19   ` Jia Zhang
  6 siblings, 2 replies; 15+ messages in thread
From: Petr Vorel @ 2019-01-16  7:20 UTC (permalink / raw)
  To: Jia Zhang; +Cc: zohar, linux-integrity, ltp

Hi Jia, Mimi,

> Changelog since V2:
> - Rework patch 6. Here is a quick diff against V2:

LGTM, just waiting if Mimi reviews 1st and 6th commit.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [LTP][PATCH v3 0/6] LTP IMA fix bundle
  2019-01-16  7:20 ` [LTP][PATCH v3 0/6] LTP IMA fix bundle Petr Vorel
@ 2019-01-20 14:59   ` Jia Zhang
  2019-01-24  2:19   ` Jia Zhang
  1 sibling, 0 replies; 15+ messages in thread
From: Jia Zhang @ 2019-01-20 14:59 UTC (permalink / raw)
  To: Petr Vorel; +Cc: zohar, linux-integrity, ltp

Hi Mini,

Could you help to take a look at this V3 patch series?

Thanks,
Jia

On 2019/1/16 下午3:20, Petr Vorel wrote:
> Hi Jia, Mimi,
> 
>> Changelog since V2:
>> - Rework patch 6. Here is a quick diff against V2:
> 
> LGTM, just waiting if Mimi reviews 1st and 6th commit.
> 
> Kind regards,
> Petr
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log
  2019-01-16  2:57 ` [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
@ 2019-01-20 18:13   ` Mimi Zohar
  2019-01-20 18:37   ` Mimi Zohar
  1 sibling, 0 replies; 15+ messages in thread
From: Mimi Zohar @ 2019-01-20 18:13 UTC (permalink / raw)
  To: Jia Zhang, pvorel; +Cc: linux-integrity, ltp

On Wed, 2019-01-16 at 10:57 +0800, Jia Zhang wrote:
> 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>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.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 {


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log
  2019-01-16  2:57 ` [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
  2019-01-20 18:13   ` Mimi Zohar
@ 2019-01-20 18:37   ` Mimi Zohar
  1 sibling, 0 replies; 15+ messages in thread
From: Mimi Zohar @ 2019-01-20 18:37 UTC (permalink / raw)
  To: Jia Zhang, pvorel; +Cc: linux-integrity, ltp

On Wed, 2019-01-16 at 10:57 +0800, Jia Zhang wrote:
> 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>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.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 {


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v3 6/6] ima/ima_violations: Temporarily remove the printk rate limit
  2019-01-16  2:57 ` [PATCH v3 6/6] ima/ima_violations: Temporarily remove the printk rate limit Jia Zhang
@ 2019-01-20 18:38   ` Mimi Zohar
  0 siblings, 0 replies; 15+ messages in thread
From: Mimi Zohar @ 2019-01-20 18:38 UTC (permalink / raw)
  To: Jia Zhang, pvorel; +Cc: linux-integrity, ltp

On Wed, 2019-01-16 at 10:57 +0800, Jia Zhang wrote:
> The output frequency of audit log is limited by printk_ratelimit()
> in kernel if auditd not used. Thus, the test cases heavily depending
> on searching certain keywords in log file may fail if the matching
> patterns are exactly suppressed by printk_ratelimit().
> 
> In order to fix such a sort of failure, just temporarily remove the
> printk rate limit, and restore its original setting when doing
> cleanup.
> 
> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

Thanks, I wasn't aware of the sysctl.  If the message isn't in
/var/log/messages or /var/log/audit/audit.log, do we now need to also
check journalctl?

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

> ---
>  .../kernel/security/integrity/ima/tests/ima_setup.sh      |  2 +-
>  .../kernel/security/integrity/ima/tests/ima_violations.sh | 15 +++++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> index 6dfb4d2..fe60981 100644
> --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> @@ -20,7 +20,7 @@
>  TST_TESTFUNC="test"
>  TST_SETUP_CALLER="$TST_SETUP"
>  TST_SETUP="ima_setup"
> -TST_CLEANUP="ima_cleanup"
> +TST_CLEANUP="${TST_CLEANUP:-ima_cleanup}"
>  TST_NEEDS_TMPDIR=1
>  TST_NEEDS_ROOT=1
>  
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> index f3f40d4..74223c2 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> @@ -20,6 +20,7 @@
>  # Test whether ToMToU and open_writer violations invalidatethe PCR and are logged.
>  
>  TST_SETUP="setup"
> +TST_CLEANUP="cleanup"
>  TST_CNT=3
>  TST_NEEDS_DEVICE=1
>  
> @@ -31,15 +32,29 @@ setup()
>  	FILE="test.txt"
>  	IMA_VIOLATIONS="$SECURITYFS/ima/violations"
>  	LOG="/var/log/messages"
> +	PRINTK_RATE_LIMIT="0"
>  
>  	if status_daemon auditd; then
>  		LOG="/var/log/audit/audit.log"
> +	else
> +		tst_check_cmds sysctl
> +
> +		PRINTK_RATE_LIMIT=`sysctl -n kernel.printk_ratelimit`
> +		sysctl -wq kernel.printk_ratelimit=0
>  	fi
>  	[ -f "$LOG" ] || \
>  		tst_brk TBROK "log $LOG does not exist (bug in detection?)"
>  	tst_res TINFO "using log $LOG"
>  }
>  
> +cleanup()
> +{
> +	[ "$PRINTK_RATE_LIMIT" != "0" ] && \
> +		sysctl -wq kernel.printk_ratelimit=$PRINTK_RATE_LIMIT
> +
> +	ima_cleanup
> +}
> +
>  open_file_read()
>  {
>  	exec 3< $FILE || exit 1


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v3 5/6] ima: Rename the folder name for policy files to datafiles
  2019-01-16  2:57 ` [PATCH v3 5/6] ima: Rename the folder name for policy files to datafiles Jia Zhang
@ 2019-01-23 17:04   ` Petr Vorel
  2019-01-24  5:11     ` Jia Zhang
  0 siblings, 1 reply; 15+ messages in thread
From: Petr Vorel @ 2019-01-23 17:04 UTC (permalink / raw)
  To: Jia Zhang; +Cc: zohar, linux-integrity, ltp

Hi Mimi, 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>
> ---

I wasn't sure about this one as make install isn't that hard to do even during
debugging and policy is more descriptive. But other tests use datafiles as well,
therefore taken as well.

Whole patchset merged, thank you both for your work.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [LTP][PATCH v3 0/6] LTP IMA fix bundle
  2019-01-16  7:20 ` [LTP][PATCH v3 0/6] LTP IMA fix bundle Petr Vorel
  2019-01-20 14:59   ` Jia Zhang
@ 2019-01-24  2:19   ` Jia Zhang
  1 sibling, 0 replies; 15+ messages in thread
From: Jia Zhang @ 2019-01-24  2:19 UTC (permalink / raw)
  To: Petr Vorel; +Cc: zohar, linux-integrity, ltp

Hi Petr,

Is it good enough to merge this patch series?

Thanks,
Jia

On 2019/1/16 下午3:20, Petr Vorel wrote:
> Hi Jia, Mimi,
> 
>> Changelog since V2:
>> - Rework patch 6. Here is a quick diff against V2:
> 
> LGTM, just waiting if Mimi reviews 1st and 6th commit.
> 
> Kind regards,
> Petr
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v3 5/6] ima: Rename the folder name for policy files to datafiles
  2019-01-23 17:04   ` Petr Vorel
@ 2019-01-24  5:11     ` Jia Zhang
  0 siblings, 0 replies; 15+ messages in thread
From: Jia Zhang @ 2019-01-24  5:11 UTC (permalink / raw)
  To: Petr Vorel; +Cc: zohar, linux-integrity, ltp



On 2019/1/24 上午1:04, Petr Vorel wrote:
> Hi Mimi, 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>
>> ---
> 
> I wasn't sure about this one as make install isn't that hard to do even during
> debugging and policy is more descriptive. But other tests use datafiles as well,
> therefore taken as well.

Actually, this patch resolves the issue if we just follow this step to
run IMA-only test:

$ git clone https://github.com/linux-test-project/ltp.git
$ cd ltp
$ export LTP_ROOT="$PWD"
$ make autotools
$ ./configure
$ cd testcases/lib
$ make
$ cd ../kernel/security/integrity/ima
$ make
$ sudo PATH=$LTP_ROOT/testcases/lib:tests:$PATH ./tests/ima_policy.sh

A full LTP installation doesn't have this issue.

Jia

> 
> Whole patchset merged, thank you both for your work.
> 
> Kind regards,
> Petr
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-01-24  5:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-16  2:57 [LTP][PATCH v3 0/6] LTP IMA fix bundle Jia Zhang
2019-01-16  2:57 ` [PATCH v3 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
2019-01-20 18:13   ` Mimi Zohar
2019-01-20 18:37   ` Mimi Zohar
2019-01-16  2:57 ` [PATCH v3 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash Jia Zhang
2019-01-16  2:57 ` [PATCH v3 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7 Jia Zhang
2019-01-16  2:57 ` [PATCH v3 4/6] ima: Code cleanup Jia Zhang
2019-01-16  2:57 ` [PATCH v3 5/6] ima: Rename the folder name for policy files to datafiles Jia Zhang
2019-01-23 17:04   ` Petr Vorel
2019-01-24  5:11     ` Jia Zhang
2019-01-16  2:57 ` [PATCH v3 6/6] ima/ima_violations: Temporarily remove the printk rate limit Jia Zhang
2019-01-20 18:38   ` Mimi Zohar
2019-01-16  7:20 ` [LTP][PATCH v3 0/6] LTP IMA fix bundle Petr Vorel
2019-01-20 14:59   ` Jia Zhang
2019-01-24  2:19   ` Jia Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).