| Message ID | 20200617234957.10611-3-t-josne@linux.microsoft.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | IMA: Verify measurement of certificates | expand |
Hi Lachlan, LGTM, I'd just like to do some tests. That's what prevents me from merging (my notes below are just nits, I'll fix them before merging). @Mimi: would you have time to have look into these tests? Reviewed-by: Petr Vorel <pvorel@suse.cz> > Add an IMA measurement test that verifies that an x509 certificate > can be imported into the .ima keyring and measured correctly. > Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com> > --- > .../kernel/security/integrity/ima/README.md | 21 +++++++++ > .../security/integrity/ima/tests/ima_keys.sh | 47 ++++++++++++++++++- > 2 files changed, 66 insertions(+), 2 deletions(-) > diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md > index 16a1f48c3..e41f7b570 100644 > --- a/testcases/kernel/security/integrity/ima/README.md > +++ b/testcases/kernel/security/integrity/ima/README.md > @@ -16,6 +16,27 @@ CONFIG_INTEGRITY=y > CONFIG_IMA=y > ``` Thanks for a docs, I'll move it to the first commit. > +IMA Key Import test IMA Key Import tests > +------------- > + > +`ima_keys.sh` requires an x509 key to be generated and placed > +at `/etc/keys/x509_ima.der`. `ima_keys.sh` requires an x509 public key to be generated and placed > +at `/etc/keys/x509_ima.der`. > + > +The x509 public key 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. I was thinking to use non-distro link: https://www.mankier.com/1/evmctl#Generate_Trusted_Keys as Ubuntu docs is tied to certain evmctl version, but on the other hand it document what you used when wrote tests. And Ubuntu URL is probably is probably safer to use (mankier.com can vanish in the future). Thus keep this one. > + > +The test cannot be set-up automatically because the kernel must be built > +with one of the keys you generate. > + > +As well as what's required for the IMA tests, the following are also required > +in the kernel configuration: > +``` > +CONFIG_IMA_READ_POLICY=y > +CONFIG_SYSTEM_TRUSTED_KEYRING=y > +CONFIG_SYSTEM_TRUSTED_KEYS="/etc/keys/ima-local-ca.pem" > +``` > + > EVM tests > --------- > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > index 2b5324dbf..1d9824aba 100755 > --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > @@ -5,10 +5,12 @@ > # 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 > +CERT_FILE="${CERT_FILE:-/etc/keys/x509_ima.der}" > + > . ima_setup.sh > # Based on https://lkml.org/lkml/2019/12/13/564. > @@ -64,4 +66,45 @@ test1() > tst_res TPASS "specified keyrings were measured correctly" > } > + > +# Test that a cert can be imported into the ".ima" keyring correctly. > +test2() { > + local keyring_id key_id test_file=$(mktemp) > + > + [ -f $CERT_FILE ] || tst_brk TCONF "missing $CERT_FILE" > + > + if ! openssl x509 -in $CERT_FILE -inform der > /dev/null; then > + tst_brk TCONF "The suppled cert file ($CERT_FILE) is not a valid x509 certificate" > + fi > + > + tst_res TINFO "adding a cert to the .ima keyring ($CERT_FILE)" > + > + keyring_id=$(keyctl show %:.ima | sed -n 2p | \ > + sed 's/^[[:space:]]*//' | cut -d' ' -f1) || \ > + tst_btk TCONF "unable to retrieve .ima keyring id" > + > + if ! tst_is_num "$keyring_id"; then > + tst_brk TCONF "unable to parse keyring id from keyring" > + fi > + > + evmctl import $CERT_FILE "$keyring_id" > /dev/null || \ > + tst_brk TCONF "unable to import a cert into the .ima keyring" > + > + grep -F ".ima" "$ASCII_MEASUREMENTS" | tail -n1 | cut -d' ' -f6 | \ > + xxd -r -p > $test_file || \ > + tst_brk TCONF "cert not found in ascii_runtime_measurements log" > + > + if ! openssl x509 -in $test_file -inform der > /dev/null; then > + tst_brk TCONF "The cert logged in ascii_runtime_measurements is not a valid x509 certificate" > + fi > + > + if cmp -s "$test_file" $CERT_FILE; then > + tst_res TPASS "logged cert matches original cert" > + else > + tst_res TFAIL "logged cert does not match original cert" > + fi > + > + rm $test_file I guess you can avoid deleting this file. There is automatic cleanup of the test directory and even if the test is run with -i (number of iterations), it'll be unique as it's using using mktemp. > +} > + > tst_run Kind regards, Petr
Hi Lachlan, On Wed, 2020-06-17 at 19:49 -0400, Lachlan Sneff wrote: > Add an IMA measurement test that verifies that an x509 certificate > can be imported into the .ima keyring and measured correctly. Please expand this, explaining that the x509 certificate needs to be signed by a key on one of the trusted keyrings. Once there is a reliable way of adding a key to the IMA keyring, this opens up a lot of other testing possibilities. > > Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com> > --- > .../kernel/security/integrity/ima/README.md | 21 +++++++++ > .../security/integrity/ima/tests/ima_keys.sh | 47 ++++++++++++++++++- > 2 files changed, 66 insertions(+), 2 deletions(-) > > diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md > index 16a1f48c3..e41f7b570 100644 > --- a/testcases/kernel/security/integrity/ima/README.md > +++ b/testcases/kernel/security/integrity/ima/README.md > @@ -16,6 +16,27 @@ CONFIG_INTEGRITY=y > CONFIG_IMA=y > ``` > > +IMA Key Import test > +------------- > + > +`ima_keys.sh` requires an x509 key to be generated and placed > +at `/etc/keys/x509_ima.der`. The filename "/etc/keys/x509_ima.der" is configurable. It's based on CONFIG_IMA_X509_PATH Kconfig option. Perhaps extract it from the running kernel's Kconfig? > + > +The x509 public key 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 kernel must be built > +with one of the keys you generate. Please reword this to convey that the public key must be built into the kernel and loaded onto a trusted keyring (eg. .builtin_trusted_keys, .secondary_trusted_keyring) > + > +As well as what's required for the IMA tests, the following are also required > +in the kernel configuration: > +``` > +CONFIG_IMA_READ_POLICY=y > +CONFIG_SYSTEM_TRUSTED_KEYRING=y > +CONFIG_SYSTEM_TRUSTED_KEYS="/etc/keys/ima-local-ca.pem" > +``` > + > EVM tests > --------- > > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > index 2b5324dbf..1d9824aba 100755 > --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > @@ -5,10 +5,12 @@ > # > # 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 > > +CERT_FILE="${CERT_FILE:-/etc/keys/x509_ima.der}" > + > . ima_setup.sh > > # Based on https://lkml.org/lkml/2019/12/13/564. > @@ -64,4 +66,45 @@ test1() > tst_res TPASS "specified keyrings were measured correctly" > } > > + > +# Test that a cert can be imported into the ".ima" keyring correctly. > +test2() { > + local keyring_id key_id test_file=$(mktemp) > + > + [ -f $CERT_FILE ] || tst_brk TCONF "missing $CERT_FILE" > + > + if ! openssl x509 -in $CERT_FILE -inform der > /dev/null; then > + tst_brk TCONF "The suppled cert file ($CERT_FILE) is not a valid x509 certificate" > + fi > + > + tst_res TINFO "adding a cert to the .ima keyring ($CERT_FILE)" > + > + keyring_id=$(keyctl show %:.ima | sed -n 2p | \ > + sed 's/^[[:space:]]*//' | cut -d' ' -f1) || \ > + tst_btk TCONF "unable to retrieve .ima keyring id" Using "keyctl describe" returns the keyring id as the first token, making it simpler to parse. Mimi > + > + if ! tst_is_num "$keyring_id"; then > + tst_brk TCONF "unable to parse keyring id from keyring" > + fi > + > + evmctl import $CERT_FILE "$keyring_id" > /dev/null || \ > + tst_brk TCONF "unable to import a cert into the .ima keyring" > + > + grep -F ".ima" "$ASCII_MEASUREMENTS" | tail -n1 | cut -d' ' -f6 | \ > + xxd -r -p > $test_file || \ > + tst_brk TCONF "cert not found in ascii_runtime_measurements log" > + > + if ! openssl x509 -in $test_file -inform der > /dev/null; then > + tst_brk TCONF "The cert logged in ascii_runtime_measurements is not a valid x509 certificate" > + fi > + > + if cmp -s "$test_file" $CERT_FILE; then > + tst_res TPASS "logged cert matches original cert" > + else > + tst_res TFAIL "logged cert does not match original cert" > + fi > + > + rm $test_file > +} > + > tst_run
Thank you for the review, Mimi! On 6/24/20 12:41 PM, Mimi Zohar wrote: > Hi Lachlan, > > On Wed, 2020-06-17 at 19:49 -0400, Lachlan Sneff wrote: >> Add an IMA measurement test that verifies that an x509 certificate >> can be imported into the .ima keyring and measured correctly. > Please expand this, explaining that the x509 certificate needs to be > signed by a key on one of the trusted keyrings. > > Once there is a reliable way of adding a key to the IMA keyring, this > opens up a lot of other testing possibilities. This is a great idea. I definitely wasn't clear enough here. >> Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com> >> --- >> .../kernel/security/integrity/ima/README.md | 21 +++++++++ >> .../security/integrity/ima/tests/ima_keys.sh | 47 ++++++++++++++++++- >> 2 files changed, 66 insertions(+), 2 deletions(-) >> >> diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md >> index 16a1f48c3..e41f7b570 100644 >> --- a/testcases/kernel/security/integrity/ima/README.md >> +++ b/testcases/kernel/security/integrity/ima/README.md >> @@ -16,6 +16,27 @@ CONFIG_INTEGRITY=y >> CONFIG_IMA=y >> ``` >> >> +IMA Key Import test >> +------------- >> + >> +`ima_keys.sh` requires an x509 key to be generated and placed >> +at `/etc/keys/x509_ima.der`. > The filename "/etc/keys/x509_ima.der" is configurable. It's based on > CONFIG_IMA_X509_PATH Kconfig option. Perhaps extract it from the > running kernel's Kconfig? I didn't think pulling it from the kernel config. Will try this. I assume `grep "..." /boot/config-$(uname -r)` is the right way to grab a line from the config? >> + >> +The x509 public key 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 kernel must be built >> +with one of the keys you generate. > Please reword this to convey that the public key must be built into > the kernel and loaded onto a trusted keyring (eg. > .builtin_trusted_keys, .secondary_trusted_keyring) Sounds good. >> + >> +As well as what's required for the IMA tests, the following are also required >> +in the kernel configuration: >> +``` >> +CONFIG_IMA_READ_POLICY=y >> +CONFIG_SYSTEM_TRUSTED_KEYRING=y >> +CONFIG_SYSTEM_TRUSTED_KEYS="/etc/keys/ima-local-ca.pem" >> +``` >> + >> EVM tests >> --------- >> >> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh >> index 2b5324dbf..1d9824aba 100755 >> --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh >> +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh >> @@ -5,10 +5,12 @@ >> # >> # 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 >> >> +CERT_FILE="${CERT_FILE:-/etc/keys/x509_ima.der}" >> + >> . ima_setup.sh >> >> # Based on https://lkml.org/lkml/2019/12/13/564. >> @@ -64,4 +66,45 @@ test1() >> tst_res TPASS "specified keyrings were measured correctly" >> } >> >> + >> +# Test that a cert can be imported into the ".ima" keyring correctly. >> +test2() { >> + local keyring_id key_id test_file=$(mktemp) >> + >> + [ -f $CERT_FILE ] || tst_brk TCONF "missing $CERT_FILE" >> + >> + if ! openssl x509 -in $CERT_FILE -inform der > /dev/null; then >> + tst_brk TCONF "The suppled cert file ($CERT_FILE) is not a valid x509 certificate" >> + fi >> + >> + tst_res TINFO "adding a cert to the .ima keyring ($CERT_FILE)" >> + >> + keyring_id=$(keyctl show %:.ima | sed -n 2p | \ >> + sed 's/^[[:space:]]*//' | cut -d' ' -f1) || \ >> + tst_btk TCONF "unable to retrieve .ima keyring id" > Using "keyctl describe" returns the keyring id as the first token, > making it simpler to parse. Didn't realize this, will simplify the code here. > > Mimi Thanks again! Will get a patchset out with the changes asap. > >> + >> + if ! tst_is_num "$keyring_id"; then >> + tst_brk TCONF "unable to parse keyring id from keyring" >> + fi >> + >> + evmctl import $CERT_FILE "$keyring_id" > /dev/null || \ >> + tst_brk TCONF "unable to import a cert into the .ima keyring" >> + >> + grep -F ".ima" "$ASCII_MEASUREMENTS" | tail -n1 | cut -d' ' -f6 | \ >> + xxd -r -p > $test_file || \ >> + tst_brk TCONF "cert not found in ascii_runtime_measurements log" >> + >> + if ! openssl x509 -in $test_file -inform der > /dev/null; then >> + tst_brk TCONF "The cert logged in ascii_runtime_measurements is not a valid x509 certificate" >> + fi >> + >> + if cmp -s "$test_file" $CERT_FILE; then >> + tst_res TPASS "logged cert matches original cert" >> + else >> + tst_res TFAIL "logged cert does not match original cert" >> + fi >> + >> + rm $test_file >> +} >> + >> tst_run
On Wed, 2020-06-24 at 15:59 -0400, Lachlan Sneff wrote: > > >> diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md > >> index 16a1f48c3..e41f7b570 100644 > >> --- a/testcases/kernel/security/integrity/ima/README.md > >> +++ b/testcases/kernel/security/integrity/ima/README.md > >> @@ -16,6 +16,27 @@ CONFIG_INTEGRITY=y > >> CONFIG_IMA=y > >> ``` > >> > >> +IMA Key Import test > >> +------------- > >> + > >> +`ima_keys.sh` requires an x509 key to be generated and placed > >> +at `/etc/keys/x509_ima.der`. > > The filename "/etc/keys/x509_ima.der" is configurable. It's based on > > CONFIG_IMA_X509_PATH Kconfig option. Perhaps extract it from the > > running kernel's Kconfig? > I didn't think pulling it from the kernel config. Will try this. I > assume `grep "..." /boot/config-$(uname -r)` is the right way to grab a > line from the config? Try using scripts/extract-ikconfig. Mimi
Hi Mimi, Lachlan, > > >> +`ima_keys.sh` requires an x509 key to be generated and placed > > >> +at `/etc/keys/x509_ima.der`. > > > The filename "/etc/keys/x509_ima.der" is configurable. It's based on > > > CONFIG_IMA_X509_PATH Kconfig option. Perhaps extract it from the > > > running kernel's Kconfig? > > I didn't think pulling it from the kernel config. Will try this. I > > assume `grep "..." /boot/config-$(uname -r)` is the right way to grab a > > line from the config? > Try using scripts/extract-ikconfig. For now I'd just try to grep /boot/config-$(uname -r), but allow to run the test with the default value if kconfig not presented / readable (when running without root). I'm not sure if extract-ikconfig as external dependency would be suitable for LTP (understand it's great for kselftest as it's already presented). BTW there is a ticket for adding kernel config related helpers into the LTP shell API [1], I'll also note extract-ikconfig there. LTP refused for long time working with kernel config, because it it's requirement meant that SUT without it could not be tested. Always try to not make kernel config as hard dependency (various embedded or old android will be disabled; some linux distros require root for reading the config). Design in [1] also suggest to have possibility to run the test even without config. [1] https://github.com/linux-test-project/ltp/issues/700 > Mimi Kind regards, Petr
diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md index 16a1f48c3..e41f7b570 100644 --- a/testcases/kernel/security/integrity/ima/README.md +++ b/testcases/kernel/security/integrity/ima/README.md @@ -16,6 +16,27 @@ CONFIG_INTEGRITY=y CONFIG_IMA=y ``` +IMA Key Import test +------------- + +`ima_keys.sh` requires an x509 key to be generated and placed +at `/etc/keys/x509_ima.der`. + +The x509 public key 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 kernel must be built +with one of the keys you generate. + +As well as what's required for the IMA tests, the following are also required +in the kernel configuration: +``` +CONFIG_IMA_READ_POLICY=y +CONFIG_SYSTEM_TRUSTED_KEYRING=y +CONFIG_SYSTEM_TRUSTED_KEYS="/etc/keys/ima-local-ca.pem" +``` + EVM tests --------- diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh index 2b5324dbf..1d9824aba 100755 --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh @@ -5,10 +5,12 @@ # # 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 +CERT_FILE="${CERT_FILE:-/etc/keys/x509_ima.der}" + . ima_setup.sh # Based on https://lkml.org/lkml/2019/12/13/564. @@ -64,4 +66,45 @@ test1() tst_res TPASS "specified keyrings were measured correctly" } + +# Test that a cert can be imported into the ".ima" keyring correctly. +test2() { + local keyring_id key_id test_file=$(mktemp) + + [ -f $CERT_FILE ] || tst_brk TCONF "missing $CERT_FILE" + + if ! openssl x509 -in $CERT_FILE -inform der > /dev/null; then + tst_brk TCONF "The suppled cert file ($CERT_FILE) is not a valid x509 certificate" + fi + + tst_res TINFO "adding a cert to the .ima keyring ($CERT_FILE)" + + keyring_id=$(keyctl show %:.ima | sed -n 2p | \ + sed 's/^[[:space:]]*//' | cut -d' ' -f1) || \ + tst_btk TCONF "unable to retrieve .ima keyring id" + + if ! tst_is_num "$keyring_id"; then + tst_brk TCONF "unable to parse keyring id from keyring" + fi + + evmctl import $CERT_FILE "$keyring_id" > /dev/null || \ + tst_brk TCONF "unable to import a cert into the .ima keyring" + + grep -F ".ima" "$ASCII_MEASUREMENTS" | tail -n1 | cut -d' ' -f6 | \ + xxd -r -p > $test_file || \ + tst_brk TCONF "cert not found in ascii_runtime_measurements log" + + if ! openssl x509 -in $test_file -inform der > /dev/null; then + tst_brk TCONF "The cert logged in ascii_runtime_measurements is not a valid x509 certificate" + fi + + if cmp -s "$test_file" $CERT_FILE; then + tst_res TPASS "logged cert matches original cert" + else + tst_res TFAIL "logged cert does not match original cert" + fi + + rm $test_file +} + tst_run
Add an IMA measurement test that verifies that an x509 certificate can be imported into the .ima keyring and measured correctly. Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com> --- .../kernel/security/integrity/ima/README.md | 21 +++++++++ .../security/integrity/ima/tests/ima_keys.sh | 47 ++++++++++++++++++- 2 files changed, 66 insertions(+), 2 deletions(-)