Quoting Mimi Zohar (zohar@xxxxxxxxxxxxxxxxxx):
Looks fine to me, and very useful.
Acked-by: Serge E. Hallyn <serge@xxxxxxxxxx>
(for 1-3, haven't looked at 4 yet and won't tonight)
> +config TRUSTED_KEYS
> + tristate "TRUSTED KEYS"
> + depends on KEYS && TCG_TPM
> + select CRYPTO
> + select CRYPTO_HMAC
> + select CRYPTO_SHA1
> + help
> + This option provides support for creating/sealing/unsealing keys
> + in the kernel. Trusted keys are TPM generated random numbers
> + symmetric keys, RSA sealed by the TPM, and only unsealed by the
> + TPM, if boot PCRs and other criteria match. Userspace ever only
> + sees/stores encrypted blobs.
"
This option provides support for creating, sealing, and unsealing keys
in the kernel. Trusted keys are ramdon symmetric keys created
RSA-sealed, and stored in the TPM. The TPM only unseals the keys if the
boot PCRs and other criteria match. Userspace can only ever see
encrypted blobs.
"
> diff --git a/security/keys/trusted_defined.c b/security/keys/trusted_defined.c
> new file mode 100644
> index 0000000..0bd935f
> --- /dev/null
> +++ b/security/keys/trusted_defined.c
> @@ -0,0 +1,999 @@
> +/*
> + * Copyright (C) 2010 IBM Corporation
> + *
> + * Author:
> + * David Safford <safford@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + *
> + * File: trusted_defined.c
(don't put filenames in comments :)
> + * Defines a new kernel key-type called 'trusted'. Trusted keys are
> + * TPM generated random numbers, RSA sealed by the TPM, and only unsealed
> + * by the TPM, if boot PCRs and other criteria match. Trusted keys are
> + * created/sealed/unsealed in the kernel. Userspace ever only sees/stores
> + * encrypted blobs.
> + *
> + * Keys are sealed under the SRK, which must have the default
> + * authorization value (20 zeros).
What does this mean? They have to be sealed by a key that starts
with 20 zeros? (of course it doesn't mean that, but i don't understand
what it does mean :)
> This can be set at takeownership
> + * time with the trouser's utility "tpm_takeownership -u -z".
> + *
> + * Usage:
> + * keyctl add trusted name "NEW keylen [hex_pcrinfo]" ring
> + * keyctl add trusted name "LOAD hex_blob" ring
> + * keyctl update key "UPDATE hex_pcrinfo"
> + * keyctl print keyid
> + * keys can be 32 - 128 bytes, blob max is 1024 hex ascii characters
> + * binary pcrinfo max is 512 hex ascii characters
> + */
> +
> +#include <linux/uaccess.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/parser.h>
> +#include <linux/string.h>
> +#include <keys/user-type.h>
> +#include <keys/trusted-type.h>
> +#include <linux/key-type.h>
> +#include <linux/random.h>
> +#include <linux/rcupdate.h>
> +#include <linux/scatterlist.h>
> +#include <linux/crypto.h>
> +#include <linux/tpm.h>
> +
> +#include "trusted_defined.h"
> +
> +static char hmac_alg[] = "hmac(sha1)";
> +static char hash_alg[] = "sha1";
> +
> +static int init_sha1_desc(struct hash_desc *desc)
> +{
> + int rc;
> +
> + desc->tfm = crypto_alloc_hash(hash_alg, 0, CRYPTO_ALG_ASYNC);
> + if (IS_ERR(desc->tfm)) {
> + rc = PTR_ERR(desc->tfm);
> + return rc;
> + }
> + desc->flags = 0;
> + rc = crypto_hash_init(desc);
> + if (rc)
> + crypto_free_hash(desc->tfm);
> + return rc;
> +}
> +
> +static int init_hmac_desc(struct hash_desc *desc, unsigned char *key,
> + int keylen)
> +{
> + int rc;
> +
> + desc->tfm = crypto_alloc_hash(hmac_alg, 0, CRYPTO_ALG_ASYNC);
> + if (IS_ERR(desc->tfm)) {
> + rc = PTR_ERR(desc->tfm);
> + return rc;
> + }
> + desc->flags = 0;
> + crypto_hash_setkey(desc->tfm, key, keylen);
> + rc = crypto_hash_init(desc);
> + if (rc)
> + crypto_free_hash(desc->tfm);
> + return rc;
> +}
> +
> +static int TSS_sha1(unsigned char *data, int datalen, unsigned char *digest)
> +{
> + struct hash_desc desc;
> + struct scatterlist sg[1];
> + int rc;
> +
> + rc = init_sha1_desc(&desc);
> + if (rc != 0)
> + return rc;
> +
> + sg_init_one(sg, data, datalen);
> + crypto_hash_update(&desc, sg, datalen);
> + crypto_hash_final(&desc, digest);
> + crypto_free_hash(desc.tfm);
> + return rc;
In later functions where rc can only be 0, you 'return 0;', but here
you return rc. Is that an oversight, or is there something actually
wrong here (like a missing hunk)?
There are also several places where you:
> + datablob = kzalloc(datalen + 1, GFP_KERNEL);
> + if (!datablob)
> + return -ENOMEM;
> + memcpy(datablob, data, datalen);
don't need to kzalloc.
-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
