Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] IMA: Add a test to verify importing a certificate into custom keyring
@ 2020-07-17 20:57 Lachlan Sneff
  2020-07-22 20:33 ` Mimi Zohar
  2020-07-31  8:02 ` Petr Vorel
  0 siblings, 2 replies; 4+ messages in thread
From: Lachlan Sneff @ 2020-07-17 20:57 UTC (permalink / raw)
  To: zohar, pvorel, ltp; +Cc: nramas, balajib, linux-integrity

A test for verifying importing an x509 certificate into a keyring and
validating the key measurement performed by IMA is needed.

Add an IMA measurement test that verifies that an x509 certificate
can be imported into a custom keyring and measured correctly.

A certificate used by the test is included in the `datafiles/keys` directory.

Using a custom keyring for the test is preferred over using a pre-existing/built-in keyring,
such as `.ima`, since there may be restrictions to importing any cert into a built-in keyring.
For example, the `.ima` keyring requires that the imported certs be signed by a kernel private
key in certain kernel configurations.

Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com>
---
 .../kernel/security/integrity/ima/README.md   |  20 +++-----
 .../security/integrity/ima/datafiles/Makefile |  11 ++---
 .../integrity/ima/datafiles/keys/Makefile     |  31 ++++++++++++
 .../integrity/ima/datafiles/keys/x509.der     | Bin 0 -> 784 bytes
 .../integrity/ima/datafiles/policy/Makefile   |  31 ++++++++++++
 .../datafiles/{ => policy}/keycheck.policy    |   0
 .../ima/datafiles/{ => policy}/measure.policy |   0
 .../{ => policy}/measure.policy-invalid       |   0
 .../security/integrity/ima/tests/ima_keys.sh  |  45 +++++++++++++++++-
 9 files changed, 117 insertions(+), 21 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/keys/Makefile
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/keys/x509.der
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/policy/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => policy}/keycheck.policy (100%)
 rename testcases/kernel/security/integrity/ima/datafiles/{ => policy}/measure.policy (100%)
 rename testcases/kernel/security/integrity/ima/datafiles/{ => policy}/measure.policy-invalid (100%)

diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md
index 732cd912f..db8819a99 100644
--- a/testcases/kernel/security/integrity/ima/README.md
+++ b/testcases/kernel/security/integrity/ima/README.md
@@ -16,25 +16,19 @@ space, may contain equivalent measurement tcb rules, detecting them would
 require `IMA_READ_POLICY=y` therefore ignore this option.
 
 ### IMA key import test
-`ima_keys.sh` requires a x509 public key, by default in `/etc/keys/x509_ima.der`
-(defined in `CONFIG_IMA_X509_PATH` kernel config option).
-The key must be signed by the private key you generate. Follow these instructions:
-https://manpages.ubuntu.com/manpages/disco/man1/evmctl.1.html#generate%20trusted%20keys
 
-The test cannot be set-up automatically because the x509 public key must be
-built into the kernel and loaded onto a trusted keyring
-(e.g. `.builtin_trusted_keys`, `.secondary_trusted_keyring`).
-
-As well as what's required for the IMA tests, the following are also required
+As well as what's required for the IMA tests, the following is also required
 in the kernel configuration:
 ```
 CONFIG_IMA_READ_POLICY=y
-CONFIG_IMA_X509_PATH="/etc/keys/x509_ima.der"
-CONFIG_SYSTEM_TRUSTED_KEYRING=y
-CONFIG_SYSTEM_TRUSTED_KEYS="/etc/keys/ima-local-ca.pem"
 ```
 
-Test also requires loaded policy with `func=KEY_CHECK`, see example in `keycheck.policy`.
+You must also add the following line to /etc/ima/ima-policy and reboot.
+```
+measure func=KEY_CHECK keyrings=key_import_test template=ima-buf
+```
+
+The test also requires loaded policy with `func=KEY_CHECK`, see example in `keycheck.policy`.
 
 ## EVM tests
 
diff --git a/testcases/kernel/security/integrity/ima/datafiles/Makefile b/testcases/kernel/security/integrity/ima/datafiles/Makefile
index a960f9d84..5261b9321 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/Makefile
+++ b/testcases/kernel/security/integrity/ima/datafiles/Makefile
@@ -1,5 +1,5 @@
 #
-#    testcases/kernel/security/integrity/ima/policy testcases Makefile.
+#    kernel/security/integrity/ima testcase Makefile.
 #
 #    Copyright (C) 2009, Cisco Systems Inc.
 #
@@ -18,14 +18,13 @@
 #    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
 # Ngie Cooper, July 2009
+# Lachlan Sneff, July 2020
 #
 
 top_srcdir		?= ../../../../../..
 
-include	$(top_srcdir)/include/mk/env_pre.mk
+include $(top_srcdir)/include/mk/env_pre.mk
 
-INSTALL_DIR		:= testcases/data/ima_policy
+SUBDIRS			:= policy keys
 
-INSTALL_TARGETS		:= measure*
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
+include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/keys/Makefile b/testcases/kernel/security/integrity/ima/datafiles/keys/Makefile
new file mode 100644
index 000000000..3e381089c
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/keys/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.
+#
+# Lachlan Sneff, July 2020
+#
+
+top_srcdir		?= ../../../../../../..
+
+include	$(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_DIR		:= testcases/data/ima_keys
+
+INSTALL_TARGETS		:= x509.der
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/keys/x509.der b/testcases/kernel/security/integrity/ima/datafiles/keys/x509.der
new file mode 100644
index 0000000000000000000000000000000000000000..538611c7244eac17377031038a97bf5d02c53d3f
GIT binary patch
literal 784
zcmXqLV&*YuV*IjznTe5!Nkm*%!l+fPYOa%w{pGeOu7W)g|5Ocl**LY@JlekVGBR?r
zG8lv!@&kpKLs__Z*gSn5b)6jzH4M~10?a(}KmmQ%Fkgk_)S{Biw9Mqhl2nD_%=Em>
zymW=^)Jj7M12MR1oQz_e$%zh`xrw^Ti3W1wyha8Frbd>ArbZS<mQg^iArjXhz);YD
zA7U*>Mt*TgUSe*lp|pV{#A2cHjQqshOccB13}le);)f`4fN3^poR91^Mpg#q#$E=4
z#!jZjMuvqS<2&d4={o*uW**0h|9T%Tw9S&_xXFEO!9t#YW*25;x2$<5W-0eWw?OBy
z^v@jizdt`D%q<d|ReAl{bE_$0%^l11#6PRp?#-6J7PfX<qx=M^6KT?qw?*WN1?*@I
z%Cqp^b9sffLXO?miT~#3|NU~~-j<6EKTciX{lAO7bFKR(@pL9;Mh3>ku?EowJU|c1
z3NtePXJIm6FyIF9_*qz(nb;c)WI-H07BLo)gzIHKKP^@H&nbIpE!y9-E`Uo=(m)<0
zt;_-p@&=LZk_VQ~bi1@pYvM`%j}JQk-GBc`6502_h-GeKWMs(U%{uxsJ8V$|&s^U*
zuT)C?`L3+KeqYmBdG>)zfpWi7n6jqq>YI1K{O3*;XK_o1>WKamk9bck{$vuzdw37y
zvA`0c_d5A+eCHjLThIIJa@MIAVc(m-U;nXmbAi*F0<FyFA#2;;#Isxf2xngNvA$<=
z$CMi7RW8Ag|Jk3|uI6~;sc!ZA<5Qn=%P&bzdui}MisQ9MYhlhY|F7HrKV4$gX6yNt
zC8pkE<;LwsPD}g`oc<&~`(D3i*QEZk`);XW=B4u<{+H1Ux#t$eaj2M|C+nQUOz~ec
zU5)v!tl*ZMaObGw{B>uG+jphhdu=ZLZTh9(PmEsIm`5}$d$_dpOSj%iU{I%d0|3@i
BE~)?k

literal 0
HcmV?d00001

diff --git a/testcases/kernel/security/integrity/ima/datafiles/policy/Makefile b/testcases/kernel/security/integrity/ima/datafiles/policy/Makefile
new file mode 100644
index 000000000..692bf61ab
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/policy/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/keycheck.policy b/testcases/kernel/security/integrity/ima/datafiles/policy/keycheck.policy
similarity index 100%
rename from testcases/kernel/security/integrity/ima/datafiles/keycheck.policy
rename to testcases/kernel/security/integrity/ima/datafiles/policy/keycheck.policy
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/policy/measure.policy
similarity index 100%
rename from testcases/kernel/security/integrity/ima/datafiles/measure.policy
rename to testcases/kernel/security/integrity/ima/datafiles/policy/measure.policy
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/policy/measure.policy-invalid
similarity index 100%
rename from testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
rename to testcases/kernel/security/integrity/ima/datafiles/policy/measure.policy-invalid
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
index 904b7515b..101d2ea92 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
@@ -5,8 +5,8 @@
 #
 # Verify that keys are measured correctly based on policy.
 
-TST_NEEDS_CMDS="grep mktemp cut sed tr"
-TST_CNT=1
+TST_NEEDS_CMDS="grep mktemp cut sed tr xxd keyctl evmctl openssl cmp"
+TST_CNT=2
 TST_NEEDS_DEVICE=1
 
 . ima_setup.sh
@@ -65,4 +65,45 @@ test1()
 	tst_res TPASS "specified keyrings were measured correctly"
 }
 
+# Create a custom keyring and import a certficate to that
+test2() {
+	local custom_keyring_id temp_file="file.txt" \
+		cert_file="$TST_DATAROOT/x509.der"
+
+	tst_res TINFO "checking for correct IMA policy"
+
+	[ -f $IMA_POLICY ] || tst_brk TCONF "missing $IMA_POLICY"
+
+	[ -r $IMA_POLICY ] || tst_brk TCONF "cannot read IMA policy (CONFIG_IMA_READ_POLICY=y required)"
+
+	if ! grep "measure func=KEY_CHECK.*keyrings=.*key_import_test" $IMA_POLICY > /dev/null; then
+		tst_brk TCONF "the IMA policy does not include the key_import_test keyring. See the LTP IMA README."
+	fi
+
+	tst_res TINFO "creating a custom keyring (key_import_test keyring)"
+
+	custom_keyring_id=$(keyctl newring key_import_test @s) || \
+		tst_brk TCONF "unable to create a new keyring"
+
+	tst_is_num "$custom_keyring_id" || \
+		tst_brk TCONF "unable to parse custom keyring id"
+
+	evmctl import $cert_file "$custom_keyring_id" > /dev/null || \
+		tst_brk TCONF "unable to import a cert into the key_import_test keyring"
+
+	grep "key_import_test" "$ASCII_MEASUREMENTS" | tail -n1 | cut -d' ' -f6 | \
+		xxd -r -p > $temp_file || \
+		tst_brk TCONF "keyring not found in $ASCII_MEASUREMENTS"
+
+	if ! openssl x509 -in $temp_file -inform der > /dev/null; then
+		tst_brk TCONF "the cert logged in $ASCII_MEASUREMENTS is not a valid x509 certificate"
+	fi
+
+	if cmp -s $temp_file $cert_file; then
+		tst_res TPASS "logged cert matches original cert"
+	else
+		tst_res TFAIL "logged cert does not match original cert"
+	fi
+}
+
 tst_run
-- 
2.25.1


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

* Re: [PATCH] IMA: Add a test to verify importing a certificate into custom keyring
  2020-07-17 20:57 [PATCH] IMA: Add a test to verify importing a certificate into custom keyring Lachlan Sneff
@ 2020-07-22 20:33 ` Mimi Zohar
  2020-07-31  8:02 ` Petr Vorel
  1 sibling, 0 replies; 4+ messages in thread
From: Mimi Zohar @ 2020-07-22 20:33 UTC (permalink / raw)
  To: Lachlan Sneff, pvorel, ltp; +Cc: nramas, balajib, linux-integrity

On Fri, 2020-07-17 at 16:57 -0400, Lachlan Sneff wrote:
> A test for verifying importing an x509 certificate into a keyring and
> validating the key measurement performed by IMA is needed.

Isn't the main purpose of this patch to verify the certificate being
loaded onto a keyring is properly measured.  The rest is just details.
 Please update the Subject line accordingly.

> Add an IMA measurement test that verifies that an x509 certificate
> can be imported into a custom keyring and measured correctly.
> 
> A certificate used by the test is included in the `datafiles/keys` directory.
> 
> Using a custom keyring for the test is preferred over using a pre-existing/built-in keyring,
> such as `.ima`, since there may be restrictions to importing any cert into a built-in keyring.
> For example, the `.ima` keyring requires that the imported certs be signed by a kernel private
> key in certain kernel configurations.
> 

First describe the problem, then provide the solution.  In this case,
perhaps something like:

There may be restrictions importing a certificate onto a builtin
trusted keyring.  For example ...   For this reason, this test defines
a userspace keyring.

(Please remember to limit the patch description line width to 72
chars.)


> Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com>
> ---
>  .../kernel/security/integrity/ima/README.md   |  20 +++-----
>  .../security/integrity/ima/datafiles/Makefile |  11 ++---
>  .../integrity/ima/datafiles/keys/Makefile     |  31 ++++++++++++
>  .../integrity/ima/datafiles/keys/x509.der     | Bin 0 -> 784 bytes
>  .../integrity/ima/datafiles/policy/Makefile   |  31 ++++++++++++
>  .../datafiles/{ => policy}/keycheck.policy    |   0
>  .../ima/datafiles/{ => policy}/measure.policy |   0
>  .../{ => policy}/measure.policy-invalid       |   0
>  .../security/integrity/ima/tests/ima_keys.sh  |  45 +++++++++++++++++-
>  9 files changed, 117 insertions(+), 21 deletions(-)
>  create mode 100644 testcases/kernel/security/integrity/ima/datafiles/keys/Makefile
>  create mode 100644 testcases/kernel/security/integrity/ima/datafiles/keys/x509.der
>  create mode 100644 testcases/kernel/security/integrity/ima/datafiles/policy/Makefile
>  rename testcases/kernel/security/integrity/ima/datafiles/{ => policy}/keycheck.policy (100%)
>  rename testcases/kernel/security/integrity/ima/datafiles/{ => policy}/measure.policy (100%)
>  rename testcases/kernel/security/integrity/ima/datafiles/{ => policy}/measure.policy-invalid (100%)
> 
> diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md
> index 732cd912f..db8819a99 100644
> --- a/testcases/kernel/security/integrity/ima/README.md
> +++ b/testcases/kernel/security/integrity/ima/README.md
> @@ -16,25 +16,19 @@ space, may contain equivalent measurement tcb rules, detecting them would
>  require `IMA_READ_POLICY=y` therefore ignore this option.
>  
>  ### IMA key import test
> -`ima_keys.sh` requires a x509 public key, by default in `/etc/keys/x509_ima.der`
> -(defined in `CONFIG_IMA_X509_PATH` kernel config option).
> -The key must be signed by the private key you generate. Follow these instructions:
> -https://manpages.ubuntu.com/manpages/disco/man1/evmctl.1.html#generate%20trusted%20keys
>  
> -The test cannot be set-up automatically because the x509 public key must be
> -built into the kernel and loaded onto a trusted keyring
> -(e.g. `.builtin_trusted_keys`, `.secondary_trusted_keyring`).
> -
> -As well as what's required for the IMA tests, the following are also required

This patch adds a new test, that shouldn't affect the existing test.
 If test1 changes, then it needs to be highlighted in the patch
description.

> +As well as what's required for the IMA tests, the following is also required
>  in the kernel configuration:
>  ```
>  CONFIG_IMA_READ_POLICY=y
> -CONFIG_IMA_X509_PATH="/etc/keys/x509_ima.der"
> -CONFIG_SYSTEM_TRUSTED_KEYRING=y
> -CONFIG_SYSTEM_TRUSTED_KEYS="/etc/keys/ima-local-ca.pem"
>  ```
>  
> -Test also requires loaded policy with `func=KEY_CHECK`, see example in `keycheck.policy`.
> +You must also add the following line to /etc/ima/ima-policy and reboot.

Documentation should be written in the third person.  For example,
instead of "you must", "A key measurement rule must be defined in the
IMA policy. For example, ...."

> +```
> +measure func=KEY_CHECK keyrings=key_import_test template=ima-buf
> +```
> +
> +The test also requires loaded policy with `func=KEY_CHECK`, see example in `keycheck.policy`.
>  
>  ## EVM tests
>  
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/Makefile b/testcases/kernel/security/integrity/ima/datafiles/Makefile
> index a960f9d84..5261b9321 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/Makefile
> +++ b/testcases/kernel/security/integrity/ima/datafiles/Makefile
> @@ -1,5 +1,5 @@
>  #
> -#    testcases/kernel/security/integrity/ima/policy testcases Makefile.
> +#    kernel/security/integrity/ima testcase Makefile.

This change doesn't look right.

>  #
>  #    Copyright (C) 2009, Cisco Systems Inc.
>  #
> @@ -18,14 +18,13 @@
>  #    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>  #
>  # Ngie Cooper, July 2009
> +# Lachlan Sneff, July 2020
>  #
>  
>  top_srcdir		?= ../../../../../..
>  
> -include	$(top_srcdir)/include/mk/env_pre.mk
> +include $(top_srcdir)/include/mk/env_pre.mk

Please review the actual patch before posting it.  Changes like this
are unnecessary.

>  
> -INSTALL_DIR		:= testcases/data/ima_policy
> +SUBDIRS			:= policy keys
>  
> -INSTALL_TARGETS		:= measure*
> -
> -include $(top_srcdir)/include/mk/generic_leaf_target.mk
> +include $(top_srcdir)/include/mk/generic_trunk_target.mk

When making changes to someone else's code, please Cc them on the
change.  Use "git blame" to get the commit author.

> diff --git a/testcases/kernel/security/integrity/ima/datafiles/keys/Makefile b/testcases/kernel/security/integrity/ima/datafiles/keys/Makefile
> new file mode 100644
> index 000000000..3e381089c
> --- /dev/null
> +++ b/testcases/kernel/security/integrity/ima/datafiles/keys/Makefile
> @@ -0,0 +1,31 @@
> +#
> +#    testcases/kernel/security/integrity/ima/policy testcases Makefile.
> +#
> +#    Copyright (C) 2009, Cisco Systems Inc.

Hm, aren't you working for Microsoft?  If you're moving existing code,
please move that code separately from any other changes.  Newer files
probably use the SPDX format.

> +#
> +#    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.
> +#
> +# Lachlan Sneff, July 2020
> +#
> +
> +top_srcdir		?= ../../../../../../..
> +
> +include	$(top_srcdir)/include/mk/env_pre.mk
> +
> +INSTALL_DIR		:= testcases/data/ima_keys
> +
> +INSTALL_TARGETS		:= x509.der
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/keys/x509.der b/testcases/kernel/security/integrity/ima/datafiles/keys/x509.der
> new file mode 100644
> index 0000000000000000000000000000000000000000..538611c7244eac17377031038a97bf5d02c53d3f
> GIT binary patch
> literal 784
> zcmXqLV&*YuV*IjznTe5!Nkm*%!l+fPYOa%w{pGeOu7W)g|5Ocl**LY@JlekVGBR?r
> zG8lv!@&kpKLs__Z*gSn5b)6jzH4M~10?a(}KmmQ%Fkgk_)S{Biw9Mqhl2nD_%=Em>
> zymW=^)Jj7M12MR1oQz_e$%zh`xrw^Ti3W1wyha8Frbd>ArbZS<mQg^iArjXhz);YD
> zA7U*>Mt*TgUSe*lp|pV{#A2cHjQqshOccB13}le);)f`4fN3^poR91^Mpg#q#$E=4
> z#!jZjMuvqS<2&d4={o*uW**0h|9T%Tw9S&_xXFEO!9t#YW*25;x2$<5W-0eWw?OBy
> z^v@jizdt`D%q<d|ReAl{bE_$0%^l11#6PRp?#-6J7PfX<qx=M^6KT?qw?*WN1?*@I
> z%Cqp^b9sffLXO?miT~#3|NU~~-j<6EKTciX{lAO7bFKR(@pL9;Mh3>ku?EowJU|c1
> z3NtePXJIm6FyIF9_*qz(nb;c)WI-H07BLo)gzIHKKP^@H&nbIpE!y9-E`Uo=(m)<0
> zt;_-p@&=LZk_VQ~bi1@pYvM`%j}JQk-GBc`6502_h-GeKWMs(U%{uxsJ8V$|&s^U*
> zuT)C?`L3+KeqYmBdG>)zfpWi7n6jqq>YI1K{O3*;XK_o1>WKamk9bck{$vuzdw37y
> zvA`0c_d5A+eCHjLThIIJa@MIAVc(m-U;nXmbAi*F0<FyFA#2;;#Isxf2xngNvA$<=
> z$CMi7RW8Ag|Jk3|uI6~;sc!ZA<5Qn=%P&bzdui}MisQ9MYhlhY|F7HrKV4$gX6yNt
> zC8pkE<;LwsPD}g`oc<&~`(D3i*QEZk`);XW=B4u<{+H1Ux#t$eaj2M|C+nQUOz~ec
> zU5)v!tl*ZMaObGw{B>uG+jphhdu=ZLZTh9(PmEsIm`5}$d$_dpOSj%iU{I%d0|3@i
> BE~)?k
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/policy/Makefile b/testcases/kernel/security/integrity/ima/datafiles/policy/Makefile
> new file mode 100644
> index 000000000..692bf61ab
> --- /dev/null
> +++ b/testcases/kernel/security/integrity/ima/datafiles/policy/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/keycheck.policy b/testcases/kernel/security/integrity/ima/datafiles/policy/keycheck.policy
> similarity index 100%
> rename from testcases/kernel/security/integrity/ima/datafiles/keycheck.policy
> rename to testcases/kernel/security/integrity/ima/datafiles/policy/keycheck.policy
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/policy/measure.policy
> similarity index 100%
> rename from testcases/kernel/security/integrity/ima/datafiles/measure.policy
> rename to testcases/kernel/security/integrity/ima/datafiles/policy/measure.policy
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/policy/measure.policy-invalid
> similarity index 100%
> rename from testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
> rename to testcases/kernel/security/integrity/ima/datafiles/policy/measure.policy-invalid
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> index 904b7515b..101d2ea92 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> @@ -5,8 +5,8 @@
>  #
>  # Verify that keys are measured correctly based on policy.
>  
> -TST_NEEDS_CMDS="grep mktemp cut sed tr"
> -TST_CNT=1
> +TST_NEEDS_CMDS="grep mktemp cut sed tr xxd keyctl evmctl openssl cmp"
> +TST_CNT=2
>  TST_NEEDS_DEVICE=1
>  
>  . ima_setup.sh
> @@ -65,4 +65,45 @@ test1()
>  	tst_res TPASS "specified keyrings were measured correctly"
>  }
>  
> +# Create a custom keyring and import a certficate to that

What is a custom keyring?  Try inverting the sentence - "Import a
certificate onto a newly created keyring."

> +test2() {

The patch description should mention this patch defines an additional
ima_keys.sh test.  Another difference between test1 and test2 is that
doesn't require root privileges.


> +	local custom_keyring_id temp_file="file.txt" \
> +		cert_file="$TST_DATAROOT/x509.der"
> +
> +	tst_res TINFO "checking for correct IMA policy"
> +
> +	[ -f $IMA_POLICY ] || tst_brk TCONF "missing $IMA_POLICY"
> +
> +	[ -r $IMA_POLICY ] || tst_brk TCONF "cannot read IMA policy (CONFIG_IMA_READ_POLICY=y required)"
> +
> +	if ! grep "measure func=KEY_CHECK.*keyrings=.*key_import_test" $IMA_POLICY > /dev/null; then
> +		tst_brk TCONF "the IMA policy does not include the key_import_test keyring. See the LTP IMA README."
> +	fi
> +
> +	tst_res TINFO "creating a custom keyring (key_import_test keyring)"
> +
> +	custom_keyring_id=$(keyctl newring key_import_test @s) || \
> +		tst_brk TCONF "unable to create a new keyring"
> +
> +	tst_is_num "$custom_keyring_id" || \
> +		tst_brk TCONF "unable to parse custom keyring id"
> +
> +	evmctl import $cert_file "$custom_keyring_id" > /dev/null || \
> +		tst_brk TCONF "unable to import a cert into the key_import_test keyring"
> +
> +	grep "key_import_test" "$ASCII_MEASUREMENTS" | tail -n1 | cut -d' ' -f6 | \
> +		xxd -r -p > $temp_file || \
> +		tst_brk TCONF "keyring not found in $ASCII_MEASUREMENTS"
> +
> +	if ! openssl x509 -in $temp_file -inform der > /dev/null; then
> +		tst_brk TCONF "the cert logged in $ASCII_MEASUREMENTS is not a valid x509 certificate"
> +	fi
> +
> +	if cmp -s $temp_file $cert_file; then
> +		tst_res TPASS "logged cert matches original cert"
> +	else
> +		tst_res TFAIL "logged cert does not match original cert"
> +	fi
> +}
> +
>  tst_run

Normally when code is duplicated with only minor changes, it implies
that the code can be refactored.

Mimi



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

* Re: [PATCH] IMA: Add a test to verify importing a certificate into custom keyring
  2020-07-17 20:57 [PATCH] IMA: Add a test to verify importing a certificate into custom keyring Lachlan Sneff
  2020-07-22 20:33 ` Mimi Zohar
@ 2020-07-31  8:02 ` Petr Vorel
  2020-07-31 21:31   ` Lakshmi Ramasubramanian
  1 sibling, 1 reply; 4+ messages in thread
From: Petr Vorel @ 2020-07-31  8:02 UTC (permalink / raw)
  To: Lachlan Sneff; +Cc: zohar, ltp, nramas, balajib, linux-integrity

Hi Lachlan,

> A test for verifying importing an x509 certificate into a keyring and
> validating the key measurement performed by IMA is needed.

I suppose you're going to send new version of this patch (rebased + fix
according to Mimi's comments).

IMHO that should be your last not yet merged patch.

FYI: I'm planning to fix ima_tpm.sh and then implement autoloading IMA policy
(when possible).

Kind regards,
Petr

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

* Re: [PATCH] IMA: Add a test to verify importing a certificate into custom keyring
  2020-07-31  8:02 ` Petr Vorel
@ 2020-07-31 21:31   ` Lakshmi Ramasubramanian
  0 siblings, 0 replies; 4+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-31 21:31 UTC (permalink / raw)
  To: Petr Vorel, Lachlan Sneff; +Cc: zohar, ltp, balajib, linux-integrity

On 7/31/20 1:02 AM, Petr Vorel wrote:

Hi Petr,

>> A test for verifying importing an x509 certificate into a keyring and
>> validating the key measurement performed by IMA is needed.
> 
> I suppose you're going to send new version of this patch (rebased + fix
> according to Mimi's comments).
> 
> IMHO that should be your last not yet merged patch.
> 
> FYI: I'm planning to fix ima_tpm.sh and then implement autoloading IMA policy
> (when possible).
> 

Yes - Lachlan will send the new version of the patch set (rebased to 
your latest changes and also addressing Mimi's comments).

thanks,
  -lakshmi


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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 20:57 [PATCH] IMA: Add a test to verify importing a certificate into custom keyring Lachlan Sneff
2020-07-22 20:33 ` Mimi Zohar
2020-07-31  8:02 ` Petr Vorel
2020-07-31 21:31   ` Lakshmi Ramasubramanian

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org
	public-inbox-index linux-integrity

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git