[LTP] [PATCH] syscalls/keyctl09: test encrypted keys.

[Yael Tiomkin]

On Fri, Dec 17, 2021 at 8:56 AM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Yael,
>
> > Test that encrypted keys can be instantiated using
> > both user-provided decrypted data
> > (
> https://lore.kernel.org/linux-integrity/20211213192030.125091-1-yaelt@google.com/
> ),
> > or kernel-generated numbers.
>
> Thanks a lot for adding this test. There are few missing things:
>
> BTW all mentioned here is implemented here to speedup your work:
> https://github.com/pevik/ltp/tree/yael_tiomkin/keyctl09.fixes
> Below is diff of my suggested changes.
>
> What I didn't solve is broken test when run with more than 1 iteration:
>
> ./keyctl09 -i2
> tst_test.c:1423: TINFO: Timeout per run is 0h 05m 00s
> keyctl09.c:47: TPASS: Encrypted keys were successfully instantiated and
> read
> keyctl09.c:31: TBROK: Failed to instantiate encrypted key using payload
> decrypted data: EINVAL (22)
>
> NOTE: you can specify test setup to do things only once. You should also
> cleanup
> after test run with test cleanup.
>
> https://github.com/linux-test-project/ltp/wiki/C-Test-API#11-basic-test-structure
>
> You also need to add entry to testcases/kernel/syscalls/keyctl/.gitignore
> /keyctl09
>
> and runtest/syscalls
> keyctl09 keyctl09
>
> ...
> > +/*
> > + * Description: This tests that encrypted keys can be instantiated using
> > + * user-provided decrypted data (plaintext), and separately, using
> > + * kernel-generated key material.
> > + */
>
> I'd rephrase it a bit and use LTP metadata formatting
> /*\
>  * [Description]
>  * Test that encrypted keys can be instantiated using user-provided
> decrypted
>  * data (plaintext), and separately, using kernel-generated key material.
>  */
>
> FYI metadata is LTP documentation - here is the output:
>
> https://github.com/linux-test-project/ltp/releases/download/20210121/metadata.20210121.html
>
> https://github.com/linux-test-project/ltp/releases/download/20210121/metadata.20210121.pdf
>
> You can run make in docparse/ directory and then see in output in
> metadata/metadata.{html,pdf}.
>
> > +
> > +#include <errno.h>
> > +#include <stdint.h>
> IMHO these 2 aren't needed.
> > +
> > +#include "tst_test.h"
> > +#include "lapi/keyctl.h"
> > +
> > +static void do_test(void)
> > +{
> > +     key_serial_t masterkey;
> > +     key_serial_t encryptedkey1;
> > +     key_serial_t encryptedkey2;
> > +     char buffer[128];
> > +
> > +     masterkey = add_key("user", "user:masterkey", "foo", 3,
> KEY_SPEC_PROCESS_KEYRING);
> > +     if (masterkey == -1)
> > +             tst_brk(TBROK | TERRNO, "Failed to add user key");
> > +
> > +     encryptedkey1 = add_key("encrypted", "ltptestkey1", "new enc32
> user:masterkey 32 plaintext12345678901234567890123", 60,
> KEY_SPEC_PROCESS_KEYRING);
> nit: It'd be nice to keep 80 lines per line (or 100 with string).
> Maybe use #define for payload, which makes it shorter?
>
> > +     if (encryptedkey1 == -1)
> > +             tst_brk(TBROK | TERRNO, "Failed to instantiate encrypted
> key using payload decrypted data");
> > +
> > +     TEST(keyctl(KEYCTL_READ, encryptedkey1, buffer, sizeof(buffer)));
> > +     if (TST_RET < 0)
> > +             tst_brk(TBROK | TTERRNO, "KEYCTL_READ failed for
> encryptedkey1");
> > +
> > +     encryptedkey2 = add_key("encrypted", "ltptestkey2", "new enc32
> user:masterkey 32", 27, KEY_SPEC_PROCESS_KEYRING);
> > +     if (encryptedkey2 == -1)
> > +             tst_brk(TBROK | TERRNO, "Failed to instantiate encrypted
> key using kernel-generated key material");
> > +
> > +     TEST(keyctl(KEYCTL_READ, encryptedkey2, buffer, sizeof(buffer)));
> > +     if (TST_RET < 0)
> > +             tst_brk(TBROK | TTERRNO, "KEYCTL_READ failed for
> encryptedkey2");
> At least one of these should be TFAIL - failing test. TBROK is used when
> test
> fails in test preparation phase. Also have look at
> include/tst_test_macros.h
> you can use e.g. TST_EXP_PASS_SILENT().
>
> Kind regards,
> Petr
>
> diff --git runtest/syscalls runtest/syscalls
> index bcf3d56c9c..ccea1ddbdb 100644
> --- runtest/syscalls
> +++ runtest/syscalls
> @@ -643,6 +643,7 @@ keyctl05 keyctl05
>  keyctl06 keyctl06
>  keyctl07 keyctl07
>  keyctl08 keyctl08
> +keyctl09 keyctl09
>
>  kcmp01 kcmp01
>  kcmp02 kcmp02
> diff --git testcases/kernel/syscalls/keyctl/.gitignore
> testcases/kernel/syscalls/keyctl/.gitignore
> index 3544ac79ce..f9948c1766 100644
> --- testcases/kernel/syscalls/keyctl/.gitignore
> +++ testcases/kernel/syscalls/keyctl/.gitignore
> @@ -6,3 +6,4 @@
>  /keyctl06
>  /keyctl07
>  /keyctl08
> +/keyctl09
> diff --git testcases/kernel/syscalls/keyctl/keyctl09.c
> testcases/kernel/syscalls/keyctl/keyctl09.c
> index 4589ef3679..7481526c67 100644
> --- testcases/kernel/syscalls/keyctl/keyctl09.c
> +++ testcases/kernel/syscalls/keyctl/keyctl09.c
> @@ -3,15 +3,12 @@
>   * Copyright (c) 2021 Google, Inc.
>   */
>
> -/*
> - * Description: This tests that encrypted keys can be instantiated using
> - * user-provided decrypted data (plaintext), and separately, using
> - * kernel-generated key material.
> +/*\
> + * [Description]
> + * Test that encrypted keys can be instantiated using user-provided
> decrypted
> + * data (plaintext), and separately, using kernel-generated key material.
>   */
>
> -#include <errno.h>
> -#include <stdint.h>
> -
>  #include "tst_test.h"
>  #include "lapi/keyctl.h"
>
> @@ -22,11 +19,14 @@ static void do_test(void)
>         key_serial_t encryptedkey2;
>         char buffer[128];
>
> -       masterkey = add_key("user", "user:masterkey", "foo", 3,
> KEY_SPEC_PROCESS_KEYRING);
> +       masterkey = add_key("user", "user:masterkey", "foo", 3,
> +                           KEY_SPEC_PROCESS_KEYRING);
>         if (masterkey == -1)
>                 tst_brk(TBROK | TERRNO, "Failed to add user key");
>
> -       encryptedkey1 = add_key("encrypted", "ltptestkey1", "new enc32
> user:masterkey 32 plaintext12345678901234567890123", 60,
> KEY_SPEC_PROCESS_KEYRING);
> +       encryptedkey1 = add_key("encrypted", "ltptestkey1",
> +                               "new enc32 user:masterkey 32
> plaintext12345678901234567890123",
> +                               60, KEY_SPEC_PROCESS_KEYRING);
>         if (encryptedkey1 == -1)
>                 tst_brk(TBROK | TERRNO, "Failed to instantiate encrypted
> key using payload decrypted data");
>
> @@ -34,9 +34,11 @@ static void do_test(void)
>         if (TST_RET < 0)
>                 tst_brk(TBROK | TTERRNO, "KEYCTL_READ failed for
> encryptedkey1");
>
> -       encryptedkey2 = add_key("encrypted", "ltptestkey2", "new enc32
> user:masterkey 32", 27, KEY_SPEC_PROCESS_KEYRING);
> +       encryptedkey2 = add_key("encrypted", "ltptestkey2", "new enc32
> user:masterkey 32",
> +                               27, KEY_SPEC_PROCESS_KEYRING);
>         if (encryptedkey2 == -1)
> -               tst_brk(TBROK | TERRNO, "Failed to instantiate encrypted
> key using kernel-generated key material");
> +               tst_brk(TBROK | TERRNO,
> +                       "Failed to instantiate encrypted key using
> kernel-generated key material");
>
>         TEST(keyctl(KEYCTL_READ, encryptedkey2, buffer, sizeof(buffer)));
>         if (TST_RET < 0)
>

Thank you Petr for the feedback!

I have implemented your suggestions and will repost the patch shortly.
The test failed when multiple iterations were run because the same
encrypted key cannot be added more than once. The updated test also revokes
the keys and it now passes with multiple iterations.

Yael
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20211220/5078e1e1/attachment-0001.htm>