From: Eric Biggers <ebiggers3@gmail.com> To: keyrings@vger.kernel.org Cc: linux-security-module@vger.kernel.org, David Howells <dhowells@redhat.com>, linux-kernel@vger.kernel.org, Eric Biggers <ebiggers@google.com>, Mimi Zohar <zohar@linux.vnet.ibm.com>, David Safford <safford@us.ibm.com> Subject: [PATCH 4/5] KEYS: trusted: sanitize all key material Date: Fri, 21 Apr 2017 01:30:36 -0700 [thread overview] Message-ID: <20170421083037.12746-5-ebiggers3@gmail.com> (raw) In-Reply-To: <20170421083037.12746-1-ebiggers3@gmail.com> From: Eric Biggers <ebiggers@google.com> As the previous patch did for encrypted-keys, zero sensitive any potentially sensitive data related to the "trusted" key type before it is freed. Notably, we were not zeroing the tpm_buf structures in which the actual key is stored for TPM seal and unseal, nor were we zeroing the trusted_key_payload in certain error paths. Cc: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: David Safford <safford@us.ibm.com> Signed-off-by: Eric Biggers <ebiggers@google.com> --- security/keys/trusted.c | 50 ++++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/security/keys/trusted.c b/security/keys/trusted.c index 2ae31c5a87de..435e86e13879 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -70,7 +70,7 @@ static int TSS_sha1(const unsigned char *data, unsigned int datalen, } ret = crypto_shash_digest(&sdesc->shash, data, datalen, digest); - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -114,7 +114,7 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key, if (!ret) ret = crypto_shash_final(&sdesc->shash, digest); out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -165,7 +165,7 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key, paramdigest, TPM_NONCE_SIZE, h1, TPM_NONCE_SIZE, h2, 1, &c, 0, 0); out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -246,7 +246,7 @@ static int TSS_checkhmac1(unsigned char *buffer, if (memcmp(testhmac, authdata, SHA1_DIGEST_SIZE)) ret = -EINVAL; out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -347,7 +347,7 @@ static int TSS_checkhmac2(unsigned char *buffer, if (memcmp(testhmac2, authdata2, SHA1_DIGEST_SIZE)) ret = -EINVAL; out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -564,7 +564,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype, *bloblen = storedsize; } out: - kfree(td); + kzfree(td); return ret; } @@ -678,7 +678,7 @@ static int key_seal(struct trusted_key_payload *p, if (ret < 0) pr_info("trusted_key: srkseal failed (%d)\n", ret); - kfree(tb); + kzfree(tb); return ret; } @@ -703,7 +703,7 @@ static int key_unseal(struct trusted_key_payload *p, /* pull migratable flag out of sealed key */ p->migratable = p->key[--p->key_len]; - kfree(tb); + kzfree(tb); return ret; } @@ -1037,12 +1037,12 @@ static int trusted_instantiate(struct key *key, if (!ret && options->pcrlock) ret = pcrlock(options->pcrlock); out: - kfree(datablob); - kfree(options); + kzfree(datablob); + kzfree(options); if (!ret) rcu_assign_keypointer(key, payload); else - kfree(payload); + kzfree(payload); return ret; } @@ -1051,8 +1051,7 @@ static void trusted_rcu_free(struct rcu_head *rcu) struct trusted_key_payload *p; p = container_of(rcu, struct trusted_key_payload, rcu); - memset(p->key, 0, p->key_len); - kfree(p); + kzfree(p); } /* @@ -1094,13 +1093,13 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) ret = datablob_parse(datablob, new_p, new_o); if (ret != Opt_update) { ret = -EINVAL; - kfree(new_p); + kzfree(new_p); goto out; } if (!new_o->keyhandle) { ret = -EINVAL; - kfree(new_p); + kzfree(new_p); goto out; } @@ -1114,22 +1113,22 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) ret = key_seal(new_p, new_o); if (ret < 0) { pr_info("trusted_key: key_seal failed (%d)\n", ret); - kfree(new_p); + kzfree(new_p); goto out; } if (new_o->pcrlock) { ret = pcrlock(new_o->pcrlock); if (ret < 0) { pr_info("trusted_key: pcrlock failed (%d)\n", ret); - kfree(new_p); + kzfree(new_p); goto out; } } rcu_assign_keypointer(key, new_p); call_rcu(&p->rcu, trusted_rcu_free); out: - kfree(datablob); - kfree(new_o); + kzfree(datablob); + kzfree(new_o); return ret; } @@ -1158,24 +1157,19 @@ static long trusted_read(const struct key *key, char __user *buffer, for (i = 0; i < p->blob_len; i++) bufp = hex_byte_pack(bufp, p->blob[i]); if ((copy_to_user(buffer, ascii_buf, 2 * p->blob_len)) != 0) { - kfree(ascii_buf); + kzfree(ascii_buf); return -EFAULT; } - kfree(ascii_buf); + kzfree(ascii_buf); return 2 * p->blob_len; } /* - * trusted_destroy - before freeing the key, clear the decrypted data + * trusted_destroy - clear and free the key's payload */ static void trusted_destroy(struct key *key) { - struct trusted_key_payload *p = key->payload.data[0]; - - if (!p) - return; - memset(p->key, 0, p->key_len); - kfree(key->payload.data[0]); + kzfree(key->payload.data[0]); } struct key_type key_type_trusted = { -- 2.12.2
WARNING: multiple messages have this Message-ID (diff)
From: ebiggers3@gmail.com (Eric Biggers) To: linux-security-module@vger.kernel.org Subject: [PATCH 4/5] KEYS: trusted: sanitize all key material Date: Fri, 21 Apr 2017 01:30:36 -0700 [thread overview] Message-ID: <20170421083037.12746-5-ebiggers3@gmail.com> (raw) In-Reply-To: <20170421083037.12746-1-ebiggers3@gmail.com> From: Eric Biggers <ebiggers@google.com> As the previous patch did for encrypted-keys, zero sensitive any potentially sensitive data related to the "trusted" key type before it is freed. Notably, we were not zeroing the tpm_buf structures in which the actual key is stored for TPM seal and unseal, nor were we zeroing the trusted_key_payload in certain error paths. Cc: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: David Safford <safford@us.ibm.com> Signed-off-by: Eric Biggers <ebiggers@google.com> --- security/keys/trusted.c | 50 ++++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/security/keys/trusted.c b/security/keys/trusted.c index 2ae31c5a87de..435e86e13879 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -70,7 +70,7 @@ static int TSS_sha1(const unsigned char *data, unsigned int datalen, } ret = crypto_shash_digest(&sdesc->shash, data, datalen, digest); - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -114,7 +114,7 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key, if (!ret) ret = crypto_shash_final(&sdesc->shash, digest); out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -165,7 +165,7 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key, paramdigest, TPM_NONCE_SIZE, h1, TPM_NONCE_SIZE, h2, 1, &c, 0, 0); out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -246,7 +246,7 @@ static int TSS_checkhmac1(unsigned char *buffer, if (memcmp(testhmac, authdata, SHA1_DIGEST_SIZE)) ret = -EINVAL; out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -347,7 +347,7 @@ static int TSS_checkhmac2(unsigned char *buffer, if (memcmp(testhmac2, authdata2, SHA1_DIGEST_SIZE)) ret = -EINVAL; out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -564,7 +564,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype, *bloblen = storedsize; } out: - kfree(td); + kzfree(td); return ret; } @@ -678,7 +678,7 @@ static int key_seal(struct trusted_key_payload *p, if (ret < 0) pr_info("trusted_key: srkseal failed (%d)\n", ret); - kfree(tb); + kzfree(tb); return ret; } @@ -703,7 +703,7 @@ static int key_unseal(struct trusted_key_payload *p, /* pull migratable flag out of sealed key */ p->migratable = p->key[--p->key_len]; - kfree(tb); + kzfree(tb); return ret; } @@ -1037,12 +1037,12 @@ static int trusted_instantiate(struct key *key, if (!ret && options->pcrlock) ret = pcrlock(options->pcrlock); out: - kfree(datablob); - kfree(options); + kzfree(datablob); + kzfree(options); if (!ret) rcu_assign_keypointer(key, payload); else - kfree(payload); + kzfree(payload); return ret; } @@ -1051,8 +1051,7 @@ static void trusted_rcu_free(struct rcu_head *rcu) struct trusted_key_payload *p; p = container_of(rcu, struct trusted_key_payload, rcu); - memset(p->key, 0, p->key_len); - kfree(p); + kzfree(p); } /* @@ -1094,13 +1093,13 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) ret = datablob_parse(datablob, new_p, new_o); if (ret != Opt_update) { ret = -EINVAL; - kfree(new_p); + kzfree(new_p); goto out; } if (!new_o->keyhandle) { ret = -EINVAL; - kfree(new_p); + kzfree(new_p); goto out; } @@ -1114,22 +1113,22 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) ret = key_seal(new_p, new_o); if (ret < 0) { pr_info("trusted_key: key_seal failed (%d)\n", ret); - kfree(new_p); + kzfree(new_p); goto out; } if (new_o->pcrlock) { ret = pcrlock(new_o->pcrlock); if (ret < 0) { pr_info("trusted_key: pcrlock failed (%d)\n", ret); - kfree(new_p); + kzfree(new_p); goto out; } } rcu_assign_keypointer(key, new_p); call_rcu(&p->rcu, trusted_rcu_free); out: - kfree(datablob); - kfree(new_o); + kzfree(datablob); + kzfree(new_o); return ret; } @@ -1158,24 +1157,19 @@ static long trusted_read(const struct key *key, char __user *buffer, for (i = 0; i < p->blob_len; i++) bufp = hex_byte_pack(bufp, p->blob[i]); if ((copy_to_user(buffer, ascii_buf, 2 * p->blob_len)) != 0) { - kfree(ascii_buf); + kzfree(ascii_buf); return -EFAULT; } - kfree(ascii_buf); + kzfree(ascii_buf); return 2 * p->blob_len; } /* - * trusted_destroy - before freeing the key, clear the decrypted data + * trusted_destroy - clear and free the key's payload */ static void trusted_destroy(struct key *key) { - struct trusted_key_payload *p = key->payload.data[0]; - - if (!p) - return; - memset(p->key, 0, p->key_len); - kfree(key->payload.data[0]); + kzfree(key->payload.data[0]); } struct key_type key_type_trusted = { -- 2.12.2 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2017-04-21 8:34 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-04-21 8:30 [PATCH 0/5] KEYS: sanitize key payloads Eric Biggers 2017-04-21 8:30 ` Eric Biggers 2017-04-21 8:30 ` [PATCH 1/5] KEYS: sanitize add_key() and keyctl() " Eric Biggers 2017-04-21 8:30 ` Eric Biggers 2017-04-28 17:57 ` Eric Biggers 2017-04-28 17:57 ` Eric Biggers 2017-04-21 8:30 ` [PATCH 2/5] KEYS: user_defined: sanitize " Eric Biggers 2017-04-21 8:30 ` Eric Biggers 2017-04-21 8:30 ` [PATCH 3/5] KEYS: encrypted: sanitize all key material Eric Biggers 2017-04-21 8:30 ` Eric Biggers 2017-04-21 8:30 ` Eric Biggers [this message] 2017-04-21 8:30 ` [PATCH 4/5] KEYS: trusted: " Eric Biggers 2017-04-21 8:30 ` [PATCH 5/5] KEYS: sanitize key structs before freeing Eric Biggers 2017-04-21 8:30 ` Eric Biggers 2017-04-21 13:57 ` [PATCH 2/5] KEYS: user_defined: sanitize key payloads David Howells 2017-04-21 13:57 ` David Howells 2017-04-21 18:34 ` Eric Biggers 2017-04-21 18:34 ` Eric Biggers 2017-04-24 14:14 ` David Howells 2017-04-24 14:14 ` David Howells 2017-04-21 14:31 ` [PATCH 3/5] KEYS: encrypted: sanitize all key material David Howells 2017-04-21 14:31 ` David Howells 2017-04-21 18:24 ` Eric Biggers 2017-04-21 18:24 ` Eric Biggers 2017-04-24 14:14 ` David Howells 2017-04-24 14:14 ` David Howells 2017-04-27 15:09 ` [PATCH 0/5] KEYS: sanitize key payloads David Howells 2017-04-27 15:09 ` David Howells 2017-04-27 17:43 ` Eric Biggers 2017-04-27 17:43 ` Eric Biggers 2017-06-02 15:34 ` [PATCH 1/5] KEYS: sanitize add_key() and keyctl() " David Howells 2017-06-02 15:34 ` David Howells 2017-06-02 15:34 ` David Howells 2017-06-02 17:24 ` Eric Biggers 2017-06-02 17:24 ` Eric Biggers 2017-06-02 17:24 ` Eric Biggers
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20170421083037.12746-5-ebiggers3@gmail.com \ --to=ebiggers3@gmail.com \ --cc=dhowells@redhat.com \ --cc=ebiggers@google.com \ --cc=keyrings@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=safford@us.ibm.com \ --cc=zohar@linux.vnet.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.