mbox series

[v3,0/4] IMA: verify measurement of certificate imported into a keyring

Message ID 20200817130916.27634-1-pvorel@suse.cz (mailing list archive)
Headers show
Series IMA: verify measurement of certificate imported into a keyring | expand

Message

Petr Vorel Aug. 17, 2020, 1:09 p.m. UTC
Hi Mimi, Lakshmi,

changes v2->v3:
fixed regression in my third commit.
(please verify it on installed LTP, or at least run make install in
testcases/kernel/security/integrity/ima/datafiles/ima_keys/)

Kind regards,
Petr

Lachlan Sneff (1):
  IMA: Add a test to verify measurement of certificate imported into a
    keyring

Petr Vorel (3):
  IMA/ima_keys.sh: Fix policy content check usage
  IMA: Refactor datafiles directory
  IMA/ima_keys.sh: Enhance policy checks

 .../kernel/security/integrity/ima/README.md   |  12 +-
 .../security/integrity/ima/datafiles/Makefile |  10 +-
 .../ima/datafiles/ima_kexec/Makefile          |  11 ++
 .../datafiles/{ => ima_kexec}/kexec.policy    |   0
 .../integrity/ima/datafiles/ima_keys/Makefile |  11 ++
 .../datafiles/{ => ima_keys}/keycheck.policy  |   2 +-
 .../ima/datafiles/ima_keys/x509_ima.der       | Bin 0 -> 650 bytes
 .../ima/datafiles/ima_policy/Makefile         |  11 ++
 .../datafiles/{ => ima_policy}/measure.policy |   0
 .../{ => ima_policy}/measure.policy-invalid   |   0
 .../security/integrity/ima/tests/ima_keys.sh  | 104 +++++++++++++++---
 11 files changed, 133 insertions(+), 28 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_kexec/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_kexec}/kexec.policy (100%)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_keys}/keycheck.policy (59%)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_policy/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy (100%)
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy-invalid (100%)

Comments

Lakshmi Ramasubramanian Aug. 17, 2020, 2:37 p.m. UTC | #1
On 8/17/20 6:09 AM, Petr Vorel wrote:

Hi Petr,

> Hi Mimi, Lakshmi,
> 
> changes v2->v3:
> fixed regression in my third commit.
> (please verify it on installed LTP, or at least run make install in
> testcases/kernel/security/integrity/ima/datafiles/ima_keys/)
> 

Verified keys tests and also kexec tests. Thanks.

Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>

> 
> Lachlan Sneff (1):
>    IMA: Add a test to verify measurement of certificate imported into a
>      keyring
> 
> Petr Vorel (3):
>    IMA/ima_keys.sh: Fix policy content check usage
>    IMA: Refactor datafiles directory
>    IMA/ima_keys.sh: Enhance policy checks
> 
>   .../kernel/security/integrity/ima/README.md   |  12 +-
>   .../security/integrity/ima/datafiles/Makefile |  10 +-
>   .../ima/datafiles/ima_kexec/Makefile          |  11 ++
>   .../datafiles/{ => ima_kexec}/kexec.policy    |   0
>   .../integrity/ima/datafiles/ima_keys/Makefile |  11 ++
>   .../datafiles/{ => ima_keys}/keycheck.policy  |   2 +-
>   .../ima/datafiles/ima_keys/x509_ima.der       | Bin 0 -> 650 bytes
>   .../ima/datafiles/ima_policy/Makefile         |  11 ++
>   .../datafiles/{ => ima_policy}/measure.policy |   0
>   .../{ => ima_policy}/measure.policy-invalid   |   0
>   .../security/integrity/ima/tests/ima_keys.sh  | 104 +++++++++++++++---
>   11 files changed, 133 insertions(+), 28 deletions(-)
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_kexec/Makefile
>   rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_kexec}/kexec.policy (100%)
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
>   rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_keys}/keycheck.policy (59%)
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_policy/Makefile
>   rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy (100%)
>   rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy-invalid (100%)
>
Mimi Zohar Aug. 17, 2020, 7:18 p.m. UTC | #2
On Mon, 2020-08-17 at 15:09 +0200, Petr Vorel wrote:
> Hi Mimi, Lakshmi,
> 
> changes v2->v3:
> fixed regression in my third commit.
> (please verify it on installed LTP, or at least run make install in
> testcases/kernel/security/integrity/ima/datafiles/ima_keys/)

I did, but nothing changed.  I probably need to set an environment
variable.

After building and installing LTP, it's finding the file, but some of
the issues still exist:

ima_keys 1 TINFO: $TMPDIR is on tmpfs => run on loop device
ima_keys 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
ima_keys 1 TINFO: verify key measurement for keyrings and templates specified in IMA policy
grep: Unmatched ( or \(
ima_keys 1 TPASS: specified keyrings were measured correctly
ima_keys 2 TINFO: verify measurement of certificate imported into a keyring
keyctl_session_to_parent: Operation not permitted
ima_keys 2 TPASS: logged certificate matches the original

IMA policy:
measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf 
measure func=KEY_CHECK keyrings=key_import_test template=ima-buf 

Mimi
Petr Vorel Aug. 17, 2020, 7:52 p.m. UTC | #3
> On Mon, 2020-08-17 at 15:09 +0200, Petr Vorel wrote:
> > Hi Mimi, Lakshmi,

> > changes v2->v3:
> > fixed regression in my third commit.
> > (please verify it on installed LTP, or at least run make install in
> > testcases/kernel/security/integrity/ima/datafiles/ima_keys/)

> I did, but nothing changed.  I probably need to set an environment
> variable.
You also need to set LTPROOT (prefix, e.g. /opt/ltp).

> After building and installing LTP, it's finding the file, but some of
> the issues still exist:

> ima_keys 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> ima_keys 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
> ima_keys 1 TINFO: verify key measurement for keyrings and templates specified in IMA policy
> grep: Unmatched ( or \(
This should be fixed by v3 (fixed by for loop and sort -u)
https://patchwork.ozlabs.org/project/ltp/patch/20200817130916.27634-5-pvorel@suse.cz/
But I'll test it tomorrow with your IMA policy.

Thank you for testing!

Kind regards,
Petr

> ima_keys 1 TPASS: specified keyrings were measured correctly
> ima_keys 2 TINFO: verify measurement of certificate imported into a keyring
> keyctl_session_to_parent: Operation not permitted
> ima_keys 2 TPASS: logged certificate matches the original

> IMA policy:
> measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf 
> measure func=KEY_CHECK keyrings=key_import_test template=ima-buf 

> Mimi
Lakshmi Ramasubramanian Aug. 17, 2020, 8:02 p.m. UTC | #4
On 8/17/20 12:18 PM, Mimi Zohar wrote:

> On Mon, 2020-08-17 at 15:09 +0200, Petr Vorel wrote:
>> Hi Mimi, Lakshmi,
>>
>> changes v2->v3:
>> fixed regression in my third commit.
>> (please verify it on installed LTP, or at least run make install in
>> testcases/kernel/security/integrity/ima/datafiles/ima_keys/)
> 
> I did, but nothing changed.  I probably need to set an environment
> variable.
> 
> After building and installing LTP, it's finding the file, but some of
> the issues still exist:
> 
> ima_keys 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> ima_keys 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
> ima_keys 1 TINFO: verify key measurement for keyrings and templates specified in IMA policy
> grep: Unmatched ( or \(
> ima_keys 1 TPASS: specified keyrings were measured correctly
> ima_keys 2 TINFO: verify measurement of certificate imported into a keyring
> keyctl_session_to_parent: Operation not permitted
> ima_keys 2 TPASS: logged certificate matches the original
> 
> IMA policy:
> measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
> measure func=KEY_CHECK keyrings=key_import_test template=ima-buf
> 

I think I see the problem

keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \
		sed "s/\./\\\./g" | cut -d'=' -f2; done | sort -u

The above line generates the list of keyrings (read from the IMA policy) 
with a newline after the first policy entry with "keyrings=". Please see 
below:

ima_keys 1 TINFO: \.ima|\.builtin_trusted_keys
key_import_test

When this is checked in the "do-done" loop grep returns "mismatched (" 
due to the newline.

I tried with "(" removed from the following line and that fixes the problem:

grep -E "($templates)*($keyrings)" $ASCII_MEASUREMENTS | while read line

But a better fix might be to remove the "newline" in $keyrings. I'll try 
that.

Regarding the following error:
keyctl_session_to_parent: Operation not permitted

The following line in test2() can be removed. Not sure if this is needed.
	keyctl new_session > /dev/null

thanks,
  -lakshmi
Petr Vorel Aug. 17, 2020, 8:39 p.m. UTC | #5
> On 8/17/20 12:18 PM, Mimi Zohar wrote:

> > On Mon, 2020-08-17 at 15:09 +0200, Petr Vorel wrote:
> > > Hi Mimi, Lakshmi,

> > > changes v2->v3:
> > > fixed regression in my third commit.
> > > (please verify it on installed LTP, or at least run make install in
> > > testcases/kernel/security/integrity/ima/datafiles/ima_keys/)

> > I did, but nothing changed.  I probably need to set an environment
> > variable.

> > After building and installing LTP, it's finding the file, but some of
> > the issues still exist:

> > ima_keys 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> > ima_keys 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
> > ima_keys 1 TINFO: verify key measurement for keyrings and templates specified in IMA policy
> > grep: Unmatched ( or \(
> > ima_keys 1 TPASS: specified keyrings were measured correctly
> > ima_keys 2 TINFO: verify measurement of certificate imported into a keyring
> > keyctl_session_to_parent: Operation not permitted
> > ima_keys 2 TPASS: logged certificate matches the original

> > IMA policy:
> > measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
> > measure func=KEY_CHECK keyrings=key_import_test template=ima-buf


> I think I see the problem

> keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \
> 		sed "s/\./\\\./g" | cut -d'=' -f2; done | sort -u

> The above line generates the list of keyrings (read from the IMA policy)
> with a newline after the first policy entry with "keyrings=". Please see
> below:

> ima_keys 1 TINFO: \.ima|\.builtin_trusted_keys
> key_import_test

> When this is checked in the "do-done" loop grep returns "mismatched (" due
> to the newline.

> I tried with "(" removed from the following line and that fixes the problem:

> grep -E "($templates)*($keyrings)" $ASCII_MEASUREMENTS | while read line

> But a better fix might be to remove the "newline" in $keyrings. I'll try
> that.
OK, I was too aggressive when removing tr dependency. I'll add another sed
command to remove new lines.

> Regarding the following error:
> keyctl_session_to_parent: Operation not permitted
I missed this problem.

> The following line in test2() can be removed. Not sure if this is needed.
> 	keyctl new_session > /dev/null
Not sure, I guess it'd work in existing session. But then it'd be good to
do cleanup.

> thanks,
>  -lakshmi

Kind regards,
Petr