On Mon, Nov 21 2016 at  9:58P -0500,
Ondrej Kozina <okoz...@redhat.com> wrote:

> Changes since v2:
>   - moved rcu_read_lock() closer to key payload processing (thanks Mikulas)
>   - updated dm-crypt documentation
>   - updated code comments and unified parameter names in kernel keyring 
> function stubs
>     (#CONFIG_KEYS undefined)
> 
> The kernel key service is a generic way to store keys for the use of
> other subsystems. Currently there is no way to use kernel keys in dm-crypt.
> This patch aims to fix that. Instead of key userspace may pass a key
> description with preceding ':'. So message that constructs encryption
> mapping now looks like this:
> 
>   <cipher> [<key>|:<key_string>] <iv_offset> <dev_path> <start> 
> [<#opt_params> <opt_params>]
> 
> where <key_string> is in format: <key_size>:<key_type>:<key_description>
> 
> Currently we only support two elementary key types: 'user' and 'logon'.
> Keys may be loaded in dm-crypt either via <key_string> or using
> classical method and pass the key in hex representation directly.
> 
> dm-crypt device initialised with a key passed in hex representation may be
> replaced with key passed in key_string format and vice versa.
> 
> (Patch is based on original work by Andrey Ryabinin)
> 
> Signed-off-by: Ondrej Kozina <okoz...@redhat.com>

Other than the following cleanup patch that I've put ontop of your v3,
looks good to me.

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 7c2bbef..8fcc924 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1508,12 +1508,8 @@ static int crypt_set_keyring_key(struct crypt_config 
*cc, const char *key_string
        if (!new_key_string)
                return -ENOMEM;
 
-       /*
-        * FIXME: are there any key descriptions we should disallow users
-        * from loading to dm-crypt? i.e.: kernel keys starting with '.'
-        */
-
-       key = request_key(strncmp(key_string, "user", 4) ? &key_type_logon : 
&key_type_user, key_desc + 1, NULL);
+       key = request_key(strncmp(key_string, "user", 4) ? &key_type_logon : 
&key_type_user,
+                         key_desc + 1, NULL);
        if (IS_ERR(key)) {
                kzfree(new_key_string);
                return PTR_ERR(key);
@@ -1602,25 +1598,25 @@ static int crypt_set_key(struct crypt_config *cc, char 
*key)
        if (!cc->key_size && strcmp(key, "-"))
                goto out;
 
-       /* ':' means that the key is in kernel keyring */
-       if (key[0] == ':')
+       /* ':' means the key is in kernel keyring, short-circuit normal key 
processing */
+       if (key[0] == ':') {
                r = crypt_set_keyring_key(cc, key + 1);
-       else {
-               /* clear the flag since following operations may invalidate 
previously valid key */
-               clear_bit(DM_CRYPT_KEY_VALID, &cc->flags);
+               goto out;
+       }
 
-               /* wipe references to any kernel keyring key */
-               kzfree(cc->key_string);
-               cc->key_string = NULL;
+       /* clear the flag since following operations may invalidate previously 
valid key */
+       clear_bit(DM_CRYPT_KEY_VALID, &cc->flags);
 
-               if (cc->key_size &&
-                   crypt_decode_key(cc->key, key, cc->key_size) < 0)
-                       goto out;
+       /* wipe references to any kernel keyring key */
+       kzfree(cc->key_string);
+       cc->key_string = NULL;
 
-               r = crypt_setkey(cc);
-               if (!r)
-                       set_bit(DM_CRYPT_KEY_VALID, &cc->flags);
-       }
+       if (cc->key_size && crypt_decode_key(cc->key, key, cc->key_size) < 0)
+               goto out;
+
+       r = crypt_setkey(cc);
+       if (!r)
+               set_bit(DM_CRYPT_KEY_VALID, &cc->flags);
 out:
        /* Hex key string not needed after here, so wipe it. */
        memset(key, '0', key_string_len);

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to