For details see
Documentation/security/keys-derived.txt
Signed-off-by: Kirill Marinushkin <[email protected]>
---
Documentation/security/keys-derived.txt | 82 +++++
include/keys/derived-type.h | 33 ++
security/keys/Kconfig | 11 +
security/keys/Makefile | 1 +
security/keys/derived.c | 577 ++++++++++++++++++++++++++++++++
5 files changed, 704 insertions(+)
create mode 100644 Documentation/security/keys-derived.txt
create mode 100644 include/keys/derived-type.h
create mode 100644 security/keys/derived.c
diff --git a/Documentation/security/keys-derived.txt b/Documentation/security/keys-derived.txt
new file mode 100644
index 0000000..3c1d65c
--- /dev/null
+++ b/Documentation/security/keys-derived.txt
@@ -0,0 +1,82 @@
+ Derived Keys
+
+Derived is a keytype of the kernel keyring facility.
+The key secret is derived from the secret value given by user.
+Optionally user may specify:
+ - hash function used for derivation;
+ - salt value;
+ - number of iterations.
+Both secret value and salt value may be given in one of the formats:
+ - plain data;
+ - hex string;
+ - size of data to generate randomly.
+If no optional parameters are specified, the key is derived from
+the plain secret value with sha256, no salt, 1 iteration.
+Derived keys store as a payload:
+ - derived key;
+ - salt;
+ - number of iterations;
+ - name of derivation algorithm;
+ - name of RNG algorithm.
+From userspace only the derived key value is returned on read.
+
+Usage:
+ keyctl add derived name "key [options]" ring
+
+ mandatory parameter:
+ key - key secret value
+
+ options:
+ kf=, keyformat= - key secret value format, see dataformat below
+ s=, salt= - salt value,
+ default is empty (no salt)
+ sf=, saltformat= - salt value format, see dataformat below
+ i=, iterations= - number of itaretions,
+ default is 1, maximum is 0x000FFFFF
+ a=, algorithm= - name of crypto API hash derivation algorithm,
+ default is sha256
+ r=, rng= - name of crypto API RNG algorithm,
+ default is stdrng
+
+ dataformat:
+ plain - data is a plain value, used by default
+ hex - data is a hex string
+ rand - data is a size of random value to be generated
+
+Examples:
+
+Create a simple derived key
+
+ $ keyctl add derived key0 secret0 @u
+ 925448848
+
+ $ keyctl read 925448848
+ 32 bytes of data in key:
+ 97699b7c c0a0ed83 b78b2002 f0e57046 ee561be6 942bec25 6fe201ab ba552a9e
+
+Create a derived key from plain secret, hex salt
+
+ $ keyctl add derived key0 "secret0 s=65a4fe09 sf=hex" @u
+ 847728695
+
+ $ keyctl read 847728695
+ 32 bytes of data in key:
+ 1c64cbb9 cc4dffff a94f8efe dce813d0 5def4a28 97c02336 6c95737b f2b152be
+
+Create a derived key from hex secret value, 32-byte random salt, 65536 iterations
+
+ $keyctl add derived key0 "09afde6781ff kf=hex s=32 sf=rand i=65536" @u
+ 604146072
+
+ $ keyctl read 604146072
+ 32 bytes of data in key:
+ a5b494b3 b6e3e26c bb9511b1 b16ce60e 99edf63e d8fbc3c2 ba38b195 229e3f43
+
+Create a derived key with sha1 algorithm
+
+ $ keyctl add derived key0 "secret0 a=sha1" @u
+ 56670858
+
+ $ keyctl read 56670858
+ 20 bytes of data in key:
+ d16cd26f bc3d44a6 16b8d0b2 ce8b5ddc c93e964d
diff --git a/include/keys/derived-type.h b/include/keys/derived-type.h
new file mode 100644
index 0000000..24772a1
--- /dev/null
+++ b/include/keys/derived-type.h
@@ -0,0 +1,33 @@
+/*
+ * Derived key type
+ *
+ * For details see
+ * Documentation/security/keys-derived.txt
+ *
+ * Copyright (C) 2016
+ * Written by Kirill Marinushkin ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ *
+ */
+
+#ifndef INCLUDE_KEYS_DERIVED_TYPE_H_
+#define INCLUDE_KEYS_DERIVED_TYPE_H_
+
+#include <linux/key.h>
+
+extern struct key_type key_type_derived;
+
+extern int derived_instantiate(struct key *key,
+ struct key_preparsed_payload *prep);
+extern int derived_update(struct key *key,
+ struct key_preparsed_payload *prep);
+extern long derived_read(const struct key *key,
+ char __user *buffer, size_t buflen);
+extern void derived_revoke(struct key *key);
+extern void derived_destroy(struct key *key);
+
+#endif /* INCLUDE_KEYS_DERIVED_TYPE_H_ */
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index fe4d74e..261994f 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -81,3 +81,14 @@ config ENCRYPTED_KEYS
Userspace only ever sees/stores encrypted blobs.
If you are unsure as to whether this is required, answer N.
+
+config DERIVED_KEYS
+ tristate "Derived keys"
+ depends on KEYS
+ select CRYPTO
+ select CRYPTO_SHA256
+ select CRYPTO_RNG
+ help
+ This option provides support for derived key type.
+
+ If you are unsure as to whether this is required, answer N.
diff --git a/security/keys/Makefile b/security/keys/Makefile
index dfb3a7b..fbe954d 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -26,3 +26,4 @@ obj-$(CONFIG_PERSISTENT_KEYRINGS) += persistent.o
obj-$(CONFIG_BIG_KEYS) += big_key.o
obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/
+obj-$(CONFIG_DERIVED_KEYS) += derived.o
diff --git a/security/keys/derived.c b/security/keys/derived.c
new file mode 100644
index 0000000..18085ce
--- /dev/null
+++ b/security/keys/derived.c
@@ -0,0 +1,577 @@
+/*
+ * Derived key type
+ *
+ * For details see
+ * Documentation/security/keys-derived.txt
+ *
+ * Copyright (C) 2016
+ * Written by Kirill Marinushkin ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/seq_file.h>
+#include <linux/err.h>
+#include <linux/parser.h>
+#include <linux/key.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+#include <keys/user-type.h>
+#include <keys/derived-type.h>
+#include <crypto/hash.h>
+#include <crypto/rng.h>
+#include "internal.h"
+
+/* KERN_ERR prefix */
+#define PREFIX "derived: "
+
+/* Limits */
+#define ITER_MAX_VAL 0x000FFFFF
+#define SALT_MAX_SIZE 1024
+#define RAND_MAX_SIZE 1024
+
+/* Default values */
+#define ITER_DEFAULT 1
+#define ALG_NAME_DEFAULT "sha256"
+#define RNG_NAME_DEFAULT "stdrng"
+
+/* Options */
+enum {
+ OPT_SHORT_SALT,
+ OPT_LONG_SALT,
+ OPT_SHORT_ITER,
+ OPT_LONG_ITER,
+ OPT_SHORT_ALG,
+ OPT_LONG_ALG,
+ OPT_SHORT_RNG,
+ OPT_LONG_RNG,
+ OPT_SHORT_KEY_F,
+ OPT_LONG_KEY_F,
+ OPT_SHORT_SALT_F,
+ OPT_LONG_SALT_F
+};
+
+/* Options data formats */
+enum derived_opt_format {
+ OPT_FORMAT_ERR = -1,
+ OPT_FORMAT_PLAIN,
+ OPT_FORMAT_HEX,
+ OPT_FORMAT_RAND
+};
+
+/* Options data index */
+enum {
+ OPT_IND_KEY = 0,
+ OPT_IND_SALT,
+ OPT_IND_NUM /* number of indexes */
+};
+
+struct derived_blob {
+ u8 *data;
+ size_t *lenp;
+};
+
+struct derived_f_blob {
+ enum derived_opt_format format;
+ struct derived_blob *b;
+};
+
+struct derived_key_payload {
+ struct rcu_head rcu; /* RCU destructor */
+ char *alg_name; /* null-terminated digest algorithm name */
+ char *rng_name; /* null-terminated random generator algorithm name */
+ u64 iter; /* number of iterations */
+ unsigned int saltlen; /* length of salt */
+ unsigned char *salt; /* salt */
+ unsigned int datalen; /* length of derived data */
+ unsigned char *data; /* derived data */
+};
+
+/* Get option data format specified by user */
+static enum derived_opt_format get_opt_format(const char *arg)
+{
+ if (!strcmp(arg, "plain"))
+ return OPT_FORMAT_PLAIN;
+ if (!strcmp(arg, "hex"))
+ return OPT_FORMAT_HEX;
+ if (!strcmp(arg, "rand"))
+ return OPT_FORMAT_RAND;
+ return OPT_FORMAT_ERR;
+}
+
+/* Generate random data */
+static int gen_random(const char *rnd_name, u8 *buf, unsigned int len)
+{
+ int ret = -EINVAL;
+ struct crypto_rng *rng = NULL;
+
+ rng = crypto_alloc_rng(rnd_name, 0, 0);
+ if (IS_ERR(rng)) {
+ pr_err(PREFIX "RNG alloc failed");
+ return -EINVAL;
+ }
+
+ ret = crypto_rng_get_bytes(rng, buf, len);
+ if (ret < 0) {
+ pr_err(PREFIX "RNG get bytes failed");
+ ret = -EFAULT;
+ }
+
+ if (rng)
+ crypto_free_rng(rng);
+ return ret;
+}
+
+/* Parse options specified by user */
+static int parse_options(char **args_str,
+ struct derived_key_payload *payload, struct derived_blob *ukey)
+{
+ int ret = -EINVAL;
+ substring_t args[MAX_OPT_ARGS];
+ char *p = *args_str;
+ int token;
+ int i;
+ unsigned short templen;
+ unsigned int tempu;
+ u64 tempul;
+ struct derived_blob usalt = {NULL};
+ struct derived_f_blob v[OPT_IND_NUM] = {
+ {OPT_FORMAT_PLAIN, NULL}
+ };
+ const match_table_t key_tokens = {
+ {OPT_SHORT_SALT, "s=%s"},
+ {OPT_LONG_SALT, "salt=%s"},
+ {OPT_SHORT_ITER, "i=%u"},
+ {OPT_LONG_ITER, "iterations=%u"},
+ {OPT_SHORT_ALG, "a=%s"},
+ {OPT_LONG_ALG, "algorithm=%s"},
+ {OPT_SHORT_RNG, "r=%s"},
+ {OPT_LONG_RNG, "rng=%s"},
+ {OPT_SHORT_KEY_F, "kf=%s"},
+ {OPT_LONG_KEY_F, "keyformat=%s"},
+ {OPT_SHORT_SALT_F, "sf=%s"},
+ {OPT_LONG_SALT_F, "saltformat=%s"}
+ };
+
+ /* set defaults */
+ payload->iter = ITER_DEFAULT;
+ payload->alg_name = kstrdup(ALG_NAME_DEFAULT, GFP_KERNEL);
+ if (!payload->alg_name) {
+ pr_err(PREFIX "default algorithm name alloc failed");
+ return -ENOMEM;
+ }
+ payload->rng_name = kstrdup(RNG_NAME_DEFAULT, GFP_KERNEL);
+ if (!payload->rng_name) {
+ pr_err(PREFIX "default RNG name alloc failed");
+ return -ENOMEM;
+ }
+
+ /* parse key */
+ ukey->data = strsep(args_str, " \t");
+ if (!ukey->data) {
+ pr_err(PREFIX "input string separation failed");
+ return -EINVAL;
+ }
+ ukey->lenp = kmalloc(sizeof(*ukey->lenp), GFP_KERNEL);
+ if (!ukey->lenp) {
+ pr_err(PREFIX "input key secret alloc failed");
+ return -ENOMEM;
+ }
+ *ukey->lenp = strlen(ukey->data);
+
+ /* prepare format blob array */
+ v[OPT_IND_KEY].b = ukey;
+ v[OPT_IND_SALT].b = &usalt;
+
+ /* parse options */
+ while ((p = strsep(args_str, " \t"))) {
+ if (*p == '\0' || *p == ' ' || *p == '\t')
+ continue;
+
+ token = match_token(p, key_tokens, args);
+
+ switch (token) {
+
+ case OPT_SHORT_SALT: /* salt */
+ case OPT_LONG_SALT:
+ templen = args[0].to - args[0].from;
+ if (templen < 0 || templen > SALT_MAX_SIZE) {
+ pr_err(PREFIX "invalid salt length");
+ return -EINVAL;
+ }
+ payload->salt = kstrndup(args[0].from, templen, GFP_KERNEL);
+ if (!payload->salt) {
+ pr_err(PREFIX "salt alloc failed");
+ return -ENOMEM;
+ }
+ payload->saltlen = templen;
+ usalt.data = payload->salt;
+ usalt.lenp = &payload->saltlen;
+ break;
+
+ case OPT_SHORT_ITER: /* iterations */
+ case OPT_LONG_ITER:
+ if (kstrtou64(args[0].from, 0, &tempul)
+ || tempul == 0
+ || tempul > ITER_MAX_VAL) {
+ pr_err(PREFIX "invalid iterations number");
+ return -EINVAL;
+ }
+ payload->iter = tempul;
+ break;
+
+ case OPT_SHORT_ALG: /* alg name */
+ case OPT_LONG_ALG:
+ payload->alg_name = kstrdup(args[0].from, GFP_KERNEL);
+ if (!payload->alg_name) {
+ pr_err(PREFIX "algorithm name alloc failed");
+ return -ENOMEM;
+ }
+ break;
+
+ case OPT_SHORT_RNG: /* rng name */
+ case OPT_LONG_RNG:
+ payload->rng_name = kstrdup(args[0].from, GFP_KERNEL);
+ if (!payload->rng_name) {
+ pr_err(PREFIX "RNG name alloc failed");
+ return -ENOMEM;
+ }
+ break;
+
+ case OPT_SHORT_KEY_F: /* key format */
+ case OPT_LONG_KEY_F:
+ v[OPT_IND_KEY].format = get_opt_format(args[0].from);
+ if (v[OPT_IND_KEY].format == OPT_FORMAT_ERR) {
+ pr_err(PREFIX "invalid key format");
+ return -EINVAL;
+ }
+ break;
+
+ case OPT_SHORT_SALT_F: /* salt format */
+ case OPT_LONG_SALT_F:
+ v[OPT_IND_SALT].format = get_opt_format(args[0].from);
+ if (v[OPT_IND_SALT].format == OPT_FORMAT_ERR) {
+ pr_err(PREFIX "invalid salt format");
+ return -EINVAL;
+ }
+ break;
+
+ default:
+ pr_err(PREFIX "unsupported option");
+ return -EINVAL;
+ }
+ }
+
+ /* modify options according to format */
+ for (i = 0; i < OPT_IND_NUM; i++) {
+ if (!v[i].b || !v[i].b->data)
+ continue;
+
+ switch (v[i].format) {
+
+ case OPT_FORMAT_HEX:
+ if (*v[i].b->lenp % 2) {
+ pr_err(PREFIX "invalid hex string");
+ return -EINVAL;
+ }
+ *v[i].b->lenp /= 2;
+ ret = hex2bin(v[i].b->data, v[i].b->data, *v[i].b->lenp);
+ if (ret) {
+ pr_err(PREFIX "invalid hex string");
+ return -EINVAL;
+ }
+ break;
+
+ case OPT_FORMAT_RAND:
+ if (kstrtouint(v[i].b->data, 0, &tempu)
+ || tempu == 0
+ || tempu > RAND_MAX_SIZE) {
+ pr_err(PREFIX "invalid random size");
+ return -EINVAL;
+ }
+ v[i].b->data = kmalloc(tempu, GFP_KERNEL);
+ if (!v[i].b->data) {
+ pr_err(PREFIX "random data alloc failed");
+ return -ENOMEM;
+ }
+ *v[i].b->lenp = tempu;
+ ret = gen_random(payload->rng_name, v[i].b->data, *v[i].b->lenp);
+ if (ret)
+ return ret;
+ break;
+
+ default:
+ break;
+ }
+ }
+
+ return 0;
+}
+
+/* Free and zero payload fields */
+static void free_payload_content(struct derived_key_payload *payload)
+{
+ if (payload->alg_name)
+ kzfree(payload->alg_name);
+ if (payload->rng_name)
+ kzfree(payload->rng_name);
+ if (payload->data)
+ kzfree(payload->data);
+ if (payload->salt)
+ kzfree(payload->salt);
+}
+
+/* Fill derived key payload with data specified by user */
+static int fill_payload(struct derived_key_payload *payload,
+ struct key_preparsed_payload *prep)
+{
+ int ret = -EINVAL;
+ char *args_str = NULL;
+ struct derived_blob ukey = {NULL};
+ struct crypto_shash *sh = NULL;
+ struct shash_desc *sdesc = NULL;
+ unsigned int i;
+
+ if (!payload || prep->datalen <= 0 || prep->datalen > 32767 || !prep->data) {
+ pr_err(PREFIX "invalid data for payload");
+ return -EINVAL;
+ }
+
+ args_str = kstrndup(prep->data, prep->datalen, GFP_KERNEL);
+ if (!args_str) {
+ pr_err(PREFIX "input arguments alloc failed");
+ return -EINVAL;
+ }
+
+ ret = parse_options(&args_str, payload, &ukey);
+ if (ret)
+ return ret;
+ if (!ukey.data || !ukey.lenp) {
+ pr_err(PREFIX "invalid key input parsed");
+ return -EINVAL;
+ }
+
+ /* start derivation */
+ sh = crypto_alloc_shash(payload->alg_name, 0, 0);
+ if (IS_ERR(sh)) {
+ pr_err(PREFIX "shash alloc failed");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ sdesc = kzalloc(sizeof(struct shash_desc) + crypto_shash_descsize(sh), GFP_KERNEL);
+ if (!sdesc) {
+ pr_err(PREFIX "sdesc alloc failed");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ sdesc->tfm = sh;
+ sdesc->flags = 0;
+
+ payload->datalen = crypto_shash_digestsize(sh);
+ if (payload->data)
+ kzfree(payload->data);
+ payload->data = kmalloc(payload->datalen, GFP_KERNEL);
+ if (!payload->data) {
+ pr_err(PREFIX "payload data alloc failed");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < payload->iter; i++) {
+ ret = crypto_shash_init(sdesc);
+ if (ret) {
+ pr_err(PREFIX "shash init failed");
+ goto out;
+ }
+
+ if (i == 0) {
+ /* first iteration */
+ ret = crypto_shash_update(sdesc, ukey.data, *ukey.lenp);
+ if (ret) {
+ pr_err(PREFIX "shash update failed");
+ goto out;
+ }
+
+ ret = crypto_shash_update(sdesc, payload->salt, payload->saltlen);
+ if (ret) {
+ pr_err(PREFIX "shash update failed");
+ goto out;
+ }
+ } else {
+ /* next iterations */
+ ret = crypto_shash_update(sdesc, payload->data, payload->datalen);
+ if (ret) {
+ pr_err(PREFIX "shash update failed");
+ goto out;
+ }
+ }
+
+ ret = crypto_shash_final(sdesc, payload->data);
+ if (ret) {
+ pr_err(PREFIX "shash final failed");
+ goto out;
+ }
+
+ }
+
+out:
+ if (sdesc)
+ kzfree(sdesc);
+ if (!IS_ERR(sh))
+ crypto_free_shash(sh);
+ if (args_str)
+ kzfree(args_str);
+ return ret;
+}
+
+/* Reserve payload for derived key */
+static int reserve_derived_payload(struct key *key,
+ struct derived_key_payload *payload)
+{
+ return key_payload_reserve(key, sizeof(*payload)
+ + payload->datalen + payload->saltlen
+ + strlen(payload->alg_name) + strlen(payload->rng_name) + 2);
+}
+
+/* Derived key instantiate */
+int derived_instantiate(struct key *key, struct key_preparsed_payload *prep)
+{
+ int ret = -EINVAL;
+ struct derived_key_payload *payload = NULL;
+
+ if (prep->datalen <= 0 || prep->datalen > 32767 || !prep->data) {
+ pr_err(PREFIX "invalid input data");
+ return -EINVAL;
+ }
+
+ payload = kzalloc(sizeof(*payload), GFP_KERNEL);
+ if (!payload) {
+ pr_err(PREFIX "payload alloc failed");
+ return -ENOMEM;
+ }
+
+ /* fill payload */
+ ret = fill_payload(payload, prep);
+ if (!ret)
+ ret = reserve_derived_payload(key, payload);
+
+ /* assign key if succeed */
+ if (!ret)
+ rcu_assign_keypointer(key, payload);
+ else
+ kzfree(key->payload.data);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(derived_instantiate);
+
+/* Derived key update */
+int derived_update(struct key *key, struct key_preparsed_payload *prep)
+{
+ int ret = -EINVAL;
+ struct derived_key_payload *payload =
+ (struct derived_key_payload *)key->payload.data;
+
+ /* free current payload */
+ free_payload_content(payload);
+ memset(payload, 0x00, sizeof(*payload));
+
+ ret = fill_payload(payload, prep);
+ if (!ret)
+ ret = reserve_derived_payload(key, payload);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(derived_update);
+
+/* Derived key read */
+long derived_read(const struct key *key, char __user *buffer, size_t buflen)
+{
+ long len = -1;
+ struct derived_key_payload *payload = rcu_dereference_key(key);
+
+ if (!payload) {
+ pr_err(PREFIX "invalid key payload");
+ return -EINVAL;
+ }
+
+ len = payload->datalen;
+ if (buffer && buflen > 0) {
+ /* copy to buffer */
+ if (buflen < payload->datalen
+ || copy_to_user(buffer, payload->data, payload->datalen)) {
+ pr_err(PREFIX "read key data failed");
+ return -EFAULT;
+ }
+ } /* else return without copy */
+
+ return len;
+}
+EXPORT_SYMBOL_GPL(derived_read);
+
+/* Derived key revoke */
+void derived_revoke(struct key *key)
+{
+ struct derived_key_payload *payload =
+ (struct derived_key_payload *)key->payload.data;
+
+ /* clear the quota */
+ key_payload_reserve(key, 0);
+
+ if (payload) {
+ rcu_assign_keypointer(key, NULL);
+ kfree_rcu(payload, rcu);
+ }
+}
+EXPORT_SYMBOL(derived_revoke);
+
+/* Derived key destroy */
+void derived_destroy(struct key *key)
+{
+ struct derived_key_payload *payload =
+ (struct derived_key_payload *)key->payload.data;
+
+ if (!payload)
+ return;
+
+ free_payload_content(payload);
+
+ kzfree(payload);
+}
+EXPORT_SYMBOL_GPL(derived_destroy);
+
+struct key_type key_type_derived = {
+ .name = "derived",
+ .instantiate = derived_instantiate,
+ .update = derived_update,
+ .destroy = derived_destroy,
+ .revoke = derived_revoke,
+ .describe = user_describe,
+ .read = derived_read,
+};
+EXPORT_SYMBOL_GPL(key_type_derived);
+
+static int __init init_derived(void)
+{
+ return register_key_type(&key_type_derived);
+}
+
+static void __exit cleanup_derived(void)
+{
+ unregister_key_type(&key_type_derived);
+}
+
+late_initcall(init_derived);
+module_exit(cleanup_derived);
+
+MODULE_LICENSE("GPL");
--
1.9.1
Hi Kirill,
[auto build test WARNING on v4.5-rc7]
[also build test WARNING on next-20160321]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Kirill-Marinushkin/Security-Keys-Added-derived-keytype/20160322-084809
config: sparc64-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All warnings (new ones prefixed by >>):
security/keys/derived.c: In function 'parse_options':
>> security/keys/derived.c:217:15: warning: assignment from incompatible pointer type
usalt.lenp = &payload->saltlen;
^
vim +217 security/keys/derived.c
201 switch (token) {
202
203 case OPT_SHORT_SALT: /* salt */
204 case OPT_LONG_SALT:
205 templen = args[0].to - args[0].from;
206 if (templen < 0 || templen > SALT_MAX_SIZE) {
207 pr_err(PREFIX "invalid salt length");
208 return -EINVAL;
209 }
210 payload->salt = kstrndup(args[0].from, templen, GFP_KERNEL);
211 if (!payload->salt) {
212 pr_err(PREFIX "salt alloc failed");
213 return -ENOMEM;
214 }
215 payload->saltlen = templen;
216 usalt.data = payload->salt;
> 217 usalt.lenp = &payload->saltlen;
218 break;
219
220 case OPT_SHORT_ITER: /* iterations */
221 case OPT_LONG_ITER:
222 if (kstrtou64(args[0].from, 0, &tempul)
223 || tempul == 0
224 || tempul > ITER_MAX_VAL) {
225 pr_err(PREFIX "invalid iterations number");
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Kirill Marinushkin <[email protected]> wrote:
> For details see
> Documentation/security/keys-derived.txt
Usage?
David
>> For details see
>> Documentation/security/keys-derived.txt
>
> Usage?
>
> David
You maybe didn't receive my answer sent from gmail web-interface, so I repeat it now from mutt.
Usages of derived keys that I can see:
kernel space:
derive keys from "trusted" (with possibility to access from user space if proper permissions are set);
user space:
store passwords within keyrings;
randomly generated keys, keys with payload given as hex string.
What's your opinion on having derived keytype?
--
Best Regards,
Kirill Marinushkin
Kirill Marinushkin <[email protected]> wrote:
> kernel space:
> derive keys from "trusted" (with possibility to access from user space if proper permissions are set);
> user space:
> store passwords within keyrings;
> randomly generated keys, keys with payload given as hex string.
>
> What's your opinion on having derived keytype?
Ummm... I'm not keen on the name; it doesn't really capture what the key is
for.
Apart from that, let me have another look through the patch.
David
Kirill Marinushkin <[email protected]> wrote:
> For details see
> Documentation/security/keys-derived.txt
Please include at least a summary in the patch description, not just a pointer
to the documentation file.
> + Derived Keys
> +
> +Derived is a keytype of the kernel keyring facility.
> +The key secret is derived from the secret value given by user.
I'm not keen on the type name "derived" as it's totally non-obvious. How
about "secret", "shared-secret" or "salted" or something like that.
> + i=, iterations= - number of itaretions,
"iterations"
> +#ifndef INCLUDE_KEYS_DERIVED_TYPE_H_
> +#define INCLUDE_KEYS_DERIVED_TYPE_H_
I would drop the initial "INCLUDE_" from that.
> +extern int derived_instantiate(struct key *key,
> + struct key_preparsed_payload *prep);
> +extern int derived_update(struct key *key,
> + struct key_preparsed_payload *prep);
> +extern long derived_read(const struct key *key,
> + char __user *buffer, size_t buflen);
> +extern void derived_revoke(struct key *key);
> +extern void derived_destroy(struct key *key);
Is there a reason you're exporting all the methods?
> +struct derived_key_payload {
Should this struct go in your type header?
> + struct rcu_head rcu; /* RCU destructor */
> + char *alg_name; /* null-terminated digest algorithm name */
> + char *rng_name; /* null-terminated random generator algorithm name */
> + u64 iter; /* number of iterations */
Isn't the max value for this 0x000FFFFF? If so, why is it u64?
> + unsigned int saltlen; /* length of salt */
> + unsigned char *salt; /* salt */
> + unsigned int datalen; /* length of derived data */
> + unsigned char *data; /* derived data */
Reorder these to put saltlen and datalen next to each other, thereby
eliminating two holes in the struct on a 64-bit machine.
> +static int gen_random(const char *rnd_name, u8 *buf, unsigned int len)
Prefix with "derived_" please.
> + case OPT_FORMAT_RAND:
> + if (kstrtouint(v[i].b->data, 0, &tempu)
> + || tempu == 0
> + || tempu > RAND_MAX_SIZE) {
> + pr_err(PREFIX "invalid random size");
> + return -EINVAL;
> + }
> + v[i].b->data = kmalloc(tempu, GFP_KERNEL);
> + if (!v[i].b->data) {
> + pr_err(PREFIX "random data alloc failed");
> + return -ENOMEM;
> + }
> + *v[i].b->lenp = tempu;
> + ret = gen_random(payload->rng_name, v[i].b->data, *v[i].b->lenp);
I would move the kmalloc() inside the gen_random() function.
> +static void free_payload_content(struct derived_key_payload *payload)
> +{
> + if (payload->alg_name)
> + kzfree(payload->alg_name);
> + if (payload->rng_name)
> + kzfree(payload->rng_name);
> + if (payload->data)
> + kzfree(payload->data);
> + if (payload->salt)
> + kzfree(payload->salt);
> +}
kzfree() can handle a NULL pointer. You've got more instances of this.
Your functions should all be prefixed with "derived_".
> + sdesc = kzalloc(sizeof(struct shash_desc) + crypto_shash_descsize(sh), GFP_KERNEL);
Do you need some wrappers on this to get the alignment correct?
> + if (!sdesc) {
> + pr_err(PREFIX "sdesc alloc failed");
Don't print an error here.
> + ret = -ENOMEM;
> + goto out;
> + }
You should stick a label about four lines below "out:" and go there instead.
Then you can get rid of the conditionalisation in the following:
+ if (!IS_ERR(sh))
+ crypto_free_shash(sh);
> + payload = kzalloc(sizeof(*payload), GFP_KERNEL);
> + if (!payload) {
> + pr_err(PREFIX "payload alloc failed");
> + return -ENOMEM;
> + }
> +
> + /* fill payload */
> + ret = fill_payload(payload, prep);
Move the kzalloc() call into fill_payload().
> +int derived_update(struct key *key, struct key_preparsed_payload *prep)
> +{
> + int ret = -EINVAL;
> + struct derived_key_payload *payload =
> + (struct derived_key_payload *)key->payload.data;
> +
> + /* free current payload */
> + free_payload_content(payload);
> + memset(payload, 0x00, sizeof(*payload));
> +
> + ret = fill_payload(payload, prep);
> + if (!ret)
> + ret = reserve_derived_payload(key, payload);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(derived_update);
This is *not* RCU safe.
You should implement the ->preparse() method and do the argument parsing and
creation and filling in of struct derived_key_payload there. Take a look at
user_preparse(). I would start by renaming fill_payload() to
derived_preparse() - it's almost exactly what you want.
You will also need to implement ->free_preparse().
You can then get rid of reserve_derived_payload() and just put the quota
amount into prep->quotalen and the payload into prep->payload.data[0].
derived_instantiate() can then be replaced with generic_key_instantiate.
Since derived_preparse() would be called prior to derived_update(), the latter
can just replace where prep->payload.data[0] points using
rcu_assign_keypointer() and then call_rcu() on the old payload.
David