| Message ID | 20200717205721.18173-1-t-josne@linux.microsoft.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | IMA: Add a test to verify importing a certificate into custom keyring | expand |
Gentle ping to Mimi and Petr to review. On 7/17/20 4:57 PM, Lachlan Sneff wrote: > 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
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
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
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
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
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%)