[LTP] [PATCH] syscalls/keyctl/keyctl09.c: fix test encrypted key

[LTP] [PATCH] syscalls/keyctl/keyctl09.c: fix test encrypted key

[Nikolaus Voss]

This commit fixes the test for adding encrypted keys with unencrypted data.
Unencryted data must be provided hex-ascii encoding. Due to a kernel
bug, the unencypted data was not decoded to binary thus the length of
the key was only half the specified key size. This patch doubles the key
size and adds a test with a wrong key size to trigger a corresponding
error.

This patch must be used with the kernel fix
https://lore.kernel.org/lkml/20220919072317.E41421357@mail.steuer-voss.de

test output:
~ # ./keyctl09
tst_kconfig.c:82: TINFO: Parsing kernel config '/proc/config.gz'
tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
keyctl09.c:28: TPASS: add_key("user", "user:masterkey", "foo", 3, KEY_SPEC_PROCESS_KEYRING) returned 837616982
keyctl09.c:34: TPASS: add_key("encrypted", "ltptestkey1", ENCRYPTED_KEY_PREAMBLE HEXASCII_UNENCRYPTED_DATA, strlen(ENCRYPTED_KEY_PREAMBLE HEXASCII_UNENCRYPTED_DATA), KEY_SPEC_PROCESS_KEYRING) returned 449585633
keyctl09.c:44: TPASS: keyctl(KEYCTL_READ, TST_RET, buffer, sizeof(buffer)) returned 186
[ 3031.280283] trusted_key: encrypted key: decrypted data provided must contain only hexadecimal characters
keyctl09.c:50: TPASS: add_key("encrypted", "ltptestkey2", ENCRYPTED_KEY_PREAMBLE ASCII_UNENCRYPTED_DATA, strlen(ENCRYPTED_KEY_PREAMBLE ASCII_UNENCRYPTED_DATA), KEY_SPEC_PROCESS_KEYRING) : EINVAL (22)
[ 3031.284871] trusted_key: encrypted key: decrypted data provided does not match decrypted data length provided
keyctl09.c:58: TPASS: add_key("encrypted", "ltptestkey3", ENCRYPTED_KEY_PREAMBLE SHORT_HEXASC_UNENCRYPTED_DATA, strlen(ENCRYPTED_KEY_PREAMBLE SHORT_HEXASC_UNENCRYPTED_DATA), KEY_SPEC_PROCESS_KEYRING) : EINVAL (22)

Fixes: 342e7a0dd ("syscalls/keyctl09: test encrypted keys with provided decrypted data.")
Signed-off-by: Nikolaus Voss <nikolaus.voss@haag-streit.com>
---
 testcases/kernel/syscalls/keyctl/keyctl09.c | 30 +++++++++++++++++----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/keyctl/keyctl09.c b/testcases/kernel/syscalls/keyctl/keyctl09.c
index c88c481b9..c5165a280 100644
--- a/testcases/kernel/syscalls/keyctl/keyctl09.c
+++ b/testcases/kernel/syscalls/keyctl/keyctl09.c
@@ -13,8 +13,13 @@
 #include "tst_test.h"
 #include "lapi/keyctl.h"
 
-#define ENCRYPTED_KEY_VALID_PAYLOAD	"new enc32 user:masterkey 32 abcdefABCDEF1234567890aaaaaaaaaa"
-#define ENCRYPTED_KEY_INVALID_PAYLOAD	"new enc32 user:masterkey 32 plaintext123@123!123@123!123@123"
+#define ENCRYPTED_KEY_PREAMBLE		"new enc32 user:masterkey 32 "
+#define SHORT_HEXASC_UNENCRYPTED_DATA	"abcdefABCDEF1234567890aaaaaaaaaa"
+#define HEXASCII_UNENCRYPTED_DATA	SHORT_HEXASC_UNENCRYPTED_DATA \
+					SHORT_HEXASC_UNENCRYPTED_DATA
+#define SHORT_ASCII_UNENCRYPTED_DATA	"plaintext123@123!123@123!123@123"
+#define ASCII_UNENCRYPTED_DATA		SHORT_ASCII_UNENCRYPTED_DATA \
+					SHORT_ASCII_UNENCRYPTED_DATA
 
 static void do_test(void)
 {
@@ -27,8 +32,11 @@ static void do_test(void)
 		return;
 
 	TST_EXP_POSITIVE(add_key("encrypted", "ltptestkey1",
-			    ENCRYPTED_KEY_VALID_PAYLOAD,
-			    60, KEY_SPEC_PROCESS_KEYRING));
+			    ENCRYPTED_KEY_PREAMBLE
+			    HEXASCII_UNENCRYPTED_DATA,
+			    strlen(ENCRYPTED_KEY_PREAMBLE
+				   HEXASCII_UNENCRYPTED_DATA),
+			    KEY_SPEC_PROCESS_KEYRING));
 
 	if (!TST_PASS)
 		return;
@@ -38,8 +46,20 @@ static void do_test(void)
 	if (!TST_PASS)
 		return;
 
+	/* key not hex-ascii encoded */
 	TST_EXP_FAIL2(add_key("encrypted", "ltptestkey2",
-			    ENCRYPTED_KEY_INVALID_PAYLOAD, 60,
+			    ENCRYPTED_KEY_PREAMBLE
+			    ASCII_UNENCRYPTED_DATA,
+			    strlen(ENCRYPTED_KEY_PREAMBLE
+				   ASCII_UNENCRYPTED_DATA),
+			    KEY_SPEC_PROCESS_KEYRING), EINVAL);
+
+	/* key size mismatch */
+	TST_EXP_FAIL2(add_key("encrypted", "ltptestkey3",
+			    ENCRYPTED_KEY_PREAMBLE
+			    SHORT_HEXASC_UNENCRYPTED_DATA,
+			    strlen(ENCRYPTED_KEY_PREAMBLE
+				   SHORT_HEXASC_UNENCRYPTED_DATA),
 			    KEY_SPEC_PROCESS_KEYRING), EINVAL);
 
 	keyctl(KEYCTL_CLEAR, KEY_SPEC_PROCESS_KEYRING);
-- 
2.34.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] syscalls/keyctl/keyctl09.c: fix test encrypted key

[Mimi Zohar]

Hi Nikolaus,

On Thu, 2022-10-06 at 08:15 +0200, Nikolaus Voss wrote:
> This commit fixes the test for adding encrypted keys with unencrypted data.
> Unencryted data must be provided hex-ascii encoding. Due to a kernel
> bug, the unencypted data was not decoded to binary thus the length of
> the key was only half the specified key size. This patch doubles the key
> size and adds a test with a wrong key size to trigger a corresponding
> error.
> 
> This patch must be used with the kernel fix
> https://lore.kernel.org/lkml/20220919072317.E41421357@mail.steuer-voss.de

Petr, please correct me if I'm wrong.  Changing an existing LTP test so
that it only works on kernels with the kernel patch applied, doesn't
sound right.  The test should emit a warning if the original "valid
payload" successfully loads.

As previously suggested, instead of replacing the existing valid
payload, define a new valid payload as the hex-ascii representation of
the existing one.  The kernel decrypted data would then be the same on
systems with and without the patch.

 #define ENCRYPTED_KEY_VALID_PAYLOAD    "new enc32 user:masterkey 32 abcdefABCDE
F1234567890aaaaaaaaaa"
+#define ENCRYPTED_KEY_VALID_PAYLOAD_NEW   "new enc32 user:masterkey 32 b61626364656
64142434445463132333435363738393061616161616161616161"

-- 
thanks,

Mimi


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] syscalls/keyctl/keyctl09.c: fix test encrypted key

[Cyril Hrubis]

Hi!
> > This commit fixes the test for adding encrypted keys with unencrypted data.
> > Unencryted data must be provided hex-ascii encoding. Due to a kernel
> > bug, the unencypted data was not decoded to binary thus the length of
> > the key was only half the specified key size. This patch doubles the key
> > size and adds a test with a wrong key size to trigger a corresponding
> > error.
> > 
> > This patch must be used with the kernel fix
> > https://lore.kernel.org/lkml/20220919072317.E41421357@mail.steuer-voss.de
> 
> Petr, please correct me if I'm wrong.  Changing an existing LTP test so
> that it only works on kernels with the kernel patch applied, doesn't
> sound right.  The test should emit a warning if the original "valid
> payload" successfully loads.

We also have a policy not to work around any kernel bugs. So if this
really fixes a kernel bug it's okay that the test will fail on older
kernels without this fix and the patch that fixes kernel should be added
as a tag to the test.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] syscalls/keyctl/keyctl09.c: fix test encrypted key

[Petr Vorel]

Hi all,

> Hi!
> > > This commit fixes the test for adding encrypted keys with unencrypted data.
> > > Unencryted data must be provided hex-ascii encoding. Due to a kernel
> > > bug, the unencypted data was not decoded to binary thus the length of
> > > the key was only half the specified key size. This patch doubles the key
> > > size and adds a test with a wrong key size to trigger a corresponding
> > > error.

> > > This patch must be used with the kernel fix
> > > https://lore.kernel.org/lkml/20220919072317.E41421357@mail.steuer-voss.de

> > Petr, please correct me if I'm wrong.  Changing an existing LTP test so
> > that it only works on kernels with the kernel patch applied, doesn't
> > sound right.  The test should emit a warning if the original "valid
> > payload" successfully loads.

> We also have a policy not to work around any kernel bugs. So if this
> really fixes a kernel bug it's okay that the test will fail on older
> kernels without this fix and the patch that fixes kernel should be added
> as a tag to the test.

@Nikolaus, FYI:
https://github.com/linux-test-project/ltp/wiki/C-Test-API#138-test-tags

But the patchset has not been even accepted by kernel maintainer - searching in
the kernel thread, looking into
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/
and to the next tree.
Or am I wrong and it has been accepted?

Even once (if ever) the patch is accepted I agree with Mimi it'd be better to
add new test than change existing old (better for diagnostic what exactly went
wrong).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] syscalls/keyctl/keyctl09.c: fix test encrypted key

[Nikolaus Voss]

On Wed, 12 Oct 2022, Petr Vorel wrote:
> Hi all,
>
>> Hi!
>>>> This commit fixes the test for adding encrypted keys with unencrypted data.
>>>> Unencryted data must be provided hex-ascii encoding. Due to a kernel
>>>> bug, the unencypted data was not decoded to binary thus the length of
>>>> the key was only half the specified key size. This patch doubles the key
>>>> size and adds a test with a wrong key size to trigger a corresponding
>>>> error.
>
>>>> This patch must be used with the kernel fix
>>>> https://lore.kernel.org/lkml/20220919072317.E41421357@mail.steuer-voss.de
>
>>> Petr, please correct me if I'm wrong.  Changing an existing LTP test so
>>> that it only works on kernels with the kernel patch applied, doesn't
>>> sound right.  The test should emit a warning if the original "valid
>>> payload" successfully loads.
>
>> We also have a policy not to work around any kernel bugs. So if this
>> really fixes a kernel bug it's okay that the test will fail on older
>> kernels without this fix and the patch that fixes kernel should be added
>> as a tag to the test.
>
> @Nikolaus, FYI:
> https://github.com/linux-test-project/ltp/wiki/C-Test-API#138-test-tags
>
> But the patchset has not been even accepted by kernel maintainer - searching in
> the kernel thread, looking into
> https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/
> and to the next tree.
> Or am I wrong and it has been accepted?
>
> Even once (if ever) the patch is accepted I agree with Mimi it'd be better to
> add new test than change existing old (better for diagnostic what exactly went
> wrong).

ok, thanks, I will change the test and repost when the kernel patch has 
been accepted!

Niko


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp