* Re: [LTP] [PATCH] syscalls/keyctl09: test encrypted keys.
@ 2021-12-17 13:56 ` Petr Vorel
0 siblings, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2021-12-17 13:56 UTC (permalink / raw)
To: Yael Tiomkin; +Cc: linux-integrity, ltp
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)
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [LTP] [PATCH] syscalls/keyctl09: test encrypted keys.
2021-12-17 13:56 ` [LTP] " Petr Vorel
(?)
@ 2021-12-21 2:35 ` Yael Tiomkin via ltp
-1 siblings, 0 replies; 5+ messages in thread
From: Yael Tiomkin via ltp @ 2021-12-21 2:35 UTC (permalink / raw)
To: Petr Vorel; +Cc: linux-integrity, ltp
[-- Attachment #1.1: Type: text/plain, Size: 7460 bytes --]
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
[-- Attachment #1.2: Type: text/html, Size: 9807 bytes --]
[-- Attachment #2: Type: text/plain, Size: 60 bytes --]
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 5+ messages in thread