Re: [RFC PATCH v2 5/5] test-crypto-secret: add 'syskey' object tests.

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Apr 16, 2020 at 01:25:25AM +0300, Alexey Krasikov wrote:
> * test_secret_seckey_bad_key_access_right() is not working yet.
>   We don't know yet if this due a bag in the Linux kernel or
>   whether it's normal syscall behavior.
>   We've requested information from kernel maintainer.
> 
> Signed-off-by: Alexey Krasikov <alex-krasikov@yandex-team.ru>
> ---
>  tests/test-crypto-secret.c | 138 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 138 insertions(+)
> 
> diff --git a/tests/test-crypto-secret.c b/tests/test-crypto-secret.c
> index 13fc6c4c75..6b17fe3a81 100644
> --- a/tests/test-crypto-secret.c
> +++ b/tests/test-crypto-secret.c
> @@ -22,8 +22,10 @@
>  
>  #include "crypto/init.h"
>  #include "crypto/secret.h"
> +#include "crypto/linux_keyring.h"
>  #include "qapi/error.h"
>  #include "qemu/module.h"
> +#include <keyutils.h>
>  
>  static void test_secret_direct(void)
>  {
> @@ -125,6 +127,132 @@ static void test_secret_indirect_emptyfile(void)
>  }
>  
>  
> +#define DESCRIPTION "qemu_test_secret"
> +#define PAYLOAD "Test Payload"
> +
> +
> +static void test_secret_seckey_good(void)
> +{
> +    char key_str[16];
> +    Object *sec;
> +    key_serial_t key = add_key("user", DESCRIPTION, PAYLOAD,
> +                               strlen(PAYLOAD), KEY_SPEC_PROCESS_KEYRING);
> +
> +    g_assert(key >= 0);
> +
> +    snprintf(key_str, sizeof(key_str), "0x%08x", key);
> +    sec = object_new_with_props(
> +        TYPE_QCRYPTO_SECRET_LINUX_KEYRING,
> +        object_get_objects_root(),
> +        "sec0",
> +        &error_abort,
> +        "serial", key_str,
> +        NULL);
> +
> +    assert(0 <= keyctl_unlink(key, KEY_SPEC_PROCESS_KEYRING));
> +    char *pw = qcrypto_secret_lookup_as_utf8("sec0",
> +                                             &error_abort);
> +    g_assert_cmpstr(pw, ==, PAYLOAD);
> +
> +    object_unparent(sec);
> +    g_free(pw);
> +}
> +
> +
> +static void test_secret_seckey_revoked_key(void)
> +{
> +    char key_str[16];
> +    Object *sec;
> +    key_serial_t key = add_key("user", DESCRIPTION, PAYLOAD,
> +                               strlen(PAYLOAD), KEY_SPEC_PROCESS_KEYRING);
> +    g_assert(key >= 0);
> +    g_assert_false(keyctl_revoke(key));
> +
> +    snprintf(key_str, sizeof(key_str), "0x%08x", key);
> +    sec = object_new_with_props(
> +        TYPE_QCRYPTO_SECRET_LINUX_KEYRING,
> +        object_get_objects_root(),
> +        "sec0",
> +        NULL,
> +        "serial", key_str,
> +        NULL);
> +
> +    g_assert(errno == EKEYREVOKED);
> +    g_assert(sec == NULL);
> +
> +    keyctl_unlink(key, KEY_SPEC_PROCESS_KEYRING);
> +}
> +
> +
> +static void test_secret_seckey_expired_key(void)
> +{
> +    char key_str[16];
> +    Object *sec;
> +    key_serial_t key = add_key("user", DESCRIPTION, PAYLOAD,
> +                               strlen(PAYLOAD), KEY_SPEC_PROCESS_KEYRING);
> +    g_assert(key >= 0);
> +    g_assert_false(keyctl_set_timeout(key, 1));
> +    sleep(1);
> +
> +    snprintf(key_str, sizeof(key_str), "0x%08x", key);
> +    sec = object_new_with_props(
> +        TYPE_QCRYPTO_SECRET_LINUX_KEYRING,
> +        object_get_objects_root(),
> +        "sec0",
> +        NULL,
> +        "serial", key_str,
> +        NULL);
> +
> +    g_assert(errno == EKEYEXPIRED);
> +    g_assert(sec == NULL);
> +
> +    keyctl_unlink(key, KEY_SPEC_PROCESS_KEYRING);
> +}
> +
> +
> +static void test_secret_seckey_bad_serial_key(void)
> +{
> +    Object *sec;
> +
> +    sec = object_new_with_props(
> +        TYPE_QCRYPTO_SECRET,
> +        object_get_objects_root(),
> +        "sec0",
> +        NULL,
> +        "serial", "1",
> +        NULL);
> +
> +    g_assert(errno == ENOKEY);
> +    g_assert(sec == NULL);
> +}
> +
> +
> +static void test_secret_seckey_bad_key_access_right(void)
> +{
> +    char key_str[16];
> +    Object *sec;
> +    key_serial_t key = add_key("user", DESCRIPTION, PAYLOAD,
> +                               strlen(PAYLOAD), KEY_SPEC_PROCESS_KEYRING);
> +    g_assert(key >= 0);
> +    g_assert_false(keyctl_setperm(key, KEY_POS_ALL & (~KEY_POS_READ)));
> +
> +    snprintf(key_str, sizeof(key_str), "0x%08x", key);
> +
> +    sec = object_new_with_props(
> +        TYPE_QCRYPTO_SECRET_LINUX_KEYRING,
> +        object_get_objects_root(),
> +        "sec0",
> +        NULL,
> +        "serial", key_str,
> +        NULL);
> +
> +    g_assert(errno == EACCES);
> +    g_assert(sec == NULL);
> +
> +    keyctl_unlink(key, KEY_SPEC_PROCESS_KEYRING);
> +}
> +
> +
>  static void test_secret_noconv_base64_good(void)
>  {
>      Object *sec = object_new_with_props(
> @@ -425,6 +553,16 @@ int main(int argc, char **argv)
>                      test_secret_indirect_badfile);
>      g_test_add_func("/crypto/secret/indirect/emptyfile",
>                      test_secret_indirect_emptyfile);
> +    g_test_add_func("/crypto/secret/seckey/good",
> +                    test_secret_seckey_good);
> +    g_test_add_func("/crypto/secret/seckey/revoked_key",
> +                    test_secret_seckey_revoked_key);
> +    g_test_add_func("/crypto/secret/seckey/expired_key",
> +                    test_secret_seckey_expired_key);
> +    g_test_add_func("/crypto/secret/seckey/bad_serial_key",
> +                    test_secret_seckey_bad_serial_key);
> +    g_test_add_func("/crypto/secret/seckey/bad_key_access_right",
> +                    test_secret_seckey_bad_key_access_right);

Again, we'll need to make this all conditional based on the result
of a configure check to avoid breaking non-Linux.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|