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

[Petr Vorel <pvorel@suse.cz>]

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)